Closed
Bug 1088843
Opened 10 years ago
Closed 6 years ago
taskcluster-client: PulseConnection and PulseListener need to support reconnecting
Categories
(Taskcluster :: Services, defect)
Taskcluster
Services
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1335953
People
(Reporter: jonasfj, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
text/x-github-pull-request
|
Details |
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.
Updated•9 years ago
|
Component: TaskCluster → General
Product: Testing → Taskcluster
Updated•9 years ago
|
Component: General → Client Libraries
Reporter | ||
Comment 1•9 years ago
|
||
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.
Comment 2•9 years ago
|
||
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 3•9 years ago
|
||
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)
Reporter | ||
Updated•7 years ago
|
Assignee: jopsen → nobody
Status: ASSIGNED → NEW
Comment 4•7 years ago
|
||
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.
Updated•7 years ago
|
Assignee: nobody → dustin
Comment 5•7 years ago
|
||
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.
Comment 6•7 years ago
|
||
Consumer support in tc-lib-pulse: https://github.com/taskcluster/taskcluster-lib-pulse/pull/3
Comment 7•7 years ago
|
||
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
Updated•6 years ago
|
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Updated•6 years ago
|
Component: Client Libraries → Services
You need to log in
before you can comment on or make changes to this bug.
Description
•