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

Delay picker updates while updating children in priority, RLS, xds_cluster_manager, and weighted_target #5211

Closed
ejona86 opened this issue Feb 28, 2022 · 4 comments · Fixed by #5539
Assignees
Labels
Type: Feature New features or improvements in behavior

Comments

@ejona86
Copy link
Member

ejona86 commented Feb 28, 2022

To allow localities moving priorities within xDS atomically, when processing a config update, the priority policy should inhibit upward picker updates of its own until it has provided the configs to all of its children. Children update their picker synchronously when receiving the config update, so after passing down the new configuration to all children the priority policy will have updated pickers for each and should at that point propagate its own picker update upward. This was decided in the cross-language TL discussion on 2022-02-11.

We need to audit the code to ensure existing LBs update their picker synchronously, and tweak the docs.

grpc/grpc#28984 grpc/grpc-java#8952

@markdroth
Copy link
Member

We've realized that we also need to do this for the xds_cluster_manager and weighted_target policies.

(Basically, our rule should be that we do this for any policy that has multiple active children at the same time.)

@markdroth
Copy link
Member

Need this in the rls policy too.

@dfawley dfawley changed the title Priority policy should delay picker updates while updating children Delay picker updates while updating children in priority, RLS, xds_cluster_manager, and weighted_target Jun 14, 2022
@dfawley
Copy link
Member

dfawley commented Jul 20, 2022

Update: xds_cluster_manager uses balancergroup which synchronously handles UpdateState calls from children, and UpdateClientConnState is handled synchronously as well, so I believe this is already being handled properly.

Edit: weighted_target follows this same pattern exactly.

@dfawley dfawley assigned easwars and unassigned dfawley Jul 20, 2022
@dfawley
Copy link
Member

dfawley commented Jul 20, 2022

@easwars will take care of RLS, which will require some extra consideration because everything is handled asynchronously today.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants