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

rls: suppress picker updates from children when handling config updates #5539

Merged
merged 4 commits into from
Aug 1, 2022

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Jul 26, 2022

Summary of changes:

  • Use a single channel to process the different events handled by the run() goroutine.
  • Handling config updates from parent: UpdateClientConnState()
    • Handles config updates inline instead of pushing them on to a channel.
    • Sets a flag to inhibit picker updates.
    • After handling the config changes and pushing the required configs to child policies, queues an event to resume picker updates.
    • Waits for the above event to be acted upon, and then returns a new picker.
  • Handling state updates from child: UpdateState()
    • Handled asynchronously as before. The only change is that message is pushed on the single channel used by run() goroutine.
  • Pushing picker updates to the parent: SendNewPickerLocked()
    • If picker updates are inhibited (by the flag set in UpdateClientConnState), a new picker is built but is not pushed up.

What these change means is that when the RLS LB policy is handling a config update from its parent, no picker updates will be pushed up. This includes picker updates caused by events such as RLS response, cache timer expiry, backoff timer expiry etc.

Fixes #5211

RELEASE NOTES: n/a

@easwars easwars requested a review from dfawley July 26, 2022 23:03
@easwars easwars added the Type: Feature New features or improvements in behavior label Jul 26, 2022
@easwars easwars added this to the 1.49 Release milestone Jul 26, 2022
// update is being processed by RLS LB policy and its child policies.
//
// The test uses a wrapping balancer as the top-level LB policy on the channel.
// The wrapping balancing wraps an RLS LB policy as a child policy and forwards
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

balancer*

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

b.inhibitPickerUpdates = false
b.sendNewPickerLocked()
close(update.done)
b.stateMu.Unlock()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defensive programming: default: logger.Errorf("%T", update)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

type connectivityStateCh struct{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we find a better name? controlChannelReady?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@dfawley dfawley assigned easwars and unassigned dfawley Jul 29, 2022
@easwars easwars merged commit c14e29e into grpc:master Aug 1, 2022
@easwars easwars deleted the delay_picker_update_rls branch August 1, 2022 23:10
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 29, 2023
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 this pull request may close these issues.

Delay picker updates while updating children in priority, RLS, xds_cluster_manager, and weighted_target
3 participants