Repository: googleapis/java-bigtable-hbase (java)

Issue:
Sample row keys timeout is too short
Sample row keys uses the same default timeouts as other unary operations, which is too short for large enough tables. This should be closer to the default settings for the Veneer client

Pull request description:
fix: Bring SampleRowKeys default timeout closer in line to the Veneer…
This brings the default timeout for SampleRowKeys to the same values as the Veneer client

Fixes #4450 ☕️


Patch:
diff --git a/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/wrappers/veneer/BigtableHBaseVeneerSettings.java b/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/wrappers/veneer/BigtableHBaseVeneerSettings.java
index 64ec3c027..15608cff2 100644
--- a/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/wrappers/veneer/BigtableHBaseVeneerSettings.java
+++ b/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/wrappers/veneer/BigtableHBaseVeneerSettings.java
@@ -136,6 +136,10 @@ public class BigtableHBaseVeneerSettings extends BigtableHBaseSettings {
           /* bulkMutateTimeouts = */ new OperationTimeouts(
               Optional.absent(),
               Optional.of(Duration.ofMinutes(1)),
+              Optional.of(Duration.ofMinutes(10))),
+          /* sampleRowKeysTimeouts = */ new OperationTimeouts(
+              Optional.absent(),
+              Optional.of(Duration.ofMinutes(5)),
               Optional.of(Duration.ofMinutes(10))));
   private static final int MAX_CONSECUTIVE_SCAN_ATTEMPTS = 10;
 
