Skip to content

Commit 6bad600

Browse files
authored
xds: Use getWatchers more often in XdsDepManager
This provides better type and missing-map handling. Note that getWatchers() now implicitly creates the map if it doesn't exist, instead of just returning an empty map. That makes it a bit easier to use and more importantly avoids accidents where a bug tries to modify the immutable map.
1 parent 379fbaa commit 6bad600

File tree

1 file changed

+25
-48
lines changed

1 file changed

+25
-48
lines changed

xds/src/main/java/io/grpc/xds/XdsDependencyManager.java

Lines changed: 25 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,7 @@ private <T extends ResourceUpdate> void addWatcher(XdsWatcherBase<T> watcher) {
112112
XdsResourceType<T> type = watcher.type;
113113
String resourceName = watcher.resourceName;
114114

115-
@SuppressWarnings("unchecked")
116-
TypeWatchers<T> typeWatchers = (TypeWatchers<T>)resourceWatchers.get(type);
117-
if (typeWatchers == null) {
118-
typeWatchers = new TypeWatchers<>(type);
119-
resourceWatchers.put(type, typeWatchers);
120-
}
121-
122-
typeWatchers.add(resourceName, watcher);
115+
getWatchers(type).put(resourceName, watcher);
123116
xdsClient.watchXdsResource(type, resourceName, watcher, syncContext);
124117
}
125118

@@ -158,16 +151,12 @@ private <T extends ResourceUpdate> void cancelWatcher(XdsWatcherBase<T> watcher)
158151
XdsResourceType<T> type = watcher.type;
159152
String resourceName = watcher.resourceName;
160153

161-
@SuppressWarnings("unchecked")
162-
TypeWatchers<T> typeWatchers = (TypeWatchers<T>)resourceWatchers.get(type);
163-
if (typeWatchers == null) {
164-
logger.log(DEBUG, "Trying to cancel watcher {0}, but type not watched", watcher);
154+
if (getWatchers(type).remove(resourceName) == null) {
155+
logger.log(DEBUG, "Trying to cancel watcher {0}, but it isn't watched", watcher);
165156
return;
166157
}
167158

168-
typeWatchers.watchers.remove(resourceName);
169159
xdsClient.cancelXdsResourceWatch(type, resourceName, watcher);
170-
171160
}
172161

