feat: enable channel priming by default (#1555)

mutianf · web-flow · commit 303959cc88e8 · 2022-12-16T16:38:16.000Z
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) Fixes #<issue_number_goes_here> ☕️ If you write sample code, please follow the [samples format]( https://togithub.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md).
diff --git a/README.md b/README.md
@@ -49,7 +49,7 @@ If you are using Maven without BOM, add this to your dependencies:
 If you are using Gradle 5.x or later, add this to your dependencies:
 
 ```Groovy
-implementation platform('com.google.cloud:libraries-bom:26.1.5')
+implementation platform('com.google.cloud:libraries-bom:26.2.0')
 
 implementation 'com.google.cloud:google-cloud-bigtable'
 ```
diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java
@@ -124,6 +124,8 @@ public static Builder newBuilderForEmulator(String hostname, int port) {
         .stubSettings()
         .setCredentialsProvider(NoCredentialsProvider.create())
         .setEndpoint(hostname + ":" + port)
+        // disable channel refreshing when creating an emulator
+        .setRefreshingChannel(false)
         .setTransportChannelProvider(
             InstantiatingGrpcChannelProvider.newBuilder()
                 .setMaxInboundMessageSize(256 * 1024 * 1024)
@@ -244,8 +246,12 @@ public String getAppProfileId() {
     return stubSettings.getAppProfileId();
   }
 
-  /** Gets if channels will gracefully refresh connections to Cloud Bigtable service */
-  @BetaApi("Channel priming is not currently stable and may change in the future")
+  /**
+   * Gets if channels will gracefully refresh connections to Cloud Bigtable service
+   *
+   * @deprecated Channel refreshing is enabled by default and this method will be deprecated.
+   */
+  @Deprecated
   public boolean isRefreshingChannel() {
     return stubSettings.isRefreshingChannel();
   }
@@ -395,19 +401,25 @@ public CredentialsProvider getCredentialsProvider() {
     /**
      * Configure periodic gRPC channel refreshes.
      *
-     * <p>This feature will gracefully refresh connections to the Cloud Bigtable service. This is an
-     * experimental feature to address tail latency caused by the service dropping long lived gRPC
-     * connections, which causes the client to renegotiate the gRPC connection in the request path,
-     * which causes periodic spikes in latency
+     * <p>This feature will gracefully refresh connections to the Cloud Bigtable service. This is a
+     * feature to address tail latency caused by the service dropping long lived gRPC connections,
+     * which causes the client to renegotiate the gRPC connection in the request path, which causes
+     * periodic spikes in latency.
+     *
+     * @deprecated Channel refreshing is enabled by default and this method will be deprecated.
      */
-    @BetaApi("Channel priming is not currently stable and may change in the future")
+    @Deprecated
     public Builder setRefreshingChannel(boolean isRefreshingChannel) {
       stubSettings.setRefreshingChannel(isRefreshingChannel);
       return this;
     }
 
-    /** Gets if channels will gracefully refresh connections to Cloud Bigtable service */
-    @BetaApi("Channel priming is not currently stable and may change in the future")
+    /**
+     * Gets if channels will gracefully refresh connections to Cloud Bigtable service.
+     *
+     * @deprecated Channel refreshing is enabled by default and this method will be deprecated.
+     */
+    @Deprecated
     public boolean isRefreshingChannel() {
       return stubSettings.isRefreshingChannel();
     }
diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableChannelPrimer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableChannelPrimer.java
@@ -51,6 +51,8 @@ static BigtableChannelPrimer create(
             .setInstanceId(instanceId)
             .setAppProfileId(appProfileId)
             .setCredentialsProvider(FixedCredentialsProvider.create(credentials))
+            // Disable refreshing channel here to avoid creating settings in a loop
+            .setRefreshingChannel(false)
             .setExecutorProvider(
                 InstantiatingExecutorProvider.newBuilder().setExecutorThreadCount(1).build());
 
diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java
@@ -15,7 +15,6 @@
  */
 package com.google.cloud.bigtable.data.v2.stub;
 
-import com.google.api.core.BetaApi;
 import com.google.api.core.InternalApi;
 import com.google.api.gax.batching.BatchingCallSettings;
 import com.google.api.gax.batching.BatchingSettings;
@@ -236,8 +235,12 @@ public String getAppProfileId() {
     return appProfileId;
   }
 
-  /** Returns if channels will gracefully refresh connections to Cloud Bigtable service */
-  @BetaApi("This API depends on experimental gRPC APIs")
+  /**
+   * Returns if channels will gracefully refresh connections to Cloud Bigtable service
+   *
+   * @deprecated Channel refreshing is enabled by default and this method will be deprecated.
+   */
+  @Deprecated
   public boolean isRefreshingChannel() {
     return isRefreshingChannel;
   }
@@ -545,7 +548,7 @@ public static class Builder extends StubSettings.Builder<EnhancedBigtableStubSet
      */
     private Builder() {
       this.appProfileId = SERVER_DEFAULT_APP_PROFILE_ID;
-      this.isRefreshingChannel = false;
+      this.isRefreshingChannel = true;
       primedTableIds = ImmutableList.of();
       jwtAudienceMapping = DEFAULT_JWT_AUDIENCE_MAPPING;
       setCredentialsProvider(defaultCredentialsProviderBuilder().build());
@@ -757,11 +760,12 @@ public String getAppProfileId() {
      * Sets if channels will gracefully refresh connections to Cloud Bigtable service.
      *
      * <p>When enabled, this will wait for the connection to complete the SSL handshake and warm up
-     * serverside caches for all the tables of the instance.
+     * serverside caches for all the tables of the instance. This feature is enabled by default.
      *
      * @see com.google.cloud.bigtable.data.v2.BigtableDataSettings.Builder#setRefreshingChannel
+     * @deprecated Channel refreshing is enabled by default and this method will be deprecated.
      */
-    @BetaApi("This API depends on experimental gRPC APIs")
+    @Deprecated
     public Builder setRefreshingChannel(boolean isRefreshingChannel) {
       this.isRefreshingChannel = isRefreshingChannel;
       return this;
@@ -777,8 +781,12 @@ public Builder setPrimedTableIds(String... tableIds) {
       return this;
     }
 
-    /** Gets if channels will gracefully refresh connections to Cloud Bigtable service */
-    @BetaApi("This API depends on experimental gRPC APIs")
+    /**
+     * Gets if channels will gracefully refresh connections to Cloud Bigtable service.
+     *
+     * @deprecated Channel refreshing is enabled by default and this method will be deprecated.
+     */
+    @Deprecated
     public boolean isRefreshingChannel() {
       return isRefreshingChannel;
     }
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataSettingsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataSettingsTest.java
@@ -33,6 +33,9 @@ public void testToString() {
             .setInstanceId("our-instance-85")
             .setAppProfileId("our-appProfile-06")
             .enableBatchMutationLatencyBasedThrottling(10)
+            // disable channel priming so we won't need authentication
+            // for sending the prime request since we're only testing the settings.
+            .setRefreshingChannel(false)
             .build();
     EnhancedBigtableStubSettings stubSettings = settings.getStubSettings();
     assertThat(settings.toString())
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubCloseTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubCloseTest.java
@@ -65,7 +65,6 @@ public void setUp() throws Exception {
             .setProjectId(PROJECT_ID)
             .setInstanceId(INSTANCE_ID)
             .setCredentialsProvider(NoCredentialsProvider.create())
-            .setRefreshingChannel(false)
             .build()
             .getStubSettings();
 
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java
@@ -185,7 +185,10 @@ public void readRowsIsNotLostTest() {
     EnhancedBigtableStubSettings.Builder builder =
         EnhancedBigtableStubSettings.newBuilder()
             .setProjectId(dummyProjectId)
-            .setInstanceId(dummyInstanceId);
+            .setInstanceId(dummyInstanceId)
+            // Here and everywhere in this test, disable channel priming so we won't need
+            // authentication for sending the prime request since we're only testing the settings.
+            .setRefreshingChannel(false);
 
     RetrySettings retrySettings =
         RetrySettings.newBuilder()
@@ -243,7 +246,8 @@ public void readRowIsNotLostTest() {
     EnhancedBigtableStubSettings.Builder builder =
         EnhancedBigtableStubSettings.newBuilder()
             .setProjectId("my-project")
-            .setInstanceId("my-instance");
+            .setInstanceId("my-instance")
+            .setRefreshingChannel(false);
 
     RetrySettings retrySettings =
         RetrySettings.newBuilder()
@@ -295,7 +299,8 @@ public void readRowRetryCodesMustMatch() {
     EnhancedBigtableStubSettings.Builder builder =
         EnhancedBigtableStubSettings.newBuilder()
             .setProjectId("my-project")
-            .setInstanceId("my-instance");
+            .setInstanceId("my-instance")
+            .setRefreshingChannel(false);
 
     builder.readRowsSettings().setRetryableCodes(Code.DEADLINE_EXCEEDED);
 
@@ -329,7 +334,8 @@ public void sampleRowKeysSettingsAreNotLostTest() {
     EnhancedBigtableStubSettings.Builder builder =
         EnhancedBigtableStubSettings.newBuilder()
             .setProjectId(dummyProjectId)
-            .setInstanceId(dummyInstanceId);
+            .setInstanceId(dummyInstanceId)
+            .setRefreshingChannel(false);
 
     RetrySettings retrySettings =
         RetrySettings.newBuilder()
@@ -376,7 +382,8 @@ public void mutateRowSettingsAreNotLostTest() {
     EnhancedBigtableStubSettings.Builder builder =
         EnhancedBigtableStubSettings.newBuilder()
             .setProjectId(dummyProjectId)
-            .setInstanceId(dummyInstanceId);
+            .setInstanceId(dummyInstanceId)
+            .setRefreshingChannel(false);
 
     RetrySettings retrySettings =
         RetrySettings.newBuilder()
@@ -423,7 +430,8 @@ public void bulkMutateRowsSettingsAreNotLostTest() {
     EnhancedBigtableStubSettings.Builder builder =
         EnhancedBigtableStubSettings.newBuilder()
             .setProjectId(dummyProjectId)
-            .setInstanceId(dummyInstanceId);
+            .setInstanceId(dummyInstanceId)
+            .setRefreshingChannel(false);
 
     assertThat(builder.bulkMutateRowsSettings().isLatencyBasedThrottlingEnabled()).isFalse();
 
@@ -536,7 +544,8 @@ public void bulkReadRowsSettingsAreNotLostTest() {
     EnhancedBigtableStubSettings.Builder builder =
         EnhancedBigtableStubSettings.newBuilder()
             .setProjectId(dummyProjectId)
-            .setInstanceId(dummyInstanceId);
+            .setInstanceId(dummyInstanceId)
+            .setRefreshingChannel(false);
 
     RetrySettings retrySettings =
         RetrySettings.newBuilder()
@@ -611,7 +620,8 @@ public void checkAndMutateRowSettingsAreNotLostTest() {
     EnhancedBigtableStubSettings.Builder builder =
         EnhancedBigtableStubSettings.newBuilder()
             .setProjectId(dummyProjectId)
-            .setInstanceId(dummyInstanceId);
+            .setInstanceId(dummyInstanceId)
+            .setRefreshingChannel(false);
 
     RetrySettings retrySettings = RetrySettings.newBuilder().build();
     builder
@@ -677,9 +687,7 @@ public void isRefreshingChannelDefaultValueTest() {
         EnhancedBigtableStubSettings.newBuilder()
             .setProjectId(dummyProjectId)
             .setInstanceId(dummyInstanceId);
-    assertThat(builder.isRefreshingChannel()).isFalse();
-    assertThat(builder.build().isRefreshingChannel()).isFalse();
-    assertThat(builder.build().toBuilder().isRefreshingChannel()).isFalse();
+    assertThat(builder.isRefreshingChannel()).isTrue();
   }
 
   @Test
@@ -721,6 +729,7 @@ public void testToString() {
             .setProjectId("our-project-85")
             .setInstanceId("our-instance-06")
             .setAppProfileId("our-appProfile-06")
+            .setRefreshingChannel(false)
             .build();
 
     checkToString(defaultSettings);
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java
@@ -162,7 +162,6 @@ public void testJwtAudience()
             .setCredentialsProvider(FixedCredentialsProvider.create(jwtCreds))
             .build();
     enhancedBigtableStub = EnhancedBigtableStub.create(settings);
-
     // Send rpc and grab the credentials sent
     enhancedBigtableStub.readRowCallable().futureCall(Query.create("fake-table")).get();
     Metadata metadata = metadataInterceptor.headers.take();
@@ -208,6 +207,9 @@ public void testBatchJwtAudience()
               .setTransportChannelProvider(
                   FixedTransportChannelProvider.create(
                       GrpcTransportChannel.create(emulatorChannel)))
+              // Channel refreshing doesn't work with FixedTransportChannelProvider. Disable it for
+              // the test
+              .setRefreshingChannel(false)
               .build();
       enhancedBigtableStub = EnhancedBigtableStub.create(settings);
       // Send rpc and grab the credentials sent
@@ -342,7 +344,7 @@ public void export(Collection<SpanData> collection) {
   @Test
   public void testBulkMutationFlowControllerConfigured() throws Exception {
     BigtableDataSettings.Builder settings =
-        BigtableDataSettings.newBuilder()
+        BigtableDataSettings.newBuilderForEmulator(server.getPort())
             .setProjectId("my-project")
             .setInstanceId("my-instance")
             .setCredentialsProvider(defaultSettings.getCredentialsProvider())
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/HeadersTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/HeadersTest.java
@@ -233,6 +233,7 @@ public void checkAndMutateRow(
         CheckAndMutateRowRequest request,
         StreamObserver<CheckAndMutateRowResponse> responseObserver) {
       responseObserver.onNext(CheckAndMutateRowResponse.getDefaultInstance());
+      responseObserver.onCompleted();
     }
 
     @Override
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsRetryTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsRetryTest.java
@@ -69,6 +69,8 @@ public void setUp() throws IOException {
         .setTransportChannelProvider(
             FixedTransportChannelProvider.create(
                 GrpcTransportChannel.create(serverRule.getChannel())))
+        // channel priming doesn't work with FixedTransportChannelProvider. Disable it for the test
+        .setRefreshingChannel(false)
         .build();
 
     this.client = BigtableDataClient.create(settings.build());
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsRetryTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsRetryTest.java
@@ -81,6 +81,8 @@ public void setUp() throws IOException {
         .setTransportChannelProvider(
             FixedTransportChannelProvider.create(
                 GrpcTransportChannel.create(serverRule.getChannel())))
+        // Refreshing channel doesn't work with FixedTransportChannelProvider
+        .setRefreshingChannel(false)
         .build();
 
     client = BigtableDataClient.create(settings.build());
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/CloudEnv.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/CloudEnv.java
@@ -120,7 +120,10 @@ private CloudEnv(
     this.kmsKeyName = kmsKeyName;
 
     this.dataSettings =
-        BigtableDataSettings.newBuilder().setProjectId(projectId).setInstanceId(instanceId);
+        BigtableDataSettings.newBuilder()
+            .setProjectId(projectId)
+            .setInstanceId(instanceId)
+            .setRefreshingChannel(false);
     if (!Strings.isNullOrEmpty(dataEndpoint)) {
       dataSettings.stubSettings().setEndpoint(dataEndpoint);
     }
diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/EmulatorEnv.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/EmulatorEnv.java
@@ -61,6 +61,7 @@ void start() throws Exception {
         BigtableDataSettings.newBuilderForEmulator(emulator.getPort())
             .setProjectId("fake-project")
             .setInstanceId("fake-instance")
+            .setRefreshingChannel(false)
             .build();
 
     dataClient = BigtableDataClient.create(dataSettings);
