diff --git a/balancer/weightedtarget/weightedaggregator/aggregator.go b/balancer/weightedtarget/weightedaggregator/aggregator.go index 38bd9b223f80..969f634818d5 100644 --- a/balancer/weightedtarget/weightedaggregator/aggregator.go +++ b/balancer/weightedtarget/weightedaggregator/aggregator.go @@ -245,6 +245,7 @@ func (wbsa *Aggregator) build() balancer.State { // state. var readyN, connectingN, idleN int readyPickerWithWeights := make([]weightedPickerState, 0, len(m)) + errorPickers := make([]weightedPickerState, 0, len(m)) for _, ps := range m { switch ps.stateToAggregate { case connectivity.Ready: @@ -254,6 +255,8 @@ func (wbsa *Aggregator) build() balancer.State { connectingN++ case connectivity.Idle: idleN++ + case connectivity.TransientFailure: + errorPickers = append(errorPickers, *ps) } } var aggregatedState connectivity.State @@ -272,7 +275,7 @@ func (wbsa *Aggregator) build() balancer.State { var picker balancer.Picker switch aggregatedState { case connectivity.TransientFailure: - picker = base.NewErrPicker(balancer.ErrTransientFailure) + picker = newWeightedPickerGroup(errorPickers, wbsa.newWRR) case connectivity.Connecting: picker = base.NewErrPicker(balancer.ErrNoSubConnAvailable) default: diff --git a/balancer/weightedtarget/weightedtarget_test.go b/balancer/weightedtarget/weightedtarget_test.go index cc9235264224..f4fd492dab25 100644 --- a/balancer/weightedtarget/weightedtarget_test.go +++ b/balancer/weightedtarget/weightedtarget_test.go @@ -21,6 +21,7 @@ package weightedtarget import ( "encoding/json" "fmt" + "strings" "testing" "time" @@ -130,6 +131,10 @@ var ( const testBackendAddrsCount = 12 +const scConnErrorPrefix = "last connection error:" +const testSCConnErrMsg = "this is definitely a connection error. please check your wiring" +var mergedErrMsg = fmt.Sprintf("%s %s", scConnErrorPrefix, testSCConnErrMsg) + func init() { balancer.Register(newTestConfigBalancerBuilder()) for i := 0; i < testBackendAddrsCount; i++ { @@ -569,7 +574,9 @@ func (s) TestWeightedTarget_TwoSubBalancers_MoreBackends(t *testing.T) { } // Turn sc1's connection down. - wtb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure}) + wtb.UpdateSubConnState(sc1, balancer.SubConnState{ + ConnectivityState: connectivity.TransientFailure, + ConnectionError: fmt.Errorf(testSCConnErrMsg)}) p = <-cc.NewPickerCh want = []balancer.SubConn{sc4} if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { @@ -586,11 +593,13 @@ func (s) TestWeightedTarget_TwoSubBalancers_MoreBackends(t *testing.T) { } // Turn all connections down. - wtb.UpdateSubConnState(sc4, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure}) + wtb.UpdateSubConnState(sc4, balancer.SubConnState{ + ConnectivityState: connectivity.TransientFailure, + ConnectionError: fmt.Errorf(testSCConnErrMsg)}) p = <-cc.NewPickerCh for i := 0; i < 5; i++ { - if _, err := p.Pick(balancer.PickInfo{}); err != balancer.ErrTransientFailure { - t.Fatalf("want pick error %v, got %v", balancer.ErrTransientFailure, err) + if _, err := p.Pick(balancer.PickInfo{}); strings.Compare(err.Error(), mergedErrMsg) != 0{ + t.Fatalf("want pick error '%v', got error '%v'", mergedErrMsg, err) } } } @@ -793,7 +802,9 @@ func (s) TestWeightedTarget_ThreeSubBalancers_RemoveBalancer(t *testing.T) { } // Move balancer 3 into transient failure. - wtb.UpdateSubConnState(sc3, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure}) + wtb.UpdateSubConnState(sc3, balancer.SubConnState{ + ConnectivityState: connectivity.TransientFailure, + ConnectionError: fmt.Errorf(testSCConnErrMsg)}) <-cc.NewPickerCh // Remove the first balancer, while the third is transient failure. @@ -827,8 +838,8 @@ func (s) TestWeightedTarget_ThreeSubBalancers_RemoveBalancer(t *testing.T) { t.Fatalf("RemoveSubConn, want %v, got %v", sc1, scRemoved) } for i := 0; i < 5; i++ { - if _, err := p.Pick(balancer.PickInfo{}); err != balancer.ErrTransientFailure { - t.Fatalf("want pick error %v, got %v", balancer.ErrTransientFailure, err) + if _, err := p.Pick(balancer.PickInfo{}); strings.Compare(err.Error(), mergedErrMsg) != 0 { + t.Fatalf("want pick error '%v', got error '%v'", mergedErrMsg, err) } } } @@ -1064,15 +1075,18 @@ func (s) TestBalancerGroup_SubBalancerTurnsConnectingFromTransientFailure(t *tes // Set both subconn to TransientFailure, this will put both sub-balancers in // transient failure. - wtb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure}) + wtb.UpdateSubConnState(sc1, balancer.SubConnState{ + ConnectivityState: connectivity.TransientFailure, + ConnectionError: fmt.Errorf(testSCConnErrMsg)}) <-cc.NewPickerCh - wtb.UpdateSubConnState(sc2, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure}) + wtb.UpdateSubConnState(sc2, balancer.SubConnState{ + ConnectivityState: connectivity.TransientFailure, + ConnectionError: fmt.Errorf(testSCConnErrMsg)}) p := <-cc.NewPickerCh for i := 0; i < 5; i++ { - r, err := p.Pick(balancer.PickInfo{}) - if err != balancer.ErrTransientFailure { - t.Fatalf("want pick to fail with %v, got result %v, err %v", balancer.ErrTransientFailure, r, err) + if _, err := p.Pick(balancer.PickInfo{}); strings.Compare(err.Error(), mergedErrMsg) != 0 { + t.Fatalf("want pick error '%s', got error '%v'", mergedErrMsg, err) } } @@ -1086,8 +1100,8 @@ func (s) TestBalancerGroup_SubBalancerTurnsConnectingFromTransientFailure(t *tes for i := 0; i < 5; i++ { r, err := p.Pick(balancer.PickInfo{}) - if err != balancer.ErrTransientFailure { - t.Fatalf("want pick to fail with %v, got result %v, err %v", balancer.ErrTransientFailure, r, err) + if strings.Compare(err.Error(), mergedErrMsg) != 0 { + t.Fatalf("want pick error '%s', got result %v, err %v", mergedErrMsg, r, err) } } } diff --git a/xds/internal/balancer/clusterresolver/priority_test.go b/xds/internal/balancer/clusterresolver/priority_test.go index b08b82089898..b528442d8d4c 100644 --- a/xds/internal/balancer/clusterresolver/priority_test.go +++ b/xds/internal/balancer/clusterresolver/priority_test.go @@ -247,7 +247,9 @@ func (s) TestEDSPriority_SwitchPriority(t *testing.T) { } // Turn down 1, use 2 - edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure}) + edsb.UpdateSubConnState(sc1, balancer.SubConnState{ + ConnectivityState: connectivity.TransientFailure, + ConnectionError: fmt.Errorf("this is definitely a connection issue")}) addrs2 := <-cc.NewSubConnAddrsCh if got, want := addrs2[0].Addr, testEndpointAddrs[2]; got != want { t.Fatalf("sc is created with addr %v, want %v", got, want) @@ -274,7 +276,7 @@ func (s) TestEDSPriority_SwitchPriority(t *testing.T) { } // Should get an update with 1's old picker, to override 2's old picker. - if err := testErrPickerFromCh(cc.NewPickerCh, balancer.ErrTransientFailure); err != nil { + if err := testErrPickerFromCh(cc.NewPickerCh, fmt.Errorf("last connection error: this is definitely a connection issue")); err != nil { t.Fatal(err) } @@ -305,10 +307,12 @@ func (s) TestEDSPriority_HigherDownWhileAddingLower(t *testing.T) { } sc1 := <-cc.NewSubConnCh // Turn down 1, pick should error. - edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure}) + edsb.UpdateSubConnState(sc1, balancer.SubConnState{ + ConnectivityState: connectivity.TransientFailure, + ConnectionError: fmt.Errorf("this is definitely a connection issue")}) // Test pick failure. - if err := testErrPickerFromCh(cc.NewPickerCh, balancer.ErrTransientFailure); err != nil { + if err := testErrPickerFromCh(cc.NewPickerCh, fmt.Errorf("last connection error: this is definitely a connection issue")); err != nil { t.Fatal(err) }