Explicitly set value of 0 in RouteAction is different than unset. Fallback should not happen if the timeout value in RouteAction is set (including 0).

Author: voidzcy

xds/src/main/java/io/grpc/xds/EnvoyProtoData.java

@@ -1136,24 +1136,23 @@ static StructOrError<HeaderMatcher> convertEnvoyProtoHeaderMatcher(
    * See corresponding Envoy proto message {@link io.envoyproxy.envoy.config.route.v3.RouteAction}.
    */
   static final class RouteAction {
-    private final long timeoutNano;
+    @Nullable
+    private final Long timeoutNano;
     // Exactly one of the following fields is non-null.
     @Nullable
     private final String cluster;
     @Nullable
     private final List<ClusterWeight> weightedClusters;
 
     @VisibleForTesting
-    RouteAction(
-        long timeoutNano,
-        @Nullable String cluster,
+    RouteAction(@Nullable Long timeoutNano, @Nullable String cluster,
         @Nullable List<ClusterWeight> weightedClusters) {
       this.timeoutNano = timeoutNano;
       this.cluster = cluster;
       this.weightedClusters = weightedClusters;
     }
 
-
+    @Nullable
     Long getTimeoutNano() {
       return timeoutNano;
     }
@@ -1190,7 +1189,9 @@ public int hashCode() {
     @Override
     public String toString() {
       ToStringHelper toStringHelper = MoreObjects.toStringHelper(this);
-      toStringHelper.add("timeout", timeoutNano + "ns");
+      if (timeoutNano != null) {
+        toStringHelper.add("timeout", timeoutNano + "ns");
+      }
       if (cluster != null) {
         toStringHelper.add("cluster", cluster);
       }
@@ -1230,7 +1231,7 @@ static StructOrError<RouteAction> fromEnvoyProtoRouteAction(
           return StructOrError.fromError(
               "Unknown cluster specifier: " + proto.getClusterSpecifierCase());
       }
-      long timeoutNano = 0L;
+      Long timeoutNano = null;
       if (proto.hasMaxStreamDuration()) {
         io.envoyproxy.envoy.config.route.v3.RouteAction.MaxStreamDuration maxStreamDuration
             = proto.getMaxStreamDuration();

xds/src/main/java/io/grpc/xds/XdsNameResolver.java

@@ -359,8 +359,8 @@ public Result selectConfig(PickSubchannelArgs args) {
       // TODO(chengyuanzhang): avoid service config generation and parsing for each call.
       Map<String, ?> rawServiceConfig = Collections.emptyMap();
       if (enableTimeout) {
-        long timeoutNano = selectedRoute.getRouteAction().getTimeoutNano();
-        if (timeoutNano == 0) {
+        Long timeoutNano = selectedRoute.getRouteAction().getTimeoutNano();
+        if (timeoutNano == null) {
           timeoutNano = routingConfig.fallbackTimeoutNano;
         }
         if (timeoutNano > 0) {

xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java

@@ -214,7 +214,7 @@ public void convertRoute() {
             new Route(
                 new RouteMatch(new PathMatcher("/service/method", null, null),
                     Collections.<HeaderMatcher>emptyList(), null),
-                new RouteAction(0, "cluster-foo", null)));
+                new RouteAction(null, "cluster-foo", null)));
 
     io.envoyproxy.envoy.config.route.v3.Route unsupportedProto =
         io.envoyproxy.envoy.config.route.v3.Route.newBuilder()
@@ -477,7 +477,7 @@ public void convertRouteAction_timeoutUnset() {
             .setCluster("cluster-foo")
             .build();
     StructOrError<RouteAction> struct = RouteAction.fromEnvoyProtoRouteAction(proto);
-    assertThat(struct.getStruct().getTimeoutNano()).isEqualTo(0);
+    assertThat(struct.getStruct().getTimeoutNano()).isNull();
   }
 
   @Test

xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java

@@ -297,7 +297,7 @@ public void resolved_noTimeout() {
     resolver.start(mockListener);
     FakeXdsClient xdsClient = (FakeXdsClient) resolver.getXdsClient();
     Route route = new Route(new RouteMatch(null, call1.getFullMethodNameForPath()),
-        new RouteAction(0L, cluster1, null));  // per-route timeout unset
+        new RouteAction(null, cluster1, null));  // per-route timeout unset
     VirtualHost virtualHost = new VirtualHost("does not matter",
         Collections.singletonList(AUTHORITY), Collections.singletonList(route));
     xdsClient.deliverLdsUpdate(AUTHORITY, 0L, Collections.singletonList(virtualHost));
@@ -317,7 +317,7 @@ public void resolved_fallbackToHttpMaxStreamDurationAsTimeout() {
     resolver.start(mockListener);
     FakeXdsClient xdsClient = (FakeXdsClient) resolver.getXdsClient();
     Route route = new Route(new RouteMatch(null, call1.getFullMethodNameForPath()),
-        new RouteAction(0L, cluster1, null));  // per-route timeout unset
+        new RouteAction(null, cluster1, null));  // per-route timeout unset
     VirtualHost virtualHost = new VirtualHost("does not matter",
         Collections.singletonList(AUTHORITY), Collections.singletonList(route));
     xdsClient.deliverLdsUpdate(AUTHORITY, TimeUnit.SECONDS.toNanos(5L),