Add ExitIdle to the Balancer interface

Author: arjan-bal

balancer/balancer.go

@@ -360,15 +360,19 @@ 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
 // implemented, ExitIdle will be called when ClientConn.Connect is called, if
 // 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

balancer/endpointsharding/endpointsharding.go

@@ -27,21 +27,16 @@ package endpointsharding
 
 import (
 	"errors"
-	"fmt"
 	rand "math/rand/v2"
 	"sync"
 	"sync/atomic"
 
 	"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

balancer/endpointsharding/endpointsharding_test.go

@@ -103,7 +103,7 @@ type fakePetiole struct {
 }
 
 func (fp *fakePetiole) ExitIdle() {
-	fp.Balancer.(balancer.ExitIdler).ExitIdle()
+	fp.Balancer.(balancer.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)

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)

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)

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 {

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 {

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

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 {

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()
 }