feat: enable channel priming by default (#1555)

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).
googleapis · Dec 16, 2022 · 303959c · 303959c
1 parent 3e5ad15
commit 303959c
Show file tree

Hide file tree

Showing 13 changed files with 77 additions and 33 deletions.
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/...-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java b/...-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/...-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableChannelPrimer.java b/...-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/...le/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/...le/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/...ud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataSettingsTest.java b/...ud-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/...e/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubCloseTest.java b/...e/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/...rc/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java b/...rc/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/...gtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java b/...gtable/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/.../src/test/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsRetryTest.java b/.../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/...able/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsRetryTest.java b/...able/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/...-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/EmulatorEnv.java b/...-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);