From 19490352e8c1c7de044844858d4e04b0362c3c2f Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Tue, 6 Dec 2022 11:59:35 -0800 Subject: [PATCH] ringhash: add logs to surface information about ring creation (#5832) Fixes https://github.com/grpc/grpc-go/issues/5781 --- xds/internal/balancer/ringhash/ring.go | 7 ++++++- xds/internal/balancer/ringhash/ring_test.go | 6 +++--- xds/internal/balancer/ringhash/ringhash.go | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/xds/internal/balancer/ringhash/ring.go b/xds/internal/balancer/ringhash/ring.go index 71d31eaeb8b0..3e35556d8a73 100644 --- a/xds/internal/balancer/ringhash/ring.go +++ b/xds/internal/balancer/ringhash/ring.go @@ -24,6 +24,7 @@ import ( "strconv" xxhash "github.com/cespare/xxhash/v2" + "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/resolver" ) @@ -65,9 +66,12 @@ type ringEntry struct { // and first item with hash >= given hash will be returned. // // Must be called with a non-empty subConns map. -func newRing(subConns *resolver.AddressMap, minRingSize, maxRingSize uint64) *ring { +func newRing(subConns *resolver.AddressMap, minRingSize, maxRingSize uint64, logger *grpclog.PrefixLogger) *ring { + logger.Debugf("newRing: number of subConns is %d, minRingSize is %d, maxRingSize is %d", subConns.Len(), minRingSize, maxRingSize) + // https://github.com/envoyproxy/envoy/blob/765c970f06a4c962961a0e03a467e165b276d50f/source/common/upstream/ring_hash_lb.cc#L114 normalizedWeights, minWeight := normalizeWeights(subConns) + logger.Debugf("newRing: normalized subConn weights is %v", normalizedWeights) // Normalized weights for {3,3,4} is {0.3,0.3,0.4}. @@ -78,6 +82,7 @@ func newRing(subConns *resolver.AddressMap, minRingSize, maxRingSize uint64) *ri scale := math.Min(math.Ceil(minWeight*float64(minRingSize))/minWeight, float64(maxRingSize)) ringSize := math.Ceil(scale) items := make([]*ringEntry, 0, int(ringSize)) + logger.Debugf("newRing: creating new ring of size %v", ringSize) // For each entry, scale*weight nodes are generated in the ring. // diff --git a/xds/internal/balancer/ringhash/ring_test.go b/xds/internal/balancer/ringhash/ring_test.go index b1d987609903..9c6eb0c242ff 100644 --- a/xds/internal/balancer/ringhash/ring_test.go +++ b/xds/internal/balancer/ringhash/ring_test.go @@ -52,7 +52,7 @@ func (s) TestRingNew(t *testing.T) { for _, min := range []uint64{3, 4, 6, 8} { for _, max := range []uint64{20, 8} { t.Run(fmt.Sprintf("size-min-%v-max-%v", min, max), func(t *testing.T) { - r := newRing(testSubConnMap, min, max) + r := newRing(testSubConnMap, min, max, nil) totalCount := len(r.items) if totalCount < int(min) || totalCount > int(max) { t.Fatalf("unexpect size %v, want min %v, max %v", totalCount, min, max) @@ -82,7 +82,7 @@ func equalApproximately(x, y float64) bool { } func (s) TestRingPick(t *testing.T) { - r := newRing(testSubConnMap, 10, 20) + r := newRing(testSubConnMap, 10, 20, nil) for _, h := range []uint64{xxhash.Sum64String("1"), xxhash.Sum64String("2"), xxhash.Sum64String("3"), xxhash.Sum64String("4")} { t.Run(fmt.Sprintf("picking-hash-%v", h), func(t *testing.T) { e := r.pick(h) @@ -100,7 +100,7 @@ func (s) TestRingPick(t *testing.T) { } func (s) TestRingNext(t *testing.T) { - r := newRing(testSubConnMap, 10, 20) + r := newRing(testSubConnMap, 10, 20, nil) for _, e := range r.items { ne := r.next(e) diff --git a/xds/internal/balancer/ringhash/ringhash.go b/xds/internal/balancer/ringhash/ringhash.go index 6f91ff303317..b9caefa63a2d 100644 --- a/xds/internal/balancer/ringhash/ringhash.go +++ b/xds/internal/balancer/ringhash/ringhash.go @@ -293,7 +293,7 @@ func (b *ringhashBalancer) UpdateClientConnState(s balancer.ClientConnState) err if regenerateRing { // Ring creation is guaranteed to not fail because we call newRing() // with a non-empty subConns map. - b.ring = newRing(b.subConns, b.config.MinRingSize, b.config.MaxRingSize) + b.ring = newRing(b.subConns, b.config.MinRingSize, b.config.MaxRingSize, b.logger) b.regeneratePicker() b.cc.UpdateState(balancer.State{ConnectivityState: b.state, Picker: b.picker}) }