Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util: Use acceptResolvedAddresses() for MultiChildLb children #11894

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

ejona86
Copy link
Member

@ejona86 ejona86 commented Feb 13, 2025

A failing Status from acceptResolvedAddresses means something is wrong with the config, but parts of the config may still have been applied. Thus there are now two possible flows: errors that should prevent updateOverallBalancingState() and errors that should have no effect other than the return code. To manage that, MultChildLb must always be responsible for calling updateOverallBalancingState(). acceptResolvedAddressesInternal() was inlined to make that error processing easier. No existing usages actually needed to have logic between updating the children and regenerating the picker.

RingHashLb already was verifying that the address list was not empty, so the short-circuiting when acceptResolvedAddressesInternal() returned an error was impossible to trigger. WrrLb's updateWeightTask() calls the last picker, so it can run before acceptResolvedAddressesInternal(); the only part that matters is re-creating the weightUpdateTimer.


When reviewing, you'll want to disable whitespace changes, especially for ring hash. There aren't that many changes, but most of this is more subtle than you might assume. I am quite happy to be able to remove other classes calling acceptResolvedAddressesInternal(), though.

A failing Status from acceptResolvedAddresses means something is wrong
with the config, but parts of the config may still have been applied.
Thus there are now two possible flows: errors that should prevent
updateOverallBalancingState() and errors that should have no effect
other than the return code. To manage that, MultChildLb must always be
responsible for calling updateOverallBalancingState().
acceptResolvedAddressesInternal() was inlined to make that error
processing easier. No existing usages actually needed to have logic
between updating the children and regenerating the picker.

RingHashLb already was verifying that the address list was not empty, so
the short-circuiting when acceptResolvedAddressesInternal() returned an
error was impossible to trigger. WrrLb's updateWeightTask() calls the
last picker, so it can run before acceptResolvedAddressesInternal(); the
only part that matters is re-creating the weightUpdateTimer.
@ejona86 ejona86 requested a review from larry-safran February 13, 2025 21:49
@ejona86
Copy link
Member Author

ejona86 commented Feb 14, 2025

I'll merge this after the branch cut.

@ejona86 ejona86 merged commit 7136070 into grpc:master Feb 18, 2025
15 of 16 checks passed
@ejona86 ejona86 deleted the multichild-acceptresolveaddresses branch February 18, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants