From 035e7363ff6737976dc7b7cc49089721d68036ea Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 14 Dec 2022 11:20:27 -0500 Subject: [PATCH 1/8] feat: enable channel priming by default --- .../cloud/bigtable/data/v2/BigtableDataSettings.java | 11 ++++------- .../data/v2/stub/EnhancedBigtableStubSettings.java | 8 ++------ 2 files changed, 6 insertions(+), 13 deletions(-) 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 index db34f5fa6520..487b7d66ec6c 100644 --- 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 @@ -245,7 +245,6 @@ public String 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") public boolean isRefreshingChannel() { return stubSettings.isRefreshingChannel(); } @@ -395,19 +394,17 @@ public CredentialsProvider getCredentialsProvider() { /** * Configure periodic gRPC channel refreshes. * - *

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 + *

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. This feature is enabled by default. */ - @BetaApi("Channel priming is not currently stable and may change in the future") 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") public boolean isRefreshingChannel() { return stubSettings.isRefreshingChannel(); } 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 index f2ae486d9ec9..b4747513a2e4 100644 --- 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; @@ -237,7 +236,6 @@ public String getAppProfileId() { } /** Returns if channels will gracefully refresh connections to Cloud Bigtable service */ - @BetaApi("This API depends on experimental gRPC APIs") public boolean isRefreshingChannel() { return isRefreshingChannel; } @@ -545,7 +543,7 @@ public static class Builder extends StubSettings.BuilderWhen 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 */ - @BetaApi("This API depends on experimental gRPC APIs") public Builder setRefreshingChannel(boolean isRefreshingChannel) { this.isRefreshingChannel = isRefreshingChannel; return this; @@ -778,7 +775,6 @@ public Builder setPrimedTableIds(String... tableIds) { } /** Gets if channels will gracefully refresh connections to Cloud Bigtable service */ - @BetaApi("This API depends on experimental gRPC APIs") public boolean isRefreshingChannel() { return isRefreshingChannel; } From 123a9fa6b094413b0b6b2e34aff93793cd191c9e Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 15 Dec 2022 13:03:47 -0500 Subject: [PATCH 2/8] fix tests --- .../data/v2/BigtableDataSettings.java | 1 + .../data/v2/stub/BigtableChannelPrimer.java | 2 + .../stub/EnhancedBigtableStubCloseTest.java | 1 - .../EnhancedBigtableStubSettingsTest.java | 6 +- .../v2/stub/EnhancedBigtableStubTest.java | 81 ++++++++----------- .../bigtable/data/v2/stub/HeadersTest.java | 1 + .../stub/mutaterows/MutateRowsRetryTest.java | 2 + .../v2/stub/readrows/ReadRowsRetryTest.java | 2 + 8 files changed, 43 insertions(+), 53 deletions(-) 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 index 487b7d66ec6c..2fbfdecfd0fb 100644 --- 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,7 @@ public static Builder newBuilderForEmulator(String hostname, int port) { .stubSettings() .setCredentialsProvider(NoCredentialsProvider.create()) .setEndpoint(hostname + ":" + port) + .setRefreshingChannel(false) .setTransportChannelProvider( InstantiatingGrpcChannelProvider.newBuilder() .setMaxInboundMessageSize(256 * 1024 * 1024) 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 index 303f52a8e2dc..ecbef85be5ec 100644 --- 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/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 index 597b109b0bf1..e918378691b5 100644 --- 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 index d5a22884b639..5564184d9157 100644 --- 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 @@ -677,9 +677,9 @@ 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(); + assertThat(builder.build().isRefreshingChannel()).isTrue(); + assertThat(builder.build().toBuilder().isRefreshingChannel()).isTrue(); } @Test 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 index 8176435f1545..8886c88e41ee 100644 --- 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 @@ -274,6 +276,8 @@ public void testChannelPrimerConfigured() throws IOException { public void testUserAgent() throws InterruptedException { ServerStreamingCallable streamingCallable = enhancedBigtableStub.createReadRowsCallable(new DefaultRowAdapter()); + // Clear the headers which will have the priming request + metadataInterceptor.headers.clear(); Query request = Query.create("table-id").rowKey("row-key"); streamingCallable.call(request).iterator().next(); @@ -342,7 +346,7 @@ public void export(Collection 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()) @@ -424,64 +428,43 @@ public void testBulkMutationFlowControllerConfigured() throws Exception { @Test public void testCallContextPropagatedInMutationBatcher() - throws IOException, InterruptedException, ExecutionException { - EnhancedBigtableStubSettings settings = - defaultSettings - .toBuilder() - .setRefreshingChannel(true) - .setPrimedTableIds("table1", "table2") - .build(); - - try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings)) { - // clear the previous contexts - contextInterceptor.contexts.clear(); + throws InterruptedException, ExecutionException { + contextInterceptor.contexts.clear(); - // Override the timeout - GrpcCallContext clientCtx = - GrpcCallContext.createDefault().withTimeout(Duration.ofMinutes(10)); - - // Send a batch - try (Batcher batcher = - stub.newMutateRowsBatcher("table1", clientCtx)) { - batcher.add(RowMutationEntry.create("key").deleteRow()).get(); - } + // Override the timeout + GrpcCallContext clientCtx = GrpcCallContext.createDefault().withTimeout(Duration.ofMinutes(10)); - // Ensure that the server got the overriden deadline - Context serverCtx = contextInterceptor.contexts.poll(); - assertThat(serverCtx).isNotNull(); - assertThat(serverCtx.getDeadline()).isAtLeast(Deadline.after(8, TimeUnit.MINUTES)); + // Send a batch + try (Batcher batcher = + enhancedBigtableStub.newMutateRowsBatcher("table1", clientCtx)) { + batcher.add(RowMutationEntry.create("key").deleteRow()).get(); } + + // Ensure that the server got the overriden deadline + Context serverCtx = contextInterceptor.contexts.poll(); + assertThat(serverCtx).isNotNull(); + assertThat(serverCtx.getDeadline()).isAtLeast(Deadline.after(8, TimeUnit.MINUTES)); } @Test public void testCallContextPropagatedInReadBatcher() throws IOException, InterruptedException, ExecutionException { - EnhancedBigtableStubSettings settings = - defaultSettings - .toBuilder() - .setRefreshingChannel(true) - .setPrimedTableIds("table1", "table2") - .build(); + // clear the previous contexts + contextInterceptor.contexts.clear(); - try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings)) { - // clear the previous contexts - contextInterceptor.contexts.clear(); + // Override the timeout + GrpcCallContext clientCtx = GrpcCallContext.createDefault().withTimeout(Duration.ofMinutes(10)); - // Override the timeout - GrpcCallContext clientCtx = - GrpcCallContext.createDefault().withTimeout(Duration.ofMinutes(10)); - - // Send a batch - try (Batcher batcher = - stub.newBulkReadRowsBatcher(Query.create("table1"), clientCtx)) { - batcher.add(ByteString.copyFromUtf8("key")).get(); - } - - // Ensure that the server got the overriden deadline - Context serverCtx = contextInterceptor.contexts.poll(); - assertThat(serverCtx).isNotNull(); - assertThat(serverCtx.getDeadline()).isAtLeast(Deadline.after(8, TimeUnit.MINUTES)); + // Send a batch + try (Batcher batcher = + enhancedBigtableStub.newBulkReadRowsBatcher(Query.create("table1"), clientCtx)) { + batcher.add(ByteString.copyFromUtf8("key")).get(); } + + // Ensure that the server got the overriden deadline + Context serverCtx = contextInterceptor.contexts.poll(); + assertThat(serverCtx).isNotNull(); + assertThat(serverCtx.getDeadline()).isAtLeast(Deadline.after(8, TimeUnit.MINUTES)); } private static class MetadataInterceptor implements ServerInterceptor { 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 index f1655e2880d5..020c0a1c27d9 100644 --- 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 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 index 19ab6413a0c6..5d15dd521907 100644 --- 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 index 54a3af5cf5b3..0f9ad21fa0d1 100644 --- 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()); From 79b1e353dc710d94e7d5023508d259b4d7d445b7 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 15 Dec 2022 13:14:28 -0500 Subject: [PATCH 3/8] updates --- .../data/v2/BigtableDataSettings.java | 1 + .../v2/stub/EnhancedBigtableStubTest.java | 75 ++++++++++++------- 2 files changed, 48 insertions(+), 28 deletions(-) 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 index 2fbfdecfd0fb..d265843a02aa 100644 --- 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,7 @@ 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() 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 index 8886c88e41ee..16100ad044f0 100644 --- 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 @@ -276,8 +276,6 @@ public void testChannelPrimerConfigured() throws IOException { public void testUserAgent() throws InterruptedException { ServerStreamingCallable streamingCallable = enhancedBigtableStub.createReadRowsCallable(new DefaultRowAdapter()); - // Clear the headers which will have the priming request - metadataInterceptor.headers.clear(); Query request = Query.create("table-id").rowKey("row-key"); streamingCallable.call(request).iterator().next(); @@ -428,43 +426,64 @@ public void testBulkMutationFlowControllerConfigured() throws Exception { @Test public void testCallContextPropagatedInMutationBatcher() - throws InterruptedException, ExecutionException { - contextInterceptor.contexts.clear(); + throws IOException, InterruptedException, ExecutionException { + EnhancedBigtableStubSettings settings = + defaultSettings + .toBuilder() + .setRefreshingChannel(true) + .setPrimedTableIds("table1", "table2") + .build(); - // Override the timeout - GrpcCallContext clientCtx = GrpcCallContext.createDefault().withTimeout(Duration.ofMinutes(10)); + try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings)) { + // clear the previous contexts + contextInterceptor.contexts.clear(); - // Send a batch - try (Batcher batcher = - enhancedBigtableStub.newMutateRowsBatcher("table1", clientCtx)) { - batcher.add(RowMutationEntry.create("key").deleteRow()).get(); - } + // Override the timeout + GrpcCallContext clientCtx = + GrpcCallContext.createDefault().withTimeout(Duration.ofMinutes(10)); + + // Send a batch + try (Batcher batcher = + stub.newMutateRowsBatcher("table1", clientCtx)) { + batcher.add(RowMutationEntry.create("key").deleteRow()).get(); + } - // Ensure that the server got the overriden deadline - Context serverCtx = contextInterceptor.contexts.poll(); - assertThat(serverCtx).isNotNull(); - assertThat(serverCtx.getDeadline()).isAtLeast(Deadline.after(8, TimeUnit.MINUTES)); + // Ensure that the server got the overriden deadline + Context serverCtx = contextInterceptor.contexts.poll(); + assertThat(serverCtx).isNotNull(); + assertThat(serverCtx.getDeadline()).isAtLeast(Deadline.after(8, TimeUnit.MINUTES)); + } } @Test public void testCallContextPropagatedInReadBatcher() throws IOException, InterruptedException, ExecutionException { - // clear the previous contexts - contextInterceptor.contexts.clear(); + EnhancedBigtableStubSettings settings = + defaultSettings + .toBuilder() + .setRefreshingChannel(true) + .setPrimedTableIds("table1", "table2") + .build(); - // Override the timeout - GrpcCallContext clientCtx = GrpcCallContext.createDefault().withTimeout(Duration.ofMinutes(10)); + try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings)) { + // clear the previous contexts + contextInterceptor.contexts.clear(); - // Send a batch - try (Batcher batcher = - enhancedBigtableStub.newBulkReadRowsBatcher(Query.create("table1"), clientCtx)) { - batcher.add(ByteString.copyFromUtf8("key")).get(); - } + // Override the timeout + GrpcCallContext clientCtx = + GrpcCallContext.createDefault().withTimeout(Duration.ofMinutes(10)); + + // Send a batch + try (Batcher batcher = + stub.newBulkReadRowsBatcher(Query.create("table1"), clientCtx)) { + batcher.add(ByteString.copyFromUtf8("key")).get(); + } - // Ensure that the server got the overriden deadline - Context serverCtx = contextInterceptor.contexts.poll(); - assertThat(serverCtx).isNotNull(); - assertThat(serverCtx.getDeadline()).isAtLeast(Deadline.after(8, TimeUnit.MINUTES)); + // Ensure that the server got the overriden deadline + Context serverCtx = contextInterceptor.contexts.poll(); + assertThat(serverCtx).isNotNull(); + assertThat(serverCtx.getDeadline()).isAtLeast(Deadline.after(8, TimeUnit.MINUTES)); + } } private static class MetadataInterceptor implements ServerInterceptor { From 3da5f7d91f55bb73c247df7998636967dd9be46d Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 15 Dec 2022 13:27:14 -0500 Subject: [PATCH 4/8] fix more tests --- .../data/v2/BigtableDataSettingsTest.java | 3 ++ .../EnhancedBigtableStubSettingsTest.java | 29 ++++++++++++------- 2 files changed, 22 insertions(+), 10 deletions(-) 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 index 2b95bf821fc7..de4fbe92fc5c 100644 --- 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/EnhancedBigtableStubSettingsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java index 5564184d9157..466355f8922f 100644 --- 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 @@ -678,8 +688,6 @@ public void isRefreshingChannelDefaultValueTest() { .setProjectId(dummyProjectId) .setInstanceId(dummyInstanceId); assertThat(builder.isRefreshingChannel()).isTrue(); - assertThat(builder.build().isRefreshingChannel()).isTrue(); - assertThat(builder.build().toBuilder().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); From cd9472be4801b11c2a40991747a5b1afc931ccd5 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 15 Dec 2022 13:54:19 -0500 Subject: [PATCH 5/8] update --- .../data/v2/BigtableDataSettings.java | 19 ++++++++++++++++--- .../v2/stub/EnhancedBigtableStubSettings.java | 17 +++++++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) 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 index d265843a02aa..28265b282d75 100644 --- 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 @@ -246,7 +246,12 @@ public String getAppProfileId() { return stubSettings.getAppProfileId(); } - /** Gets if channels will gracefully refresh connections to Cloud Bigtable service */ + /** + * 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(); } @@ -399,14 +404,22 @@ public CredentialsProvider getCredentialsProvider() { *

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. This feature is enabled by default. + * periodic spikes in latency. + * + * @deprecated Channel refreshing is enabled by default and this method will be deprecated. */ + @Deprecated public Builder setRefreshingChannel(boolean isRefreshingChannel) { stubSettings.setRefreshingChannel(isRefreshingChannel); return this; } - /** Gets if channels will gracefully refresh connections to Cloud Bigtable service */ + /** + * 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/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index b4747513a2e4..bc20e13bfc9d 100644 --- 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 @@ -235,7 +235,12 @@ public String getAppProfileId() { return appProfileId; } - /** Returns if channels will gracefully refresh connections to Cloud Bigtable service */ + /** + * 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; } @@ -758,7 +763,10 @@ public String getAppProfileId() { * 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. */ + @Deprecated public Builder setRefreshingChannel(boolean isRefreshingChannel) { this.isRefreshingChannel = isRefreshingChannel; return this; @@ -774,7 +782,12 @@ public Builder setPrimedTableIds(String... tableIds) { return this; } - /** Gets if channels will gracefully refresh connections to Cloud Bigtable service */ + /** + * 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; } From 6fa4acc85456e08a2e61894a6fb819575e1c0c6a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 15 Dec 2022 14:14:28 -0500 Subject: [PATCH 6/8] fix format --- .../bigtable/data/v2/stub/EnhancedBigtableStubSettings.java | 1 - 1 file changed, 1 deletion(-) 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 index bc20e13bfc9d..c78bdafbf356 100644 --- 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 @@ -763,7 +763,6 @@ public String getAppProfileId() { * 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. */ @Deprecated From 3228a829c64abf2d413b08cfec28de5e59f1919c Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 15 Dec 2022 15:25:54 -0500 Subject: [PATCH 7/8] disable channel refreshing for integration tests --- .../com/google/cloud/bigtable/test_helpers/env/CloudEnv.java | 5 ++++- .../google/cloud/bigtable/test_helpers/env/EmulatorEnv.java | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) 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 index ba0fda8b2ca9..f1308f3520af 100644 --- 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 index e4b0a26515cd..bec3e0eef2f8 100644 --- 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); From 93bf09c02fd27da00bbff84f27d9741b617b0670 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 16 Dec 2022 16:16:15 +0000 Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e74ebc210bee..993a28f454a4 100644 --- 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' ```