Skip to content

Commit

Permalink
Fix tests and some underlying bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
larry-safran committed Feb 28, 2025
1 parent 5f4813d commit 7e32c0a
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 105 deletions.
20 changes: 20 additions & 0 deletions util/src/main/java/io/grpc/util/OutlierDetectionLoadBalancer.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Random;
import java.util.Set;
import java.util.concurrent.ScheduledExecutorService;
Expand Down Expand Up @@ -1062,6 +1063,25 @@ public static class SuccessRateEjection {
this.requestVolume = requestVolume;
}

@Override
public int hashCode() {
return Objects.hash(stdevFactor, enforcementPercentage, minimumHosts, requestVolume);
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (! (obj instanceof SuccessRateEjection)) {
return false;
}
return Objects.equals(stdevFactor, ((SuccessRateEjection) obj).stdevFactor)
&& Objects.equals(enforcementPercentage, ((SuccessRateEjection) obj).enforcementPercentage)
&& Objects.equals(minimumHosts, ((SuccessRateEjection) obj).minimumHosts)
&& Objects.equals(requestVolume, ((SuccessRateEjection) obj).requestVolume);
}

/** Builds new instances of {@link SuccessRateEjection}. */
public static final class Builder {

Expand Down
32 changes: 18 additions & 14 deletions xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,16 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
}

logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses);
if (rootCdsLbState != null && this.resolvedAddresses != null
&& rootClusterName.equals(rootCdsLbState.root.name)
&& this.resolvedAddresses.equals(resolvedAddresses)) {
return Status.OK; // no changes
}

if (rootCdsLbState != null) {
rootCdsLbState.shutdown();
}

