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.
This commit is contained in:
Eric Anderson 2025-06-17 14:14:32 +00:00 committed by GitHub
parent 2604ce8a55
commit f07eb47cac
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 61 additions and 2 deletions

View File

@ -24,7 +24,9 @@ import static io.grpc.ConnectivityState.SHUTDOWN;
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.primitives.UnsignedInts;
import io.grpc.Attributes;
import io.grpc.ConnectivityState;
import io.grpc.EquivalentAddressGroup;
@ -40,6 +42,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
@ -52,6 +55,7 @@ import javax.annotation.Nullable;
public abstract class MultiChildLoadBalancer extends LoadBalancer {
private static final Logger logger = Logger.getLogger(MultiChildLoadBalancer.class.getName());
private static final int OFFSET_SEED = new Random().nextInt();
// Modify by replacing the list to release memory when no longer used.
private List<ChildLbState> childLbStates = new ArrayList<>(0);
private final Helper helper;
@ -168,9 +172,15 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
childLbState = createChildLbState(entry.getKey());
}
newChildLbStates.add(childLbState);
if (entry.getValue() != null) {
}
// Use a random start position for child updates to weakly "shuffle" connection creation order.
// The network will often add noise to the creation order, but this avoids giving earlier
// children a consistent head start.
for (ChildLbState childLbState : offsetIterable(newChildLbStates, OFFSET_SEED)) {
ResolvedAddresses addresses = newChildAddresses.get(childLbState.getKey());
if (addresses != null) {
// update child LB
Status newStatus = childLbState.lb.acceptResolvedAddresses(entry.getValue());
Status newStatus = childLbState.lb.acceptResolvedAddresses(addresses);
if (!newStatus.isOk()) {
status = newStatus;
}
@ -188,6 +198,19 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
return status;
}
@VisibleForTesting
static <T> Iterable<T> offsetIterable(Collection<T> c, int seed) {
int pos;
if (c.isEmpty()) {
pos = 0;
} else {
pos = UnsignedInts.remainder(seed, c.size());
}
return Iterables.concat(
Iterables.skip(c, pos),
Iterables.limit(c, pos));
}
@Nullable
protected static ConnectivityState aggregateState(
@Nullable ConnectivityState overallState, ConnectivityState childState) {

View File

@ -264,6 +264,42 @@ public class MultiChildLoadBalancerTest {
.testEquals();
}
@Test
public void offsetIterable_positive() {
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3, 4), 9))
.containsExactly(2, 3, 4, 1)
.inOrder();
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3, 4, 5), 9))
.containsExactly(5, 1, 2, 3, 4)
.inOrder();
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3), 3))
.containsExactly(1, 2, 3)
.inOrder();
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3), 0))
.containsExactly(1, 2, 3)
.inOrder();
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1), 123))
.containsExactly(1)
.inOrder();
}
@Test
public void offsetIterable_negative() {
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(1, 2, 3, 4), -1))
.containsExactly(4, 1, 2, 3)
.inOrder();
}
@Test
public void offsetIterable_empty() {
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(), 1))
.isEmpty();
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(), 0))
.isEmpty();
assertThat(MultiChildLoadBalancer.offsetIterable(Arrays.asList(), -1))
.isEmpty();
}
private String addressesOnlyString(EquivalentAddressGroup eag) {
if (eag == null) {
return null;