util: Remove deactivation and GracefulSwitchLb from MultiChildLb

It is easy to manage these things outside of MultiChildLb and it makes
the shared code easier and use less memory. In particular, we don't want
to use many instances of GracefulSwitchLb in virtually every policy
simply because it was needed in one or two cases.
This commit is contained in:
Eric Anderson 2024-03-07 13:47:24 -08:00
parent 7f0a1910d3
commit 9de8e44384
3 changed files with 17 additions and 106 deletions

View File

@ -234,36 +234,21 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
return new AcceptResolvedAddrRetVal(unavailableStatus, null); return new AcceptResolvedAddrRetVal(unavailableStatus, null);
} }
Collection<ChildLbState> reusedChildren = addMissingChildrenAndIdReuse(newChildren); addMissingChildren(newChildren);
// Raactivate deactivated children
for (ChildLbState reusedChild : reusedChildren) {
reusedChild.reactivate(reusedChild.getPolicyFactory());
}
updateChildrenWithResolvedAddresses(resolvedAddresses, newChildren); updateChildrenWithResolvedAddresses(resolvedAddresses, newChildren);
return new AcceptResolvedAddrRetVal(Status.OK, getRemovedChildren(newChildren.keySet())); return new AcceptResolvedAddrRetVal(Status.OK, getRemovedChildren(newChildren.keySet()));
} }
protected final Collection<ChildLbState> addMissingChildrenAndIdReuse( protected final void addMissingChildren(Map<Object, ChildLbState> newChildren) {
Map<Object, ChildLbState> newChildren) {
Collection<ChildLbState> reusedChildren = new ArrayList<>();
// Do adds and identify reused children // Do adds and identify reused children
for (Map.Entry<Object, ChildLbState> entry : newChildren.entrySet()) { for (Map.Entry<Object, ChildLbState> entry : newChildren.entrySet()) {
final Object key = entry.getKey(); final Object key = entry.getKey();
if (!childLbStates.containsKey(key)) { if (!childLbStates.containsKey(key)) {
childLbStates.put(key, entry.getValue()); childLbStates.put(key, entry.getValue());
} else {
// Reuse the existing one
ChildLbState existingChildLbState = childLbStates.get(key);
if (existingChildLbState.isDeactivated()) {
reusedChildren.add(existingChildLbState);
}
} }
} }
return reusedChildren;
} }
protected final void updateChildrenWithResolvedAddresses(ResolvedAddresses resolvedAddresses, protected final void updateChildrenWithResolvedAddresses(ResolvedAddresses resolvedAddresses,
@ -274,9 +259,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
ResolvedAddresses childAddresses = ResolvedAddresses childAddresses =
getChildAddresses(entry.getKey(), resolvedAddresses, childConfig); getChildAddresses(entry.getKey(), resolvedAddresses, childConfig);
childLbState.setResolvedAddresses(childAddresses); // update child childLbState.setResolvedAddresses(childAddresses); // update child
if (!childLbState.deactivated) { childLbState.lb.handleResolvedAddresses(childAddresses); // update child LB
childLbState.lb.handleResolvedAddresses(childAddresses); // update child LB
}
} }
} }
@ -288,8 +271,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
// Do removals // Do removals
for (Object key : ImmutableList.copyOf(childLbStates.keySet())) { for (Object key : ImmutableList.copyOf(childLbStates.keySet())) {
if (!newChildKeys.contains(key)) { if (!newChildKeys.contains(key)) {
ChildLbState childLbState = childLbStates.get(key); ChildLbState childLbState = childLbStates.remove(key);
childLbState.deactivate();
removedChildren.add(childLbState); removedChildren.add(childLbState);
} }
} }
@ -326,10 +308,6 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
return helper; return helper;
} }
protected final void removeChild(Object key) {
childLbStates.remove(key);
}
@VisibleForTesting @VisibleForTesting
public final ImmutableMap<Object, ChildLbState> getImmutableChildMap() { public final ImmutableMap<Object, ChildLbState> getImmutableChildMap() {
return ImmutableMap.copyOf(childLbStates); return ImmutableMap.copyOf(childLbStates);
@ -357,12 +335,12 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
} }
/** /**
* Filters out non-ready and deactivated child load balancers (subchannels). * Filters out non-ready child load balancers (subchannels).
*/ */
protected final List<ChildLbState> getReadyChildren() { protected final List<ChildLbState> getReadyChildren() {
List<ChildLbState> activeChildren = new ArrayList<>(); List<ChildLbState> activeChildren = new ArrayList<>();
for (ChildLbState child : getChildLbStates()) { for (ChildLbState child : getChildLbStates()) {
if (!child.isDeactivated() && child.getCurrentState() == READY) { if (child.getCurrentState() == READY) {
activeChildren.add(child); activeChildren.add(child);
} }
} }
@ -372,9 +350,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
/** /**
* This represents the state of load balancer children. Each endpoint (represented by an * This represents the state of load balancer children. Each endpoint (represented by an
* EquivalentAddressGroup or EDS string) will have a separate ChildLbState which in turn will * EquivalentAddressGroup or EDS string) will have a separate ChildLbState which in turn will
* define a GracefulSwitchLoadBalancer. When the GracefulSwitchLoadBalancer is activated, a * have a single child LoadBalancer created from the provided factory.
* single PickFirstLoadBalancer will be created which will then create a subchannel and start
* trying to connect to it.
* *
* <p>A ChildLbStateHelper is the glue between ChildLbState and the helpers associated with the * <p>A ChildLbStateHelper is the glue between ChildLbState and the helpers associated with the
* petiole policy above and the PickFirstLoadBalancer's helper below. * petiole policy above and the PickFirstLoadBalancer's helper below.
@ -387,65 +363,23 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
private ResolvedAddresses resolvedAddresses; private ResolvedAddresses resolvedAddresses;
private final Object config; private final Object config;
private final GracefulSwitchLoadBalancer lb; private final LoadBalancer lb;
private final LoadBalancer.Factory policyFactory;
private ConnectivityState currentState; private ConnectivityState currentState;
private SubchannelPicker currentPicker; private SubchannelPicker currentPicker;
private boolean deactivated;
public ChildLbState(Object key, LoadBalancer.Factory policyFactory, Object childConfig, public ChildLbState(Object key, LoadBalancer.Factory policyFactory, Object childConfig,
SubchannelPicker initialPicker) { SubchannelPicker initialPicker) {
this(key, policyFactory, childConfig, initialPicker, null, false);
}
public ChildLbState(Object key, LoadBalancer.Factory policyFactory, Object childConfig,
SubchannelPicker initialPicker, ResolvedAddresses resolvedAddrs, boolean deactivated) {
this.key = key; this.key = key;
this.policyFactory = policyFactory;
this.deactivated = deactivated;
this.currentPicker = initialPicker; this.currentPicker = initialPicker;
this.config = childConfig; this.config = childConfig;
this.lb = new GracefulSwitchLoadBalancer(createChildHelper()); this.lb = policyFactory.newLoadBalancer(createChildHelper());
this.currentState = deactivated ? IDLE : CONNECTING; this.currentState = CONNECTING;
this.resolvedAddresses = resolvedAddrs;
if (!deactivated) {
lb.switchTo(policyFactory);
}
} }
protected ChildLbStateHelper createChildHelper() { protected ChildLbStateHelper createChildHelper() {
return new ChildLbStateHelper(); return new ChildLbStateHelper();
} }
/**
* The default implementation. This not only marks the lb policy as not active, it also removes
* this child from the map of children maintained by the petiole policy.
*
* <p>Note that this does not explicitly shutdown this child. That will generally be done by
* acceptResolvedAddresses on the LB, but can also be handled by an override such as is done
* in <a href=" https://github.com/grpc/grpc-java/blob/master/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java">ClusterManagerLoadBalancer</a>.
*
* <p>If you plan to reactivate, you will probably want to override this to not call
* childLbStates.remove() and handle that cleanup another way.
*/
protected void deactivate() {
if (deactivated) {
return;
}
childLbStates.remove(key); // This means it can't be reactivated again
deactivated = true;
logger.log(Level.FINE, "Child balancer {0} deactivated", key);
}
/**
* This base implementation does nothing but reset the flag. If you really want to both
* deactivate and reactivate you should override them both.
*/
protected void reactivate(LoadBalancer.Factory policyFactory) {
deactivated = false;
}
/** /**
* Override for unique behavior such as delayed shutdowns of subchannels. * Override for unique behavior such as delayed shutdowns of subchannels.
*/ */
@ -460,8 +394,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
return "Address = " + key return "Address = " + key
+ ", state = " + currentState + ", state = " + currentState
+ ", picker type: " + currentPicker.getClass() + ", picker type: " + currentPicker.getClass()
+ ", lb: " + lb.delegate().getClass() + ", lb: " + lb;
+ (deactivated ? ", deactivated" : "");
} }
public final Object getKey() { public final Object getKey() {
@ -469,7 +402,7 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
} }
@VisibleForTesting @VisibleForTesting
public final GracefulSwitchLoadBalancer getLb() { public final LoadBalancer getLb() {
return lb; return lb;
} }
@ -478,10 +411,6 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
return currentPicker; return currentPicker;
} }
protected final LoadBalancer.Factory getPolicyFactory() {
return policyFactory;
}
protected final Subchannel getSubchannels(PickSubchannelArgs args) { protected final Subchannel getSubchannels(PickSubchannelArgs args) {
if (getCurrentPicker() == null) { if (getCurrentPicker() == null) {
return null; return null;
@ -508,18 +437,6 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
return resolvedAddresses.getAddresses().get(0); return resolvedAddresses.getAddresses().get(0);
} }
public final boolean isDeactivated() {
return deactivated;
}
protected final void setDeactivated() {
deactivated = true;
}
protected final void markReactivated() {
deactivated = false;
}
protected final void setResolvedAddresses(ResolvedAddresses newAddresses) { protected final void setResolvedAddresses(ResolvedAddresses newAddresses) {
checkNotNull(newAddresses, "Missing address list for child"); checkNotNull(newAddresses, "Missing address list for child");
resolvedAddresses = newAddresses; resolvedAddresses = newAddresses;
@ -552,17 +469,15 @@ public abstract class MultiChildLoadBalancer extends LoadBalancer {
@Override @Override
public void updateBalancingState(final ConnectivityState newState, public void updateBalancingState(final ConnectivityState newState,
final SubchannelPicker newPicker) { final SubchannelPicker newPicker) {
// If we are already in the process of resolving addresses, the overall balancing state
// will be updated at the end of it, and we don't need to trigger that update here.
if (!childLbStates.containsKey(key)) { if (!childLbStates.containsKey(key)) {
return; return;
} }
// Subchannel picker and state are saved, but will only be propagated to the channel
// when the child instance exits deactivated state.
currentState = newState; currentState = newState;
currentPicker = newPicker; currentPicker = newPicker;
if (!deactivated && !resolvingAddresses) { // If we are already in the process of resolving addresses, the overall balancing state
// will be updated at the end of it, and we don't need to trigger that update here.
if (!resolvingAddresses) {
if (newState == IDLE) { if (newState == IDLE) {
lb.requestConnection(); lb.requestConnection();
} }

View File

@ -358,9 +358,6 @@ public class MultiChildLoadBalancerTest {
ConnectivityState overallState = null; ConnectivityState overallState = null;
final Map<Object, SubchannelPicker> childPickers = new HashMap<>(); final Map<Object, SubchannelPicker> childPickers = new HashMap<>();
for (ChildLbState childLbState : getChildLbStates()) { for (ChildLbState childLbState : getChildLbStates()) {
if (childLbState.isDeactivated()) {
continue;
}
childPickers.put(childLbState.getKey(), childLbState.getCurrentPicker()); childPickers.put(childLbState.getKey(), childLbState.getCurrentPicker());
overallState = aggregateState(overallState, childLbState.getCurrentState()); overallState = aggregateState(overallState, childLbState.getCurrentState());
} }

View File

@ -99,8 +99,7 @@ final class RingHashLoadBalancer extends MultiChildLoadBalancer {
return addressValidityStatus; return addressValidityStatus;
} }
// We don't care about reuse because we don't want to activate them addMissingChildren(newChildren);
addMissingChildrenAndIdReuse(newChildren);
updateChildrenWithResolvedAddresses(resolvedAddresses, newChildren); updateChildrenWithResolvedAddresses(resolvedAddresses, newChildren);
// Now do the ringhash specific logic with weights and building the ring // Now do the ringhash specific logic with weights and building the ring