[WIP] Add Stop() method to shut down cluster. #322
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timoreimann
force-pushed
the
shut-down-cluster
branch
3 times, most recently
from
5a758ea
to
0fd6fa5
Compare
timoreimann
changed the title
|
Something is still fishy: |
This enables to terminate all pending health checks. Apart from making our tests shut down cleanly, it will also give users an opportunity to stop otherwise potentially leaking health-checking goroutines.
Add extra CloseServer method for cases where only the server should be closed.
timoreimann
force-pushed
the
shut-down-cluster
branch
from
0fd6fa5
to
76a0fe8
Compare
|
Seems like we're yet missing to terminate SSE subscriptions. |
|
Has there been any further work towards supporting cluster (or client) shutdowns? Asking because I'm about ready to start doing this myself since we're running into goroutine leaks in health checks. We currently schedule Marathon on arbitrary hosts, so it's possible that a member of the cluster will go away and reappear with a new host and port, which currently requires a new client to be allocated. So, unless I can reconfigure a client's cluster (doesn't seem like the right approach), I'd have to be able to stop the client to avoid leaking health check and other goroutines. |
|
@nilium I stopped using Marathon and subsequently go-marathon a while ago. You're more than welcome to continue the effort. I can try my best to review any PR(s) you may submit. Thank you! |
|
@timoreimann Thanks! I've got some changes that I'm testing out in our systems, so if all goes well, I'll have a pull request once I've had some time to make sure that it's working the way I expect. Just testing it on our own work to avoid too much churn in the PR. No ETA on that right now, but the WIP branch is over at https://github.com/Kochava/go-marathon/tree/cluster-stop for anyone else that wants to poke at it. (May be force pushed over, so please don't pin anything to that fork or branch even if it works for someone.) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enables to terminate all pending health checks. Apart from making our tests shut down cleanly, it will also give users an opportunity to stop otherwise potentially leaking health-checking goroutines.