Skip to content

Commit c87fc05

Browse files
YifeiZhuangYifeiZhuang
authored
xds: remove retained resources logics for RDS and EDS resources (#9724)
We use state-of-the-world approach. For LDS/CDS, the control plane must return all resources that the client has subscribed to in each request. If some LDS/CDS resources are gone in a new update, their corresponding RDS/EDS resources names will be onAbsent(), unless there is cached data that is in use by other subscribers in other components. The motivations to remove this "retained resource" logic between resource types are: 1. Already handled by the subscribers, e.g. a CDS state would shut down its childLBs on new updates. XdsResolver for LdsUpdate would cancel all existing RDS subscriptions. Therefore the onAbsent() notification is effectively no-op. 2. Complexity.
1 parent 9dac8cf commit c87fc05

File tree

8 files changed

+94
-177
lines changed

8 files changed

+94
-177
lines changed

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

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@
4848
import io.grpc.xds.LoadStatsManager2.ClusterLocalityStats;
4949
import io.grpc.xds.XdsClient.ResourceStore;
5050
import io.grpc.xds.XdsClient.XdsResponseHandler;
51-
import io.grpc.xds.XdsClusterResource.CdsUpdate;
52-
import io.grpc.xds.XdsListenerResource.LdsUpdate;
5351
import io.grpc.xds.XdsLogger.XdsLogLevel;
5452
import java.net.URI;
5553
import java.util.Collection;
@@ -107,7 +105,6 @@ public void uncaughtException(Thread t, Throwable e) {
107105
private final XdsLogger logger;
108106
private volatile boolean isShutdown;
109107

110-
// TODO(zdapeng): rename to XdsClientImpl
111108
XdsClientImpl(
112109
XdsChannelFactory xdsChannelFactory,
113110
Bootstrapper.BootstrapInfo bootstrapInfo,
@@ -398,7 +395,6 @@ private <T extends ResourceUpdate> void handleResourceUpdate(XdsResourceType.Arg
398395
xdsResourceType.typeName(), args.versionInfo, args.nonce, result.unpackedResources);
399396
Map<String, ParsedResource<T>> parsedResources = result.parsedResources;
400397
Set<String> invalidResources = result.invalidResources;
401-
Set<String> retainedResources = result.retainedResources;
402398
List<String> errors = result.errors;
403399
String errorDetail = null;
404400
if (errors.isEmpty()) {
@@ -432,16 +428,14 @@ private <T extends ResourceUpdate> void handleResourceUpdate(XdsResourceType.Arg
432428
}
433429

434430
// Nothing else to do for incremental ADS resources.
435-
if (xdsResourceType.dependentResource() == null) {
431+
if (!xdsResourceType.isFullStateOfTheWorld()) {
436432
continue;
437433
}
438434

439435
// Handle State of the World ADS: invalid resources.
440436
if (invalidResources.contains(resourceName)) {
441437
// The resource is missing. Reuse the cached resource if possible.
442-
if (subscriber.data != null) {
443-
retainDependentResource(subscriber, retainedResources);
444-
} else {
438+
if (subscriber.data == null) {
445439
// No cached data. Notify the watchers of an invalid update.
446440
subscriber.onError(Status.UNAVAILABLE.withDescription(errorDetail));
447441
}
@@ -451,49 +445,6 @@ private <T extends ResourceUpdate> void handleResourceUpdate(XdsResourceType.Arg
451445
// For State of the World services, notify watchers when their watched resource is missing
452446
// from the ADS update.
453447
subscriber.onAbsent();
454-
// Retain any dependent resources if the resource deletion is ignored
455-
// per bootstrap ignore_resource_deletion server feature.
456-
if (!subscriber.absent) {
457-
retainDependentResource(subscriber, retainedResources);
458-
}
459-
}
460-
461-
// LDS/CDS responses represents the state of the world, RDS/EDS resources not referenced in
462-
// LDS/CDS resources should be deleted.
463-
if (xdsResourceType.dependentResource() != null) {
464-
XdsResourceType<?> dependency = xdsResourceType.dependentResource();
465-
Map<String, ResourceSubscriber<? extends ResourceUpdate>> dependentSubscribers =
466-
resourceSubscribers.get(dependency);
467-
if (dependentSubscribers == null) {
468-
return;
469-
}
470-
for (String resource : dependentSubscribers.keySet()) {
471-
if (!retainedResources.contains(resource)) {
472-
dependentSubscribers.get(resource).onAbsent();
473-
}
474-
}
475-
}
476-
}
477-
478-
private void retainDependentResource(
479-
ResourceSubscriber<? extends ResourceUpdate> subscriber, Set<String> retainedResources) {
480-
if (subscriber.data == null) {
481-
return;
482-
}
483-
String resourceName = null;
484-
if (subscriber.type == XdsListenerResource.getInstance()) {
485-
LdsUpdate ldsUpdate = (LdsUpdate) subscriber.data;
486-
io.grpc.xds.HttpConnectionManager hcm = ldsUpdate.httpConnectionManager();
487-
if (hcm != null) {
488-
resourceName = hcm.rdsName();
489-
}
490-
} else if (subscriber.type == XdsClusterResource.getInstance()) {
491-
CdsUpdate cdsUpdate = (CdsUpdate) subscriber.data;
492-
resourceName = cdsUpdate.edsServiceName();
493-
}
494-
495-
if (resourceName != null) {
496-
retainedResources.add(resourceName);
497448
}
498449
}
499450

@@ -657,9 +608,7 @@ void onAbsent() {
657608
// and the resource is reusable.
658609
boolean ignoreResourceDeletionEnabled =
659610
serverInfo != null && serverInfo.ignoreResourceDeletion();
660-
boolean isStateOfTheWorld = (type == XdsListenerResource.getInstance()
661-
|| type == XdsClusterResource.getInstance());
662-
if (ignoreResourceDeletionEnabled && isStateOfTheWorld && data != null) {
611+
if (ignoreResourceDeletionEnabled && type.isFullStateOfTheWorld() && data != null) {
663612
if (!resourceDeletionIgnored) {
664613
logger.log(XdsLogLevel.FORCE_WARNING,
665614
"xds server {0}: ignoring deletion for resource type {1} name {2}}",

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

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,9 @@ String typeUrlV2() {
8888
return ADS_TYPE_URL_CDS_V2;
8989
}
9090

91-
@Nullable
9291
@Override
93-
XdsResourceType<?> dependentResource() {
94-
return XdsEndpointResource.getInstance();
92+
boolean isFullStateOfTheWorld() {
93+
return true;
9594
}
9695

9796
@Override
@@ -101,8 +100,7 @@ Class<Cluster> unpackedClassName() {
101100
}
102101

103102
@Override
104-
CdsUpdate doParse(Args args, Message unpackedMessage,
105-
Set<String> retainedResources, boolean isResourceV3)
103+
CdsUpdate doParse(Args args, Message unpackedMessage, boolean isResourceV3)
106104
throws ResourceInvalidException {
107105
if (!(unpackedMessage instanceof Cluster)) {
108106
throw new ResourceInvalidException("Invalid message type: " + unpackedMessage.getClass());
@@ -111,20 +109,20 @@ CdsUpdate doParse(Args args, Message unpackedMessage,
111109
if (args.bootstrapInfo != null && args.bootstrapInfo.certProviders() != null) {
112110
certProviderInstances = args.bootstrapInfo.certProviders().keySet();
113111
}
114-
return processCluster((Cluster) unpackedMessage, retainedResources, certProviderInstances,
112+
return processCluster((Cluster) unpackedMessage, certProviderInstances,
115113
args.serverInfo, args.loadBalancerRegistry);
116114
}
117115

118116
@VisibleForTesting
119-
static CdsUpdate processCluster(Cluster cluster, Set<String> retainedEdsResources,
117+
static CdsUpdate processCluster(Cluster cluster,
120118
Set<String> certProviderInstances,
121119
Bootstrapper.ServerInfo serverInfo,
122120
LoadBalancerRegistry loadBalancerRegistry)
123121
throws ResourceInvalidException {
124122
StructOrError<CdsUpdate.Builder> structOrError;
125123
switch (cluster.getClusterDiscoveryTypeCase()) {
126124
case TYPE:
127-
structOrError = parseNonAggregateCluster(cluster, retainedEdsResources,
125+
structOrError = parseNonAggregateCluster(cluster,
128126
certProviderInstances, serverInfo);
129127
break;
130128
case CLUSTER_TYPE:
@@ -178,8 +176,7 @@ private static StructOrError<CdsUpdate.Builder> parseAggregateCluster(Cluster cl
178176
}
179177

180178
private static StructOrError<CdsUpdate.Builder> parseNonAggregateCluster(
181-
Cluster cluster, Set<String> edsResources, Set<String> certProviderInstances,
182-
Bootstrapper.ServerInfo serverInfo) {
179+
Cluster cluster, Set<String> certProviderInstances, Bootstrapper.ServerInfo serverInfo) {
183180
String clusterName = cluster.getName();
184181
Bootstrapper.ServerInfo lrsServerInfo = null;
185182
Long maxConcurrentRequests = null;
@@ -249,9 +246,6 @@ private static StructOrError<CdsUpdate.Builder> parseNonAggregateCluster(
249246
// If the service_name field is set, that value will be used for the EDS request.
250247
if (!edsClusterConfig.getServiceName().isEmpty()) {
251248
edsServiceName = edsClusterConfig.getServiceName();
252-
edsResources.add(edsServiceName);
253-
} else {
254-
edsResources.add(clusterName);
255249
}
256250
return StructOrError.fromStruct(CdsUpdate.forEds(
257251
clusterName, edsServiceName, lrsServerInfo, maxConcurrentRequests, upstreamTlsContext,

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,9 @@ String typeUrlV2() {
7878
return ADS_TYPE_URL_EDS_V2;
7979
}
8080

81-
@Nullable
8281
@Override
83-
XdsResourceType<?> dependentResource() {
84-
return null;
82+
boolean isFullStateOfTheWorld() {
83+
return false;
8584
}
8685

8786
@Override
@@ -90,8 +89,7 @@ Class<ClusterLoadAssignment> unpackedClassName() {
9089
}
9190

9291
@Override
93-
EdsUpdate doParse(Args args, Message unpackedMessage,
94-
Set<String> retainedResources, boolean isResourceV3)
92+
EdsUpdate doParse(Args args, Message unpackedMessage, boolean isResourceV3)
9593
throws ResourceInvalidException {
9694
if (!(unpackedMessage instanceof ClusterLoadAssignment)) {
9795
throw new ResourceInvalidException("Invalid message type: " + unpackedMessage.getClass());

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

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -97,15 +97,13 @@ String typeUrlV2() {
9797
return ADS_TYPE_URL_LDS_V2;
9898
}
9999

100-
@Nullable
101100
@Override
102-
XdsResourceType<?> dependentResource() {
103-
return XdsRouteConfigureResource.getInstance();
101+
boolean isFullStateOfTheWorld() {
102+
return true;
104103
}
105104

106105
@Override
107-
LdsUpdate doParse(Args args, Message unpackedMessage, Set<String> retainedResources,
108-
boolean isResourceV3)
106+
LdsUpdate doParse(Args args, Message unpackedMessage, boolean isResourceV3)
109107
throws ResourceInvalidException {
110108
if (!(unpackedMessage instanceof Listener)) {
111109
throw new ResourceInvalidException("Invalid message type: " + unpackedMessage.getClass());
@@ -114,15 +112,14 @@ LdsUpdate doParse(Args args, Message unpackedMessage, Set<String> retainedResour
114112

115113
if (listener.hasApiListener()) {
116114
return processClientSideListener(
117-
listener, retainedResources, args, enableFaultInjection && isResourceV3);
115+
listener, args, enableFaultInjection && isResourceV3);
118116
} else {
119117
return processServerSideListener(
120-
listener, retainedResources, args, enableRbac && isResourceV3);
118+
listener, args, enableRbac && isResourceV3);
121119
}
122120
}
123121

124-
private LdsUpdate processClientSideListener(
125-
Listener listener, Set<String> rdsResources, Args args, boolean parseHttpFilter)
122+
private LdsUpdate processClientSideListener(Listener listener, Args args, boolean parseHttpFilter)
126123
throws ResourceInvalidException {
127124
// Unpack HttpConnectionManager from the Listener.
128125
HttpConnectionManager hcm;
@@ -135,24 +132,22 @@ private LdsUpdate processClientSideListener(
135132
"Could not parse HttpConnectionManager config from ApiListener", e);
136133
}
137134
return LdsUpdate.forApiListener(parseHttpConnectionManager(
138-
hcm, rdsResources, args.filterRegistry, parseHttpFilter, true /* isForClient */));
135+
hcm, args.filterRegistry, parseHttpFilter, true /* isForClient */));
139136
}
140137

141-
private LdsUpdate processServerSideListener(
142-
Listener proto, Set<String> rdsResources, Args args, boolean parseHttpFilter)
138+
private LdsUpdate processServerSideListener(Listener proto, Args args, boolean parseHttpFilter)
143139
throws ResourceInvalidException {
144140
Set<String> certProviderInstances = null;
145141
if (args.bootstrapInfo != null && args.bootstrapInfo.certProviders() != null) {
146142
certProviderInstances = args.bootstrapInfo.certProviders().keySet();
147143
}
148-
return LdsUpdate.forTcpListener(parseServerSideListener(
149-
proto, rdsResources, args.tlsContextManager, args.filterRegistry, certProviderInstances,
150-
parseHttpFilter));
144+
return LdsUpdate.forTcpListener(parseServerSideListener(proto, args.tlsContextManager,
145+
args.filterRegistry, certProviderInstances, parseHttpFilter));
151146
}
152147

153148
@VisibleForTesting
154149
static EnvoyServerProtoData.Listener parseServerSideListener(
155-
Listener proto, Set<String> rdsResources, TlsContextManager tlsContextManager,
150+
Listener proto, TlsContextManager tlsContextManager,
156151
FilterRegistry filterRegistry, Set<String> certProviderInstances, boolean parseHttpFilter)
157152
throws ResourceInvalidException {
158153
if (!proto.getTrafficDirection().equals(TrafficDirection.INBOUND)
@@ -190,13 +185,13 @@ static EnvoyServerProtoData.Listener parseServerSideListener(
190185
Set<FilterChainMatch> uniqueSet = new HashSet<>();
191186
for (io.envoyproxy.envoy.config.listener.v3.FilterChain fc : proto.getFilterChainsList()) {
192187
filterChains.add(
193-
parseFilterChain(fc, rdsResources, tlsContextManager, filterRegistry, uniqueSet,
188+
parseFilterChain(fc, tlsContextManager, filterRegistry, uniqueSet,
194189
certProviderInstances, parseHttpFilter));
195190
}
196191
FilterChain defaultFilterChain = null;
197192
if (proto.hasDefaultFilterChain()) {
198193
defaultFilterChain = parseFilterChain(
199-
proto.getDefaultFilterChain(), rdsResources, tlsContextManager, filterRegistry,
194+
proto.getDefaultFilterChain(), tlsContextManager, filterRegistry,
200195
null, certProviderInstances, parseHttpFilter);
201196
}
202197

@@ -206,7 +201,7 @@ static EnvoyServerProtoData.Listener parseServerSideListener(
206201

207202
@VisibleForTesting
208203
static FilterChain parseFilterChain(
209-
io.envoyproxy.envoy.config.listener.v3.FilterChain proto, Set<String> rdsResources,
204+
io.envoyproxy.envoy.config.listener.v3.FilterChain proto,
210205
TlsContextManager tlsContextManager, FilterRegistry filterRegistry,
211206
Set<FilterChainMatch> uniqueSet, Set<String> certProviderInstances, boolean parseHttpFilters)
212207
throws ResourceInvalidException {
@@ -235,7 +230,7 @@ static FilterChain parseFilterChain(
235230
+ filter.getName() + " failed to unpack message", e);
236231
}
237232
io.grpc.xds.HttpConnectionManager httpConnectionManager = parseHttpConnectionManager(
238-
hcmProto, rdsResources, filterRegistry, parseHttpFilters, false /* isForClient */);
233+
hcmProto, filterRegistry, parseHttpFilters, false /* isForClient */);
239234

240235
EnvoyServerProtoData.DownstreamTlsContext downstreamTlsContext = null;
241236
if (proto.hasTransportSocket()) {
@@ -466,7 +461,7 @@ private static FilterChainMatch parseFilterChainMatch(
466461

467462
@VisibleForTesting
468463
static io.grpc.xds.HttpConnectionManager parseHttpConnectionManager(
469-
HttpConnectionManager proto, Set<String> rdsResources, FilterRegistry filterRegistry,
464+
HttpConnectionManager proto, FilterRegistry filterRegistry,
470465
boolean parseHttpFilter, boolean isForClient) throws ResourceInvalidException {
471466
if (enableRbac && proto.getXffNumTrustedHops() != 0) {
472467
throw new ResourceInvalidException(
@@ -541,8 +536,6 @@ static io.grpc.xds.HttpConnectionManager parseHttpConnectionManager(
541536
throw new ResourceInvalidException(
542537
"HttpConnectionManager contains invalid RDS: must specify ADS or self ConfigSource");
543538
}
544-
// Collect the RDS resource referenced by this HttpConnectionManager.
545-
rdsResources.add(rds.getRouteConfigName());
546539
return io.grpc.xds.HttpConnectionManager.forRdsName(
547540
maxStreamDuration, rds.getRouteConfigName(), filterConfigs);
548541
}

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,12 @@ abstract class XdsResourceType<T extends ResourceUpdate> {
8585

8686
abstract String typeUrlV2();
8787

88-
// Non-null for State of the World resources.
89-
@Nullable
90-
abstract XdsResourceType<?> dependentResource();
88+
// Do not confuse with the SotW approach: it is the mechanism in which the client must specify all
89+
// resource names it is interested in with each request. Different resource types may behave
90+
// differently in this approach. For LDS and CDS resources, the server must return all resources
91+
// that the client has subscribed to in each request. For RDS and EDS, the server may only return
92+
// the resources that need an update.
93+
abstract boolean isFullStateOfTheWorld();
9194

9295
static class Args {
9396
final ServerInfo serverInfo;
@@ -125,7 +128,6 @@ ValidatedResourceUpdate<T> parse(Args args, List<Any> resources) {
125128
Set<String> unpackedResources = new HashSet<>(resources.size());
126129
Set<String> invalidResources = new HashSet<>();
127130
List<String> errors = new ArrayList<>();
128-
Set<String> retainedResources = new HashSet<>();
129131

130132
for (int i = 0; i < resources.size(); i++) {
131133
Any resource = resources.get(i);
@@ -156,7 +158,7 @@ ValidatedResourceUpdate<T> parse(Args args, List<Any> resources) {
156158

157159
T resourceUpdate;
158160
try {
159-
resourceUpdate = doParse(args, unpackedMessage, retainedResources, isResourceV3);
161+
resourceUpdate = doParse(args, unpackedMessage, isResourceV3);
160162
} catch (XdsClientImpl.ResourceInvalidException e) {
161163
errors.add(String.format("%s response %s '%s' validation error: %s",
162164
typeName(), unpackedClassName().getSimpleName(), cname, e.getMessage()));
@@ -168,12 +170,11 @@ ValidatedResourceUpdate<T> parse(Args args, List<Any> resources) {
168170
parsedResources.put(cname, new ParsedResource<T>(resourceUpdate, resource));
169171
}
170172
return new ValidatedResourceUpdate<T>(parsedResources, unpackedResources, invalidResources,
171-
errors, retainedResources);
173+
errors);
172174

173175
}
174176

175-
abstract T doParse(Args args, Message unpackedMessage, Set<String> retainedResources,
176-
boolean isResourceV3)
177+
abstract T doParse(Args args, Message unpackedMessage, boolean isResourceV3)
177178
throws ResourceInvalidException;
178179

179180
/**
@@ -231,19 +232,16 @@ static final class ValidatedResourceUpdate<T extends ResourceUpdate> {
231232
Set<String> unpackedResources;
232233
Set<String> invalidResources;
233234
List<String> errors;
234-
Set<String> retainedResources;
235235

236236
// validated resource update
237237
public ValidatedResourceUpdate(Map<String, ParsedResource<T>> parsedResources,
238238
Set<String> unpackedResources,
239239
Set<String> invalidResources,
240-
List<String> errors,
241-
Set<String> retainedResources) {
240+
List<String> errors) {
242241
this.parsedResources = parsedResources;
243242
this.unpackedResources = unpackedResources;
244243
this.invalidResources = invalidResources;
245244
this.errors = errors;
246-
this.retainedResources = retainedResources;
247245
}
248246
}
249247

0 commit comments

Comments
 (0)