Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

xds: parse timeout from RDS responses #7257

Merged
merged 5 commits into from
Jul 31, 2020
Merged

xds: parse timeout from RDS responses #7257

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4e2acd8
Parse timeout from RDS responses.
voidzcy Jul 28, 2020
138f6b4
Set timeout to infinite if maxGrpcTimeout is set to 0
voidzcy Jul 29, 2020
0cfc73b
Delete comment
voidzcy Jul 29, 2020
4c416aa
Add case for timeout disabled.
voidzcy Jul 29, 2020
910790d
Use max value instead of null.
voidzcy Jul 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 24 additions & 4 deletions xds/src/main/java/io/grpc/xds/EnvoyProtoData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.protobuf.NullValue;
import com.google.protobuf.Struct;
import com.google.protobuf.Value;
import com.google.protobuf.util.Durations;
import com.google.re2j.Pattern;
import com.google.re2j.PatternSyntaxException;
import io.envoyproxy.envoy.type.v3.FractionalPercent;
Expand All @@ -40,6 +41,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -1067,18 +1069,27 @@ static StructOrError<HeaderMatcher> convertEnvoyProtoHeaderMatcher(

/** See corresponding Envoy proto message {@link io.envoyproxy.envoy.api.v2.route.RouteAction}. */
static final class RouteAction {
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(@Nullable String cluster, @Nullable List<ClusterWeight> weightedClusters) {
RouteAction(
long timeoutNano,
@Nullable String cluster,
@Nullable List<ClusterWeight> weightedClusters) {
this.timeoutNano = timeoutNano;
this.cluster = cluster;
this.weightedClusters = weightedClusters;
}

long getTimeoutNano() {
return timeoutNano;
}

@Nullable
String getCluster() {
return cluster;
Expand All @@ -1098,18 +1109,20 @@ public boolean equals(Object o) {
return false;
}
RouteAction that = (RouteAction) o;
return Objects.equals(cluster, that.cluster)
return Objects.equals(timeoutNano, that.timeoutNano)
&& Objects.equals(cluster, that.cluster)
&& Objects.equals(weightedClusters, that.weightedClusters);
}

@Override
public int hashCode() {
return Objects.hash(cluster, weightedClusters);
return Objects.hash(timeoutNano, cluster, weightedClusters);
}

@Override
public String toString() {
ToStringHelper toStringHelper = MoreObjects.toStringHelper(this);
toStringHelper.add("timeout", timeoutNano + "ns");
if (cluster != null) {
toStringHelper.add("cluster", cluster);
}
Expand Down Expand Up @@ -1146,7 +1159,14 @@ static StructOrError<RouteAction> fromEnvoyProtoRouteAction(
return StructOrError.fromError(
"Unknown cluster specifier: " + proto.getClusterSpecifierCase());
}
return StructOrError.fromStruct(new RouteAction(cluster, weightedClusters));
long timeoutNano = TimeUnit.SECONDS.toNanos(15L); // default 15s
if (proto.hasMaxGrpcTimeout()) {
long time = Durations.toNanos(proto.getMaxGrpcTimeout());
timeoutNano = time == 0 ? Long.MAX_VALUE : time;
} else if (proto.hasTimeout()) {
timeoutNano = Durations.toNanos(proto.getTimeout());
}
return StructOrError.fromStruct(new RouteAction(timeoutNano, cluster, weightedClusters));
}
}

Expand Down
39 changes: 30 additions & 9 deletions xds/src/test/java/io/grpc/xds/EnvoyProtoDataTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.protobuf.Struct;
import com.google.protobuf.UInt32Value;
import com.google.protobuf.Value;
import com.google.protobuf.util.Durations;
import com.google.re2j.Pattern;
import io.envoyproxy.envoy.config.core.v3.RuntimeFractionalPercent;
import io.envoyproxy.envoy.config.route.v3.QueryParameterMatcher;
Expand All @@ -44,6 +45,7 @@
import io.grpc.xds.RouteMatch.PathMatcher;
import java.util.Arrays;
import java.util.Collections;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -206,7 +208,7 @@ public void convertRoute() {
new Route(
new RouteMatch(new PathMatcher("/service/method", null, null),
Collections.<HeaderMatcher>emptyList(), null),
new RouteAction("cluster-foo", null)));
new RouteAction(TimeUnit.SECONDS.toNanos(15L), "cluster-foo", null)));

