From 025055ebafb46dcdd529fc010230822fe265ed3d Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 19 May 2025 14:29:54 +0530 Subject: [PATCH 1/6] Implement ExitIdler for endpointsharding --- balancer/endpointsharding/endpointsharding.go | 11 ++++ .../endpointsharding/endpointsharding_test.go | 66 +++++++++++++++++++ balancer/leastrequest/leastrequest.go | 2 + balancer/roundrobin/roundrobin.go | 2 + balancer/weightedroundrobin/balancer.go | 2 + .../balancer/cdsbalancer/cdsbalancer.go | 2 + .../clusterresolver/clusterresolver.go | 2 + 7 files changed, 87 insertions(+) diff --git a/balancer/endpointsharding/endpointsharding.go b/balancer/endpointsharding/endpointsharding.go index cc606f4dae4e..ad03d7a726b4 100644 --- a/balancer/endpointsharding/endpointsharding.go +++ b/balancer/endpointsharding/endpointsharding.go @@ -205,6 +205,17 @@ func (es *endpointSharding) Close() { } } +func (es *endpointSharding) ExitIdle() { + es.childMu.Lock() + defer es.childMu.Unlock() + for _, bw := range es.children.Load().Values() { + ei, ok := bw.child.(balancer.ExitIdler) + if ok && !bw.isClosed { + ei.ExitIdle() + } + } +} + // updateState updates this component's state. It sends the aggregated state, // and a picker with round robin behavior with all the child states present if // needed. diff --git a/balancer/endpointsharding/endpointsharding_test.go b/balancer/endpointsharding/endpointsharding_test.go index 5567b4c9038b..93f337326df8 100644 --- a/balancer/endpointsharding/endpointsharding_test.go +++ b/balancer/endpointsharding/endpointsharding_test.go @@ -285,3 +285,69 @@ func (s) TestEndpointShardingReconnectDisabled(t *testing.T) { } } } + +// Tests that endpointsharding doesn't automatically re-connect IDLE children +// until cc.Connect() is called. The test creates an endpoint with a single +// address. The client is connected and the active server is closed to make the +// child pickfirst enter IDLE state. The test verifies that the child pickfirst +// doesn't re-connect automatically. The test calls cc.Connect() and verified +// that the balancer connects causing the channel to enter TransientFailure. +func (s) TestEndpointShardingExitIdle(t *testing.T) { + backend := stubserver.StartTestService(t, nil) + defer backend.Stop() + + mr := manual.NewBuilderWithScheme("e2e-test") + defer mr.Close() + + name := strings.ReplaceAll(strings.ToLower(t.Name()), "/", "") + bf := stub.BalancerFuncs{ + Init: func(bd *stub.BalancerData) { + epOpts := endpointsharding.Options{DisableAutoReconnect: true} + bd.Data = endpointsharding.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirstleaf.Name).Build, epOpts) + }, + UpdateClientConnState: func(bd *stub.BalancerData, ccs balancer.ClientConnState) error { + return bd.Data.(balancer.Balancer).UpdateClientConnState(ccs) + }, + Close: func(bd *stub.BalancerData) { + bd.Data.(balancer.Balancer).Close() + }, + ExitIdle: func(bd *stub.BalancerData) { + bd.Data.(balancer.ExitIdler).ExitIdle() + }, + } + stub.Register(name, bf) + + json := fmt.Sprintf(`{"loadBalancingConfig": [{"%s":{}}]}`, name) + sc := internal.ParseServiceConfig.(func(string) *serviceconfig.ParseResult)(json) + mr.InitialState(resolver.State{ + Endpoints: []resolver.Endpoint{ + {Addresses: []resolver.Address{{Addr: backend.Address}}}, + }, + ServiceConfig: sc, + }) + + cc, err := grpc.NewClient(mr.Scheme()+":///", grpc.WithResolvers(mr), grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatalf("Failed to create new client: %v", err) + } + defer cc.Close() + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + client := testgrpc.NewTestServiceClient(cc) + if _, err := client.EmptyCall(ctx, &testpb.Empty{}); err != nil { + t.Errorf("client.EmptyCall() returned unexpected error: %v", err) + } + + // On closing the first server, the first child balancer should enter + // IDLE. Since endpointsharding is configured not to auto-reconnect, it will + // remain IDLE and will not try to re-connect + backend.Stop() + testutils.AwaitState(ctx, t, cc, connectivity.Idle) + shortCtx, shortCancel := context.WithTimeout(ctx, defaultTestShortTimeout) + defer shortCancel() + testutils.AwaitNoStateChange(shortCtx, t, cc, connectivity.Idle) + + // The balancer should try to re-connect and fail. + cc.Connect() + testutils.AwaitState(ctx, t, cc, connectivity.TransientFailure) +} diff --git a/balancer/leastrequest/leastrequest.go b/balancer/leastrequest/leastrequest.go index f758f954e9f1..0ac0f43e6b35 100644 --- a/balancer/leastrequest/leastrequest.go +++ b/balancer/leastrequest/leastrequest.go @@ -127,6 +127,8 @@ func (lrb *leastRequestBalancer) ResolverError(err error) { func (lrb *leastRequestBalancer) ExitIdle() { if ei, ok := lrb.child.(balancer.ExitIdler); ok { // Should always be ok, as child is endpoint sharding. ei.ExitIdle() + } else { + lrb.logger.Errorf("Child balancer doesn't implement ExitIdler.") } } diff --git a/balancer/roundrobin/roundrobin.go b/balancer/roundrobin/roundrobin.go index 35da5d1ec9d9..d4df537fc04a 100644 --- a/balancer/roundrobin/roundrobin.go +++ b/balancer/roundrobin/roundrobin.go @@ -75,5 +75,7 @@ func (b *rrBalancer) ExitIdle() { // Should always be ok, as child is endpoint sharding. if ei, ok := b.Balancer.(balancer.ExitIdler); ok { ei.ExitIdle() + } else { + b.logger.Errorf("Child balancer doesn't implement ExitIdler.") } } diff --git a/balancer/weightedroundrobin/balancer.go b/balancer/weightedroundrobin/balancer.go index e6fe5a37acd7..9f38758921a3 100644 --- a/balancer/weightedroundrobin/balancer.go +++ b/balancer/weightedroundrobin/balancer.go @@ -406,6 +406,8 @@ func (b *wrrBalancer) Close() { func (b *wrrBalancer) ExitIdle() { if ei, ok := b.child.(balancer.ExitIdler); ok { // Should always be ok, as child is endpoint sharding. ei.ExitIdle() + } else { + b.logger.Errorf("Child balancer doesn't implement ExitIdler.") } } diff --git a/xds/internal/balancer/cdsbalancer/cdsbalancer.go b/xds/internal/balancer/cdsbalancer/cdsbalancer.go index fa34748c8822..1e1493d9d67c 100644 --- a/xds/internal/balancer/cdsbalancer/cdsbalancer.go +++ b/xds/internal/balancer/cdsbalancer/cdsbalancer.go @@ -426,6 +426,8 @@ func (b *cdsBalancer) ExitIdle() { // will be connected. if ei, ok := b.childLB.(balancer.ExitIdler); ok { ei.ExitIdle() + } else { + b.logger.Errorf("Child balancer doesn't implement ExitIdler.") } }) } diff --git a/xds/internal/balancer/clusterresolver/clusterresolver.go b/xds/internal/balancer/clusterresolver/clusterresolver.go index fe5d93dbfbdf..60d836741c04 100644 --- a/xds/internal/balancer/clusterresolver/clusterresolver.go +++ b/xds/internal/balancer/clusterresolver/clusterresolver.go @@ -341,6 +341,8 @@ func (b *clusterResolverBalancer) run() { // will be connected. if ei, ok := b.child.(balancer.ExitIdler); ok { ei.ExitIdle() + } else { + b.logger.Errorf("Child balancer doesn't implement ExitIdler.") } } case u := <-b.resourceWatcher.updateChannel: From aca03a5ee5f7d0af9d806855dc4bc44105c1e722 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 19 May 2025 14:50:26 +0530 Subject: [PATCH 2/6] Log error in endpointsharding when child doesn't implement ExitIdler --- balancer/endpointsharding/endpointsharding.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/balancer/endpointsharding/endpointsharding.go b/balancer/endpointsharding/endpointsharding.go index ad03d7a726b4..47821e195cf2 100644 --- a/balancer/endpointsharding/endpointsharding.go +++ b/balancer/endpointsharding/endpointsharding.go @@ -27,6 +27,7 @@ package endpointsharding import ( "errors" + "fmt" rand "math/rand/v2" "sync" "sync/atomic" @@ -34,9 +35,13 @@ import ( "google.golang.org/grpc/balancer" "google.golang.org/grpc/balancer/base" "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/grpclog" + internalgrpclog "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/resolver" ) +var logger = grpclog.Component("endpointsharding-lb") + // ChildState is the balancer state of a child along with the endpoint which // identifies the child balancer. type ChildState struct { @@ -73,6 +78,7 @@ func NewBalancer(cc balancer.ClientConn, opts balancer.BuildOptions, childBuilde esOpts: esOpts, childBuilder: childBuilder, } + es.logger = internalgrpclog.NewPrefixLogger(logger, fmt.Sprintf("[endpointsharding-lb %p] ", es)) es.children.Store(resolver.NewEndpointMap[*balancerWrapper]()) return es } @@ -85,6 +91,7 @@ type endpointSharding struct { bOpts balancer.BuildOptions esOpts Options childBuilder ChildBuilderFunc + logger *internalgrpclog.PrefixLogger // Prefix logger for all logging. // childMu synchronizes calls to any single child. It must be held for all // calls into a child. To avoid deadlocks, do not acquire childMu while @@ -209,9 +216,15 @@ func (es *endpointSharding) ExitIdle() { es.childMu.Lock() defer es.childMu.Unlock() for _, bw := range es.children.Load().Values() { + // this implementation assumes the child balancer supports + // exitidle (but still checks for the interface's existence to + // avoid a panic if not). If the child does not, no subconns + // will be connected. ei, ok := bw.child.(balancer.ExitIdler) if ok && !bw.isClosed { ei.ExitIdle() + } else if !ok { + es.logger.Errorf("Child balancer doesn't implement ExitIdler.") } } } @@ -337,6 +350,10 @@ func (bw *balancerWrapper) UpdateState(state balancer.State) { // ExitIdle pings an IDLE child balancer to exit idle in a new goroutine to // avoid deadlocks due to synchronous balancer state updates. func (bw *balancerWrapper) ExitIdle() { + // this implementation assumes the child balancer supports + // exitidle (but still checks for the interface's existence to + // avoid a panic if not). If the child does not, no subconns + // will be connected. if ei, ok := bw.child.(balancer.ExitIdler); ok { go func() { bw.es.childMu.Lock() @@ -345,6 +362,8 @@ func (bw *balancerWrapper) ExitIdle() { } bw.es.childMu.Unlock() }() + } else if !ok { + bw.es.logger.Errorf("Child balancer doesn't implement ExitIdler.") } } From 98ebf34bbef864a9263837d8ecfd0de0040c7352 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Tue, 20 May 2025 10:54:07 +0530 Subject: [PATCH 3/6] Ensure all Balancer implementers also implement ExitIdler --- balancer/endpointsharding/endpointsharding_test.go | 4 ++++ .../pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go | 4 +--- balancer/rls/internal/test/e2e/rls_child_policy.go | 4 ++++ balancer/weightedtarget/weightedtarget_test.go | 4 ++++ clientconn_test.go | 4 ++++ internal/balancer/gracefulswitch/gracefulswitch_test.go | 2 ++ internal/balancer/nop/nop.go | 3 +++ internal/stats/metrics_recorder_list_test.go | 4 ++++ interop/orcalb.go | 6 ++++++ test/clientconn_state_transition_test.go | 4 ++++ test/end2end_test.go | 4 ++++ xds/internal/balancer/wrrlocality/balancer.go | 9 +++++++++ 12 files changed, 49 insertions(+), 3 deletions(-) diff --git a/balancer/endpointsharding/endpointsharding_test.go b/balancer/endpointsharding/endpointsharding_test.go index 93f337326df8..ce68a80af0dc 100644 --- a/balancer/endpointsharding/endpointsharding_test.go +++ b/balancer/endpointsharding/endpointsharding_test.go @@ -102,6 +102,10 @@ type fakePetiole struct { bOpts balancer.BuildOptions } +func (fp *fakePetiole) ExitIdle() { + fp.Balancer.(balancer.ExitIdler).ExitIdle() +} + func (fp *fakePetiole) UpdateClientConnState(state balancer.ClientConnState) error { if el := state.ResolverState.Endpoints; len(el) != 2 { return fmt.Errorf("UpdateClientConnState wants two endpoints, got: %v", el) diff --git a/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go b/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go index 160783fc27d3..be8c43fa7ab5 100644 --- a/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go +++ b/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go @@ -1593,9 +1593,7 @@ func (b *stateStoringBalancer) Close() { } func (b *stateStoringBalancer) ExitIdle() { - if ib, ok := b.Balancer.(balancer.ExitIdler); ok { - ib.ExitIdle() - } + b.Balancer.(balancer.ExitIdler).ExitIdle() } type stateStoringBalancerBuilder struct { diff --git a/balancer/rls/internal/test/e2e/rls_child_policy.go b/balancer/rls/internal/test/e2e/rls_child_policy.go index 2bae34b60887..3e06d818b047 100644 --- a/balancer/rls/internal/test/e2e/rls_child_policy.go +++ b/balancer/rls/internal/test/e2e/rls_child_policy.go @@ -102,6 +102,10 @@ type RLSChildPolicyConfig struct { Random string // A random field to test child policy config changes. } +func (b *bal) ExitIdle() { + b.Balancer.(balancer.ExitIdler).ExitIdle() +} + func (b *bal) UpdateClientConnState(c balancer.ClientConnState) error { cfg, ok := c.BalancerConfig.(*RLSChildPolicyConfig) if !ok { diff --git a/balancer/weightedtarget/weightedtarget_test.go b/balancer/weightedtarget/weightedtarget_test.go index c9c78fc5a307..a9a1d4340b33 100644 --- a/balancer/weightedtarget/weightedtarget_test.go +++ b/balancer/weightedtarget/weightedtarget_test.go @@ -130,6 +130,10 @@ func getConfigKey(attr *attributes.Attributes) (string, bool) { return name, ok } +func (b *testConfigBalancer) ExitIdle() { + b.Balancer.(balancer.ExitIdler).ExitIdle() +} + func (b *testConfigBalancer) UpdateClientConnState(s balancer.ClientConnState) error { c, ok := s.BalancerConfig.(stringBalancerConfig) if !ok { diff --git a/clientconn_test.go b/clientconn_test.go index 9cca5a8eb7b7..96eb7ace51bd 100644 --- a/clientconn_test.go +++ b/clientconn_test.go @@ -1220,6 +1220,10 @@ type stateRecordingBalancer struct { balancer.Balancer } +func (b *stateRecordingBalancer) ExitIdle() { + b.Balancer.(balancer.ExitIdler).ExitIdle() +} + func (b *stateRecordingBalancer) UpdateSubConnState(sc balancer.SubConn, s balancer.SubConnState) { panic(fmt.Sprintf("UpdateSubConnState(%v, %+v) called unexpectedly", sc, s)) } diff --git a/internal/balancer/gracefulswitch/gracefulswitch_test.go b/internal/balancer/gracefulswitch/gracefulswitch_test.go index 5e6c53c74534..537f23d42761 100644 --- a/internal/balancer/gracefulswitch/gracefulswitch_test.go +++ b/internal/balancer/gracefulswitch/gracefulswitch_test.go @@ -1010,6 +1010,8 @@ func (vb *verifyBalancer) UpdateClientConnState(balancer.ClientConnState) error return nil } +func (vb *verifyBalancer) ExitIdle() {} + func (vb *verifyBalancer) ResolverError(error) {} func (vb *verifyBalancer) UpdateSubConnState(sc balancer.SubConn, state balancer.SubConnState) { diff --git a/internal/balancer/nop/nop.go b/internal/balancer/nop/nop.go index 0c96f1b81186..8c5bf96f2d27 100644 --- a/internal/balancer/nop/nop.go +++ b/internal/balancer/nop/nop.go @@ -60,3 +60,6 @@ func (b *bal) UpdateSubConnState(_ balancer.SubConn, _ balancer.SubConnState) {} // Close is a no-op. func (b *bal) Close() {} + +// ExitIdle is a no-op. +func (b *bal) ExitIdle() {} diff --git a/internal/stats/metrics_recorder_list_test.go b/internal/stats/metrics_recorder_list_test.go index 38f9472f926a..2e64cf288a00 100644 --- a/internal/stats/metrics_recorder_list_test.go +++ b/internal/stats/metrics_recorder_list_test.go @@ -124,6 +124,10 @@ type recordingLoadBalancer struct { balancer.Balancer } +func (b *recordingLoadBalancer) ExitIdle() { + b.Balancer.(balancer.ExitIdler).ExitIdle() +} + // TestMetricsRecorderList tests the metrics recorder list functionality of the // ClientConn. It configures a global and local stats handler Dial Option. These // stats handlers implement the MetricsRecorder interface. It also configures a diff --git a/interop/orcalb.go b/interop/orcalb.go index 572a7dfcd5cb..b19defb63242 100644 --- a/interop/orcalb.go +++ b/interop/orcalb.go @@ -53,6 +53,12 @@ type orcab struct { report *v3orcapb.OrcaLoadReport } +func (o *orcab) ExitIdle() { + if o.sc != nil { + o.sc.Connect() + } +} + func (o *orcab) UpdateClientConnState(s balancer.ClientConnState) error { if o.sc != nil { o.sc.UpdateAddresses(s.ResolverState.Addresses) diff --git a/test/clientconn_state_transition_test.go b/test/clientconn_state_transition_test.go index 6e179c1cd59a..57249201734c 100644 --- a/test/clientconn_state_transition_test.go +++ b/test/clientconn_state_transition_test.go @@ -474,6 +474,10 @@ type stateRecordingBalancer struct { balancer.Balancer } +func (b *stateRecordingBalancer) ExitIdle() { + b.Balancer.(balancer.ExitIdler).ExitIdle() +} + func (b *stateRecordingBalancer) Close() { b.Balancer.Close() } diff --git a/test/end2end_test.go b/test/end2end_test.go index 75b27f4c224d..396336fc78fa 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -6541,6 +6541,10 @@ type triggerRPCBlockBalancer struct { balancer.Balancer } +func (bpb *triggerRPCBlockBalancer) ExitIdle() { + bpb.Balancer.(balancer.ExitIdler).ExitIdle() +} + func (bpb *triggerRPCBlockBalancer) UpdateClientConnState(s balancer.ClientConnState) error { err := bpb.Balancer.UpdateClientConnState(s) bpb.ClientConn.UpdateState(balancer.State{ diff --git a/xds/internal/balancer/wrrlocality/balancer.go b/xds/internal/balancer/wrrlocality/balancer.go index 065de200f40c..cfc438c5a82e 100644 --- a/xds/internal/balancer/wrrlocality/balancer.go +++ b/xds/internal/balancer/wrrlocality/balancer.go @@ -154,6 +154,15 @@ type wrrLocalityBalancer struct { logger *grpclog.PrefixLogger } +func (b *wrrLocalityBalancer) ExitIdle() { + ei, ok := b.child.(balancer.ExitIdler) + if ok { + ei.ExitIdle() + } else if !ok { + b.logger.Errorf("Child balancer doesn't implement ExitIdler.") + } +} + func (b *wrrLocalityBalancer) UpdateClientConnState(s balancer.ClientConnState) error { lbCfg, ok := s.BalancerConfig.(*LBConfig) if !ok { From 111c588b1e2cecb34638f1fc6b69193c75f1dcd1 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Wed, 28 May 2025 19:04:28 +0530 Subject: [PATCH 4/6] Add ExitIdle to the Balancer interface --- balancer/balancer.go | 8 +++- balancer/endpointsharding/endpointsharding.go | 46 ++++++++----------- .../endpointsharding/endpointsharding_test.go | 4 +- balancer/lazy/lazy.go | 4 +- balancer/lazy/lazy_ext_test.go | 4 +- balancer/leastrequest/leastrequest.go | 6 +-- .../pickfirstleaf/pickfirstleaf_ext_test.go | 2 +- balancer/ringhash/ringhash.go | 4 +- .../rls/internal/test/e2e/rls_child_policy.go | 2 +- balancer/roundrobin/roundrobin.go | 7 +-- balancer/weightedroundrobin/balancer.go | 6 +-- .../weightedtarget/weightedtarget_test.go | 2 +- clientconn_test.go | 2 +- examples/features/orca/client/main.go | 8 ++++ .../balancer/gracefulswitch/gracefulswitch.go | 10 +--- .../gracefulswitch/gracefulswitch_test.go | 22 ++------- internal/stats/metrics_recorder_list_test.go | 2 +- test/balancer_test.go | 2 +- test/clientconn_state_transition_test.go | 2 +- test/end2end_test.go | 2 +- .../balancer/cdsbalancer/cdsbalancer.go | 6 +-- .../balancer/cdsbalancer/cdsbalancer_test.go | 5 +- .../clusterresolver/clusterresolver.go | 10 +--- xds/internal/balancer/wrrlocality/balancer.go | 7 +-- 24 files changed, 59 insertions(+), 114 deletions(-) diff --git a/balancer/balancer.go b/balancer/balancer.go index c9b343c71564..b1264017db1f 100644 --- a/balancer/balancer.go +++ b/balancer/balancer.go @@ -360,6 +360,10 @@ type Balancer interface { // call SubConn.Shutdown for its existing SubConns; however, this will be // required in a future release, so it is recommended. Close() + // ExitIdle instructs the LB policy to reconnect to backends / exit the + // IDLE state, if appropriate and possible. Note that SubConns that enter + // the IDLE state will not reconnect until SubConn.Connect is called. + ExitIdle() } // ExitIdler is an optional interface for balancers to implement. If @@ -367,8 +371,8 @@ type Balancer interface { // the ClientConn is idle. If unimplemented, ClientConn.Connect will cause // all SubConns to connect. // -// Notice: it will be required for all balancers to implement this in a future -// release. +// Deprecated: All balancers must implement this interface. This interface will +// be removed in a future release. type ExitIdler interface { // ExitIdle instructs the LB policy to reconnect to backends / exit the // IDLE state, if appropriate and possible. Note that SubConns that enter diff --git a/balancer/endpointsharding/endpointsharding.go b/balancer/endpointsharding/endpointsharding.go index 47821e195cf2..eb821b13a93c 100644 --- a/balancer/endpointsharding/endpointsharding.go +++ b/balancer/endpointsharding/endpointsharding.go @@ -27,7 +27,6 @@ package endpointsharding import ( "errors" - "fmt" rand "math/rand/v2" "sync" "sync/atomic" @@ -35,13 +34,9 @@ import ( "google.golang.org/grpc/balancer" "google.golang.org/grpc/balancer/base" "google.golang.org/grpc/connectivity" - "google.golang.org/grpc/grpclog" - internalgrpclog "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/resolver" ) -var logger = grpclog.Component("endpointsharding-lb") - // ChildState is the balancer state of a child along with the endpoint which // identifies the child balancer. type ChildState struct { @@ -50,7 +45,15 @@ type ChildState struct { // Balancer exposes only the ExitIdler interface of the child LB policy. // Other methods of the child policy are called only by endpointsharding. - Balancer balancer.ExitIdler + Balancer ExitIdler +} + +// ExitIdler provides access to only the ExitIdle method of the child balancer. +type ExitIdler interface { + // ExitIdle instructs the LB policy to reconnect to backends / exit the + // IDLE state, if appropriate and possible. Note that SubConns that enter + // the IDLE state will not reconnect until SubConn.Connect is called. + ExitIdle() } // Options are the options to configure the behaviour of the @@ -78,7 +81,6 @@ func NewBalancer(cc balancer.ClientConn, opts balancer.BuildOptions, childBuilde esOpts: esOpts, childBuilder: childBuilder, } - es.logger = internalgrpclog.NewPrefixLogger(logger, fmt.Sprintf("[endpointsharding-lb %p] ", es)) es.children.Store(resolver.NewEndpointMap[*balancerWrapper]()) return es } @@ -91,7 +93,6 @@ type endpointSharding struct { bOpts balancer.BuildOptions esOpts Options childBuilder ChildBuilderFunc - logger *internalgrpclog.PrefixLogger // Prefix logger for all logging. // childMu synchronizes calls to any single child. It must be held for all // calls into a child. To avoid deadlocks, do not acquire childMu while @@ -220,11 +221,8 @@ func (es *endpointSharding) ExitIdle() { // exitidle (but still checks for the interface's existence to // avoid a panic if not). If the child does not, no subconns // will be connected. - ei, ok := bw.child.(balancer.ExitIdler) - if ok && !bw.isClosed { - ei.ExitIdle() - } else if !ok { - es.logger.Errorf("Child balancer doesn't implement ExitIdler.") + if !bw.isClosed { + bw.child.ExitIdle() } } } @@ -350,21 +348,13 @@ func (bw *balancerWrapper) UpdateState(state balancer.State) { // ExitIdle pings an IDLE child balancer to exit idle in a new goroutine to // avoid deadlocks due to synchronous balancer state updates. func (bw *balancerWrapper) ExitIdle() { - // this implementation assumes the child balancer supports - // exitidle (but still checks for the interface's existence to - // avoid a panic if not). If the child does not, no subconns - // will be connected. - if ei, ok := bw.child.(balancer.ExitIdler); ok { - go func() { - bw.es.childMu.Lock() - if !bw.isClosed { - ei.ExitIdle() - } - bw.es.childMu.Unlock() - }() - } else if !ok { - bw.es.logger.Errorf("Child balancer doesn't implement ExitIdler.") - } + go func() { + bw.es.childMu.Lock() + if !bw.isClosed { + bw.child.ExitIdle() + } + bw.es.childMu.Unlock() + }() } // updateClientConnStateLocked delivers the ClientConnState to the child diff --git a/balancer/endpointsharding/endpointsharding_test.go b/balancer/endpointsharding/endpointsharding_test.go index ce68a80af0dc..8443cfe808ab 100644 --- a/balancer/endpointsharding/endpointsharding_test.go +++ b/balancer/endpointsharding/endpointsharding_test.go @@ -103,7 +103,7 @@ type fakePetiole struct { } func (fp *fakePetiole) ExitIdle() { - fp.Balancer.(balancer.ExitIdler).ExitIdle() + fp.Balancer.ExitIdle() } func (fp *fakePetiole) UpdateClientConnState(state balancer.ClientConnState) error { @@ -316,7 +316,7 @@ func (s) TestEndpointShardingExitIdle(t *testing.T) { bd.Data.(balancer.Balancer).Close() }, ExitIdle: func(bd *stub.BalancerData) { - bd.Data.(balancer.ExitIdler).ExitIdle() + bd.Data.(balancer.Balancer).ExitIdle() }, } stub.Register(name, bf) diff --git a/balancer/lazy/lazy.go b/balancer/lazy/lazy.go index 7368d8f4bf9c..314ccd1b0aa1 100644 --- a/balancer/lazy/lazy.go +++ b/balancer/lazy/lazy.go @@ -125,9 +125,7 @@ func (lb *lazyBalancer) ExitIdle() { lb.mu.Lock() defer lb.mu.Unlock() if lb.delegate != nil { - if d, ok := lb.delegate.(balancer.ExitIdler); ok { - d.ExitIdle() - } + lb.delegate.ExitIdle() return } lb.delegate = lb.childBuilder(lb.cc, lb.buildOptions) diff --git a/balancer/lazy/lazy_ext_test.go b/balancer/lazy/lazy_ext_test.go index c57ad28e1bb7..cfd4ca998463 100644 --- a/balancer/lazy/lazy_ext_test.go +++ b/balancer/lazy/lazy_ext_test.go @@ -82,7 +82,7 @@ func (s) TestExitIdle(t *testing.T) { bd.Data = lazy.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirstleaf.Name).Build) }, ExitIdle: func(bd *stub.BalancerData) { - bd.Data.(balancer.ExitIdler).ExitIdle() + bd.Data.(balancer.Balancer).ExitIdle() }, ResolverError: func(bd *stub.BalancerData, err error) { bd.Data.(balancer.Balancer).ResolverError(err) @@ -410,7 +410,7 @@ func (s) TestExitIdlePassthrough(t *testing.T) { bd.Data = lazy.NewBalancer(bd.ClientConn, bd.BuildOptions, balancer.Get(pickfirstleaf.Name).Build) }, ExitIdle: func(bd *stub.BalancerData) { - bd.Data.(balancer.ExitIdler).ExitIdle() + bd.Data.(balancer.Balancer).ExitIdle() }, ResolverError: func(bd *stub.BalancerData, err error) { bd.Data.(balancer.Balancer).ResolverError(err) diff --git a/balancer/leastrequest/leastrequest.go b/balancer/leastrequest/leastrequest.go index 0ac0f43e6b35..f9cf7ccfc1ef 100644 --- a/balancer/leastrequest/leastrequest.go +++ b/balancer/leastrequest/leastrequest.go @@ -125,11 +125,7 @@ func (lrb *leastRequestBalancer) ResolverError(err error) { } func (lrb *leastRequestBalancer) ExitIdle() { - if ei, ok := lrb.child.(balancer.ExitIdler); ok { // Should always be ok, as child is endpoint sharding. - ei.ExitIdle() - } else { - lrb.logger.Errorf("Child balancer doesn't implement ExitIdler.") - } + lrb.child.ExitIdle() } func (lrb *leastRequestBalancer) UpdateClientConnState(ccs balancer.ClientConnState) error { diff --git a/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go b/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go index be8c43fa7ab5..aca6c82e2fad 100644 --- a/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go +++ b/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go @@ -1593,7 +1593,7 @@ func (b *stateStoringBalancer) Close() { } func (b *stateStoringBalancer) ExitIdle() { - b.Balancer.(balancer.ExitIdler).ExitIdle() + b.Balancer.ExitIdle() } type stateStoringBalancerBuilder struct { diff --git a/balancer/ringhash/ringhash.go b/balancer/ringhash/ringhash.go index 8f20410ecf07..2defa5ff1301 100644 --- a/balancer/ringhash/ringhash.go +++ b/balancer/ringhash/ringhash.go @@ -263,7 +263,7 @@ func (b *ringhashBalancer) updatePickerLocked() { sort.Slice(endpointStates, func(i, j int) bool { return endpointStates[i].hashKey < endpointStates[j].hashKey }) - var idleBalancer balancer.ExitIdler + var idleBalancer endpointsharding.ExitIdler for _, es := range endpointStates { connState := es.state.ConnectivityState if connState == connectivity.Connecting { @@ -394,7 +394,7 @@ type endpointState struct { // overridden, for example based on EDS endpoint metadata. hashKey string weight uint32 - balancer balancer.ExitIdler + balancer endpointsharding.ExitIdler // state is updated by the balancer while receiving resolver updates from // the channel and picker updates from its children. Access to it is guarded diff --git a/balancer/rls/internal/test/e2e/rls_child_policy.go b/balancer/rls/internal/test/e2e/rls_child_policy.go index 3e06d818b047..bb611efe863a 100644 --- a/balancer/rls/internal/test/e2e/rls_child_policy.go +++ b/balancer/rls/internal/test/e2e/rls_child_policy.go @@ -103,7 +103,7 @@ type RLSChildPolicyConfig struct { } func (b *bal) ExitIdle() { - b.Balancer.(balancer.ExitIdler).ExitIdle() + b.Balancer.ExitIdle() } func (b *bal) UpdateClientConnState(c balancer.ClientConnState) error { diff --git a/balancer/roundrobin/roundrobin.go b/balancer/roundrobin/roundrobin.go index d4df537fc04a..03477217df60 100644 --- a/balancer/roundrobin/roundrobin.go +++ b/balancer/roundrobin/roundrobin.go @@ -72,10 +72,5 @@ func (b *rrBalancer) UpdateClientConnState(ccs balancer.ClientConnState) error { } func (b *rrBalancer) ExitIdle() { - // Should always be ok, as child is endpoint sharding. - if ei, ok := b.Balancer.(balancer.ExitIdler); ok { - ei.ExitIdle() - } else { - b.logger.Errorf("Child balancer doesn't implement ExitIdler.") - } + b.Balancer.ExitIdle() } diff --git a/balancer/weightedroundrobin/balancer.go b/balancer/weightedroundrobin/balancer.go index 9f38758921a3..fe8ebff1f5b5 100644 --- a/balancer/weightedroundrobin/balancer.go +++ b/balancer/weightedroundrobin/balancer.go @@ -404,11 +404,7 @@ func (b *wrrBalancer) Close() { } func (b *wrrBalancer) ExitIdle() { - if ei, ok := b.child.(balancer.ExitIdler); ok { // Should always be ok, as child is endpoint sharding. - ei.ExitIdle() - } else { - b.logger.Errorf("Child balancer doesn't implement ExitIdler.") - } + b.child.ExitIdle() } // picker is the WRR policy's picker. It uses live-updating backend weights to diff --git a/balancer/weightedtarget/weightedtarget_test.go b/balancer/weightedtarget/weightedtarget_test.go index a9a1d4340b33..96b5bbfab368 100644 --- a/balancer/weightedtarget/weightedtarget_test.go +++ b/balancer/weightedtarget/weightedtarget_test.go @@ -131,7 +131,7 @@ func getConfigKey(attr *attributes.Attributes) (string, bool) { } func (b *testConfigBalancer) ExitIdle() { - b.Balancer.(balancer.ExitIdler).ExitIdle() + b.Balancer.ExitIdle() } func (b *testConfigBalancer) UpdateClientConnState(s balancer.ClientConnState) error { diff --git a/clientconn_test.go b/clientconn_test.go index 96eb7ace51bd..6ec4f630b92e 100644 --- a/clientconn_test.go +++ b/clientconn_test.go @@ -1221,7 +1221,7 @@ type stateRecordingBalancer struct { } func (b *stateRecordingBalancer) ExitIdle() { - b.Balancer.(balancer.ExitIdler).ExitIdle() + b.Balancer.ExitIdle() } func (b *stateRecordingBalancer) UpdateSubConnState(sc balancer.SubConn, s balancer.SubConnState) { diff --git a/examples/features/orca/client/main.go b/examples/features/orca/client/main.go index 60a91f4c0dc6..e21f30965f02 100644 --- a/examples/features/orca/client/main.go +++ b/examples/features/orca/client/main.go @@ -95,6 +95,7 @@ func (orcaLBBuilder) Build(cc balancer.ClientConn, _ balancer.BuildOptions) bala // designed to run within. type orcaLB struct { cc balancer.ClientConn + sc balancer.SubConn } func (o *orcaLB) UpdateClientConnState(ccs balancer.ClientConnState) error { @@ -112,6 +113,7 @@ func (o *orcaLB) UpdateClientConnState(ccs balancer.ClientConnState) error { return fmt.Errorf("orcaLB: error creating SubConn: %v", err) } sc.Connect() + o.sc = sc // Register a simple ORCA OOB listener on the SubConn. We request a 1 // second report interval, but in this example the server indicated the @@ -124,6 +126,12 @@ func (o *orcaLB) UpdateClientConnState(ccs balancer.ClientConnState) error { func (o *orcaLB) ResolverError(error) {} +func (o *orcaLB) ExitIdle() { + if o.sc != nil { + o.sc.Connect() + } +} + // TODO: unused; remove when no longer required. func (o *orcaLB) UpdateSubConnState(balancer.SubConn, balancer.SubConnState) {} diff --git a/internal/balancer/gracefulswitch/gracefulswitch.go b/internal/balancer/gracefulswitch/gracefulswitch.go index fbc1ca356ab9..4c266ce2198b 100644 --- a/internal/balancer/gracefulswitch/gracefulswitch.go +++ b/internal/balancer/gracefulswitch/gracefulswitch.go @@ -223,15 +223,7 @@ func (gsb *Balancer) ExitIdle() { // There is no need to protect this read with a mutex, as the write to the // Balancer field happens in SwitchTo, which completes before this can be // called. - if ei, ok := balToUpdate.Balancer.(balancer.ExitIdler); ok { - ei.ExitIdle() - return - } - gsb.mu.Lock() - defer gsb.mu.Unlock() - for sc := range balToUpdate.subconns { - sc.Connect() - } + balToUpdate.Balancer.ExitIdle() } // updateSubConnState forwards the update to the appropriate child. diff --git a/internal/balancer/gracefulswitch/gracefulswitch_test.go b/internal/balancer/gracefulswitch/gracefulswitch_test.go index 537f23d42761..9f536e8586f0 100644 --- a/internal/balancer/gracefulswitch/gracefulswitch_test.go +++ b/internal/balancer/gracefulswitch/gracefulswitch_test.go @@ -795,9 +795,7 @@ func (s) TestInlineCallbackInBuild(t *testing.T) { } } -// TestExitIdle tests the ExitIdle operation on the Graceful Switch Balancer for -// both possible codepaths, one where the child implements ExitIdler interface -// and one where the child doesn't implement ExitIdler interface. +// TestExitIdle tests the ExitIdle operation on the Graceful Switch Balancer. func (s) TestExitIdle(t *testing.T) { _, gsb := setup(t) // switch to a balancer that implements ExitIdle{} (will populate current). @@ -811,22 +809,6 @@ func (s) TestExitIdle(t *testing.T) { if err := currBal.waitForExitIdle(ctx); err != nil { t.Fatal(err) } - - // switch to a balancer that doesn't implement ExitIdle{} (will populate - // pending). - gsb.SwitchTo(verifyBalancerBuilder{}) - // call exitIdle concurrently with newSubConn to make sure there is not a - // data race. - done := make(chan struct{}) - go func() { - gsb.ExitIdle() - close(done) - }() - pendBal := gsb.balancerPending.Balancer.(*verifyBalancer) - for i := 0; i < 10; i++ { - pendBal.newSubConn([]resolver.Address{}, balancer.NewSubConnOptions{}) - } - <-done } const balancerName1 = "mock_balancer_1" @@ -1070,6 +1052,8 @@ func (bcb *buildCallbackBal) UpdateClientConnState(balancer.ClientConnState) err func (bcb *buildCallbackBal) ResolverError(error) {} +func (bcb *buildCallbackBal) ExitIdle() {} + func (bcb *buildCallbackBal) UpdateSubConnState(sc balancer.SubConn, state balancer.SubConnState) { panic(fmt.Sprintf("UpdateSubConnState(%v, %+v) called unexpectedly", sc, state)) } diff --git a/internal/stats/metrics_recorder_list_test.go b/internal/stats/metrics_recorder_list_test.go index 2e64cf288a00..4ad6aa4ebf92 100644 --- a/internal/stats/metrics_recorder_list_test.go +++ b/internal/stats/metrics_recorder_list_test.go @@ -125,7 +125,7 @@ type recordingLoadBalancer struct { } func (b *recordingLoadBalancer) ExitIdle() { - b.Balancer.(balancer.ExitIdler).ExitIdle() + b.Balancer.ExitIdle() } // TestMetricsRecorderList tests the metrics recorder list functionality of the diff --git a/test/balancer_test.go b/test/balancer_test.go index 8c26ea1b92cf..049aeb7f5c23 100644 --- a/test/balancer_test.go +++ b/test/balancer_test.go @@ -1034,7 +1034,7 @@ func (s) TestSubConn_RegisterHealthListener(t *testing.T) { return bd.Data.(balancer.Balancer).UpdateClientConnState(ccs) }, ExitIdle: func(bd *stub.BalancerData) { - bd.Data.(balancer.ExitIdler).ExitIdle() + bd.Data.(balancer.Balancer).ExitIdle() }, } diff --git a/test/clientconn_state_transition_test.go b/test/clientconn_state_transition_test.go index 57249201734c..568618f909d6 100644 --- a/test/clientconn_state_transition_test.go +++ b/test/clientconn_state_transition_test.go @@ -475,7 +475,7 @@ type stateRecordingBalancer struct { } func (b *stateRecordingBalancer) ExitIdle() { - b.Balancer.(balancer.ExitIdler).ExitIdle() + b.Balancer.ExitIdle() } func (b *stateRecordingBalancer) Close() { diff --git a/test/end2end_test.go b/test/end2end_test.go index 396336fc78fa..8e436877db8f 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -6542,7 +6542,7 @@ type triggerRPCBlockBalancer struct { } func (bpb *triggerRPCBlockBalancer) ExitIdle() { - bpb.Balancer.(balancer.ExitIdler).ExitIdle() + bpb.Balancer.ExitIdle() } func (bpb *triggerRPCBlockBalancer) UpdateClientConnState(s balancer.ClientConnState) error { diff --git a/xds/internal/balancer/cdsbalancer/cdsbalancer.go b/xds/internal/balancer/cdsbalancer/cdsbalancer.go index 1e1493d9d67c..3a0e3f9e6be7 100644 --- a/xds/internal/balancer/cdsbalancer/cdsbalancer.go +++ b/xds/internal/balancer/cdsbalancer/cdsbalancer.go @@ -424,11 +424,7 @@ func (b *cdsBalancer) ExitIdle() { // ExitIdle (but still checks for the interface's existence to // avoid a panic if not). If the child does not, no subconns // will be connected. - if ei, ok := b.childLB.(balancer.ExitIdler); ok { - ei.ExitIdle() - } else { - b.logger.Errorf("Child balancer doesn't implement ExitIdler.") - } + b.childLB.ExitIdle() }) } diff --git a/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go b/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go index 87888c734d7a..2cebfe52db52 100644 --- a/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go +++ b/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go @@ -138,8 +138,7 @@ func registerWrappedClusterResolverPolicy(t *testing.T) (chan serviceconfig.Load bal.ResolverError(err) }, ExitIdle: func(bd *stub.BalancerData) { - bal := bd.Data.(balancer.Balancer) - bal.(balancer.ExitIdler).ExitIdle() + bd.Data.(balancer.Balancer).ExitIdle() close(exitIdleCh) }, Close: func(bd *stub.BalancerData) { @@ -1110,7 +1109,7 @@ func (s) TestExitIdle(t *testing.T) { case <-ctx.Done(): t.Fatal("Timeout when waiting for cds LB policy to be created") } - cdsBal.(balancer.ExitIdler).ExitIdle() + cdsBal.ExitIdle() // Wait for ExitIdle to be called on the child policy. select { diff --git a/xds/internal/balancer/clusterresolver/clusterresolver.go b/xds/internal/balancer/clusterresolver/clusterresolver.go index 60d836741c04..f9ce57293393 100644 --- a/xds/internal/balancer/clusterresolver/clusterresolver.go +++ b/xds/internal/balancer/clusterresolver/clusterresolver.go @@ -335,15 +335,7 @@ func (b *clusterResolverBalancer) run() { } break } - // This implementation assumes the child balancer supports - // ExitIdle (but still checks for the interface's existence to - // avoid a panic if not). If the child does not, no subconns - // will be connected. - if ei, ok := b.child.(balancer.ExitIdler); ok { - ei.ExitIdle() - } else { - b.logger.Errorf("Child balancer doesn't implement ExitIdler.") - } + b.child.ExitIdle() } case u := <-b.resourceWatcher.updateChannel: b.handleResourceUpdate(u) diff --git a/xds/internal/balancer/wrrlocality/balancer.go b/xds/internal/balancer/wrrlocality/balancer.go index cfc438c5a82e..2d03a9c75e7b 100644 --- a/xds/internal/balancer/wrrlocality/balancer.go +++ b/xds/internal/balancer/wrrlocality/balancer.go @@ -155,12 +155,7 @@ type wrrLocalityBalancer struct { } func (b *wrrLocalityBalancer) ExitIdle() { - ei, ok := b.child.(balancer.ExitIdler) - if ok { - ei.ExitIdle() - } else if !ok { - b.logger.Errorf("Child balancer doesn't implement ExitIdler.") - } + b.child.ExitIdle() } func (b *wrrLocalityBalancer) UpdateClientConnState(s balancer.ClientConnState) error { From fb056d633b92c210fe1029ffbec2009c2b329319 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Wed, 28 May 2025 23:28:57 +0530 Subject: [PATCH 5/6] Remove unecessary overrides --- balancer/rls/internal/test/e2e/rls_child_policy.go | 4 ---- clientconn_test.go | 4 ---- internal/stats/metrics_recorder_list_test.go | 4 ---- test/clientconn_state_transition_test.go | 4 ---- test/end2end_test.go | 4 ---- 5 files changed, 20 deletions(-) diff --git a/balancer/rls/internal/test/e2e/rls_child_policy.go b/balancer/rls/internal/test/e2e/rls_child_policy.go index bb611efe863a..2bae34b60887 100644 --- a/balancer/rls/internal/test/e2e/rls_child_policy.go +++ b/balancer/rls/internal/test/e2e/rls_child_policy.go @@ -102,10 +102,6 @@ type RLSChildPolicyConfig struct { Random string // A random field to test child policy config changes. } -func (b *bal) ExitIdle() { - b.Balancer.ExitIdle() -} - func (b *bal) UpdateClientConnState(c balancer.ClientConnState) error { cfg, ok := c.BalancerConfig.(*RLSChildPolicyConfig) if !ok { diff --git a/clientconn_test.go b/clientconn_test.go index 6ec4f630b92e..9cca5a8eb7b7 100644 --- a/clientconn_test.go +++ b/clientconn_test.go @@ -1220,10 +1220,6 @@ type stateRecordingBalancer struct { balancer.Balancer } -func (b *stateRecordingBalancer) ExitIdle() { - b.Balancer.ExitIdle() -} - func (b *stateRecordingBalancer) UpdateSubConnState(sc balancer.SubConn, s balancer.SubConnState) { panic(fmt.Sprintf("UpdateSubConnState(%v, %+v) called unexpectedly", sc, s)) } diff --git a/internal/stats/metrics_recorder_list_test.go b/internal/stats/metrics_recorder_list_test.go index 4ad6aa4ebf92..38f9472f926a 100644 --- a/internal/stats/metrics_recorder_list_test.go +++ b/internal/stats/metrics_recorder_list_test.go @@ -124,10 +124,6 @@ type recordingLoadBalancer struct { balancer.Balancer } -func (b *recordingLoadBalancer) ExitIdle() { - b.Balancer.ExitIdle() -} - // TestMetricsRecorderList tests the metrics recorder list functionality of the // ClientConn. It configures a global and local stats handler Dial Option. These // stats handlers implement the MetricsRecorder interface. It also configures a diff --git a/test/clientconn_state_transition_test.go b/test/clientconn_state_transition_test.go index 568618f909d6..6e179c1cd59a 100644 --- a/test/clientconn_state_transition_test.go +++ b/test/clientconn_state_transition_test.go @@ -474,10 +474,6 @@ type stateRecordingBalancer struct { balancer.Balancer } -func (b *stateRecordingBalancer) ExitIdle() { - b.Balancer.ExitIdle() -} - func (b *stateRecordingBalancer) Close() { b.Balancer.Close() } diff --git a/test/end2end_test.go b/test/end2end_test.go index 8e436877db8f..75b27f4c224d 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -6541,10 +6541,6 @@ type triggerRPCBlockBalancer struct { balancer.Balancer } -func (bpb *triggerRPCBlockBalancer) ExitIdle() { - bpb.Balancer.ExitIdle() -} - func (bpb *triggerRPCBlockBalancer) UpdateClientConnState(s balancer.ClientConnState) error { err := bpb.Balancer.UpdateClientConnState(s) bpb.ClientConn.UpdateState(balancer.State{ From a08fdccbfb8413fbbfd9ff1111e3012669132d18 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Fri, 30 May 2025 09:41:48 +0530 Subject: [PATCH 6/6] Remove unecessary overrides --- balancer/endpointsharding/endpointsharding.go | 4 ---- balancer/endpointsharding/endpointsharding_test.go | 4 ---- balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go | 4 ---- balancer/weightedtarget/weightedtarget_test.go | 4 ---- internal/balancer/gracefulswitch/gracefulswitch.go | 2 +- 5 files changed, 1 insertion(+), 17 deletions(-) diff --git a/balancer/endpointsharding/endpointsharding.go b/balancer/endpointsharding/endpointsharding.go index eb821b13a93c..0ad6bb1f2203 100644 --- a/balancer/endpointsharding/endpointsharding.go +++ b/balancer/endpointsharding/endpointsharding.go @@ -217,10 +217,6 @@ func (es *endpointSharding) ExitIdle() { es.childMu.Lock() defer es.childMu.Unlock() for _, bw := range es.children.Load().Values() { - // this implementation assumes the child balancer supports - // exitidle (but still checks for the interface's existence to - // avoid a panic if not). If the child does not, no subconns - // will be connected. if !bw.isClosed { bw.child.ExitIdle() } diff --git a/balancer/endpointsharding/endpointsharding_test.go b/balancer/endpointsharding/endpointsharding_test.go index 8443cfe808ab..0b9955211b74 100644 --- a/balancer/endpointsharding/endpointsharding_test.go +++ b/balancer/endpointsharding/endpointsharding_test.go @@ -102,10 +102,6 @@ type fakePetiole struct { bOpts balancer.BuildOptions } -func (fp *fakePetiole) ExitIdle() { - fp.Balancer.ExitIdle() -} - func (fp *fakePetiole) UpdateClientConnState(state balancer.ClientConnState) error { if el := state.ResolverState.Endpoints; len(el) != 2 { return fmt.Errorf("UpdateClientConnState wants two endpoints, got: %v", el) diff --git a/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go b/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go index aca6c82e2fad..25bb29444d8a 100644 --- a/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go +++ b/balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go @@ -1592,10 +1592,6 @@ func (b *stateStoringBalancer) Close() { b.Balancer.Close() } -func (b *stateStoringBalancer) ExitIdle() { - b.Balancer.ExitIdle() -} - type stateStoringBalancerBuilder struct { balancer chan *stateStoringBalancer } diff --git a/balancer/weightedtarget/weightedtarget_test.go b/balancer/weightedtarget/weightedtarget_test.go index 96b5bbfab368..c9c78fc5a307 100644 --- a/balancer/weightedtarget/weightedtarget_test.go +++ b/balancer/weightedtarget/weightedtarget_test.go @@ -130,10 +130,6 @@ func getConfigKey(attr *attributes.Attributes) (string, bool) { return name, ok } -func (b *testConfigBalancer) ExitIdle() { - b.Balancer.ExitIdle() -} - func (b *testConfigBalancer) UpdateClientConnState(s balancer.ClientConnState) error { c, ok := s.BalancerConfig.(stringBalancerConfig) if !ok { diff --git a/internal/balancer/gracefulswitch/gracefulswitch.go b/internal/balancer/gracefulswitch/gracefulswitch.go index 4c266ce2198b..ba25b8988718 100644 --- a/internal/balancer/gracefulswitch/gracefulswitch.go +++ b/internal/balancer/gracefulswitch/gracefulswitch.go @@ -223,7 +223,7 @@ func (gsb *Balancer) ExitIdle() { // There is no need to protect this read with a mutex, as the write to the // Balancer field happens in SwitchTo, which completes before this can be // called. - balToUpdate.Balancer.ExitIdle() + balToUpdate.ExitIdle() } // updateSubConnState forwards the update to the appropriate child.