Avoid multiple picker updates when resetting backoffs when RLS control plane transitions to READY.

Author: kannanjgithub

rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java

@@ -226,11 +226,18 @@ public void run() {
         } else if (wasInTransientFailure && currentState == ConnectivityState.READY) {
           wasInTransientFailure = false;
           synchronized (lock) {
+            boolean anyBackoffsCanceled = false;
             for (CacheEntry value : linkedHashLruCache.values()) {
               if (value instanceof BackoffCacheEntry) {
-                refreshBackoffEntry((BackoffCacheEntry) value);
+                if (((BackoffCacheEntry) value).scheduledFuture.cancel(false)) {
+                  anyBackoffsCanceled = true;
+                }
               }
             }
+            if (anyBackoffsCanceled) {
+              // Cache updated. updateBalancingState() to reattempt picks
+              helper.triggerPendingRpcProcessing();
+            }
           }
         }
         rlsChannel.notifyWhenStateChanged(currentState, this);

rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java

@@ -484,12 +484,18 @@ public void run() {
     fakeThrottler.nextResult = true;
     fakeBackoffProvider.nextPolicy = createBackoffPolicy(10, TimeUnit.MILLISECONDS);
 
-    // Cause a cache miss by using a new request key. This will create a back-off Rls cache entry.
+    // Cause two cache misses by using new request keys. This will create back-off Rls cache
+    // entries. RLS control plane state transitioning to READY should reset both back-offs but
+    // update picker only once.
     RlsProtoData.RouteLookupRequestKey routeLookupRequestKey2 =
         RlsProtoData.RouteLookupRequestKey.create(ImmutableMap.of(
         "server", "bigtable.googleapis.com", "service-key", "foo2", "method-key", "bar"));
     resp = getInSyncContext(routeLookupRequestKey2);
-
+    assertThat(resp.hasError()).isTrue();
+    RlsProtoData.RouteLookupRequestKey routeLookupRequestKey3 =
+        RlsProtoData.RouteLookupRequestKey.create(ImmutableMap.of(
+            "server", "bigtable.googleapis.com", "service-key", "foo3", "method-key", "bar"));
+    resp = getInSyncContext(routeLookupRequestKey3);
     assertThat(resp.hasError()).isTrue();
 
     fakeHelper.createServerAndRegister("service1");
@@ -508,20 +514,24 @@ public void run() {
       ArgumentCaptor<ConnectivityState> stateCaptor =
           ArgumentCaptor.forClass(ConnectivityState.class);
 
-      inOrder.verify(helper, times(5))
+      inOrder.verify(helper, times(4))
           .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture());
       assertThat(new HashSet<>(pickerCaptor.getAllValues())).hasSize(1);
       assertThat(stateCaptor.getAllValues())
           .containsExactly(ConnectivityState.TRANSIENT_FAILURE, ConnectivityState.CONNECTING,
-              ConnectivityState.CONNECTING, ConnectivityState.CONNECTING,
-              ConnectivityState.CONNECTING);
+              ConnectivityState.CONNECTING, ConnectivityState.CONNECTING);
       Metadata headers = new Metadata();
       PickResult pickResult = getPickResultForCreate(pickerCaptor, headers);
       assertThat(pickResult.getStatus().getCode()).isEqualTo(Status.Code.UNAVAILABLE);
       assertThat(pickResult.getStatus().getDescription()).isEqualTo("fallback not available");
       isSuccess.set(true);
     }).get();
     assertThat(isSuccess.get()).isTrue();
+
+    fakeThrottler.nextResult = false;
+    // Rpcs are not backed off now.
+    assertThat(getInSyncContext(routeLookupRequestKey2).isPending()).isTrue();
+    assertThat(getInSyncContext(routeLookupRequestKey3).isPending()).isTrue();
   }
 
   @Test