balancer: add StateListener to NewSubConnOptions for SubConn state updates #6481
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
dfawley
added
the
Type: API Change
easwars
reviewed
Jul 28, 2023
Comment on lines
+234
to
+236
| // UpdateSubConnState forwards the update to the appropriate child. | ||
| func (gsb *Balancer) UpdateSubConnState(sc balancer.SubConn, state balancer.SubConnState) { | ||
| gsb.updateSubConnState(sc, state, nil) |
There was a problem hiding this comment.
Since ccb is switched to calling subConn.UpdateState, when and how will this be ever called? From tests maybe (which need to be updated)?
There was a problem hiding this comment.
Yes, from tests... Loads of tests call these methods. I cleaned up some, but there are more.
There was a problem hiding this comment.
Do we have a TODO or an issue to track that?
There was a problem hiding this comment.
That will need to be done as part of #6472. Once this PR lands and I start migrating more things, I'll make UpdateSubConnState for each balancer into a nop and stop calling it from tests at that time. I'll likely do every balancer in its own PR. In hindsight, I probably would have just left the tests I changed in this PR alone to keep the changes smaller, but it's also a nice preview into how it will look, so I left it.
easwars
approved these changes
Jul 28, 2023
mdibaiee
added a commit
to gazette/core
that referenced
this pull request
Nov 15, 2023
See grpc/grpc-go#6481, specifically weightedtarget_test.go as an example of how tests need to be migrated now that UpdateSubConnState is deprecated. I have followed the same example in our dispatcher_test.go
mdibaiee
added a commit
to gazette/core
that referenced
this pull request
Nov 15, 2023
See grpc/grpc-go#6481, specifically weightedtarget_test.go as an example of how tests need to be migrated now that UpdateSubConnState is deprecated. I have followed the same example in our dispatcher_test.go
mdibaiee
added a commit
to gazette/core
that referenced
this pull request
Nov 17, 2023
See grpc/grpc-go#6481, specifically weightedtarget_test.go as an example of how tests need to be migrated now that UpdateSubConnState is deprecated. I have followed the same example in our dispatcher_test.go
mdibaiee
added a commit
to gazette/core
that referenced
this pull request
Nov 17, 2023
See grpc/grpc-go#6481, specifically weightedtarget_test.go as an example of how tests need to be migrated now that UpdateSubConnState is deprecated. I have followed the same example in our dispatcher_test.go
mdibaiee
added a commit
to gazette/core
that referenced
this pull request
Nov 21, 2023
See grpc/grpc-go#6481, specifically weightedtarget_test.go as an example of how tests need to be migrated now that UpdateSubConnState is deprecated. I have followed the same example in our dispatcher_test.go
mdibaiee
added a commit
to gazette/core
that referenced
this pull request
Nov 21, 2023
See grpc/grpc-go#6481, specifically weightedtarget_test.go as an example of how tests need to be migrated now that UpdateSubConnState is deprecated. I have followed the same example in our dispatcher_test.go
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 change is backward-compatible. However I've labeled it as "API Change" since we are deprecating
Balancer.UpdateSubConnState.This also does not fully migrate all our code to stop using
UpdateSubConnState. In particular, many tests still invoke this method, and will need to be migrated over slowly. However, it is the minimal amount of changes required to keep everything working while providing support for this optional feature throughout. It also shows an example of how the other tests can be updated for this transition ingracefulswitch_test.goandweightedtarget_test.go, which now calls theSubConns directly to update their state. Future PRs will migrate all our balancers to use this functionality.#6472
RELEASE NOTES:
StateListenertoNewSubConnOptionsforSubConnstate updates and deprecateBalancer.UpdateSubConnState--- NOTICE: TheBalancer.UpdateSubConnStatemethod will be deleted in a future release andNewSubConnOptions.StateListenerwill be required.