Skip to content

Commit 303959c

Browse files
mutianfmutianf
authored
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://github.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://github.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md).
1 parent 3e5ad15 commit 303959c

File tree

13 files changed

+77
-33
lines changed

13 files changed

+77
-33
lines changed

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ If you are using Maven without BOM, add this to your dependencies:
4949
If you are using Gradle 5.x or later, add this to your dependencies:
5050

5151
```Groovy
52-
implementation platform('com.google.cloud:libraries-bom:26.1.5')
52+
implementation platform('com.google.cloud:libraries-bom:26.2.0')
5353
5454
implementation 'com.google.cloud:google-cloud-bigtable'
5555
```

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java

+21-9
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ public static Builder newBuilderForEmulator(String hostname, int port) {
124124
.stubSettings()
125125
.setCredentialsProvider(NoCredentialsProvider.create())
126126
.setEndpoint(hostname + ":" + port)
127+
// disable channel refreshing when creating an emulator
128+
.setRefreshingChannel(false)
127129
.setTransportChannelProvider(
128130
InstantiatingGrpcChannelProvider.newBuilder()
129131
.setMaxInboundMessageSize(256 * 1024 * 1024)
@@ -244,8 +246,12 @@ public String getAppProfileId() {
244246
return stubSettings.getAppProfileId();
245247
}
246248

247-
/** Gets if channels will gracefully refresh connections to Cloud Bigtable service */
248-
@BetaApi("Channel priming is not currently stable and may change in the future")
249+
/**
250+
* Gets if channels will gracefully refresh connections to Cloud Bigtable service
251+
*
252+
* @deprecated Channel refreshing is enabled by default and this method will be deprecated.
253+
*/
254+
@Deprecated
249255
public boolean isRefreshingChannel() {
250256
return stubSettings.isRefreshingChannel();
251257
}
@@ -395,19 +401,25 @@ public CredentialsProvider getCredentialsProvider() {
395401
/**
396402
* Configure periodic gRPC channel refreshes.
397403
*
398-
* <p>This feature will gracefully refresh connections to the Cloud Bigtable service. This is an
399-
* experimental feature to address tail latency caused by the service dropping long lived gRPC
400-
* connections, which causes the client to renegotiate the gRPC connection in the request path,
401-
* which causes periodic spikes in latency
404+
* <p>This feature will gracefully refresh connections to the Cloud Bigtable service. This is a
405+
* feature to address tail latency caused by the service dropping long lived gRPC connections,
406+
* which causes the client to renegotiate the gRPC connection in the request path, which causes
407+
* periodic spikes in latency.
408+
*
409+
* @deprecated Channel refreshing is enabled by default and this method will be deprecated.
402410
*/
403-
@BetaApi("Channel priming is not currently stable and may change in the future")
411+
@Deprecated
404412
public Builder setRefreshingChannel(boolean isRefreshingChannel) {
405413
stubSettings.setRefreshingChannel(isRefreshingChannel);
406414
return this;
407415
}
408416

409-
/** Gets if channels will gracefully refresh connections to Cloud Bigtable service */
410-
@BetaApi("Channel priming is not currently stable and may change in the future")
417+
/**
418+
* Gets if channels will gracefully refresh connections to Cloud Bigtable service.
419+
*
420+
* @deprecated Channel refreshing is enabled by default and this method will be deprecated.
421+
*/
422+
@Deprecated
411423
public boolean isRefreshingChannel() {
412424
return stubSettings.isRefreshingChannel();
413425
}

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableChannelPrimer.java

+2
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ static BigtableChannelPrimer create(
5151
.setInstanceId(instanceId)
5252
.setAppProfileId(appProfileId)
5353
.setCredentialsProvider(FixedCredentialsProvider.create(credentials))
54+
// Disable refreshing channel here to avoid creating settings in a loop
55+
.setRefreshingChannel(false)
5456
.setExecutorProvider(
5557
InstantiatingExecutorProvider.newBuilder().setExecutorThreadCount(1).build());
5658

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java

+16-8
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package com.google.cloud.bigtable.data.v2.stub;
1717

18-
import com.google.api.core.BetaApi;
1918
import com.google.api.core.InternalApi;
2019
import com.google.api.gax.batching.BatchingCallSettings;
2120
import com.google.api.gax.batching.BatchingSettings;
@@ -236,8 +235,12 @@ public String getAppProfileId() {
236235
return appProfileId;
237236
}
238237

239-
/** Returns if channels will gracefully refresh connections to Cloud Bigtable service */
240-
@BetaApi("This API depends on experimental gRPC APIs")
238+
/**
239+
* Returns if channels will gracefully refresh connections to Cloud Bigtable service
240+
*
241+
* @deprecated Channel refreshing is enabled by default and this method will be deprecated.
242+
*/
243+
@Deprecated
241244
public boolean isRefreshingChannel() {
242245
return isRefreshingChannel;
243246
}
@@ -545,7 +548,7 @@ public static class Builder extends StubSettings.Builder<EnhancedBigtableStubSet
545548
*/
546549
private Builder() {
547550
this.appProfileId = SERVER_DEFAULT_APP_PROFILE_ID;
548-
this.isRefreshingChannel = false;
551+
this.isRefreshingChannel = true;
549552
primedTableIds = ImmutableList.of();
550553
jwtAudienceMapping = DEFAULT_JWT_AUDIENCE_MAPPING;
551554
setCredentialsProvider(defaultCredentialsProviderBuilder().build());
@@ -757,11 +760,12 @@ public String getAppProfileId() {
757760
* Sets if channels will gracefully refresh connections to Cloud Bigtable service.
758761
*
759762
* <p>When enabled, this will wait for the connection to complete the SSL handshake and warm up
760-
* serverside caches for all the tables of the instance.
763+
* serverside caches for all the tables of the instance. This feature is enabled by default.
761764
*
762765
* @see com.google.cloud.bigtable.data.v2.BigtableDataSettings.Builder#setRefreshingChannel
766+
* @deprecated Channel refreshing is enabled by default and this method will be deprecated.
763767
*/
764-
@BetaApi("This API depends on experimental gRPC APIs")
768+
@Deprecated
765769
public Builder setRefreshingChannel(boolean isRefreshingChannel) {
766770
this.isRefreshingChannel = isRefreshingChannel;
767771
return this;
@@ -777,8 +781,12 @@ public Builder setPrimedTableIds(String... tableIds) {
777781
return this;
778782
}
779783

780-
/** Gets if channels will gracefully refresh connections to Cloud Bigtable service */
781-
@BetaApi("This API depends on experimental gRPC APIs")
784+
/**
785+
* Gets if channels will gracefully refresh connections to Cloud Bigtable service.
786+
*
787+
* @deprecated Channel refreshing is enabled by default and this method will be deprecated.
788+
*/
789+
@Deprecated
782790
public boolean isRefreshingChannel() {
783791
return isRefreshingChannel;
784792
}

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataSettingsTest.java

+3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ public void testToString() {
3333
.setInstanceId("our-instance-85")
3434
.setAppProfileId("our-appProfile-06")
3535
.enableBatchMutationLatencyBasedThrottling(10)
36+
// disable channel priming so we won't need authentication
37+
// for sending the prime request since we're only testing the settings.
38+
.setRefreshingChannel(false)
3639
.build();
3740
EnhancedBigtableStubSettings stubSettings = settings.getStubSettings();
3841
assertThat(settings.toString())

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubCloseTest.java

-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ public void setUp() throws Exception {
6565
.setProjectId(PROJECT_ID)
6666
.setInstanceId(INSTANCE_ID)
6767
.setCredentialsProvider(NoCredentialsProvider.create())
68-
.setRefreshingChannel(false)
6968
.build()
7069
.getStubSettings();
7170

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java

+20-11
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,10 @@ public void readRowsIsNotLostTest() {
185185
EnhancedBigtableStubSettings.Builder builder =
186186
EnhancedBigtableStubSettings.newBuilder()
187187
.setProjectId(dummyProjectId)
188-
.setInstanceId(dummyInstanceId);
188+
.setInstanceId(dummyInstanceId)
189+
// Here and everywhere in this test, disable channel priming so we won't need
190+
// authentication for sending the prime request since we're only testing the settings.
191+
.setRefreshingChannel(false);
189192

190193
RetrySettings retrySettings =
191194
RetrySettings.newBuilder()
@@ -243,7 +246,8 @@ public void readRowIsNotLostTest() {
243246
EnhancedBigtableStubSettings.Builder builder =
244247
EnhancedBigtableStubSettings.newBuilder()
245248
.setProjectId("my-project")
246-
.setInstanceId("my-instance");
249+
.setInstanceId("my-instance")
250+
.setRefreshingChannel(false);
247251

248252
RetrySettings retrySettings =
249253
RetrySettings.newBuilder()
@@ -295,7 +299,8 @@ public void readRowRetryCodesMustMatch() {
295299
EnhancedBigtableStubSettings.Builder builder =
296300
EnhancedBigtableStubSettings.newBuilder()
297301
.setProjectId("my-project")
298-
.setInstanceId("my-instance");
302+
.setInstanceId("my-instance")
303+
.setRefreshingChannel(false);
299304

300305
builder.readRowsSettings().setRetryableCodes(Code.DEADLINE_EXCEEDED);
301306

@@ -329,7 +334,8 @@ public void sampleRowKeysSettingsAreNotLostTest() {
329334
EnhancedBigtableStubSettings.Builder builder =
330335
EnhancedBigtableStubSettings.newBuilder()
331336
.setProjectId(dummyProjectId)
332-
.setInstanceId(dummyInstanceId);
337+
.setInstanceId(dummyInstanceId)
338+
.setRefreshingChannel(false);
333339

334340
RetrySettings retrySettings =
335341
RetrySettings.newBuilder()
@@ -376,7 +382,8 @@ public void mutateRowSettingsAreNotLostTest() {
376382
EnhancedBigtableStubSettings.Builder builder =
377383
EnhancedBigtableStubSettings.newBuilder()
378384
.setProjectId(dummyProjectId)
379-
.setInstanceId(dummyInstanceId);
385+
.setInstanceId(dummyInstanceId)
386+
.setRefreshingChannel(false);
380387

381388
RetrySettings retrySettings =
382389
RetrySettings.newBuilder()
@@ -423,7 +430,8 @@ public void bulkMutateRowsSettingsAreNotLostTest() {
423430
EnhancedBigtableStubSettings.Builder builder =
424431
EnhancedBigtableStubSettings.newBuilder()
425432
.setProjectId(dummyProjectId)
426-
.setInstanceId(dummyInstanceId);
433+
.setInstanceId(dummyInstanceId)
434+
.setRefreshingChannel(false);
427435

428436
assertThat(builder.bulkMutateRowsSettings().isLatencyBasedThrottlingEnabled()).isFalse();
429437

@@ -536,7 +544,8 @@ public void bulkReadRowsSettingsAreNotLostTest() {
536544
EnhancedBigtableStubSettings.Builder builder =
537545
EnhancedBigtableStubSettings.newBuilder()
538546
.setProjectId(dummyProjectId)
539-
.setInstanceId(dummyInstanceId);
547+
.setInstanceId(dummyInstanceId)
548+
.setRefreshingChannel(false);
540549

541550
RetrySettings retrySettings =
542551
RetrySettings.newBuilder()
@@ -611,7 +620,8 @@ public void checkAndMutateRowSettingsAreNotLostTest() {
611620
EnhancedBigtableStubSettings.Builder builder =
612621
EnhancedBigtableStubSettings.newBuilder()
613622
.setProjectId(dummyProjectId)
614-
.setInstanceId(dummyInstanceId);
623+
.setInstanceId(dummyInstanceId)
624+
.setRefreshingChannel(false);
615625

616626
RetrySettings retrySettings = RetrySettings.newBuilder().build();
617627
builder
@@ -677,9 +687,7 @@ public void isRefreshingChannelDefaultValueTest() {
677687
EnhancedBigtableStubSettings.newBuilder()
678688
.setProjectId(dummyProjectId)
679689
.setInstanceId(dummyInstanceId);
680-
assertThat(builder.isRefreshingChannel()).isFalse();
681-
assertThat(builder.build().isRefreshingChannel()).isFalse();
682-
assertThat(builder.build().toBuilder().isRefreshingChannel()).isFalse();
690+
assertThat(builder.isRefreshingChannel()).isTrue();
683691
}
684692

685693
@Test
@@ -721,6 +729,7 @@ public void testToString() {
721729
.setProjectId("our-project-85")
722730
.setInstanceId("our-instance-06")
723731
.setAppProfileId("our-appProfile-06")
732+
.setRefreshingChannel(false)
724733
.build();
725734

726735
checkToString(defaultSettings);

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ public void testJwtAudience()
162162
.setCredentialsProvider(FixedCredentialsProvider.create(jwtCreds))
163163
.build();
164164
enhancedBigtableStub = EnhancedBigtableStub.create(settings);
165-
166165
// Send rpc and grab the credentials sent
167166
enhancedBigtableStub.readRowCallable().futureCall(Query.create("fake-table")).get();
168167
Metadata metadata = metadataInterceptor.headers.take();
@@ -208,6 +207,9 @@ public void testBatchJwtAudience()
208207
.setTransportChannelProvider(
209208
FixedTransportChannelProvider.create(
210209
GrpcTransportChannel.create(emulatorChannel)))
210+
// Channel refreshing doesn't work with FixedTransportChannelProvider. Disable it for
211+
// the test
212+
.setRefreshingChannel(false)
211213
.build();
212214
enhancedBigtableStub = EnhancedBigtableStub.create(settings);
213215
// Send rpc and grab the credentials sent
@@ -342,7 +344,7 @@ public void export(Collection<SpanData> collection) {
342344
@Test
343345
public void testBulkMutationFlowControllerConfigured() throws Exception {
344346
BigtableDataSettings.Builder settings =
345-
BigtableDataSettings.newBuilder()
347+
BigtableDataSettings.newBuilderForEmulator(server.getPort())
346348
.setProjectId("my-project")
347349
.setInstanceId("my-instance")
348350
.setCredentialsProvider(defaultSettings.getCredentialsProvider())

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/HeadersTest.java

+1
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ public void checkAndMutateRow(
233233
CheckAndMutateRowRequest request,
234234
StreamObserver<CheckAndMutateRowResponse> responseObserver) {
235235
responseObserver.onNext(CheckAndMutateRowResponse.getDefaultInstance());
236+
responseObserver.onCompleted();
236237
}
237238

238239
@Override

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsRetryTest.java

+2
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ public void setUp() throws IOException {
6969
.setTransportChannelProvider(
7070
FixedTransportChannelProvider.create(
7171
GrpcTransportChannel.create(serverRule.getChannel())))
72+
// channel priming doesn't work with FixedTransportChannelProvider. Disable it for the test
73+
.setRefreshingChannel(false)
7274
.build();
7375

7476
this.client = BigtableDataClient.create(settings.build());

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsRetryTest.java

+2
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ public void setUp() throws IOException {
8181
.setTransportChannelProvider(
8282
FixedTransportChannelProvider.create(
8383
GrpcTransportChannel.create(serverRule.getChannel())))
84+
// Refreshing channel doesn't work with FixedTransportChannelProvider
85+
.setRefreshingChannel(false)
8486
.build();
8587

8688
client = BigtableDataClient.create(settings.build());

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/CloudEnv.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,10 @@ private CloudEnv(
120120
this.kmsKeyName = kmsKeyName;
121121

122122
this.dataSettings =
123-
BigtableDataSettings.newBuilder().setProjectId(projectId).setInstanceId(instanceId);
123+
BigtableDataSettings.newBuilder()
124+
.setProjectId(projectId)
125+
.setInstanceId(instanceId)
126+
.setRefreshingChannel(false);
124127
if (!Strings.isNullOrEmpty(dataEndpoint)) {
125128
dataSettings.stubSettings().setEndpoint(dataEndpoint);
126129
}

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/EmulatorEnv.java

+1
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ void start() throws Exception {
6161
BigtableDataSettings.newBuilderForEmulator(emulator.getPort())
6262
.setProjectId("fake-project")
6363
.setInstanceId("fake-instance")
64+
.setRefreshingChannel(false)
6465
.build();
6566

6667
dataClient = BigtableDataClient.create(dataSettings);

0 commit comments

Comments
 (0)