Correctly remove boundary references when worker leaves #2073
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
We were not removing all routes to a boundary to a worker that left the cluster. This led to problems in in-flight message acking, since we would try to request acks to boundaries that were no longer connected.
jtfmumm
force-pushed
the
boundary-removal
branch
from
290e6ce
to
3d3bbbc
Compare
|
@SeanTAllen I only saw this causing a problem with the in-flight message barrier work, so I'm not sure what test we would use here. I think the fact that it passes our current tests indicates that it's not introducing a new problem. |
|
I think this is only waiting on your review @SeanTAllen |
|
Code looks reasonable. I don't know how to test. |
|
@SeanTAllen As I say, I can't think of a test since this only created a problem with my in-flight message work. It was doing the incorrect thing before, it just didn't happen to matter. I think the fact that it's passing our existing tests means it's ok to merge. |
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.
We were not removing all routes to a boundary to a worker
that left the cluster. This led to problems in in-flight
message acking, since we would try to request acks to
boundaries that were no longer connected.
Closes #2074