173162
private static void throwIfParentContextsNotEmpty(XdsWatcherBase<?> watcher) {
@@ -213,8 +202,8 @@ private void releaseSubscription(ClusterSubscription subscription) {
213202
return;
214203
}
215204
subscription.closed = true;
216-
XdsWatcherBase<?> cdsWatcher =
217-
resourceWatchers.get(CLUSTER_RESOURCE).watchers.get(clusterName);
205+
XdsWatcherBase<XdsClusterResource.CdsUpdate> cdsWatcher
206+
= getWatchers(CLUSTER_RESOURCE).get(clusterName);
218207
if (cdsWatcher == null) {
219208
return; // shutdown() called
220209
}
@@ -236,14 +225,12 @@ private void cancelClusterWatcherTree(CdsWatcher root, Object parentContext) {
236225
switch (cdsUpdate.clusterType()) {
237226
case EDS:
238227
String edsServiceName = root.getEdsServiceName();
239-
EdsWatcher edsWatcher =
240-
(EdsWatcher) resourceWatchers.get(ENDPOINT_RESOURCE).watchers.get(edsServiceName);
228+
EdsWatcher edsWatcher = (EdsWatcher) getWatchers(ENDPOINT_RESOURCE).get(edsServiceName);
241229
cancelEdsWatcher(edsWatcher, root);
242230
break;
243231
case AGGREGATE:
244232
for (String cluster : cdsUpdate.prioritizedClusterNames()) {
245-
CdsWatcher clusterWatcher =
246-
(CdsWatcher) resourceWatchers.get(CLUSTER_RESOURCE).watchers.get(cluster);
233+
CdsWatcher clusterWatcher = (CdsWatcher) getWatchers(CLUSTER_RESOURCE).get(cluster);
247234
if (clusterWatcher != null) {
248235
cancelClusterWatcherTree(clusterWatcher, root);
249236
}
@@ -348,7 +335,8 @@ private <T extends ResourceUpdate> Map<String, XdsWatcherBase<T>> getWatchers(
348335
XdsResourceType<T> resourceType) {
349336
TypeWatchers<?> typeWatchers = resourceWatchers.get(resourceType);
350337
if (typeWatchers == null) {
351-
return Collections.emptyMap();
338+
typeWatchers = new TypeWatchers<T>(resourceType);
339+
resourceWatchers.put(resourceType, typeWatchers);
352340
}
353341
assert typeWatchers.resourceType == resourceType;
354342
@SuppressWarnings("unchecked")
@@ -470,25 +458,22 @@ public String toString() {
470458

471459
// Returns true if the watcher was added, false if it already exists
472460
private boolean addEdsWatcher(String edsServiceName, CdsWatcher parentContext) {
473-
TypeWatchers<?> typeWatchers = resourceWatchers.get(XdsEndpointResource.getInstance());
474-
if (typeWatchers == null || !typeWatchers.watchers.containsKey(edsServiceName)) {
475-
addWatcher(new EdsWatcher(edsServiceName, parentContext));
476-
return true;
461+
EdsWatcher watcher
462+
= (EdsWatcher) getWatchers(XdsEndpointResource.getInstance()).get(edsServiceName);
463+
if (watcher != null) {
464+
watcher.addParentContext(parentContext); // Is a set, so don't need to check for existence
465+
return false;
477466
}
478467

479-
EdsWatcher watcher = (EdsWatcher) typeWatchers.watchers.get(edsServiceName);
480-
watcher.addParentContext(parentContext); // Is a set, so don't need to check for existence
481-
return false;
468+
addWatcher(new EdsWatcher(edsServiceName, parentContext));
469+
return true;
482470
}
483471

484472
private void addClusterWatcher(String clusterName, Object parentContext, int depth) {
485-
TypeWatchers<?> clusterWatchers = resourceWatchers.get(CLUSTER_RESOURCE);
486-
if (clusterWatchers != null) {
487-
CdsWatcher watcher = (CdsWatcher) clusterWatchers.watchers.get(clusterName);
488-
if (watcher != null) {
489-
watcher.parentContexts.put(parentContext, depth);
490-
return;
491-
}
473+
CdsWatcher watcher = (CdsWatcher) getWatchers(CLUSTER_RESOURCE).get(clusterName);
474+
if (watcher != null) {
475+
watcher.parentContexts.put(parentContext, depth);
476+
return;
492477
}
493478

494479
addWatcher(new CdsWatcher(clusterName, parentContext, depth));
@@ -546,7 +531,7 @@ private static Set<String> getClusterNamesFromVirtualHost(VirtualHost virtualHos
546531
}
547532

548533
private CdsWatcher getCluster(String clusterName) {
549-
return (CdsWatcher) resourceWatchers.get(CLUSTER_RESOURCE).watchers.get(clusterName);
534+
return (CdsWatcher) getWatchers(CLUSTER_RESOURCE).get(clusterName);
550535
}
551536

552537
private static class TypeWatchers<T extends ResourceUpdate> {
@@ -557,10 +542,6 @@ private static class TypeWatchers<T extends ResourceUpdate> {
557542
TypeWatchers(XdsResourceType<T> resourceType) {
558543
this.resourceType = resourceType;
559544
}
560-
561-
public void add(String resourceName, XdsWatcherBase<T> watcher) {
562-
watchers.put(resourceName, watcher);
563-
}
564545
}
565546

566547
public interface XdsConfigWatcher {
@@ -738,11 +719,11 @@ private void cleanUpRdsWatcher() {
738719
logger.log(XdsLogger.XdsLogLevel.DEBUG, "Stop watching RDS resource {0}", rdsName);
739720

740721
// Cleanup clusters (as appropriate) that had the old rds watcher as a parent
741-
if (!oldRdsWatcher.hasDataValue() || resourceWatchers.get(CLUSTER_RESOURCE) == null) {
722+
if (!oldRdsWatcher.hasDataValue()) {
742723
return;
743724
}
744-
for (XdsWatcherBase<?> watcher :
745-
resourceWatchers.get(CLUSTER_RESOURCE).watchers.values()) {
725+
for (XdsWatcherBase<XdsClusterResource.CdsUpdate> watcher :
726+
getWatchers(CLUSTER_RESOURCE).values()) {
746727
cancelCdsWatcher((CdsWatcher) watcher, oldRdsWatcher);
747728
}
748729
}
@@ -752,11 +733,7 @@ private RdsWatcher getRdsWatcher() {
752733
if (rdsName == null) {
753734
return null;
754735
}
755-
TypeWatchers<?> watchers = resourceWatchers.get(XdsRouteConfigureResource.getInstance());
756-
if (watchers == null) {
757-
return null;
758-
}
759-
return (RdsWatcher) watchers.watchers.get(rdsName);
736+
return (RdsWatcher) getWatchers(XdsRouteConfigureResource.getInstance()).get(rdsName);
760737
}
761738

762739
public RdsUpdateSupplier getRouteSource() {

0 commit comments

Comments
 (0)