balancer/weightedtarget: fix ConnStateEvltr to ignore transition from TF to Connecting (#5747)

balancer/weightedtarget/weightedaggregator/aggregator.go

@@ -185,23 +185,22 @@ func (wbsa *Aggregator) ResumeStateUpdates() {
 func (wbsa *Aggregator) UpdateState(id string, newState balancer.State) {
 	wbsa.mu.Lock()
 	defer wbsa.mu.Unlock()
-	oldState, ok := wbsa.idToPickerState[id]
+	state, ok := wbsa.idToPickerState[id]
 	if !ok {
 		// All state starts with an entry in pickStateMap. If ID is not in map,
 		// it's either removed, or never existed.
 		return
 	}
 
-	wbsa.csEvltr.RecordTransition(oldState.stateToAggregate, newState.ConnectivityState)
-
-	if !(oldState.state.ConnectivityState == connectivity.TransientFailure && newState.ConnectivityState == connectivity.Connecting) {
+	if !(state.state.ConnectivityState == connectivity.TransientFailure && newState.ConnectivityState == connectivity.Connecting) {
 		// If old state is TransientFailure, and new state is Connecting, don't
 		// update the state, to prevent the aggregated state from being always
 		// CONNECTING. Otherwise, stateToAggregate is the same as
 		// state.ConnectivityState.
-		oldState.stateToAggregate = newState.ConnectivityState
+		wbsa.csEvltr.RecordTransition(state.stateToAggregate, newState.ConnectivityState)
+		state.stateToAggregate = newState.ConnectivityState
 	}
-	oldState.state = newState
+	state.state = newState
 
 	wbsa.buildAndUpdateLocked()
 }

balancer/weightedtarget/weightedtarget_test.go

@@ -1249,6 +1249,46 @@ func (s) TestInitialIdle(t *testing.T) {
 	}
 }
 
+// TestIgnoreSubBalancerStateTransitions covers the case that if the child reports a
+// transition from TF to Connecting, the overall state will still be TF.
+func (s) TestIgnoreSubBalancerStateTransitions(t *testing.T) {
+	cc := &tcc{TestClientConn: testutils.NewTestClientConn(t)}
+
+	wtb := wtbBuilder.Build(cc, balancer.BuildOptions{})
+	defer wtb.Close()
+
+	config, err := wtbParser.ParseConfig([]byte(`
+{
+  "targets": {
+    "cluster_1": {
+      "weight":1,
+      "childPolicy": [{"round_robin": ""}]
+    }
+  }
+}`))
+	if err != nil {
+		t.Fatalf("failed to parse balancer config: %v", err)
+	}
+
+	// Send the config, and an address with hierarchy path ["cluster_1"].
+	addr := resolver.Address{Addr: testBackendAddrStrs[0], Attributes: nil}
+	if err := wtb.UpdateClientConnState(balancer.ClientConnState{
+		ResolverState:  resolver.State{Addresses: []resolver.Address{hierarchy.Set(addr, []string{"cluster_1"})}},
+		BalancerConfig: config,
+	}); err != nil {
+		t.Fatalf("failed to update ClientConn state: %v", err)
+	}
+
+	sc := <-cc.NewSubConnCh
+	wtb.UpdateSubConnState(sc, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure})
+	wtb.UpdateSubConnState(sc, balancer.SubConnState{ConnectivityState: connectivity.Connecting})
+
+	// Verify that the SubConnState update from TF to Connecting is ignored.
+	if len(cc.states) != 2 || cc.states[0].ConnectivityState != connectivity.Connecting || cc.states[1].ConnectivityState != connectivity.TransientFailure {
+		t.Fatalf("cc.states = %v; want [Connecting, TransientFailure]", cc.states)
+	}
+}
+
 // tcc wraps a testutils.TestClientConn but stores all state transitions in a
 // slice.
 type tcc struct {