From edf301dadcfc75436d7bd26e98f2f81eba12048d Mon Sep 17 00:00:00 2001 From: Timur Rakhmatullin <174210871+TimurRakhmatullin86@users.noreply.github.com> Date: Fri, 11 Sep 2026 20:22:38 -0700 Subject: [PATCH] xds: Wrap around the ring in RingHashLoadBalancer.getTargetIndex RingHashPicker.getTargetIndex binary-searches the Ketama ring for the smallest ring entry whose hash is >= the request hash. When the request hash is greater than every entry on the ring, the search stops at the last entry and returns it instead of wrapping clockwise back to the first entry as a ring requires. As a result, requests whose hash falls in the gap above the largest ring point are routed to the last ring entry's host as the primary pick (and demote the first entry's host), diverging from the affinity Envoy and gRPC C-core produce for the same hash and skewing load onto the last host. That window is a deterministic slice of the 64-bit hash space and is always reachable, since the request hash is an independent 64-bit value. Return index 0 when the request hash exceeds the largest ring entry so the ring wraps around; a hash equal to the largest entry still resolves to that entry (strict comparison). Add a test asserting that a hash above the maximum and a hash below the minimum select the same host. --- .../io/grpc/xds/RingHashLoadBalancer.java | 6 +++++ .../io/grpc/xds/RingHashLoadBalancerTest.java | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java index 322723c5ba1..fab1e5786bb 100644 --- a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java @@ -396,6 +396,12 @@ private int getTargetIndex(long requestHash) { return 0; } + // A hash greater than the largest ring entry wraps clockwise back to the first entry; + // otherwise the binary search below stops at the last entry instead of wrapping around. + if (requestHash > ring.get(ring.size() - 1).hash) { + return 0; + } + int low = 0; int high = ring.size() - 1; int mid = (low + high) / 2; diff --git a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java index da11df24af4..37e8fbc5bc2 100644 --- a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java @@ -478,6 +478,32 @@ public void deterministicPickWithRequestHashHeader_multipleHeaderValues() { assertThat(result.getSubchannel().getAddresses()).isEqualTo(servers.get(1)); } + @Test + public void pickWithHashAboveMaxRingEntry_wrapsAroundToFirstEntry() { + // Map each server address to exactly one ring entry, so the first and last ring entries + // belong to different hosts. + RingHashConfig config = new RingHashConfig(3, 3, ""); + List servers = createWeightedServerAddrs(1, 1, 1); + initializeLbSubchannels(config, servers); + InOrder inOrder = Mockito.inOrder(helper); + + // Bring all subchannels to READY so that any pick resolves to its ring entry's host. + for (Subchannel subchannel : subchannels.values()) { + deliverSubchannelState(subchannel, CSI_READY); + inOrder.verify(helper).updateBalancingState(eq(READY), pickerCaptor.capture()); + } + SubchannelPicker picker = pickerCaptor.getValue(); + + // Long.MIN_VALUE sorts below every ring entry, so it hits the first entry on the ring. + Subchannel firstEntryHost = + picker.pickSubchannel(getDefaultPickSubchannelArgs(Long.MIN_VALUE)).getSubchannel(); + // Long.MAX_VALUE sorts above every ring entry; on a ring it must wrap clockwise back to the + // first entry rather than sticking to the last one. + Subchannel wrappedHost = + picker.pickSubchannel(getDefaultPickSubchannelArgs(Long.MAX_VALUE)).getSubchannel(); + assertThat(wrappedHost).isSameInstanceAs(firstEntryHost); + } + @Test public void pickWithRandomHash_allSubchannelsReady() { loadBalancer = new RingHashLoadBalancer(helper, new FakeRandom());