xds/priority: start the init timer when a child switch to Connecting …

…from non-transient-failure state (#5334)

xds/internal/balancer/priority/balancer.go

@@ -100,12 +100,6 @@ type priorityBalancer struct {
 	childToPriority map[string]int
 	// children is a map from child name to sub-balancers.
 	children map[string]*childBalancer
-	// The timer to give a priority some time to connect. And if the priority
-	// doesn't go into Ready/Failure, the next priority will be started.
-	//
-	// One timer is enough because there can be at most one priority in init
-	// state.
-	priorityInitTimer *timerWrapper
 }
 
 func (b *priorityBalancer) UpdateClientConnState(s balancer.ClientConnState) error {
@@ -198,27 +192,17 @@ func (b *priorityBalancer) Close() {
 	// Clear states of the current child in use, so if there's a race in picker
 	// update, it will be dropped.
 	b.childInUse = ""
-	b.stopPriorityInitTimer()
+	// Stop the child policies, this is necessary to stop the init timers in the
+	// children.
+	for _, child := range b.children {
+		child.stop()
+	}
 }
 
 func (b *priorityBalancer) ExitIdle() {
 	b.bg.ExitIdle()
 }
 
-// stopPriorityInitTimer stops the priorityInitTimer if it's not nil, and set it
-// to nil.
-//
-// Caller must hold b.mu.
-func (b *priorityBalancer) stopPriorityInitTimer() {
-	timerW := b.priorityInitTimer
-	if timerW == nil {
-		return
-	}
-	b.priorityInitTimer = nil
-	timerW.stopped = true
-	timerW.timer.Stop()
-}
-
 // UpdateState implements balancergroup.BalancerStateAggregator interface. The
 // balancer group sends new connectivity state and picker here.
 func (b *priorityBalancer) UpdateState(childName string, state balancer.State) {

xds/internal/balancer/priority/balancer_child.go

@@ -19,6 +19,8 @@
 package priority
 
 import (
+	"time"
+
 	"google.golang.org/grpc/balancer"
 	"google.golang.org/grpc/balancer/base"
 	"google.golang.org/grpc/connectivity"
@@ -36,7 +38,16 @@ type childBalancer struct {
 	rState                     resolver.State
 
 	started bool
-	state   balancer.State
+	// This is set when the child reports TransientFailure, and unset when it
+	// reports Ready or Idle. It is used to decide whether the failover timer
+	// should start when the child is transitioning into Connecting. The timer
+	// will be restarted if the child has not reported TF more recently than it
+	// reported Ready or Idle.
+	reportedTF bool
+	state      balancer.State
+	// The timer to give a priority some time to connect. And if the priority
+	// doesn't go into Ready/Failure, the next priority will be started.
+	initTimer *timerWrapper
 }
 
 // newChildBalancer creates a child balancer place holder, but doesn't
@@ -79,6 +90,7 @@ func (cb *childBalancer) start() {
 	}
 	cb.started = true
 	cb.parent.bg.Add(cb.name, cb.bb)
+	cb.startInitTimer()
 }
 
 // sendUpdate sends the addresses and config to the child balancer.
@@ -103,10 +115,46 @@ func (cb *childBalancer) stop() {
 	if !cb.started {
 		return
 	}
+	cb.stopInitTimer()
 	cb.parent.bg.Remove(cb.name)
 	cb.started = false
 	cb.state = balancer.State{
 		ConnectivityState: connectivity.Connecting,
 		Picker:            base.NewErrPicker(balancer.ErrNoSubConnAvailable),
 	}
+	// Clear child.reportedTF, so that if this child is started later, it will
+	// be given time to connect.
+	cb.reportedTF = false
+}
+
+func (cb *childBalancer) startInitTimer() {
+	if cb.initTimer != nil {
+		return
+	}
+	// Need this local variable to capture timerW in the AfterFunc closure
+	// to check the stopped boolean.
+	timerW := &timerWrapper{}
+	cb.initTimer = timerW
+	timerW.timer = time.AfterFunc(DefaultPriorityInitTimeout, func() {
+		cb.parent.mu.Lock()
+		defer cb.parent.mu.Unlock()
+		if timerW.stopped {
+			return
+		}
+		cb.initTimer = nil
+		// Re-sync the priority. This will switch to the next priority if
+		// there's any. Note that it's important sync() is called after setting
+		// initTimer to nil.
+		cb.parent.syncPriority()
+	})
+}
+
+func (cb *childBalancer) stopInitTimer() {
+	timerW := cb.initTimer
+	if timerW == nil {
+		return
+	}
+	cb.initTimer = nil
+	timerW.stopped = true
+	timerW.timer.Stop()
 }