From 24d03d9f769106b3c96b4145244ce682999d3d88 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Tue, 27 Apr 2021 15:22:25 -0700 Subject: [PATCH] xds/priority: add ignore reresolution boolean to config (#4275) --- xds/internal/balancer/priority/balancer.go | 6 +- .../balancer/priority/balancer_child.go | 20 +- .../balancer/priority/balancer_test.go | 266 ++++++++++++++---- xds/internal/balancer/priority/config.go | 3 +- xds/internal/balancer/priority/config_test.go | 9 +- .../balancer/priority/ignore_resolve_now.go | 73 +++++ .../priority/ignore_resolve_now_test.go | 106 +++++++ 7 files changed, 417 insertions(+), 66 deletions(-) create mode 100644 xds/internal/balancer/priority/ignore_resolve_now.go create mode 100644 xds/internal/balancer/priority/ignore_resolve_now_test.go diff --git a/xds/internal/balancer/priority/balancer.go b/xds/internal/balancer/priority/balancer.go index d5c99b0b914..e12a5068737 100644 --- a/xds/internal/balancer/priority/balancer.go +++ b/xds/internal/balancer/priority/balancer.go @@ -131,7 +131,7 @@ func (b *priorityBalancer) UpdateClientConnState(s balancer.ClientConnState) err // the balancer isn't built, because this child can be a low // priority. If necessary, it will be built when syncing priorities. cb := newChildBalancer(name, b, bb) - cb.updateConfig(newSubConfig.Config.Config, resolver.State{ + cb.updateConfig(newSubConfig, resolver.State{ Addresses: addressesSplit[name], ServiceConfig: s.ResolverState.ServiceConfig, Attributes: s.ResolverState.Attributes, @@ -146,13 +146,13 @@ func (b *priorityBalancer) UpdateClientConnState(s balancer.ClientConnState) err // rebuild, rebuild will happen when syncing priorities. if currentChild.bb.Name() != bb.Name() { currentChild.stop() - currentChild.bb = bb + currentChild.updateBuilder(bb) } // Update config and address, but note that this doesn't send the // updates to child balancer (the child balancer might not be built, if // it's a low priority). - currentChild.updateConfig(newSubConfig.Config.Config, resolver.State{ + currentChild.updateConfig(newSubConfig, resolver.State{ Addresses: addressesSplit[name], ServiceConfig: s.ResolverState.ServiceConfig, Attributes: s.ResolverState.Attributes, diff --git a/xds/internal/balancer/priority/balancer_child.go b/xds/internal/balancer/priority/balancer_child.go index d012ad4e459..600705da01a 100644 --- a/xds/internal/balancer/priority/balancer_child.go +++ b/xds/internal/balancer/priority/balancer_child.go @@ -29,10 +29,11 @@ import ( type childBalancer struct { name string parent *priorityBalancer - bb balancer.Builder + bb *ignoreResolveNowBalancerBuilder - config serviceconfig.LoadBalancingConfig - rState resolver.State + ignoreReresolutionRequests bool + config serviceconfig.LoadBalancingConfig + rState resolver.State started bool state balancer.State @@ -44,7 +45,7 @@ func newChildBalancer(name string, parent *priorityBalancer, bb balancer.Builder return &childBalancer{ name: name, parent: parent, - bb: bb, + bb: newIgnoreResolveNowBalancerBuilder(bb, false), started: false, // Start with the connecting state and picker with re-pick error, so // that when a priority switch causes this child picked before it's @@ -56,10 +57,16 @@ func newChildBalancer(name string, parent *priorityBalancer, bb balancer.Builder } } +// updateBuilder updates builder for the child, but doesn't build. +func (cb *childBalancer) updateBuilder(bb balancer.Builder) { + cb.bb = newIgnoreResolveNowBalancerBuilder(bb, cb.ignoreReresolutionRequests) +} + // updateConfig sets childBalancer's config and state, but doesn't send update to // the child balancer. -func (cb *childBalancer) updateConfig(config serviceconfig.LoadBalancingConfig, rState resolver.State) { - cb.config = config +func (cb *childBalancer) updateConfig(child *Child, rState resolver.State) { + cb.ignoreReresolutionRequests = child.IgnoreReresolutionRequests + cb.config = child.Config.Config cb.rState = rState } @@ -76,6 +83,7 @@ func (cb *childBalancer) start() { // sendUpdate sends the addresses and config to the child balancer. func (cb *childBalancer) sendUpdate() { + cb.bb.updateIgnoreResolveNow(cb.ignoreReresolutionRequests) // TODO: return and aggregate the returned error in the parent. err := cb.parent.bg.UpdateClientConnState(cb.name, balancer.ClientConnState{ ResolverState: cb.rState, diff --git a/xds/internal/balancer/priority/balancer_test.go b/xds/internal/balancer/priority/balancer_test.go index d546216123d..61e3dee94d9 100644 --- a/xds/internal/balancer/priority/balancer_test.go +++ b/xds/internal/balancer/priority/balancer_test.go @@ -21,6 +21,7 @@ package priority import ( + "context" "fmt" "testing" "time" @@ -99,8 +100,8 @@ func (s) TestPriority_HighPriorityReady(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -136,9 +137,9 @@ func (s) TestPriority_HighPriorityReady(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-2": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-2": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1", "child-2"}, }, @@ -166,8 +167,8 @@ func (s) TestPriority_HighPriorityReady(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -206,8 +207,8 @@ func (s) TestPriority_SwitchPriority(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -273,9 +274,9 @@ func (s) TestPriority_SwitchPriority(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-2": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-2": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1", "child-2"}, }, @@ -332,8 +333,8 @@ func (s) TestPriority_SwitchPriority(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -389,8 +390,8 @@ func (s) TestPriority_HighPriorityToConnectingFromReady(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -484,8 +485,8 @@ func (s) TestPriority_HigherDownWhileAddingLower(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -538,9 +539,9 @@ func (s) TestPriority_HigherDownWhileAddingLower(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-2": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-2": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1", "child-2"}, }, @@ -596,9 +597,9 @@ func (s) TestPriority_HigherReadyCloseAllLower(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-2": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-2": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1", "child-2"}, }, @@ -711,8 +712,8 @@ func (s) TestPriority_InitTimeout(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -781,8 +782,8 @@ func (s) TestPriority_RemovesAllPriorities(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -842,8 +843,8 @@ func (s) TestPriority_RemovesAllPriorities(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -886,7 +887,7 @@ func (s) TestPriority_RemovesAllPriorities(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0"}, }, @@ -949,8 +950,8 @@ func (s) TestPriority_HighPriorityNoEndpoints(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -984,8 +985,8 @@ func (s) TestPriority_HighPriorityNoEndpoints(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -1047,7 +1048,7 @@ func (s) TestPriority_FirstPriorityUnavailable(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0"}, }, @@ -1091,8 +1092,8 @@ func (s) TestPriority_MoveChildToHigherPriority(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -1128,8 +1129,8 @@ func (s) TestPriority_MoveChildToHigherPriority(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-1", "child-0"}, }, @@ -1192,8 +1193,8 @@ func (s) TestPriority_MoveReadyChildToHigherPriority(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -1244,8 +1245,8 @@ func (s) TestPriority_MoveReadyChildToHigherPriority(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-1", "child-0"}, }, @@ -1292,8 +1293,8 @@ func (s) TestPriority_RemoveReadyLowestChild(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, - "child-1": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-1": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0", "child-1"}, }, @@ -1342,7 +1343,7 @@ func (s) TestPriority_RemoveReadyLowestChild(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0"}, }, @@ -1399,7 +1400,7 @@ func (s) TestPriority_ReadyChildRemovedButInCache(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0"}, }, @@ -1458,7 +1459,7 @@ func (s) TestPriority_ReadyChildRemovedButInCache(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0"}, }, @@ -1502,7 +1503,7 @@ func (s) TestPriority_ChildPolicyChange(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}}, }, Priorities: []string{"child-0"}, }, @@ -1537,7 +1538,7 @@ func (s) TestPriority_ChildPolicyChange(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: testRRBalancerName}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: testRRBalancerName}}, }, Priorities: []string{"child-0"}, }, @@ -1602,7 +1603,7 @@ func (s) TestPriority_ChildPolicyUpdatePickerInline(t *testing.T) { }, BalancerConfig: &LBConfig{ Children: map[string]*Child{ - "child-0": {&internalserviceconfig.BalancerConfig{Name: inlineUpdateBalancerName}}, + "child-0": {Config: &internalserviceconfig.BalancerConfig{Name: inlineUpdateBalancerName}}, }, Priorities: []string{"child-0"}, }, @@ -1618,3 +1619,164 @@ func (s) TestPriority_ChildPolicyUpdatePickerInline(t *testing.T) { } } } + +// When the child policy's configured to ignore reresolution requests, the +// ResolveNow() calls from this child should be all ignored. +func (s) TestPriority_IgnoreReresolutionRequest(t *testing.T) { + cc := testutils.NewTestClientConn(t) + bb := balancer.Get(Name) + pb := bb.Build(cc, balancer.BuildOptions{}) + defer pb.Close() + + // One children, with priorities [0], with one backend, reresolution is + // ignored. + if err := pb.UpdateClientConnState(balancer.ClientConnState{ + ResolverState: resolver.State{ + Addresses: []resolver.Address{ + hierarchy.Set(resolver.Address{Addr: testBackendAddrStrs[0]}, []string{"child-0"}), + }, + }, + BalancerConfig: &LBConfig{ + Children: map[string]*Child{ + "child-0": { + Config: &internalserviceconfig.BalancerConfig{Name: resolveNowBalancerName}, + IgnoreReresolutionRequests: true, + }, + }, + Priorities: []string{"child-0"}, + }, + }); err != nil { + t.Fatalf("failed to update ClientConn state: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + // This is the balancer.ClientConn that the inner resolverNowBalancer is + // built with. + balancerCCI, err := resolveNowBalancerCCCh.Receive(ctx) + if err != nil { + t.Fatalf("timeout waiting for ClientConn from balancer builder") + } + balancerCC := balancerCCI.(balancer.ClientConn) + + // Since IgnoreReresolutionRequests was set to true, all ResolveNow() calls + // should be ignored. + for i := 0; i < 5; i++ { + balancerCC.ResolveNow(resolver.ResolveNowOptions{}) + } + select { + case <-cc.ResolveNowCh: + t.Fatalf("got unexpected ResolveNow() call") + case <-time.After(time.Millisecond * 100): + } + + // Send another update to set IgnoreReresolutionRequests to false. + if err := pb.UpdateClientConnState(balancer.ClientConnState{ + ResolverState: resolver.State{ + Addresses: []resolver.Address{ + hierarchy.Set(resolver.Address{Addr: testBackendAddrStrs[0]}, []string{"child-0"}), + }, + }, + BalancerConfig: &LBConfig{ + Children: map[string]*Child{ + "child-0": { + Config: &internalserviceconfig.BalancerConfig{Name: resolveNowBalancerName}, + IgnoreReresolutionRequests: false, + }, + }, + Priorities: []string{"child-0"}, + }, + }); err != nil { + t.Fatalf("failed to update ClientConn state: %v", err) + } + + // Call ResolveNow() on the CC, it should be forwarded. + balancerCC.ResolveNow(resolver.ResolveNowOptions{}) + select { + case <-cc.ResolveNowCh: + case <-time.After(time.Second): + t.Fatalf("timeout waiting for ResolveNow()") + } + +} + +// When the child policy's configured to ignore reresolution requests, the +// ResolveNow() calls from this child should be all ignored, from the other +// children are forwarded. +func (s) TestPriority_IgnoreReresolutionRequestTwoChildren(t *testing.T) { + cc := testutils.NewTestClientConn(t) + bb := balancer.Get(Name) + pb := bb.Build(cc, balancer.BuildOptions{}) + defer pb.Close() + + // One children, with priorities [0, 1], each with one backend. + // Reresolution is ignored for p0. + if err := pb.UpdateClientConnState(balancer.ClientConnState{ + ResolverState: resolver.State{ + Addresses: []resolver.Address{ + hierarchy.Set(resolver.Address{Addr: testBackendAddrStrs[0]}, []string{"child-0"}), + hierarchy.Set(resolver.Address{Addr: testBackendAddrStrs[1]}, []string{"child-1"}), + }, + }, + BalancerConfig: &LBConfig{ + Children: map[string]*Child{ + "child-0": { + Config: &internalserviceconfig.BalancerConfig{Name: resolveNowBalancerName}, + IgnoreReresolutionRequests: true, + }, + "child-1": { + Config: &internalserviceconfig.BalancerConfig{Name: resolveNowBalancerName}, + }, + }, + Priorities: []string{"child-0", "child-1"}, + }, + }); err != nil { + t.Fatalf("failed to update ClientConn state: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + // This is the balancer.ClientConn from p0. + balancerCCI0, err := resolveNowBalancerCCCh.Receive(ctx) + if err != nil { + t.Fatalf("timeout waiting for ClientConn from balancer builder 0") + } + balancerCC0 := balancerCCI0.(balancer.ClientConn) + + // Set p0 to transient failure, p1 will be started. + addrs0 := <-cc.NewSubConnAddrsCh + if got, want := addrs0[0].Addr, testBackendAddrStrs[0]; got != want { + t.Fatalf("sc is created with addr %v, want %v", got, want) + } + sc0 := <-cc.NewSubConnCh + pb.UpdateSubConnState(sc0, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure}) + + // This is the balancer.ClientConn from p1. + ctx1, cancel1 := context.WithTimeout(context.Background(), time.Second) + defer cancel1() + balancerCCI1, err := resolveNowBalancerCCCh.Receive(ctx1) + if err != nil { + t.Fatalf("timeout waiting for ClientConn from balancer builder 1") + } + balancerCC1 := balancerCCI1.(balancer.ClientConn) + + // Since IgnoreReresolutionRequests was set to true for p0, ResolveNow() + // from p0 should all be ignored. + for i := 0; i < 5; i++ { + balancerCC0.ResolveNow(resolver.ResolveNowOptions{}) + } + select { + case <-cc.ResolveNowCh: + t.Fatalf("got unexpected ResolveNow() call") + case <-time.After(time.Millisecond * 100): + } + + // But IgnoreReresolutionRequests was false for p1, ResolveNow() from p1 + // should be forwarded. + balancerCC1.ResolveNow(resolver.ResolveNowOptions{}) + select { + case <-cc.ResolveNowCh: + case <-time.After(time.Second): + t.Fatalf("timeout waiting for ResolveNow()") + } +} diff --git a/xds/internal/balancer/priority/config.go b/xds/internal/balancer/priority/config.go index 7704f21d13b..c9cb16e323f 100644 --- a/xds/internal/balancer/priority/config.go +++ b/xds/internal/balancer/priority/config.go @@ -28,7 +28,8 @@ import ( // Child is a child of priority balancer. type Child struct { - Config *internalserviceconfig.BalancerConfig `json:"config,omitempty"` + Config *internalserviceconfig.BalancerConfig `json:"config,omitempty"` + IgnoreReresolutionRequests bool } // LBConfig represents priority balancer's config. diff --git a/xds/internal/balancer/priority/config_test.go b/xds/internal/balancer/priority/config_test.go index f3a09fe3a32..42498bfa2b7 100644 --- a/xds/internal/balancer/priority/config_test.go +++ b/xds/internal/balancer/priority/config_test.go @@ -65,7 +65,7 @@ func TestParseConfig(t *testing.T) { js: `{ "priorities": ["child-1", "child-2", "child-3"], "children": { - "child-1": {"config": [{"round_robin":{}}]}, + "child-1": {"config": [{"round_robin":{}}], "ignoreReresolutionRequests": true}, "child-2": {"config": [{"round_robin":{}}]}, "child-3": {"config": [{"round_robin":{}}]} } @@ -74,17 +74,18 @@ func TestParseConfig(t *testing.T) { want: &LBConfig{ Children: map[string]*Child{ "child-1": { - &internalserviceconfig.BalancerConfig{ + Config: &internalserviceconfig.BalancerConfig{ Name: roundrobin.Name, }, + IgnoreReresolutionRequests: true, }, "child-2": { - &internalserviceconfig.BalancerConfig{ + Config: &internalserviceconfig.BalancerConfig{ Name: roundrobin.Name, }, }, "child-3": { - &internalserviceconfig.BalancerConfig{ + Config: &internalserviceconfig.BalancerConfig{ Name: roundrobin.Name, }, }, diff --git a/xds/internal/balancer/priority/ignore_resolve_now.go b/xds/internal/balancer/priority/ignore_resolve_now.go new file mode 100644 index 00000000000..9a9f4777269 --- /dev/null +++ b/xds/internal/balancer/priority/ignore_resolve_now.go @@ -0,0 +1,73 @@ +/* + * + * Copyright 2021 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package priority + +import ( + "sync/atomic" + + "google.golang.org/grpc/balancer" + "google.golang.org/grpc/resolver" +) + +type ignoreResolveNowBalancerBuilder struct { + balancer.Builder + ignoreResolveNow *uint32 +} + +// If `ignore` is true, all `ResolveNow()` from the balancer built from this +// builder will be ignored. +// +// `ignore` can be updated later by `updateIgnoreResolveNow`, and the update +// will be propagated to all the old and new balancers built with this. +func newIgnoreResolveNowBalancerBuilder(bb balancer.Builder, ignore bool) *ignoreResolveNowBalancerBuilder { + ret := &ignoreResolveNowBalancerBuilder{ + Builder: bb, + ignoreResolveNow: new(uint32), + } + ret.updateIgnoreResolveNow(ignore) + return ret +} + +func (irnbb *ignoreResolveNowBalancerBuilder) updateIgnoreResolveNow(b bool) { + if b { + atomic.StoreUint32(irnbb.ignoreResolveNow, 1) + return + } + atomic.StoreUint32(irnbb.ignoreResolveNow, 0) + +} + +func (irnbb *ignoreResolveNowBalancerBuilder) Build(cc balancer.ClientConn, opts balancer.BuildOptions) balancer.Balancer { + return irnbb.Builder.Build(&ignoreResolveNowClientConn{ + ClientConn: cc, + ignoreResolveNow: irnbb.ignoreResolveNow, + }, opts) +} + +type ignoreResolveNowClientConn struct { + balancer.ClientConn + ignoreResolveNow *uint32 +} + +func (i ignoreResolveNowClientConn) ResolveNow(o resolver.ResolveNowOptions) { + if atomic.LoadUint32(i.ignoreResolveNow) != 0 { + return + } + i.ClientConn.ResolveNow(o) +} diff --git a/xds/internal/balancer/priority/ignore_resolve_now_test.go b/xds/internal/balancer/priority/ignore_resolve_now_test.go new file mode 100644 index 00000000000..452753de6c7 --- /dev/null +++ b/xds/internal/balancer/priority/ignore_resolve_now_test.go @@ -0,0 +1,106 @@ +// +build go1.12 + +/* + * + * Copyright 2021 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package priority + +import ( + "context" + "testing" + "time" + + "google.golang.org/grpc/balancer" + "google.golang.org/grpc/balancer/roundrobin" + grpctestutils "google.golang.org/grpc/internal/testutils" + "google.golang.org/grpc/resolver" + "google.golang.org/grpc/xds/internal/testutils" +) + +const resolveNowBalancerName = "test-resolve-now-balancer" + +var resolveNowBalancerCCCh = grpctestutils.NewChannel() + +type resolveNowBalancerBuilder struct { + balancer.Builder +} + +func (r *resolveNowBalancerBuilder) Build(cc balancer.ClientConn, opts balancer.BuildOptions) balancer.Balancer { + resolveNowBalancerCCCh.Send(cc) + return r.Builder.Build(cc, opts) +} + +func (r *resolveNowBalancerBuilder) Name() string { + return resolveNowBalancerName +} + +func init() { + balancer.Register(&resolveNowBalancerBuilder{ + Builder: balancer.Get(roundrobin.Name), + }) +} + +func (s) TestIgnoreResolveNowBalancerBuilder(t *testing.T) { + resolveNowBB := balancer.Get(resolveNowBalancerName) + // Create a build wrapper, but will not ignore ResolveNow(). + ignoreResolveNowBB := newIgnoreResolveNowBalancerBuilder(resolveNowBB, false) + + cc := testutils.NewTestClientConn(t) + tb := ignoreResolveNowBB.Build(cc, balancer.BuildOptions{}) + defer tb.Close() + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + // This is the balancer.ClientConn that the inner resolverNowBalancer is + // built with. + balancerCCI, err := resolveNowBalancerCCCh.Receive(ctx) + if err != nil { + t.Fatalf("timeout waiting for ClientConn from balancer builder") + } + balancerCC := balancerCCI.(balancer.ClientConn) + + // Call ResolveNow() on the CC, it should be forwarded. + balancerCC.ResolveNow(resolver.ResolveNowOptions{}) + select { + case <-cc.ResolveNowCh: + case <-time.After(time.Second): + t.Fatalf("timeout waiting for ResolveNow()") + } + + // Update ignoreResolveNow to true, call ResolveNow() on the CC, they should + // all be ignored. + ignoreResolveNowBB.updateIgnoreResolveNow(true) + for i := 0; i < 5; i++ { + balancerCC.ResolveNow(resolver.ResolveNowOptions{}) + } + select { + case <-cc.ResolveNowCh: + t.Fatalf("got unexpected ResolveNow() call") + case <-time.After(time.Millisecond * 100): + } + + // Update ignoreResolveNow to false, new ResolveNow() calls should be + // forwarded. + ignoreResolveNowBB.updateIgnoreResolveNow(false) + balancerCC.ResolveNow(resolver.ResolveNowOptions{}) + select { + case <-cc.ResolveNowCh: + case <-time.After(time.Second): + t.Fatalf("timeout waiting for ResolveNow()") + } +}