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());