From d007af18e6d3fb18bd97f13ffa84e5467fbef879 Mon Sep 17 00:00:00 2001
From: Chengyuan Zhang <chengyuanzhang@google.com>
Date: Mon, 16 Dec 2019 10:43:24 -0800
Subject: [PATCH] xds: fix bug of concluding retry state without checking retry
 task status (#6519)

---
 .../main/java/io/grpc/xds/XdsClientImpl.java  | 10 +++++-----
 .../java/io/grpc/xds/XdsClientImplTest.java   | 20 ++++++++++++++++---
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java
index 9ed76dd7c7..6ed68f19b1 100644
--- a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java
+++ b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java
@@ -181,7 +181,7 @@ final class XdsClientImpl extends XdsClient {
     } else {
       ldsResourceName = hostName + ":" + port;
     }
-    if (rpcRetryTimer != null) {
+    if (rpcRetryTimer != null && rpcRetryTimer.isPending()) {
       // Currently in retry backoff.
       return;
     }
@@ -210,7 +210,7 @@ final class XdsClientImpl extends XdsClient {
     if (clusterNamesToClusterUpdates.containsKey(clusterName)) {
       watcher.onClusterChanged(clusterNamesToClusterUpdates.get(clusterName));
     }
-    if (rpcRetryTimer != null) {
+    if (rpcRetryTimer != null && rpcRetryTimer.isPending()) {
       // Currently in retry backoff.
       return;
     }
@@ -241,7 +241,7 @@ final class XdsClientImpl extends XdsClient {
       }
       // No longer interested in this cluster, send an updated CDS request to unsubscribe
       // this resource.
-      if (rpcRetryTimer != null) {
+      if (rpcRetryTimer != null && rpcRetryTimer.isPending()) {
         // Currently in retry backoff.
         return;
       }
@@ -271,7 +271,7 @@ final class XdsClientImpl extends XdsClient {
     if (clusterNamesToEndpointUpdates.containsKey(clusterName)) {
       watcher.onEndpointChanged(clusterNamesToEndpointUpdates.get(clusterName));
     }
-    if (rpcRetryTimer != null) {
+    if (rpcRetryTimer != null && rpcRetryTimer.isPending()) {
       // Currently in retry backoff.
       return;
     }
@@ -299,7 +299,7 @@ final class XdsClientImpl extends XdsClient {
       clusterNamesToEndpointUpdates.remove(clusterName);
       // No longer interested in this cluster, send an updated EDS request to unsubscribe
       // this resource.
-      if (rpcRetryTimer != null) {
+      if (rpcRetryTimer != null && rpcRetryTimer.isPending()) {
         // Currently in retry backoff.
         return;
       }
diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java
index 38893f81d3..869ddc4d1e 100644
--- a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java
+++ b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java
@@ -2543,6 +2543,20 @@ public class XdsClientImplTest {
 
     // Client sent an CDS ACK request (Omitted).
 
+    // No longer interested in endpoint information after RPC resumes.
+    xdsClient.cancelEndpointDataWatch("cluster.googleapis.com", endpointWatcher);
+    // Client updates EDS resource subscription immediately.
+    verify(requestObserver)
+        .onNext(eq(buildDiscoveryRequest(NODE, "", ImmutableList.<String>of(),
+            XdsClientImpl.ADS_TYPE_URL_EDS, "")));
+
+    // Become interested in endpoints of another cluster.
+    xdsClient.watchEndpointData("cluster2.googleapis.com", endpointWatcher);
+    // Client updates EDS resource subscription immediately.
+    verify(requestObserver)
+        .onNext(eq(buildDiscoveryRequest(NODE, "", "cluster2.googleapis.com",
+            XdsClientImpl.ADS_TYPE_URL_EDS, "")));
+
     // Management server closes the RPC stream again.
     responseObserver.onCompleted();
 
@@ -2560,7 +2574,7 @@ public class XdsClientImplTest {
         .onNext(eq(buildDiscoveryRequest(NODE, "", "cluster.googleapis.com",
             XdsClientImpl.ADS_TYPE_URL_CDS, "")));
     verify(requestObserver)
-        .onNext(eq(buildDiscoveryRequest(NODE, "", "cluster.googleapis.com",
+        .onNext(eq(buildDiscoveryRequest(NODE, "", "cluster2.googleapis.com",
             XdsClientImpl.ADS_TYPE_URL_EDS, "")));
 
     // Management server becomes unreachable again.
@@ -2570,7 +2584,7 @@ public class XdsClientImplTest {
 
     // No longer interested in previous cluster and endpoints in that cluster.
     xdsClient.cancelClusterDataWatch("cluster.googleapis.com", clusterWatcher);
-    xdsClient.cancelEndpointDataWatch("cluster.googleapis.com", endpointWatcher);
+    xdsClient.cancelEndpointDataWatch("cluster2.googleapis.com", endpointWatcher);
 
     // Retry after backoff.
     fakeClock.forwardNanos(19L);
@@ -2587,7 +2601,7 @@ public class XdsClientImplTest {
         .onNext(eq(buildDiscoveryRequest(NODE, "", "cluster.googleapis.com",
             XdsClientImpl.ADS_TYPE_URL_CDS, "")));
     verify(requestObserver, never())
-        .onNext(eq(buildDiscoveryRequest(NODE, "", "cluster.googleapis.com",
+        .onNext(eq(buildDiscoveryRequest(NODE, "", "cluster2.googleapis.com",
             XdsClientImpl.ADS_TYPE_URL_EDS, "")));
 
     verifyNoMoreInteractions(mockedDiscoveryService, backoffPolicyProvider, backoffPolicy1,