Fix rebase problems

This commit is contained in:
larry-safran 2023-11-09 11:26:44 -08:00
parent 1dbf0363e0
commit 3ee56ea1a0
6 changed files with 91 additions and 77 deletions

View File

@ -146,15 +146,15 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
* Override to completely replace the default logic or to do additional activities.
*/
@Override
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
try {
resolvingAddresses = true;
// process resolvedAddresses to update children
AcceptResolvedAddressRetVal acceptRetVal =
acceptResolvedAddressesInternal(resolvedAddresses);
if (!acceptRetVal.valid) {
return false;
if (!acceptRetVal.status.isOk()) {
return acceptRetVal.status;
}
// Update the picker and our connectivity state
@ -162,7 +162,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
// shutdown removed children
shutdownRemoved(acceptRetVal.removedChildren);
return true;
return acceptRetVal.status;
} finally {
resolvingAddresses = false;
}
@ -213,9 +213,10 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
Map<Object, ChildLbState> newChildren = createChildLbMap(resolvedAddresses);
if (newChildren.isEmpty()) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
"NameResolver returned no usable address. " + resolvedAddresses));
return new AcceptResolvedAddressRetVal(false, null);
Status unavailableStatus = Status.UNAVAILABLE.withDescription(
"NameResolver returned no usable address. " + resolvedAddresses);
handleNameResolutionError(unavailableStatus);
return new AcceptResolvedAddressRetVal(unavailableStatus, null);
}
// Do adds and updates
@ -251,7 +252,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
}
}
return new AcceptResolvedAddressRetVal(true, removedChildren);
return new AcceptResolvedAddressRetVal(Status.OK, removedChildren);
}
protected void shutdownRemoved(List<ChildLbState> removedChildren) {
@ -594,11 +595,11 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
}
protected static class AcceptResolvedAddressRetVal {
public final boolean valid;
public final Status status;
public final List<ChildLbState> removedChildren;
public AcceptResolvedAddressRetVal(boolean valid, List<ChildLbState> removedChildren) {
this.valid = valid;
public AcceptResolvedAddressRetVal(Status status, List<ChildLbState> removedChildren) {
this.status = status;
this.removedChildren = removedChildren;
}
}

View File

@ -98,21 +98,21 @@ class ClusterManagerLoadBalancer extends MultiChildLoadBalancer {
* to be done by the timer.
*/
@Override
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
try {
resolvingAddresses = true;
// process resolvedAddresses to update children
AcceptResolvedAddressRetVal acceptRetVal =
acceptResolvedAddressesInternal(resolvedAddresses);
if (!acceptRetVal.valid) {
return false;
if (!acceptRetVal.status.isOk()) {
return acceptRetVal.status;
}
// Update the picker
updateOverallBalancingState();
return true;
return acceptRetVal.status;
} finally {
resolvingAddresses = false;
}

View File

@ -145,7 +145,7 @@ final class LeastRequestLoadBalancer extends MultiChildLoadBalancer {
@Override
protected ChildLbState createChildLbState(Object key, Object policyConfig,
SubchannelPicker initialPicker) {
SubchannelPicker initialPicker, ResolvedAddresses unused) {
return new LeastRequestLbState(key, pickFirstLbProvider, policyConfig, initialPicker);
}

View File

@ -79,11 +79,12 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer {
}
@Override
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses);
List<EquivalentAddressGroup> addrList = resolvedAddresses.getAddresses();
if (!validateAddrList(addrList)) {
return false;
Status addressValidityStatus = validateAddrList(addrList);
if (!addressValidityStatus.isOk()) {
return addressValidityStatus;
}
AcceptResolvedAddressRetVal acceptRetVal;
@ -91,11 +92,12 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer {
resolvingAddresses = true;
// Update the child list by creating-adding, updating addresses, and removing
acceptRetVal = super.acceptResolvedAddressesInternal(resolvedAddresses);
if (!acceptRetVal.valid) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
if (!acceptRetVal.status.isOk()) {
addressValidityStatus = Status.UNAVAILABLE.withDescription(
"Ring hash lb error: EDS resolution was successful, but was not accepted by base class"
));
return false;
+ " (" + acceptRetVal.status + ")");
handleNameResolutionError(addressValidityStatus);
return addressValidityStatus;
}
// Now do the ringhash specific logic with weights and building the ring
@ -146,7 +148,7 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer {
this.resolvingAddresses = false;
}
return true;
return Status.OK;
}
/**
@ -170,10 +172,11 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer {
checkState(!getChildLbStates().isEmpty(), "no subchannel has been created");
if (this.currentConnectivityState == SHUTDOWN) {
// Ignore changes that happen after shutdown is called
logger.log(XdsLogLevel.DEBUG, "UpdateOverallBalancingState called after shutdown");
return;
}
// Calculate the current overall state considering sticky TF
// Calculate the current overall state to report
int numIdle = 0;
int numReady = 0;
int numConnecting = 0;
@ -237,18 +240,20 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer {
getChildAddresses(key, resolvedAddresses, null));
}
private boolean validateAddrList(List<EquivalentAddressGroup> addrList) {
private Status 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;
Status unavailableStatus = Status.UNAVAILABLE.withDescription("Ring hash lb error: EDS "
+ "resolution was successful, but returned server addresses are empty.");
handleNameResolutionError(unavailableStatus);
return unavailableStatus;
}
String dupAddrString = validateNoDuplicateAddresses(addrList);
if (dupAddrString != null) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription("Ring hash lb error: EDS "
+ "resolution was successful, but there were duplicate addresses: " + dupAddrString));
return false;
Status unavailableStatus = Status.UNAVAILABLE.withDescription("Ring hash lb error: EDS "
+ "resolution was successful, but there were duplicate addresses: " + dupAddrString);
handleNameResolutionError(unavailableStatus);
return unavailableStatus;
}
long totalWeight = 0;
@ -260,29 +265,32 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer {
}
if (weight < 0) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
Status unavailableStatus = Status.UNAVAILABLE.withDescription(
String.format("Ring hash lb error: EDS resolution was successful, but returned a "
+ "negative weight for %s.", stripAttrs(eag))));
return false;
+ "negative weight for %s.", stripAttrs(eag)));
handleNameResolutionError(unavailableStatus);
return unavailableStatus;
}
if (weight > UnsignedInteger.MAX_VALUE.longValue()) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
Status unavailableStatus = 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;
+ " too large to fit in an unsigned int for %s.", stripAttrs(eag)));
handleNameResolutionError(unavailableStatus);
return unavailableStatus;
}
totalWeight += weight;
}
if (totalWeight > UnsignedInteger.MAX_VALUE.longValue()) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
Status unavailableStatus = 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;
+ " large to fit in an unsigned int (%d).", totalWeight));
handleNameResolutionError(unavailableStatus);
return unavailableStatus;
}
return true;
return Status.OK;
}
@Nullable
@ -377,7 +385,7 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer {
long midVal = ring.get(mid).hash;
long midValL = mid == 0 ? 0 : ring.get(mid - 1).hash;
if (requestHash <= midVal && requestHash > midValL) {
return mid;
break;
}
if (midVal < requestHash) {
low = mid + 1;
@ -441,13 +449,17 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer {
throw new UnsupportedOperationException("Not used by RingHash");
}
/**
* An unmodifiable view of a subchannel with state not subject to its real connectivity
* state changes.
*/
private static final class SubchannelView {
private final RingHashChildLbState childLbState;
private final ConnectivityState connectivityState;
private SubchannelView(RingHashChildLbState subchannel, ConnectivityState connectivityState) {
this.childLbState = subchannel;
this.connectivityState = connectivityState;
private SubchannelView(RingHashChildLbState childLbState, ConnectivityState state) {
this.childLbState = childLbState;
this.connectivityState = state;
}
}

View File

@ -97,29 +97,30 @@ final class WeightedRoundRobinLoadBalancer extends RoundRobinLoadBalancer {
@Override
protected ChildLbState createChildLbState(Object key, Object policyConfig,
SubchannelPicker initialPicker, ResolvedAddresses resolvedAddresses) {
SubchannelPicker initialPicker, ResolvedAddresses unused) {
ChildLbState childLbState = new WeightedChildLbState(key, pickFirstLbProvider, policyConfig,
initialPicker);
return childLbState;
}
@Override
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
if (resolvedAddresses.getLoadBalancingPolicyConfig() == null) {
handleNameResolutionError(Status.UNAVAILABLE.withDescription(
Status unavailableStatus = Status.UNAVAILABLE.withDescription(
"NameResolver returned no WeightedRoundRobinLoadBalancerConfig. addrs="
+ resolvedAddresses.getAddresses()
+ ", attrs=" + resolvedAddresses.getAttributes()));
return false;
+ ", attrs=" + resolvedAddresses.getAttributes());
handleNameResolutionError(unavailableStatus);
return unavailableStatus;
}
config =
(WeightedRoundRobinLoadBalancerConfig) resolvedAddresses.getLoadBalancingPolicyConfig();
AcceptResolvedAddressRetVal acceptRetVal;
try {
resolvingAddresses = true;
AcceptResolvedAddressRetVal acceptRetVal =
acceptResolvedAddressesInternal(resolvedAddresses);
if (!acceptRetVal.valid) {
return false;
acceptRetVal = acceptResolvedAddressesInternal(resolvedAddresses);
if (!acceptRetVal.status.isOk()) {
return acceptRetVal.status;
}
if (weightUpdateTimer != null && weightUpdateTimer.isPending()) {
@ -138,7 +139,7 @@ final class WeightedRoundRobinLoadBalancer extends RoundRobinLoadBalancer {
resolvingAddresses = false;
}
return true;
return acceptRetVal.status;
}
@Override

View File

@ -128,10 +128,10 @@ public class RingHashLoadBalancerTest {
public void subchannelLazyConnectUntilPicked() {
RingHashConfig config = new RingHashConfig(10, 100);
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1); // one server
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
Status addressesAcceptanceStatus = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isTrue();
assertThat(addressesAcceptanceStatus.isOk()).isTrue();
verify(helper).updateBalancingState(eq(IDLE), pickerCaptor.capture());
assertThat(subchannels.size()).isEqualTo(0);
@ -159,10 +159,10 @@ public class RingHashLoadBalancerTest {
public void subchannelNotAutoReconnectAfterReenteringIdle() {
RingHashConfig config = new RingHashConfig(10, 100);
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1); // one server
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
Status addressesAcceptanceStatus = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isTrue();
assertThat(addressesAcceptanceStatus.isOk()).isTrue();
verify(helper).updateBalancingState(eq(IDLE), pickerCaptor.capture());
RingHashChildLbState childLbState =
@ -337,10 +337,10 @@ public class RingHashLoadBalancerTest {
}
Subchannel subchannel0_old = subchannels.get(Collections.singletonList(servers.get(0)));
Subchannel subchannel1_old = subchannels.get(Collections.singletonList(servers.get(1)));
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
Status addressesAcceptanceStatus = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(updatedServers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isTrue();
assertThat(addressesAcceptanceStatus.isOk()).isTrue();
verify(subchannel0_old).updateAddresses(Collections.singletonList(updatedServers.get(0)));
verify(subchannel1_old).updateAddresses(Collections.singletonList(updatedServers.get(1)));
inOrder.verify(helper).updateBalancingState(eq(READY), pickerCaptor.capture());
@ -372,10 +372,10 @@ public class RingHashLoadBalancerTest {
assertThat(subchannel.getAddresses()).isEqualTo(servers.get(1));
servers = createWeightedServerAddrs(1, 1, 1, 1, 1); // server2, server3, server4 added
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
Status addressesAcceptanceStatus = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isTrue();
assertThat(addressesAcceptanceStatus.isOk()).isTrue();
assertThat(loadBalancer.getChildLbStates().size()).isEqualTo(5);
inOrder.verify(helper).updateBalancingState(eq(READY), pickerCaptor.capture());
assertThat(pickerCaptor.getValue().pickSubchannel(args).getSubchannel())
@ -392,11 +392,11 @@ public class RingHashLoadBalancerTest {
// Map each server address to exactly one ring entry.
RingHashConfig config = new RingHashConfig(3, 3);
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
boolean addressesAccepted =
Status addressesAcceptanceStatus =
loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isTrue();
assertThat(addressesAcceptanceStatus.isOk()).isTrue();
// Create subchannel for the first address
((RingHashChildLbState)loadBalancer.getChildLbStateEag(servers.get(0))).activate();
@ -772,32 +772,32 @@ public class RingHashLoadBalancerTest {
// Try value between max signed and max unsigned int
servers = createWeightedServerAddrs(Integer.MAX_VALUE + 100L, 100); // (MAX+100):100
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
Status addressesAcceptanceStatus = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isTrue();
assertThat(addressesAcceptanceStatus.isOk()).isTrue();
// Try a negative value
servers = createWeightedServerAddrs(10, -20, 100); // 10:-20:100
addressesAccepted = loadBalancer.acceptResolvedAddresses(
addressesAcceptanceStatus = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isFalse();
assertThat(addressesAcceptanceStatus.isOk()).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(
addressesAcceptanceStatus = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isFalse();
assertThat(addressesAcceptanceStatus.isOk()).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(
addressesAcceptanceStatus = loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
assertThat(addressesAccepted).isFalse();
assertThat(addressesAcceptanceStatus.isOk()).isFalse();
}
@Test
@ -912,17 +912,17 @@ public class RingHashLoadBalancerTest {
}
}
boolean addressesAccepted =
Status addressesAcceptanceStatus =
loadBalancer.acceptResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
if (doVerifies) {
assertThat(addressesAccepted).isTrue();
assertThat(addressesAcceptanceStatus.isOk()).isTrue();
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
}
if (!addressesAccepted) {
if (!addressesAcceptanceStatus.isOk()) {
return new ArrayList<>();
}