Skip to content

Commit 0138977

Browse files
ejona86ejona86
committed
util: Remove child policy config from MultiChildLB state
The child policy config should be refreshed every address update, so it shouldn't be stored in the ChildLbState. In addition, none of the current usages actually used what was stored in the ChildLbState in a meaningful way (it was always null). ResolvedAddresses was also removed from createChildLbState(), as nothing in it should be needed for creation; it varies over time and the values passed at creation are immutable.
1 parent 10d6002 commit 0138977

File tree

6 files changed

+23
-43
lines changed

6 files changed

+23
-43
lines changed

‎util/src/main/java/io/grpc/util/MultiChildLoadBalancer.java‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ protected Map<Object, ChildLbState> createChildLbMap(ResolvedAddresses resolvedA
9090
if (existingChildLbState != null) {
9191
childLbMap.put(endpoint, existingChildLbState);
9292
} else {
93-
childLbMap.put(endpoint, createChildLbState(endpoint, null, resolvedAddresses));
93+
childLbMap.put(endpoint, createChildLbState(endpoint));
9494
}
9595
}
9696
return childLbMap;
@@ -99,9 +99,8 @@ protected Map<Object, ChildLbState> createChildLbMap(ResolvedAddresses resolvedA
9999
/**
100100
* Override to create an instance of a subclass.
101101
*/
102-
protected ChildLbState createChildLbState(Object key, Object policyConfig,
103-
ResolvedAddresses resolvedAddresses) {
104-
return new ChildLbState(key, pickFirstLbProvider, policyConfig);
102+
protected ChildLbState createChildLbState(Object key) {
103+
return new ChildLbState(key, pickFirstLbProvider);
105104
}
106105

107106
/**
@@ -133,11 +132,9 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
133132
* Override this if your keys are not of type Endpoint.
134133
* @param key Key to identify the ChildLbState
135134
* @param resolvedAddresses list of addresses which include attributes
136-
* @param childConfig a load balancing policy config. This field is optional.
137135
* @return a fully loaded ResolvedAddresses object for the specified key
138136
*/
139-
protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses,
140-
Object childConfig) {
137+
protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses) {
141138
Endpoint endpointKey;
142139
if (key instanceof EquivalentAddressGroup) {
143140
endpointKey = new Endpoint((EquivalentAddressGroup) key);
@@ -160,7 +157,7 @@ protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses reso
160157
return resolvedAddresses.toBuilder()
161158
.setAddresses(Collections.singletonList(eagToUse))
162159
.setAttributes(Attributes.newBuilder().set(IS_PETIOLE_POLICY, true).build())
163-
.setLoadBalancingPolicyConfig(childConfig)
160+
.setLoadBalancingPolicyConfig(null)
164161
.build();
165162
}
166163

@@ -226,10 +223,8 @@ private void addMissingChildren(Map<Object, ChildLbState> newChildren) {
226223
private void updateChildrenWithResolvedAddresses(ResolvedAddresses resolvedAddresses,
227224
Map<Object, ChildLbState> newChildren) {
228225
for (Map.Entry<Object, ChildLbState> entry : newChildren.entrySet()) {
229-
Object childConfig = entry.getValue().getConfig();
230226
ChildLbState childLbState = childLbStates.get(entry.getKey());
231-
ResolvedAddresses childAddresses =
232-
getChildAddresses(entry.getKey(), resolvedAddresses, childConfig);
227+
ResolvedAddresses childAddresses = getChildAddresses(entry.getKey(), resolvedAddresses);
233228
childLbState.setResolvedAddresses(childAddresses); // update child
234229
childLbState.lb.handleResolvedAddresses(childAddresses); // update child LB
235230
}
@@ -328,15 +323,13 @@ protected final List<ChildLbState> getReadyChildren() {
328323
public class ChildLbState {
329324
private final Object key;
330325
private ResolvedAddresses resolvedAddresses;
331-
private final Object config;
332326

333327
private final LoadBalancer lb;
334328
private ConnectivityState currentState;
335329
private SubchannelPicker currentPicker = new FixedResultPicker(PickResult.withNoResult());
336330

337-
public ChildLbState(Object key, LoadBalancer.Factory policyFactory, Object childConfig) {
331+
public ChildLbState(Object key, LoadBalancer.Factory policyFactory) {
338332
this.key = key;
339-
this.config = childConfig;
340333
this.lb = policyFactory.newLoadBalancer(createChildHelper());
341334
this.currentState = CONNECTING;
342335
}
@@ -400,10 +393,6 @@ protected final void setResolvedAddresses(ResolvedAddresses newAddresses) {
400393
resolvedAddresses = newAddresses;
401394
}
402395

403-
private Object getConfig() {
404-
return config;
405-
}
406-
407396
@VisibleForTesting
408397
public final ResolvedAddresses getResolvedAddresses() {
409398
return resolvedAddresses;

‎util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,8 @@ private SubchannelPicker createReadyPicker(Collection<ChildLbState> children) {
9696
}
9797

9898
@Override
99-
protected ChildLbState createChildLbState(Object key, Object policyConfig,
100-
ResolvedAddresses resolvedAddresses) {
101-
return new ChildLbState(key, pickFirstLbProvider, policyConfig) {
99+
protected ChildLbState createChildLbState(Object key) {
100+
return new ChildLbState(key, pickFirstLbProvider) {
102101
@Override
103102
protected ChildLbStateHelper createChildHelper() {
104103
return new ChildLbStateHelper() {

‎xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ class ClusterManagerLoadBalancer extends MultiChildLoadBalancer {
7070
}
7171

7272
@Override
73-
protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses,
74-
Object unusedChildConfig) {
73+
protected ResolvedAddresses getChildAddresses(Object key, ResolvedAddresses resolvedAddresses) {
7574
ClusterManagerConfig config = (ClusterManagerConfig)
7675
resolvedAddresses.getLoadBalancingPolicyConfig();
7776
Object childConfig = config.childPolicies.get(key);
@@ -87,7 +86,7 @@ protected Map<Object, ChildLbState> createChildLbMap(ResolvedAddresses resolvedA
8786
for (String key : config.childPolicies.keySet()) {
8887
ChildLbState child = getChildLbState(key);
8988
if (child == null) {
90-
child = new ClusterManagerLbState(key, GracefulSwitchLoadBalancerFactory.INSTANCE, null);
89+
child = new ClusterManagerLbState(key, GracefulSwitchLoadBalancerFactory.INSTANCE);
9190
}
9291
newChildPolicies.put(key, child);
9392
}
@@ -204,9 +203,8 @@ private class ClusterManagerLbState extends ChildLbState {
204203
@Nullable
205204
ScheduledHandle deletionTimer;
206205

207-
public ClusterManagerLbState(Object key, LoadBalancer.Factory policyFactory,
208-
Object childConfig) {
209-
super(key, policyFactory, childConfig);
206+
public ClusterManagerLbState(Object key, LoadBalancer.Factory policyFactory) {
207+
super(key, policyFactory);
210208
}
211209

212210
@Override

‎xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,8 @@ protected void updateOverallBalancingState() {
126126
}
127127

128128
@Override
129-
protected ChildLbState createChildLbState(Object key, Object policyConfig,
130-
ResolvedAddresses unused) {
131-
return new LeastRequestLbState(key, pickFirstLbProvider, policyConfig);
129+
protected ChildLbState createChildLbState(Object key) {
130+
return new LeastRequestLbState(key, pickFirstLbProvider);
132131
}
133132

134133
private void updateBalancingState(ConnectivityState state, SubchannelPicker picker) {
@@ -320,9 +319,8 @@ public String toString() {
320319
protected class LeastRequestLbState extends ChildLbState {
321320
private final AtomicInteger activeRequests = new AtomicInteger(0);
322321

323-
public LeastRequestLbState(Object key, LoadBalancerProvider policyProvider,
324-
Object childConfig) {
325-
super(key, policyProvider, childConfig);
322+
public LeastRequestLbState(Object key, LoadBalancerProvider policyProvider) {
323+
super(key, policyProvider);
326324
}
327325

328326
int getActiveRequests() {

‎xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,8 @@ protected void updateOverallBalancingState() {
219219
}
220220

221221
@Override
222-
protected ChildLbState createChildLbState(Object key, Object policyConfig,
223-
ResolvedAddresses resolvedAddresses) {
224-
return new ChildLbState(key, lazyLbFactory, null);
222+
protected ChildLbState createChildLbState(Object key) {
223+
return new ChildLbState(key, lazyLbFactory);
225224
}
226225

227226
private Status validateAddrList(List<EquivalentAddressGroup> addrList) {

‎xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,8 @@ public WeightedRoundRobinLoadBalancer(Helper helper, Ticker ticker) {
148148
}
149149

150150
@Override
151-
protected ChildLbState createChildLbState(Object key, Object policyConfig,
152-
ResolvedAddresses unused) {
153-
ChildLbState childLbState = new WeightedChildLbState(key, pickFirstLbProvider, policyConfig);
154-
return childLbState;
151+
protected ChildLbState createChildLbState(Object key) {
152+
return new WeightedChildLbState(key, pickFirstLbProvider);
155153
}
156154

157155
@Override
@@ -289,9 +287,8 @@ final class WeightedChildLbState extends ChildLbState {
289287

290288
private OrcaReportListener orcaReportListener;
291289

292-
public WeightedChildLbState(
293-
Object key, LoadBalancerProvider policyProvider, Object childConfig) {
294-
super(key, policyProvider, childConfig);
290+
public WeightedChildLbState(Object key, LoadBalancerProvider policyProvider) {
291+
super(key, policyProvider);
295292
}
296293

297294
@Override

0 commit comments

Comments
 (0)