From 7bf6e79e7e85a68cd6381c61dca10974ff2b2640 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 3 Jan 2025 12:34:47 -0800 Subject: [PATCH] xds: Preserve nonce when unsubscribing type This fixes a regression introduced in 19c9b998. b/374697875 --- .../java/io/grpc/xds/client/ControlPlaneClient.java | 10 +++++++--- .../java/io/grpc/xds/GrpcXdsClientImplTestBase.java | 5 ++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java b/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java index 62076fb8bf1..a97e6527992 100644 --- a/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java +++ b/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java @@ -156,9 +156,10 @@ void adjustResourceSubscription(XdsResourceType resourceType) { } adsStream.sendDiscoveryRequest(resourceType, resources); if (resources.isEmpty()) { - // The resource type no longer has subscribing resources; clean up references to it + // The resource type no longer has subscribing resources; clean up references to it, except + // for nonces. If the resource type becomes used again the control plane can ignore requests + // for old/missing nonces. Old type's nonces are dropped when the ADS stream is restarted. versions.remove(resourceType); - adsStream.respNonces.remove(resourceType); } } @@ -284,7 +285,10 @@ private class AdsStream implements EventHandler { // Nonce in each response is echoed back in the following ACK/NACK request. It is // used for management server to identify which response the client is ACKing/NACking. // To avoid confusion, client-initiated requests will always use the nonce in - // most recently received responses of each resource type. + // most recently received responses of each resource type. Nonces are never deleted from the + // map; nonces are only discarded once the stream closes because xds_protocol says "the + // management server should not send a DiscoveryResponse for any DiscoveryRequest that has a + // stale nonce." private final Map, String> respNonces = new HashMap<>(); private final StreamingCall call; private final MethodDescriptor methodDescriptor = diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index 198faea7fdc..025c0a600f3 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -2899,10 +2899,9 @@ public void edsCleanupNonceAfterUnsubscription() { verifySubscribedResourcesMetadataSizes(0, 0, 0, 0); call.verifyRequest(EDS, Arrays.asList(), VERSION_1, "0000", NODE); - // When re-subscribing, the version and nonce were properly forgotten, so the request is the - // same as the initial request + // When re-subscribing, the version was forgotten but not the nonce xdsClient.watchXdsResource(XdsEndpointResource.getInstance(), "A.1", edsResourceWatcher); - call.verifyRequest(EDS, "A.1", "", "", NODE, Mockito.timeout(2000).times(2)); + call.verifyRequest(EDS, "A.1", "", "0000", NODE, Mockito.timeout(2000)); } @Test