Closed Bug 1088843 Opened 10 years ago Closed 6 years ago

taskcluster-client: PulseConnection and PulseListener need to support reconnecting

Categories

(Taskcluster :: Services, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1335953

People

(Reporter: jonasfj, Unassigned)

References

Details

Attachments

(1 file)

Currently, we just crash whenever the AMQP connection fails. This is a decent strategy... but we might want to retry the connection and send the unconfirmed messages again. There needs to be some interaction between PulseListener and PulseConnection for this to work. Ideally, we should also be able to use this logic (and PulseConnection) in taskcluster-base/exchanges so that we only need one AMQP connection per node and that one connection will reconnect if broken.
Component: TaskCluster → General
Product: Testing → Taskcluster
Component: General → Client Libraries
Attached file Github PR (deleted) —
This has been in the pipeline for a long time... I think we should have the same retry-logic in go-client, not that we're in a hurry to implement it! (IMO it needs some restructuring to support error handling) --- Note, We shouldn't publish this until the taskcluster-base exchange/publisher utilities supports reconnect. But we should review it before we fix tc-base.
Assignee: nobody → jopsen
Status: NEW → ASSIGNED
Attachment #8654454 - Flags: review?(pmoore)
Thanks for doing this! Let's have a vidyo to go through it, as there is quite a lot of change there. This will help me prepare a better review.
Comment on attachment 8654454 [details] Github PR As discussed, let's have one of the other guys look at this, who will probably have more insights than me. Sorry for holding onto it for so long! :/
Attachment #8654454 - Flags: review?(pmoore)
Assignee: jopsen → nobody
Status: ASSIGNED → NEW
Picking this back up again.. I see a few things to do here: * Abstract the pulse connection code out into a new library -- taking bits and pieces from both pulse-publisher and taskcluster-client * Add a callback for new connections so that users can (repeatedly) declare their exchanges, etc. * Otherwise use EventEmitter for a nice contained API * Support automatic reconnects, and even deliberate connection drops to exercise reconnect logic * Support various credentials * Support fake connections * Rewrite tc-client to use this library For the moment, the Go client's pulse interface is unused, so I won't try to work on that.
Assignee: nobody → dustin
Brian points out that most of the sentry exceptions are pulse-related, so even just cleaning those up to separate signal from noise would be a big win.
I'm going to drop this -- it's taking time I could be spending on redeployability. It's certainly open for others to work on (perhaps Biboswan?). There are some review comments to address in the PR.
Assignee: dustin → nobody
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Component: Client Libraries → Services
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: