Skip to content

Commit 82ca98f

Browse files
authored
xds:fix cancel xds watcher accidentally removes the url (#9809)
Fix a bug. When any of the xds subscribers for a resource has the last watcher cancelled, the bug will accidentally remove that resource type from the map, which make xds stream not accepting response update for that resource type entirely(pass through, no ACK/NACK will send).
1 parent eb391fd commit 82ca98f

File tree

2 files changed

+31
-1
lines changed

2 files changed

+31
-1
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,12 +332,13 @@ public void run() {
332332
if (!subscriber.isWatched()) {
333333
subscriber.cancelResourceWatch();
334334
resourceSubscribers.get(type).remove(resourceName);
335-
subscribedResourceTypeUrls.remove(type.typeUrl());
336335
if (subscriber.xdsChannel != null) {
337336
subscriber.xdsChannel.adjustResourceSubscription(type);
338337
}
339338
if (resourceSubscribers.get(type).isEmpty()) {
340339
resourceSubscribers.remove(type);
340+
subscribedResourceTypeUrls.remove(type.typeUrl());
341+
341342
}
342343
}
343344
}

xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,35 @@ public void ldsResourceUpdated() {
938938
assertThat(channelForEmptyAuthority).isNull();
939939
}
940940

941+
@Test
942+
public void cancelResourceWatcherNotRemoveUrlSubscribers() {
943+
DiscoveryRpcCall call = startResourceWatcher(XdsListenerResource.getInstance(), LDS_RESOURCE,
944+
ldsResourceWatcher);
945+
verifyResourceMetadataRequested(LDS, LDS_RESOURCE);
946+
947+
// Initial LDS response.
948+
call.sendResponse(LDS, testListenerVhosts, VERSION_1, "0000");
949+
call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE);
950+
verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture());
951+
verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue());
952+
verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT);
953+
954+
xdsClient.watchXdsResource(XdsListenerResource.getInstance(),
955+
LDS_RESOURCE + "1", ldsResourceWatcher);
956+
xdsClient.cancelXdsResourceWatch(XdsListenerResource.getInstance(), LDS_RESOURCE + "1",
957+
ldsResourceWatcher);
958+
959+
// Updated LDS response.
960+
Any testListenerVhosts2 = Any.pack(mf.buildListenerWithApiListener(LDS_RESOURCE,
961+
mf.buildRouteConfiguration("new", mf.buildOpaqueVirtualHosts(VHOST_SIZE))));
962+
call.sendResponse(LDS, testListenerVhosts2, VERSION_2, "0001");
963+
call.verifyRequest(LDS, LDS_RESOURCE, VERSION_2, "0001", NODE);
964+
verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture());
965+
verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue());
966+
verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts2, VERSION_2,
967+
TIME_INCREMENT * 2);
968+
}
969+
941970
@Test
942971
public void ldsResourceUpdated_withXdstpResourceName() {
943972
BootstrapperImpl.enableFederation = true;

0 commit comments

Comments
 (0)