Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 44 additions & 3 deletions xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import com.google.common.base.MoreObjects;
import com.google.common.collect.Sets;
import com.google.common.primitives.UnsignedInteger;
import io.grpc.Attributes;
import io.grpc.ConnectivityState;
import io.grpc.ConnectivityStateInfo;
Expand Down Expand Up @@ -84,11 +85,10 @@ final class RingHashLoadBalancer extends LoadBalancer {
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses);
List<EquivalentAddressGroup> addrList = resolvedAddresses.getAddresses();
if (addrList.isEmpty()) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription("Ring hash lb error: EDS "
+ "resolution was successful, but returned server addresses are empty."));
if (!validateAddrList(addrList)) {
return false;
}

Map<EquivalentAddressGroup, EquivalentAddressGroup> latestAddrs = stripAttrs(addrList);
Set<EquivalentAddressGroup> removedAddrs =
Sets.newHashSet(Sets.difference(subchannels.keySet(), latestAddrs.keySet()));
Expand Down Expand Up @@ -166,6 +166,47 @@ public void onSubchannelState(ConnectivityStateInfo newState) {
return true;
}

private boolean validateAddrList(List<EquivalentAddressGroup> addrList) {
if (addrList.isEmpty()) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription("Ring hash lb error: EDS "
+ "resolution was successful, but returned server addresses are empty."));
return false;
}

long totalWeight = 0;
for (EquivalentAddressGroup eag : addrList) {
Long weight = eag.getAttributes().get(InternalXdsAttributes.ATTR_SERVER_WEIGHT);

if (weight == null) {
weight = 1L;
}

if (weight < 0) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
String.format("Ring hash lb error: EDS resolution was successful, but returned a "
+ "negative weight for %s.", stripAttrs(eag))));
return false;
}
if (weight > UnsignedInteger.MAX_VALUE.longValue()) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
String.format("Ring hash lb error: EDS resolution was successful, but returned a weight"
+ " too large to fit in an unsigned int for %s.", stripAttrs(eag))));
return false;
}
totalWeight += weight;
}

if (totalWeight > UnsignedInteger.MAX_VALUE.longValue()) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
String.format(
"Ring hash lb error: EDS resolution was successful, but returned a sum of weights too"
+ " large to fit in an unsigned int (%d).", totalWeight)));
return false;
}

return true;
}

private static List<RingEntry> buildRing(
Map<EquivalentAddressGroup, Long> serverWeights, long totalWeight, double scale) {
List<RingEntry> ring = new ArrayList<>();
Expand Down
41 changes: 41 additions & 0 deletions xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import static org.mockito.Mockito.when;

import com.google.common.collect.Iterables;
import com.google.common.primitives.UnsignedInteger;
import io.grpc.Attributes;
import io.grpc.CallOptions;
import io.grpc.ConnectivityStateInfo;
Expand Down Expand Up @@ -961,6 +962,46 @@ public void stickyTransientFailure() {
.requestConnection();
}

@Test
public void largeWeights() {
RingHashConfig config = new RingHashConfig(10000, 100000); // large ring
List<EquivalentAddressGroup> servers =
createWeightedServerAddrs(Integer.MAX_VALUE, 10, 100); // MAX:10:100
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isTrue();

// Try value between max signed and max unsigned int
servers = createWeightedServerAddrs(Integer.MAX_VALUE + 100L, 100); // (MAX+100):100
addressesAccepted = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isTrue();

// Try a negative value
servers = createWeightedServerAddrs(10, -20, 100); // 10:-20:100
addressesAccepted = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isFalse();

// Try an individual value larger than max unsigned int
long maxUnsigned = UnsignedInteger.MAX_VALUE.longValue();
servers = createWeightedServerAddrs(maxUnsigned + 10, 10, 100); // uMAX+10:10:100
addressesAccepted = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isFalse();

// Try a sum of values larger than max unsigned int
servers = createWeightedServerAddrs(Integer.MAX_VALUE, Integer.MAX_VALUE, 100); // MAX:MAX:100
addressesAccepted = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isFalse();
}

@Test
public void hostSelectionProportionalToWeights() {
RingHashConfig config = new RingHashConfig(10000, 100000); // large ring
Expand Down