Skip to content

Commit f07eb47

Browse files
authored
util: Deliver addresses in a random order in MultiChildLb
This should often not matter much, but in b/412468630 it was cleary visible that child creation order can skew load for the first batch of RPCs. This doesn't solve all the cases, as further-away backends will still be less likely chosen initially and it is ignorant of the LB policy. But this doesn't impact correctness, is easy, and is one fewer cases to worry about.
1 parent 2604ce8 commit f07eb47

File tree

2 files changed

+61
-2
lines changed

2 files changed

+61
-2
lines changed

util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
2525

2626
import com.google.common.annotations.VisibleForTesting;
27+
import com.google.common.collect.Iterables;
2728
import com.google.common.collect.Maps;
29+
import com.google.common.primitives.UnsignedInts;
2830
import io.grpc.Attributes;
2931
import io.grpc.ConnectivityState;
3032
import io.grpc.EquivalentAddressGroup;
@@ -40,6 +42,7 @@
4042
import java.util.HashSet;
4143
import java.util.List;
4244
import java.util.Map;
45+
import java.util.Random;
4346
import java.util.logging.Level;
4447
import java.util.logging.Logger;
4548
import javax.annotation.Nullable;
@@ -52,6 +55,7 @@
5255
public abstract class MultiChildLoadBalancer extends LoadBalancer {
5356

5457
private static final Logger logger = Logger.getLogger(MultiChildLoadBalancer.class.getName());
58+
private static final int OFFSET_SEED = new Random().nextInt();
5559
// Modify by replacing the list to release memory when no longer used.
5660
private List<ChildLbState> childLbStates = new ArrayList<>(0);
5761
private final Helper helper;
@@ -168,9 +172,15 @@ private Status updateChildrenWithResolvedAddresses(
168172
childLbState = createChildLbState(entry.getKey());
169173
}
170174
newChildLbStates.add(childLbState);
171-
if (entry.getValue() != null) {
175+
}
176+
// Use a random start position for child updates to weakly "shuffle" connection creation order.
177+
// The network will often add noise to the creation order, but this avoids giving earlier
178+
// children a consistent head start.
179+
for (ChildLbState childLbState : offsetIterable(newChildLbStates, OFFSET_SEED)) {
180+
ResolvedAddresses addresses = newChildAddresses.get(childLbState.getKey());
181+
if (addresses != null) {
172182
// update child LB
173-
Status newStatus = childLbState.lb.acceptResolvedAddresses(entry.getValue());
183+
Status newStatus = childLbState.lb.acceptResolvedAddresses(addresses);
174184
if (!newStatus.isOk()) {
175185
status = newStatus;
176186
}
@@ -188,6 +198,19 @@ private Status updateChildrenWithResolvedAddresses(
188198
return status;
189199
}
190200

201+
@VisibleForTesting
202+
static <T> Iterable<T> offsetIterable(Collection<T> c, int seed) {
203+
int pos;
204+
if (c.isEmpty()) {
205+
pos = 0;
206+
} else {
207+
pos = UnsignedInts.remainder(seed, c.size());
208+
}
209+
return Iterables.concat(
210+
Iterables.skip(c, pos),
211+
Iterables.limit(c, pos));
212+
}
213+
191214
@Nullable
192215
protected static ConnectivityState aggregateState(
193216
@Nullable ConnectivityState overallState, ConnectivityState childState) {

util/src/test/java/io/grpc/util/MultiChildLoadBalancerTest.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,42 @@ public void testEndpoint_equals() {
264264
.testEquals();
265265
}
266266

267+
@Test
268+
public void offsetIterable_positive() {
269+
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3, 4), 9))
270+
.containsExactly(2, 3, 4, 1)
271+
.inOrder();
272+
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3, 4, 5), 9))
273+
.containsExactly(5, 1, 2, 3, 4)
274+
.inOrder();
275+
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3), 3))
276+
.containsExactly(1, 2, 3)
277+
.inOrder();
278+
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3), 0))
279+
.containsExactly(1, 2, 3)
280+
.inOrder();
281+
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1), 123))
282+
.containsExactly(1)
283+
.inOrder();
284+
}
285+
286+
@Test
287+
public void offsetIterable_negative() {
288+
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3, 4), -1))
289+
.containsExactly(4, 1, 2, 3)
290+
.inOrder();
291+
}
292+
293+
@Test
294+
public void offsetIterable_empty() {
295+
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(), 1))
296+
.isEmpty();
297+
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(), 0))
298+
.isEmpty();
299+
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(), -1))
300+
.isEmpty();
301+
}
302+
267303
private String addressesOnlyString(EquivalentAddressGroup eag) {
268304
if (eag == null) {
269305
return null;

0 commit comments

Comments
 (0)