diff --git a/client/src/main/java/org/asynchttpclient/netty/channel/RoundRobinAddressSelector.java b/client/src/main/java/org/asynchttpclient/netty/channel/RoundRobinAddressSelector.java index 181294c21..1684c40ee 100644 --- a/client/src/main/java/org/asynchttpclient/netty/channel/RoundRobinAddressSelector.java +++ b/client/src/main/java/org/asynchttpclient/netty/channel/RoundRobinAddressSelector.java @@ -16,7 +16,7 @@ package org.asynchttpclient.netty.channel; import java.net.InetSocketAddress; -import java.util.ArrayList; +import java.util.AbstractList; import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; @@ -67,7 +67,7 @@ public List rotate(String host, List resol AtomicInteger counter = counterFor(host); int index = (counter.getAndIncrement() & Integer.MAX_VALUE) % n; - return index == 0 ? resolved : rotateBy(resolved, index, n); + return index == 0 ? resolved : rotateBy(resolved, index); } // Visible for testing: the number of hosts currently tracked (bounded by MAX_TRACKED_HOSTS). @@ -80,11 +80,41 @@ private AtomicInteger counterFor(String host) { return counters.computeIfAbsent(host, h -> new AtomicInteger()); } - private static List rotateBy(List resolved, int index, int n) { - List rotated = new ArrayList<>(n); - rotated.addAll(resolved.subList(index, n)); - rotated.addAll(resolved.subList(0, index)); - return rotated; + // Returns a read-only view of {@code resolved} rotated left by {@code index} — element i is + // resolved.get((index + i) mod size) — instead of copying the rotation into a fresh ArrayList (plus two + // subList views), which the round-robin fast path did on ~(n-1)/n of multi-IP requests. All consumers + // only read the result (get/size/iteration — NettyChannelConnector and FailedIpCooldownHolder#reorder), + // and the resolved list is not mutated after being wrapped, so the lightweight view is safe. + private static List rotateBy(List resolved, int index) { + return new RotatedView(resolved, index); + } + + private static final class RotatedView extends AbstractList { + + private final List resolved; + private final int index; + private final int size; + + RotatedView(List resolved, int index) { + this.resolved = resolved; + this.index = index; + this.size = resolved.size(); + } + + @Override + public InetSocketAddress get(int i) { + if (i < 0 || i >= size) { + throw new IndexOutOfBoundsException("Index: " + i + ", Size: " + size); + } + // index and i are both in [0, size), so index + i < 2*size — one wrap suffices (no modulo). + int j = index + i; + return resolved.get(j < size ? j : j - size); + } + + @Override + public int size() { + return size; + } } // Keep the map bounded: when it is full, drop one arbitrary entry before a new host is added. diff --git a/client/src/test/java/org/asynchttpclient/netty/channel/RoundRobinAddressSelectorTest.java b/client/src/test/java/org/asynchttpclient/netty/channel/RoundRobinAddressSelectorTest.java index 8bb09033f..44b04770d 100644 --- a/client/src/test/java/org/asynchttpclient/netty/channel/RoundRobinAddressSelectorTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/channel/RoundRobinAddressSelectorTest.java @@ -26,7 +26,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class RoundRobinAddressSelectorTest { @@ -73,6 +75,35 @@ void rotationFollowsResolverOrder() { assertEquals("127.0.0.3", firstIp(selector.rotate("h", input))); } + @Test + void rotatedViewMatchesFullLeftRotation() { + RoundRobinAddressSelector selector = new RoundRobinAddressSelector(); + List input = Arrays.asList(addr("127.0.0.1"), addr("127.0.0.2"), + addr("127.0.0.3"), addr("127.0.0.4")); + int n = input.size(); + // Each successive rotation is the resolver order rotated left by one more; the returned view must + // reproduce the whole order element-by-element (not just the first element). + for (int start = 0; start < n; start++) { + List rotated = selector.rotate("h", input); + assertEquals(n, rotated.size()); + for (int i = 0; i < n; i++) { + assertEquals(input.get((start + i) % n), rotated.get(i), + "element " + i + " of the rotation starting at index " + start); + } + } + } + + @Test + void rotatedResultIsAReadOnlyView() { + RoundRobinAddressSelector selector = new RoundRobinAddressSelector(); + List input = Arrays.asList(addr("127.0.0.1"), addr("127.0.0.2"), addr("127.0.0.3")); + selector.rotate("h", input); // index 0 -> returns input as-is + List rotated = selector.rotate("h", input); // index 1 -> a real rotation + assertNotSame(input, rotated, "a non-zero rotation returns a distinct view"); + assertThrows(UnsupportedOperationException.class, () -> rotated.set(0, addr("127.0.0.9")), + "the rotated view must be read-only (all consumers only read it)"); + } + @Test void singleAddressReturnedUnchanged() { RoundRobinAddressSelector selector = new RoundRobinAddressSelector();