io.envoyproxy.envoy.config.route.v3.Route unsupportedProto =
io.envoyproxy.envoy.config.route.v3.Route.newBuilder()
Expand Down Expand Up @@ -400,20 +402,37 @@ public void convertRouteAction() {
.build();
StructOrError<RouteAction> struct1 = RouteAction.fromEnvoyProtoRouteAction(proto1);
assertThat(struct1.getErrorDetail()).isNull();
assertThat(struct1.getStruct().getTimeoutNano())
.isEqualTo(TimeUnit.SECONDS.toNanos(15L)); // default value
assertThat(struct1.getStruct().getCluster()).isEqualTo("cluster-foo");
assertThat(struct1.getStruct().getWeightedCluster()).isNull();

// cluster_specifier = cluster_header
io.envoyproxy.envoy.config.route.v3.RouteAction proto2 =
io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder()
.setClusterHeader("cluster-bar")
.setMaxGrpcTimeout(Durations.fromNanos(0))
.setTimeout(Durations.fromMicros(20L))
.setCluster("cluster-foo")
.build();
StructOrError<RouteAction> struct2 = RouteAction.fromEnvoyProtoRouteAction(proto2);
assertThat(struct2).isNull();
assertThat(struct2.getErrorDetail()).isNull();
assertThat(struct2.getStruct().getTimeoutNano())
.isEqualTo(Long.MAX_VALUE); // infinite
assertThat(struct2.getStruct().getCluster()).isEqualTo("cluster-foo");
assertThat(struct2.getStruct().getWeightedCluster()).isNull();

// cluster_specifier = weighted_cluster
// cluster_specifier = cluster_header
io.envoyproxy.envoy.config.route.v3.RouteAction proto3 =
io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder()
.setClusterHeader("cluster-bar")
.build();
StructOrError<RouteAction> struct3 = RouteAction.fromEnvoyProtoRouteAction(proto3);
assertThat(struct3).isNull();

// cluster_specifier = weighted_cluster
io.envoyproxy.envoy.config.route.v3.RouteAction proto4 =
io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder()
.setMaxGrpcTimeout(Durations.fromSeconds(6L))
.setTimeout(Durations.fromMicros(20L))
.setWeightedClusters(
WeightedCluster.newBuilder()
.addClusters(
Expand All @@ -422,10 +441,12 @@ public void convertRouteAction() {
.setName("cluster-baz")
.setWeight(UInt32Value.newBuilder().setValue(100))))
.build();
StructOrError<RouteAction> struct3 = RouteAction.fromEnvoyProtoRouteAction(proto3);
assertThat(struct3.getErrorDetail()).isNull();
assertThat(struct3.getStruct().getCluster()).isNull();
assertThat(struct3.getStruct().getWeightedCluster())
StructOrError<RouteAction> struct4 = RouteAction.fromEnvoyProtoRouteAction(proto4);
assertThat(struct4.getErrorDetail()).isNull();
assertThat(struct4.getStruct().getTimeoutNano())
.isEqualTo(TimeUnit.SECONDS.toNanos(6L));
assertThat(struct4.getStruct().getCluster()).isNull();
assertThat(struct4.getStruct().getWeightedCluster())
.containsExactly(new ClusterWeight("cluster-baz", 100));

// cluster_specifier unset
Expand Down
9 changes: 6 additions & 3 deletions xds/src/test/java/io/grpc/xds/XdsClientImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,14 +748,16 @@ public void resolveVirtualHostWithPathMatchingInRdsResponse() {
new io.grpc.xds.RouteMatch(
/* prefix= */ null,
/* path= */ "/service1/method1"),
new EnvoyProtoData.RouteAction("cl1.googleapis.com", null)));
new EnvoyProtoData.RouteAction(
TimeUnit.SECONDS.toNanos(15L), "cl1.googleapis.com", null)));
assertThat(routes.get(1)).isEqualTo(
new EnvoyProtoData.Route(
// path match with weighted cluster route
new io.grpc.xds.RouteMatch(
/* prefix= */ null,
/* path= */ "/service2/method2"),
new EnvoyProtoData.RouteAction(
TimeUnit.SECONDS.toNanos(15L),
null,
ImmutableList.of(
new EnvoyProtoData.ClusterWeight("cl21.googleapis.com", 30),
Expand All @@ -767,15 +769,16 @@ public void resolveVirtualHostWithPathMatchingInRdsResponse() {
new io.grpc.xds.RouteMatch(
/* prefix= */ "/service1/",
/* path= */ null),
new EnvoyProtoData.RouteAction("cl1.googleapis.com", null)));
new EnvoyProtoData.RouteAction(
TimeUnit.SECONDS.toNanos(15L), "cl1.googleapis.com", null)));
assertThat(routes.get(3)).isEqualTo(
new EnvoyProtoData.Route(
// default match with cluster route
new io.grpc.xds.RouteMatch(
/* prefix= */ "",
/* path= */ null),
new EnvoyProtoData.RouteAction(
"cluster.googleapis.com", null)));
TimeUnit.SECONDS.toNanos(15L), "cluster.googleapis.com", null)));
}

/**
Expand Down
5 changes: 3 additions & 2 deletions xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void generateXdsRoutingRawConfig() {
new RouteMatch(
new PathMatcher(null, "", null), Collections.<HeaderMatcher>emptyList(),
new FractionMatcher(10, 20)),
new RouteAction("cluster-foo", null));
new RouteAction(15L, "cluster-foo", null));
Route r2 =
new Route(
new RouteMatch(
Expand All @@ -92,6 +92,7 @@ public void generateXdsRoutingRawConfig() {
new HeaderMatcher(":scheme", "https", null, null, null, null, null, false)),
null),
new RouteAction(
15L,
null,
Arrays.asList(
new ClusterWeight("cluster-foo", 20),
Expand Down Expand Up @@ -134,7 +135,7 @@ public void generateXdsRoutingRawConfig_allowDuplicateMatchers() {
new RouteMatch(
new PathMatcher("/service/method", null, null),
Collections.<HeaderMatcher>emptyList(), null),
new RouteAction("cluster-foo", null));
new RouteAction(15L, "cluster-foo", null));

Map<String, ?> config =
XdsNameResolver.generateXdsRoutingRawConfig(Arrays.asList(route, route));
Expand Down