From cac75d4fb44049fd88e99fac7a4ca07491863575 Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Fri, 5 May 2023 13:59:12 -0700 Subject: [PATCH] review comments --- balancer/weightedroundrobin/balancer.go | 4 ++-- balancer/weightedroundrobin/balancer_test.go | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/balancer/weightedroundrobin/balancer.go b/balancer/weightedroundrobin/balancer.go index 5e9594e1fdb..35a5bd9f1d3 100644 --- a/balancer/weightedroundrobin/balancer.go +++ b/balancer/weightedroundrobin/balancer.go @@ -120,7 +120,7 @@ func (b *wrrBalancer) UpdateClientConnState(ccs balancer.ClientConnState) error b.resolverErr = nil cfg, ok := ccs.BalancerConfig.(*lbConfig) if !ok { - return fmt.Errorf("wrr: received nil or illegal BalancerConfig: %v", ccs.BalancerConfig) + return fmt.Errorf("wrr: received nil or illegal BalancerConfig (type %T): %v", ccs.BalancerConfig, ccs.BalancerConfig) } b.cfg = cfg @@ -206,7 +206,7 @@ func (b *wrrBalancer) ResolverError(err error) { func (b *wrrBalancer) UpdateSubConnState(sc balancer.SubConn, state balancer.SubConnState) { wsc := b.scMap[sc] if wsc == nil { - b.logger.Errorf("UpdateSubConnStateChange called with an unknown SubConn: %p, %v", sc, state) + b.logger.Errorf("UpdateSubConnState called with an unknown SubConn: %p, %v", sc, state) return } if b.logger.V(2) { diff --git a/balancer/weightedroundrobin/balancer_test.go b/balancer/weightedroundrobin/balancer_test.go index f4edb32f671..9d9e4283176 100644 --- a/balancer/weightedroundrobin/balancer_test.go +++ b/balancer/weightedroundrobin/balancer_test.go @@ -388,8 +388,10 @@ func (s) TestBalancer_TwoAddresses_ErrorPenalty(t *testing.T) { srv2 := startServer(t, reportOOB) // srv1 starts loaded and srv2 starts without load; ensure RPCs are routed - // disproportionately to srv2 (10:1). Errors are set (but ignored - // initially) such that RPCs will be routed 50/50. + // disproportionately to srv2 (10:1). EPS values are set (but ignored + // initially due to ErrorUtilizationPenalty=0). Later EUP will be updated + // to 0.9 which will cause the weights to be equal and RPCs to be routed + // 50/50. srv1.oobMetrics.SetQPS(10.0) srv1.oobMetrics.SetCPUUtilization(1.0) srv1.oobMetrics.SetEPS(0) @@ -432,7 +434,7 @@ func (s) TestBalancer_TwoAddresses_ErrorPenalty(t *testing.T) { } // Tests that the blackout period causes backends to use 0 as their weight -// until the backout period elapses. +// (meaning to use the average weight) until the blackout period elapses. func (s) TestBalancer_TwoAddresses_BlackoutPeriod(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() @@ -509,7 +511,8 @@ func (s) TestBalancer_TwoAddresses_BlackoutPeriod(t *testing.T) { } // Tests that the weight expiration period causes backends to use 0 as their -// weight once the expiration period elapses. +// weight (meaning to use the average weight) once the expiration period +// elapses. func (s) TestBalancer_TwoAddresses_WeightExpiration(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel()