Skip to content

Commit

Permalink
xds: Use acceptResolvedAddresses() for PriorityLb children
Browse files Browse the repository at this point in the history
PriorityLb should propagate config problems up to the name resolver so
it can refresh.
  • Loading branch information
ejona86 committed Feb 20, 2025
1 parent 892144d commit 34b0d52
Show file tree
Hide file tree
Showing 2 changed files with 207 additions and 33 deletions.
41 changes: 28 additions & 13 deletions xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
checkNotNull(config, "missing priority lb config");
priorityNames = config.priorities;
priorityConfigs = config.childConfigs;
Status status = Status.OK;
Set<String> prioritySet = new HashSet<>(config.priorities);
ArrayList<String> childKeys = new ArrayList<>(children.keySet());
for (String priority : childKeys) {
Expand All @@ -105,12 +106,18 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
for (String priority : priorityNames) {
ChildLbState childLbState = children.get(priority);
if (childLbState != null) {
childLbState.updateResolvedAddresses();
Status newStatus = childLbState.updateResolvedAddresses();
if (!newStatus.isOk()) {
status = newStatus;
}
}
}
handlingResolvedAddresses = false;
tryNextPriority();
return Status.OK;
Status newStatus = tryNextPriority();
if (!newStatus.isOk()) {
status = newStatus;
}
return status;
}

@Override
Expand Down Expand Up @@ -140,7 +147,7 @@ public void shutdown() {
children.clear();
}

private void tryNextPriority() {
private Status tryNextPriority() {
for (int i = 0; i < priorityNames.size(); i++) {
String priority = priorityNames.get(i);
if (!children.containsKey(priority)) {
Expand All @@ -151,8 +158,7 @@ private void tryNextPriority() {
// Calling the child's updateResolvedAddresses() can result in tryNextPriority() being
// called recursively. We need to be sure to be done with processing here before it is
// called.
child.updateResolvedAddresses();
return; // Give priority i time to connect.
return child.updateResolvedAddresses(); // Give priority i time to connect.
}
ChildLbState child = children.get(priority);
child.reactivate();
Expand All @@ -165,23 +171,24 @@ private void tryNextPriority() {
children.get(p).deactivate();
}
}
return;
return Status.OK;
}
if (child.failOverTimer != null && child.failOverTimer.isPending()) {
updateOverallState(priority, child.connectivityState, child.picker);
return; // Give priority i time to connect.
return Status.OK; // Give priority i time to connect.
}
if (priority.equals(currentPriority) && child.connectivityState != TRANSIENT_FAILURE) {
// If the current priority is not changed into TRANSIENT_FAILURE, keep using it.
updateOverallState(priority, child.connectivityState, child.picker);
return;
return Status.OK;

Check warning on line 183 in xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java

View check run for this annotation

Codecov / codecov/patch

xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java#L183

Added line #L183 was not covered by tests
}
}
// TODO(zdapeng): Include error details of each priority.
logger.log(XdsLogLevel.DEBUG, "All priority failed");
String lastPriority = priorityNames.get(priorityNames.size() - 1);
SubchannelPicker errorPicker = children.get(lastPriority).picker;
updateOverallState(lastPriority, TRANSIENT_FAILURE, errorPicker);
return Status.OK;
}

private void updateOverallState(
Expand Down Expand Up @@ -228,7 +235,11 @@ public void run() {
Status.UNAVAILABLE.withDescription("Connection timeout for priority " + priority)));
logger.log(XdsLogLevel.DEBUG, "Priority {0} failed over to next", priority);
currentPriority = null; // reset currentPriority to guarantee failover happen
tryNextPriority();
Status status = tryNextPriority();
if (!status.isOk()) {
// A child had a problem with the addresses/config. Request it to be refreshed
helper.refreshNameResolution();
}
}
}

Expand Down Expand Up @@ -279,10 +290,10 @@ void tearDown() {
* resolvedAddresses}, or when priority lb receives a new resolved addresses while the child
* already exists.
*/
void updateResolvedAddresses() {
Status updateResolvedAddresses() {
PriorityLbConfig config =
(PriorityLbConfig) resolvedAddresses.getLoadBalancingPolicyConfig();
lb.handleResolvedAddresses(
return lb.acceptResolvedAddresses(
resolvedAddresses.toBuilder()
.setAddresses(AddressFilter.filter(resolvedAddresses.getAddresses(), priority))
.setLoadBalancingPolicyConfig(config.childConfigs.get(priority).childConfig)
Expand Down Expand Up @@ -331,7 +342,11 @@ public void updateBalancingState(final ConnectivityState newState,
// If we are currently handling newly resolved addresses, let's not try to reconfigure as
// the address handling process will take care of that to provide an atomic config update.
if (!handlingResolvedAddresses) {
tryNextPriority();
Status status = tryNextPriority();
if (!status.isOk()) {
// A child had a problem with the addresses/config. Request it to be refreshed
helper.refreshNameResolution();
}
}
}

Expand Down
Loading

0 comments on commit 34b0d52

Please sign in to comment.