xds: populate envoy RetryPolicy with no retryOn to resolver

grpc · Sep 10, 2021 · 1190e84 · 1190e84
1 parent 7ad7876
commit 1190e84
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 11 deletions.
diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java
@@ -1281,13 +1281,10 @@ private static StructOrError<RetryPolicy> parseRetryPolicy(
       retryableStatusCodesBuilder.add(code);
     }
     List<Code> retryableStatusCodes = retryableStatusCodesBuilder.build();
-    if (!retryableStatusCodes.isEmpty()) {
-      return StructOrError.fromStruct(
-          RetryPolicy.create(
-              maxAttempts, retryableStatusCodes, initialBackoff, maxBackoff,
-              /* perAttemptRecvTimeout= */ null));
-    }
-    return null;
+    return StructOrError.fromStruct(
+        RetryPolicy.create(
+            maxAttempts, retryableStatusCodes, initialBackoff, maxBackoff,
+            /* perAttemptRecvTimeout= */ null));
   }
 
   @VisibleForTesting

diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java
@@ -182,13 +182,14 @@ public void shutdown() {
   @VisibleForTesting
   static Map<String, ?> generateServiceConfigWithMethodConfig(
       @Nullable Long timeoutNano, @Nullable RetryPolicy retryPolicy) {
-    if (timeoutNano == null && retryPolicy == null) {
+    if (timeoutNano == null
+        && (retryPolicy == null || retryPolicy.retryableStatusCodes().isEmpty())) {
       return Collections.emptyMap();
     }
     ImmutableMap.Builder<String, Object> methodConfig = ImmutableMap.builder();
     methodConfig.put(
         "name", Collections.singletonList(Collections.emptyMap()));
-    if (retryPolicy != null) {
+    if (retryPolicy != null && !retryPolicy.retryableStatusCodes().isEmpty()) {
       ImmutableMap.Builder<String, Object> rawRetryPolicy = ImmutableMap.builder();
       rawRetryPolicy.put("maxAttempts", (double) retryPolicy.maxAttempts());
       rawRetryPolicy.put("initialBackoff", Durations.toString(retryPolicy.initialBackoff()));

diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java
@@ -552,7 +552,8 @@ public void parseRouteAction_withRetryPolicy() {
         .setRetryPolicy(builder.build())
         .build();
     struct = ClientXdsClient.parseRouteAction(proto, filterRegistry, false);
-    assertThat(struct.getStruct().retryPolicy()).isNull();
+    assertThat(struct.getStruct().retryPolicy()).isNotNull();
+    assertThat(struct.getStruct().retryPolicy().retryableStatusCodes()).isEmpty();
 
     // base_interval unset
     builder

diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
@@ -988,6 +988,8 @@ public void generateServiceConfig_forPerMethodConfig() throws IOException {
     RetryPolicy retryPolicy = RetryPolicy.create(
         4, ImmutableList.of(Code.UNAVAILABLE, Code.CANCELLED), Durations.fromMillis(100),
         Durations.fromMillis(200), null);
+    RetryPolicy retryPolicyWithEmptyStatusCodes = RetryPolicy.create(
+        4, ImmutableList.<Code>of(), Durations.fromMillis(100), Durations.fromMillis(200), null);
 
     // timeout only
     String expectedServiceConfigJson = "{\n"
@@ -1001,6 +1003,11 @@ public void generateServiceConfig_forPerMethodConfig() throws IOException {
     assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(timeoutNano, null))
         .isEqualTo(expectedServiceConfig);
 
+    // timeout with empty retriable status codes
+    assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(
+            timeoutNano, retryPolicyWithEmptyStatusCodes))
+        .isEqualTo(expectedServiceConfig);
+
     // retry only
     expectedServiceConfigJson = "{\n"
         + "  \"methodConfig\": [{\n"
@@ -1021,6 +1028,7 @@ public void generateServiceConfig_forPerMethodConfig() throws IOException {
     assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(null, retryPolicy))
         .isEqualTo(expectedServiceConfig);
 
+
     // timeout and retry
     expectedServiceConfigJson = "{\n"
         + "  \"methodConfig\": [{\n"
@@ -1043,12 +1051,16 @@ public void generateServiceConfig_forPerMethodConfig() throws IOException {
         .isEqualTo(expectedServiceConfig);
 
     // no timeout and no retry
-    // timeout and retry
     expectedServiceConfigJson = "{}";
     expectedServiceConfig =
         (Map<String, ?>) JsonParser.parse(expectedServiceConfigJson);
     assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(null, null))
         .isEqualTo(expectedServiceConfig);
+
+    // retry with emtry retriable status codes only
+    assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(
+            null, retryPolicyWithEmptyStatusCodes))
+        .isEqualTo(expectedServiceConfig);
   }
 
   @Test