this.resolvedAddresses = resolvedAddresses;
rootCdsLbState =
new CdsLbState(rootClusterName, xdsConfig.getClusters(), rootClusterName);
Expand Down Expand Up @@ -376,7 +386,7 @@ private void handleEndpointResourceUpdate() {
if (childLb == null) {
childLb = lbRegistry.getProvider(PRIORITY_POLICY_NAME).newLoadBalancer(helper);
}
childLb.handleResolvedAddresses(
childLb.acceptResolvedAddresses(
resolvedAddresses.toBuilder()
.setLoadBalancingPolicyConfig(childConfig)
.setAddresses(Collections.unmodifiableList(addresses))
Expand Down Expand Up @@ -805,7 +815,7 @@ private CdsLbState(String rootCluster,
String rootName) {
root = new ClusterStateDetails(rootName, clusterConfigs.get(rootName));
clusterStates.put(rootCluster, root);
initializeChildren(clusterConfigs, root);
initializeChildren(clusterConfigs, rootName);
}

private void start() {
Expand All @@ -820,18 +830,13 @@ private void shutdown() {
}
}

// If doesn't have children is a no-op
private void initializeChildren(ImmutableMap<String,
StatusOr<XdsClusterConfig>> clusterConfigs, ClusterStateDetails curRoot) {
if (curRoot.result == null) {
return;
}
ImmutableList<String> childNames = curRoot.result.prioritizedClusterNames();
if (childNames == null) {
return;
}
private void initializeChildren(
ImmutableMap<String, StatusOr<XdsClusterConfig>> clusterConfigs, String rootName) {
for (String clusterName : clusterConfigs.keySet()) {
if (clusterName.equals(rootName)) {
continue;
}

for (String clusterName : childNames) {
StatusOr<XdsClusterConfig> configStatusOr = clusterConfigs.get(clusterName);
if (configStatusOr == null) {
logger.log(XdsLogLevel.DEBUG, "Child cluster %s of %s has no matching config",
Expand All @@ -843,7 +848,6 @@ private void initializeChildren(ImmutableMap<String,
clusterStateDetails = new ClusterStateDetails(clusterName, configStatusOr);
clusterStates.put(clusterName, clusterStateDetails);
}
initializeChildren(clusterConfigs, clusterStateDetails);
}
}

Expand Down
19 changes: 19 additions & 0 deletions xds/src/main/java/io/grpc/xds/XdsClusterResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,25 @@ private static Builder newBuilder(String clusterName) {
.parsedMetadata(ImmutableMap.of());
}

Builder toBuilder() {
return new AutoValue_XdsClusterResource_CdsUpdate.Builder()
.choiceCount(choiceCount())
.clusterName(clusterName())
.clusterType(clusterType())
.dnsHostName(dnsHostName())
.edsServiceName(edsServiceName())
.lrsServerInfo(lrsServerInfo())
.maxConcurrentRequests(maxConcurrentRequests())
.maxRingSize(maxRingSize())
.minRingSize(minRingSize())
.lbPolicyConfig(lbPolicyConfig())
.upstreamTlsContext(upstreamTlsContext())
.prioritizedClusterNames(prioritizedClusterNames())
.outlierDetection(outlierDetection())
.filterMetadata(filterMetadata())
.parsedMetadata(parsedMetadata());
}

static Builder forAggregate(String clusterName, List<String> prioritizedClusterNames) {
checkNotNull(prioritizedClusterNames, "prioritizedClusterNames");
return newBuilder(clusterName)
Expand Down
5 changes: 3 additions & 2 deletions xds/src/main/java/io/grpc/xds/XdsConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

/**
* Represents the xDS configuration tree for a specified Listener.
Expand Down Expand Up @@ -206,9 +207,9 @@ public String toString() {

// The list of leaf clusters for an aggregate cluster.
static final class AggregateConfig implements ClusterChild {
private final List<String> leafNames;
private final Set<String> leafNames;

public AggregateConfig(List<String> leafNames) {
public AggregateConfig(Set<String> leafNames) {
this.leafNames = checkNotNull(leafNames, "leafNames");
}

Expand Down
21 changes: 13 additions & 8 deletions xds/src/main/java/io/grpc/xds/XdsDependencyManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ XdsConfig buildConfig() {
cdsWatchers.values().stream()
.filter(XdsDependencyManager::isTopLevelCluster)
.map(XdsWatcherBase::resourceName)
.distinct()
.collect(Collectors.toList());

// Flatten multi-level aggregates into lists of leaf clusters
Expand Down Expand Up @@ -441,7 +442,7 @@ private static EndpointConfig buildEndpointConfig(CdsWatcher cdsWatcher,
List<Endpoints.LbEndpoint> endpoints = new ArrayList<>();
for (EquivalentAddressGroup eag : cdsWatcher.clusterState.addressGroupList) {
// TODO: should this really be health and null hostname?
endpoints.add(Endpoints.LbEndpoint.create(eag, 1, true, null));
endpoints.add(Endpoints.LbEndpoint.create(eag, 1, true, ""));
}
LocalityLbEndpoints lbEndpoints = LocalityLbEndpoints.create(endpoints, 1, 0);
localityLbEndpoints.put(locality, lbEndpoints);
Expand Down Expand Up @@ -470,9 +471,12 @@ private Set<String> addTopLevelClustersToBuilder(
XdsConfig.XdsClusterConfig.ClusterChild child;
switch (cdsUpdate.clusterType()) {
case AGGREGATE:
List<String> leafNames = getLeafNames(cdsUpdate);
Set<String> leafNames = new HashSet<>();
addLeafNames(leafNames, cdsUpdate);
child = new AggregateConfig(leafNames);
leafClusterNames.addAll(leafNames);
cdsUpdate = cdsUpdate.toBuilder().prioritizedClusterNames(ImmutableList.copyOf(leafNames))
.build();
break;
case EDS:
EdsWatcher edsWatcher = (EdsWatcher) edsWatchers.get(cdsUpdate.edsServiceName());
Expand Down Expand Up @@ -503,23 +507,24 @@ private Set<String> addTopLevelClustersToBuilder(
return leafClusterNames;
}

private List<String> getLeafNames(XdsClusterResource.CdsUpdate cdsUpdate) {
List<String> childNames = new ArrayList<>();
private void addLeafNames(Set<String> leafNames, XdsClusterResource.CdsUpdate cdsUpdate) {
Set<String> childNames = new HashSet<>();

for (String cluster : cdsUpdate.prioritizedClusterNames()) {
if (leafNames.contains(cluster)) {
continue;
}
StatusOr<XdsClusterResource.CdsUpdate> data = getCluster(cluster).getData();
if (data == null || !data.hasValue() || data.getValue() == null) {
childNames.add(cluster);
continue;
}
if (data.getValue().clusterType() == ClusterType.AGGREGATE) {
childNames.addAll(getLeafNames(data.getValue()));
addLeafNames(leafNames, data.getValue());
} else {
childNames.add(cluster);
leafNames.add(cluster);
}
}

return childNames;
}

private static boolean isTopLevelCluster(XdsWatcherBase<?> cdsWatcher) {
Expand Down
Loading

0 comments on commit 7e32c0a

Please sign in to comment.