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.

Sign up for GitHub

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

Jump to bottom

xds:Fix ConcurrentModificationException in PriorityLoadBalancer (#9728) #9744

Merged
merged 1 commit into from Dec 12, 2022
Merged

xds:Fix ConcurrentModificationException in PriorityLoadBalancer (#9728) #9744

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 17 additions & 6 deletions xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java
View file
Open in desktop
Expand Up @@ -37,6 +37,8 @@
import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig.PriorityChildConfig;
import io.grpc.xds.XdsLogger.XdsLogLevel;
import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand All @@ -59,6 +61,8 @@ final class PriorityLoadBalancer extends LoadBalancer {

// Includes all active and deactivated children. Mutable. New entries are only added from priority
// 0 up to the selected priority. An entry is only deleted 15 minutes after its deactivation.
// Note that because all configuration updates should be atomic, updates to children can happen
// outside of the synchronization context. Therefore copy values before looping over them.
private final Map<String, ChildLbState> children = new HashMap<>();

// Following fields are only null initially.
Expand Down Expand Up @@ -91,15 +95,20 @@ public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
priorityNames = config.priorities;
priorityConfigs = config.childConfigs;
Set<String> prioritySet = new HashSet<>(config.priorities);
for (String priority : children.keySet()) {
ArrayList<String> childKeys = new ArrayList<>(children.keySet());
for (String priority : childKeys) {
if (!prioritySet.contains(priority)) {
children.get(priority).deactivate();
ChildLbState childLbState = children.get(priority);
if (childLbState != null) {
childLbState.deactivate();
}
}
}
handlingResolvedAddresses = true;
for (String priority : priorityNames) {
if (children.containsKey(priority)) {
children.get(priority).updateResolvedAddresses();
ChildLbState childLbState = children.get(priority);
if (childLbState != null) {
childLbState.updateResolvedAddresses();
}
}
handlingResolvedAddresses = false;
Expand All @@ -111,7 +120,8 @@ public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
public void handleNameResolutionError(Status error) {
logger.log(XdsLogLevel.WARNING, "Received name resolution error: {0}", error);
boolean gotoTransientFailure = true;
for (ChildLbState child : children.values()) {
Collection<ChildLbState> childValues = new ArrayList<>(children.values());
for (ChildLbState child : childValues) {
if (priorityNames.contains(child.priority)) {
child.lb.handleNameResolutionError(error);
gotoTransientFailure = false;
Expand All @@ -125,7 +135,8 @@ public void handleNameResolutionError(Status error) {
@Override
public void shutdown() {
logger.log(XdsLogLevel.INFO, "Shutdown");
for (ChildLbState child : children.values()) {
Collection<ChildLbState> childValues = new ArrayList<>(children.values());
for (ChildLbState child : childValues) {
child.tearDown();
}
children.clear();
Expand Down