balance: Handle duplicate Insert events by overwriting the existing service #75
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
Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
to linkerd/linkerd2
that referenced
this pull request
May 24, 2018
hawkw
added a commit
to linkerd/linkerd2
that referenced
this pull request
May 29, 2018
Depends on tower-rs/tower#75. Required for #386 In order for the proxy to use the TLS support metadata from the Destination service correctly, we determined that the code for dynamically changing the labels on an already-bound service should be removed, and any change in metadata should cause an endpoint to be rebound. I've modified the proxy so that we no longer update the labels using `futures-watch` (as a sidenote, we no longer depend on that crate). Metadata update events now cause the `tower-discover::Discover` implementation for `DestinationSet` to re-insert the changed endpoint into the load balancer. Upstream PR tower-rs/tower#75 in tower-balance changes the load balancer to honor duplicate insertions by replacing the old endpoint rather than ignoring them; that change is necessary for the tests to pass on this branch. Signed-off-by: Eliza Weisman <[email protected]>
hawkw
added a commit
to linkerd/linkerd2
that referenced
this pull request
May 29, 2018
olix0r
pushed a commit
to linkerd/linkerd2-proxy
that referenced
this pull request
Jul 7, 2018
Depends on tower-rs/tower#75. Required for #386 In order for the proxy to use the TLS support metadata from the Destination service correctly, we determined that the code for dynamically changing the labels on an already-bound service should be removed, and any change in metadata should cause an endpoint to be rebound. I've modified the proxy so that we no longer update the labels using `futures-watch` (as a sidenote, we no longer depend on that crate). Metadata update events now cause the `tower-discover::Discover` implementation for `DestinationSet` to re-insert the changed endpoint into the load balancer. Upstream PR tower-rs/tower#75 in tower-balance changes the load balancer to honor duplicate insertions by replacing the old endpoint rather than ignoring them; that change is necessary for the tests to pass on this branch. Signed-off-by: Eliza Weisman <[email protected]>
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.
When an endpoint's state changes in some way, it may need to be rebound to a
new service, and reinserted into the load balancer. This PR changes
tower-balanceso that, rather than ignoring duplicateInserts, the newendpoint replaces the old endpoint. The new endpoint is always placed on the
not-ready list; if the replaced endpoint was on the ready list, it is removed
prior to inserting the new endpoint into the not-ready list.
Signed-off-by: Eliza Weisman [email protected]