Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: Use acceptResolvedAddresses() for PriorityLb children #11914

Merged
merged 1 commit into from
Feb 21, 2025
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
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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
* 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 @@
// 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