Merge branch 'master' into dynamic_flow_control_p2

googleapis · Mar 16, 2021 · d83fdc1 · d83fdc1
2 parents 3aa884d + 86d5c72
commit d83fdc1
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 9 deletions.
diff --git a/benchmark/build.gradle b/benchmark/build.gradle
@@ -23,7 +23,7 @@ dependencies {
     compile project(':gax-grpc')
     compile "io.grpc:grpc-netty:${libraries['version.io_grpc']}"
 
-    compile 'com.google.api.grpc:grpc-google-cloud-bigtable-v2:1.11.0'
+    compile 'com.google.api.grpc:grpc-google-cloud-bigtable-v2:1.21.0'
     compile 'com.google.api.grpc:grpc-google-cloud-pubsub-v1:1.86.0'
 }
 

diff --git a/benchmark/src/jmh/java/com/google/api/gax/grpc/ServerStreamingCallableBenchmark.java b/benchmark/src/jmh/java/com/google/api/gax/grpc/ServerStreamingCallableBenchmark.java
@@ -146,12 +146,12 @@ public void setup(BenchmarkParams benchmarkParams) throws IOException {
     stub = BigtableGrpc.newStub(grpcChannel);
 
     // Direct Callable
-    directCallable = new GrpcDirectServerStreamingCallable<>(BigtableGrpc.METHOD_READ_ROWS);
+    directCallable = new GrpcDirectServerStreamingCallable<>(BigtableGrpc.getReadRowsMethod());
 
     // Base Callable (direct + params extractor + exceptions + retries)
     GrpcCallSettings<ReadRowsRequest, ReadRowsResponse> grpcCallSettings =
         GrpcCallSettings.<ReadRowsRequest, ReadRowsResponse>newBuilder()
-            .setMethodDescriptor(BigtableGrpc.METHOD_READ_ROWS)
+            .setMethodDescriptor(BigtableGrpc.getReadRowsMethod())
             .setParamsExtractor(new FakeRequestParamsExtractor())
             .build();
 
@@ -212,7 +212,7 @@ public void teardown() {
   @Benchmark
   public void asyncGrpcListener(AsyncSettings asyncSettings, Blackhole blackhole) throws Exception {
     ClientCall<ReadRowsRequest, ReadRowsResponse> clientCall =
-        grpcChannel.newCall(BigtableGrpc.METHOD_READ_ROWS, CallOptions.DEFAULT);
+        grpcChannel.newCall(BigtableGrpc.getReadRowsMethod(), CallOptions.DEFAULT);
 
     GrpcClientCallListener listener =
         new GrpcClientCallListener(clientCall, asyncSettings.autoFlowControl, blackhole);
@@ -242,7 +242,7 @@ public void asyncGrpcObserver(AsyncSettings asyncSettings, Blackhole blackhole)
   public void syncGrpcIterator(Blackhole blackhole) {
     Iterator<ReadRowsResponse> iterator =
         ClientCalls.blockingServerStreamingCall(
-            grpcChannel, BigtableGrpc.METHOD_READ_ROWS, CallOptions.DEFAULT, request);
+            grpcChannel, BigtableGrpc.getReadRowsMethod(), CallOptions.DEFAULT, request);
 
     while (iterator.hasNext()) {
       ReadRowsResponse response = iterator.next();

diff --git a/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java b/gax/src/main/java/com/google/api/gax/rpc/ApiCallContext.java
@@ -63,9 +63,13 @@ public interface ApiCallContext extends RetryingContext {
    * Returns a new ApiCallContext with the given timeout set.
    *
    * <p>This sets the maximum amount of time a single unary RPC attempt can take. If retries are
-   * enabled, then this can take much longer. Unlike a deadline, timeouts are relative durations
-   * that are measure from the beginning of each RPC attempt. Please note that this will limit the
-   * duration of a server streaming RPC as well.
+   * enabled, then this can take much longer, as each RPC attempt will have the same constant
+   * timeout. Unlike a deadline, timeouts are relative durations that are measure from the beginning
+   * of each RPC attempt. Please note that this limits the duration of a server streaming RPC as
+   * well.
+   *
+   * <p>If a method has default {@link com.google.api.gax.retrying.RetrySettings}, the max attempts
+   * and/or total timeout is still respected when scheduling each RPC attempt.
    */
   ApiCallContext withTimeout(@Nullable Duration timeout);
 

diff --git a/gax/src/main/java/com/google/api/gax/rpc/AttemptCallable.java b/gax/src/main/java/com/google/api/gax/rpc/AttemptCallable.java
@@ -69,8 +69,9 @@ public ResponseT call() {
     ApiCallContext callContext = originalCallContext;
 
     try {
+      // Set the RPC timeout if the caller did not provide their own.
       Duration rpcTimeout = externalFuture.getAttemptSettings().getRpcTimeout();
-      if (!rpcTimeout.isZero()) {
+      if (!rpcTimeout.isZero() && callContext.getTimeout() == null) {
         callContext = callContext.withTimeout(rpcTimeout);
       }
 

diff --git a/gax/src/test/java/com/google/api/gax/rpc/AttemptCallableTest.java b/gax/src/test/java/com/google/api/gax/rpc/AttemptCallableTest.java
@@ -108,6 +108,9 @@ public void testRpcTimeoutIsNotErased() {
     Duration callerTimeout = Duration.ofMillis(10);
     ApiCallContext callerCallContext = FakeCallContext.createDefault().withTimeout(callerTimeout);
 
+    Duration timeout = Duration.ofMillis(5);
+    currentAttemptSettings = currentAttemptSettings.toBuilder().setRpcTimeout(timeout).build();
+
     AttemptCallable<String, String> callable =
         new AttemptCallable<>(mockInnerCallable, "fake-request", callerCallContext);
     callable.setExternalFuture(mockExternalFuture);