Skip to content

Commit 7fafa9e

Browse files
dfawleycjqzhao
andauthored
feat(grpc): add gracefulswitch LB policy and some other LB policy changes (#2442)
This mainly supersedes #2399, but most of the implementation is different except the tests, which are largely copied verbatim. And it's a net 200 LoC smaller in graceful switch (100 LoC in total) which hopefully is an indication that the reuse of ChildManager is preferable. In addition, I'd like to revisit the test implementation later, as I'm not sure the current way they are written is ideal. -- General: - Add `Debug` to many traits and derive/impl in structs. - Pass LB config to LB policies via `Option<LbConfig>` instead of `Option<&LbConfig>`. It should be rare that policies want to store a config except for the leaf policy. Child manager: The original assumption was that all children would be the same type/configuration, but several policies (including gracefulswitch) will not have that property. So, several changes are made: - Children are considered unique by both their identifier and their `LbPolicyBuilder`'s name(). - Make it so the sharder also can shard `LbConfig` and provide it via the `ChildUpdate.child_update` field in addition to the `ResolverUpdate`. - Make `ResolverUpdateSharder` a generic instead of `Box<dyn>`. - Add booleans so users of child manager can easily easily tell whether any child policies updated themselves, and which ones did. - Pass `&mut self` for sharder so that it can maintain and update its state if needed. - Change the sharder's output `ChildUpdate.child_update` field to an `Option`; if `None` then the child will not be called during the resolver update, but will remain in the child manager. - Change `child_states` into `children` and provide the whole `Child` struct, exposing the fields it contains. - Provide mutable access to the sharder. - Minor test cleanups Graceful switch: The previous implementation in #2399 contained a lot of logic to manage child policy delegation. It was intended that only `ChildManager` should need to have this kind of logic. - Create a new implementation of this policy that delegates to `ChildManager`. - Uses a Sharder that simply emits the active policy with no update alongside any new policy in the new `LbConfig`. - `maybe_swap` is called after every call into the `ChildManager` to determine if child updates necessitate a swap. - This logic is simple: if the active policy is not `Ready`, or if there is a new policy and it is not `Connecting`, then set the new policy as the active policy and call resolver_update on the `ChildManager`. The sharder will see that no `LbConfig` is provided and just emit the active policy with no config, causing the `ChildManager` to drop the previously active policy. If no swap is needed, update the picker of the active policy if it had an update. - Minor test cleanups/fixes vs. #2399. --------- Co-authored-by: Cathy Zhao <cathyjzhao@google.com>
1 parent 7a5b5bf commit 7fafa9e

File tree

19 files changed

+1344
-204
lines changed

19 files changed

+1344
-204
lines changed

grpc/src/client/channel.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,7 @@ impl load_balancing::ChannelController for InternalChannelController {
444444
}
445445

446446
// A channel that is not idle (connecting, ready, or erroring).
447+
#[derive(Debug)]
447448
pub(super) struct GracefulSwitchBalancer {
448449
pub(super) policy: Mutex<Option<Box<dyn LbPolicy>>>,
449450
policy_builder: Mutex<Option<Arc<dyn LbPolicyBuilder>>>,

grpc/src/client/load_balancing/child_manager.rs

Lines changed: 173 additions & 112 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)