diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index d099fb72db1..451470f8ad2 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -1272,13 +1272,10 @@ private static StructOrError parseRetryPolicy( retryableStatusCodesBuilder.add(code); } List 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 index 3ae6346c158..34b6a415200 100644 --- 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 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 methodConfig = ImmutableMap.builder(); methodConfig.put( "name", Collections.singletonList(Collections.emptyMap())); - if (retryPolicy != null) { + if (retryPolicy != null && !retryPolicy.retryableStatusCodes().isEmpty()) { ImmutableMap.Builder 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 index 5bd57c2f16b..53d8388427b 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java @@ -547,7 +547,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 index cb2b4481fad..8b604fbe6ab 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -989,6 +989,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.of(), Durations.fromMillis(100), Durations.fromMillis(200), null); // timeout only String expectedServiceConfigJson = "{\n" @@ -1002,6 +1004,11 @@ public void generateServiceConfig_forPerMethodConfig() throws IOException { assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(timeoutNano, null)) .isEqualTo(expectedServiceConfig); + // timeout and retry with empty retriable status codes + assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig( + timeoutNano, retryPolicyWithEmptyStatusCodes)) + .isEqualTo(expectedServiceConfig); + // retry only expectedServiceConfigJson = "{\n" + " \"methodConfig\": [{\n" @@ -1022,6 +1029,7 @@ public void generateServiceConfig_forPerMethodConfig() throws IOException { assertThat(XdsNameResolver.generateServiceConfigWithMethodConfig(null, retryPolicy)) .isEqualTo(expectedServiceConfig); + // timeout and retry expectedServiceConfigJson = "{\n" + " \"methodConfig\": [{\n" @@ -1044,12 +1052,16 @@ public void generateServiceConfig_forPerMethodConfig() throws IOException { .isEqualTo(expectedServiceConfig); // no timeout and no retry - // timeout and retry expectedServiceConfigJson = "{}"; expectedServiceConfig = (Map) 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