@@ -348,7 +352,8 @@ public class BigtableHBaseVeneerSettings extends BigtableHBaseSettings {
     configureRetryableCallSettings(
         dataBuilder.stubSettings().readRowSettings(), clientTimeouts.getUnaryTimeouts());
     configureRetryableCallSettings(
-        dataBuilder.stubSettings().sampleRowKeysSettings(), clientTimeouts.getUnaryTimeouts());
+        dataBuilder.stubSettings().sampleRowKeysSettings(),
+        clientTimeouts.getSampleRowKeysTimeouts());
 
     if (!configuration.getBoolean(BIGTABLE_ENABLE_CLIENT_SIDE_METRICS, true)) {
       dataBuilder.setMetricsProvider(NoopMetricsProvider.INSTANCE);
@@ -866,7 +871,16 @@ public class BigtableHBaseVeneerSettings extends BigtableHBaseSettings {
             extractDuration(BIGTABLE_RPC_TIMEOUT_MS_KEY, MAX_ELAPSED_BACKOFF_MILLIS_KEY)
                 .or(DEFAULT_TIMEOUTS.unaryTimeouts.operationTimeout));
 
-    return new ClientOperationTimeouts(unaryTimeouts, scanTimeouts, bulkMutateTimeouts);
+    OperationTimeouts sampleRowKeysTimeouts =
+        new OperationTimeouts(
+            DEFAULT_TIMEOUTS.sampleRowKeysTimeouts.responseTimeout,
+            extractDuration(BIGTABLE_RPC_ATTEMPT_TIMEOUT_MS_KEY)
+                .or(DEFAULT_TIMEOUTS.sampleRowKeysTimeouts.attemptTimeout),
+            extractDuration(BIGTABLE_RPC_TIMEOUT_MS_KEY, MAX_ELAPSED_BACKOFF_MILLIS_KEY)
+                .or(DEFAULT_TIMEOUTS.sampleRowKeysTimeouts.operationTimeout));
+
+    return new ClientOperationTimeouts(
+        unaryTimeouts, scanTimeouts, bulkMutateTimeouts, sampleRowKeysTimeouts);
   }
 
   private Optional<Duration> extractDuration(String... keys) {
@@ -911,19 +925,25 @@ public class BigtableHBaseVeneerSettings extends BigtableHBaseSettings {
 
     static final ClientOperationTimeouts EMPTY =
         new ClientOperationTimeouts(
-            OperationTimeouts.EMPTY, OperationTimeouts.EMPTY, OperationTimeouts.EMPTY);
+            OperationTimeouts.EMPTY,
+            OperationTimeouts.EMPTY,
+            OperationTimeouts.EMPTY,
+            OperationTimeouts.EMPTY);
 
     private final OperationTimeouts unaryTimeouts;
     private final OperationTimeouts scanTimeouts;
     private final OperationTimeouts bulkMutateTimeouts;
+    private final OperationTimeouts sampleRowKeysTimeouts;
 
     public ClientOperationTimeouts(
         OperationTimeouts unaryTimeouts,
         OperationTimeouts scanTimeouts,
-        OperationTimeouts bulkMutateTimeouts) {
+        OperationTimeouts bulkMutateTimeouts,
+        OperationTimeouts sampleRowKeysTimeouts) {
       this.unaryTimeouts = unaryTimeouts;
       this.scanTimeouts = scanTimeouts;
       this.bulkMutateTimeouts = bulkMutateTimeouts;
+      this.sampleRowKeysTimeouts = sampleRowKeysTimeouts;
     }
 
     public OperationTimeouts getUnaryTimeouts() {
@@ -937,6 +957,10 @@ public class BigtableHBaseVeneerSettings extends BigtableHBaseSettings {
     public OperationTimeouts getBulkMutateTimeouts() {
       return bulkMutateTimeouts;
     }
+
+    public OperationTimeouts getSampleRowKeysTimeouts() {
+      return sampleRowKeysTimeouts;
+    }
   }
 
   static class OperationTimeouts {


Test patch:
diff --git a/bigtable-client-core-parent/bigtable-hbase/src/test/java/com/google/cloud/bigtable/hbase/wrappers/veneer/TestBigtableHBaseVeneerSettings.java b/bigtable-client-core-parent/bigtable-hbase/src/test/java/com/google/cloud/bigtable/hbase/wrappers/veneer/TestBigtableHBaseVeneerSettings.java
index 2a6d2882b..9376d3ff1 100644
--- a/bigtable-client-core-parent/bigtable-hbase/src/test/java/com/google/cloud/bigtable/hbase/wrappers/veneer/TestBigtableHBaseVeneerSettings.java
+++ b/bigtable-client-core-parent/bigtable-hbase/src/test/java/com/google/cloud/bigtable/hbase/wrappers/veneer/TestBigtableHBaseVeneerSettings.java
@@ -240,6 +240,12 @@ public class TestBigtableHBaseVeneerSettings {
     assertEquals(
         Optional.of(Duration.ofMillis(perRowTimeoutMs)),
         settings.getClientTimeouts().getScanTimeouts().getResponseTimeout());
+    assertEquals(
+        Optional.of(Duration.ofMillis(rpcTimeoutMs)),
+        settings.getClientTimeouts().getSampleRowKeysTimeouts().getOperationTimeout());
+    assertEquals(
+        Optional.of(Duration.ofMillis(rpcAttemptTimeoutMs)),
+        settings.getClientTimeouts().getSampleRowKeysTimeouts().getAttemptTimeout());
 
     RetrySettings readRowRetrySettings =
         dataSettings.getStubSettings().readRowSettings().getRetrySettings();
@@ -262,6 +268,13 @@ public class TestBigtableHBaseVeneerSettings {
     assertEquals(perRowTimeoutMs, readRowsRetrySettings.getMaxRpcTimeout().toMillis());
     assertEquals(maxAttempt, readRowsRetrySettings.getMaxAttempts());
     assertEquals(readRowStreamTimeout, readRowsRetrySettings.getTotalTimeout().toMillis());
+
+    RetrySettings sampleRowKeysRetrySettings =
+        dataSettings.getStubSettings().sampleRowKeysSettings().getRetrySettings();
+    assertEquals(initialElapsedMs, sampleRowKeysRetrySettings.getInitialRetryDelay().toMillis());
+    assertEquals(rpcTimeoutMs, sampleRowKeysRetrySettings.getTotalTimeout().toMillis());
+    assertEquals(rpcAttemptTimeoutMs, sampleRowKeysRetrySettings.getInitialRpcTimeout().toMillis());
+    assertEquals(rpcAttemptTimeoutMs, sampleRowKeysRetrySettings.getMaxRpcTimeout().toMillis());
   }
 
   @Test