From 1f22527b5b432b20d58d7f253976dd44363846be Mon Sep 17 00:00:00 2001 From: Sushan Bhattarai Date: Tue, 31 Mar 2026 21:15:33 -0400 Subject: [PATCH 1/2] feat(bigtable): enable directaccess by default --- .../data/v2/stub/BigtableClientContext.java | 7 +- .../v2/stub/EnhancedBigtableStubSettings.java | 4 +- .../v2/BigtableDataClientFactoryTest.java | 97 ++++++++++++++++++- .../v2/stub/EnhancedBigtableStubTest.java | 4 +- 4 files changed, 105 insertions(+), 7 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java index c4f882e2f7..696065d236 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java @@ -33,6 +33,7 @@ import com.google.cloud.bigtable.data.v2.internal.csm.MetricsImpl; import com.google.cloud.bigtable.data.v2.internal.csm.attributes.ClientInfo; import com.google.cloud.bigtable.data.v2.internal.dp.AlwaysEnabledDirectAccessChecker; +import com.google.cloud.bigtable.data.v2.internal.dp.ClassicDirectAccessChecker; import com.google.cloud.bigtable.data.v2.internal.dp.DirectAccessChecker; import com.google.cloud.bigtable.data.v2.internal.dp.NoopDirectAccessChecker; import com.google.cloud.bigtable.data.v2.stub.metrics.CustomOpenTelemetryMetricsProvider; @@ -170,9 +171,13 @@ public static BigtableClientContext create( directAccessChecker = AlwaysEnabledDirectAccessChecker.INSTANCE; break; case FORCED_OFF: - case DEFAULT: directAccessChecker = NoopDirectAccessChecker.INSTANCE; break; + case DEFAULT: + default: + System.out.println("Using default direct access checker"); + directAccessChecker = new ClassicDirectAccessChecker(metrics.getDirectPathCompatibleTracer(), channelPrimer, backgroundExecutor); + break; } BigtableTransportChannelProvider btTransportProvider = 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 dcd0879b40..9a7944fbf3 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 @@ -182,7 +182,7 @@ public String getProjectId() { @InternalApi public DirectPathConfig getDirectPathConfig() { - return DIRECT_PATH_CONFIG; + return this.directPathConfig; } /** Returns the target instance id. */ @@ -637,7 +637,7 @@ private Builder() { // TODO: flip the bit setDirectAccessRequested and setTrafficDirectorEnabled once we make // client compatible by default. - boolean isDirectPathRequested = directPathConfig == DirectPathConfig.FORCED_ON; + boolean isDirectPathRequested = directPathConfig != DirectPathConfig.FORCED_OFF; featureFlags = FeatureFlags.newBuilder() .setReverseScans(true) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java index aa8ec31137..6709174003 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java @@ -268,8 +268,7 @@ public void testCreateWithRefreshingChannel() throws Exception { .stubSettings() .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setBackgroundExecutorProvider(executorProvider) - .setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.FORCED_ON); + .setBackgroundExecutorProvider(executorProvider); InstantiatingGrpcChannelProvider channelProvider = (InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider(); InstantiatingGrpcChannelProvider.Builder channelProviderBuilder = channelProvider.toBuilder(); @@ -281,6 +280,99 @@ public void testCreateWithRefreshingChannel() throws Exception { factory.createForAppProfile("other-appprofile"); factory.createForInstance("other-project", "other-instance"); + // Make sure that only 1 instance is created by each provider + // getCredentials was called twice, in patchCredentials and when creating the fixed credentials + // in BigtableClientContext + Mockito.verify(credentialsProvider, Mockito.times(2)).getCredentials(); + Mockito.verify(executorProvider, Mockito.times(1)).getExecutor(); + Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog(); + assertThat(warmedChannels).hasSize(poolSize+1); + assertThat(warmedChannels.values()).doesNotContain(false); + + // Wait for all the connections to close asynchronously + factory.close(); + long sleepTimeMs = 1000; + Thread.sleep(sleepTimeMs); + // Verify that all the channels are closed + assertThat(terminateAttributes).hasSize(poolSize+1); + } + + @Test + public void testCreateWithRefreshingChannelWithDirectAccessByDefault() throws Exception { + int poolSize = 3; + // TODO: remove the suppression when setRefreshingChannel can be removed + @SuppressWarnings("deprecation") + BigtableDataSettings.Builder builder = + BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .setProjectId(DEFAULT_PROJECT_ID) + .setInstanceId(DEFAULT_INSTANCE_ID) + .setAppProfileId(DEFAULT_APP_PROFILE_ID) + .setRefreshingChannel(true); + builder + .stubSettings() + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) + .setBackgroundExecutorProvider(executorProvider) + .setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.DEFAULT); + InstantiatingGrpcChannelProvider channelProvider = + (InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider(); + InstantiatingGrpcChannelProvider.Builder channelProviderBuilder = channelProvider.toBuilder(); + channelProviderBuilder.setChannelPoolSettings(ChannelPoolSettings.staticallySized(poolSize)); + builder.stubSettings().setTransportChannelProvider(channelProviderBuilder.build()); + + BigtableDataClientFactory factory = BigtableDataClientFactory.create(builder.build()); + factory.createDefault(); + factory.createForAppProfile("other-appprofile"); + factory.createForInstance("other-project", "other-instance"); + + // Make sure that only 1 instance is created by each provider + // getCredentials was called twice, in patchCredentials and when creating the fixed credentials + // in BigtableClientContext + Mockito.verify(credentialsProvider, Mockito.times(2)).getCredentials(); + Mockito.verify(executorProvider, Mockito.times(1)).getExecutor(); + Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog(); + assertThat(warmedChannels).hasSize(poolSize+1); + assertThat(warmedChannels.values()).doesNotContain(false); + + // Wait for all the connections to close asynchronously + factory.close(); + long sleepTimeMs = 1000; + Thread.sleep(sleepTimeMs); + // Verify that all the channels are closed + // If we have DEFAULT, it will add one channel temporily + assertThat(terminateAttributes).hasSize(poolSize+1); + } + + + @Test + public void testCreateWithRefreshingChannelDisableDirectAccess() throws Exception { + int poolSize = 3; + // TODO: remove the suppression when setRefreshingChannel can be removed + @SuppressWarnings("deprecation") + BigtableDataSettings.Builder builder = + BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .setProjectId(DEFAULT_PROJECT_ID) + .setInstanceId(DEFAULT_INSTANCE_ID) + .setAppProfileId(DEFAULT_APP_PROFILE_ID) + .setRefreshingChannel(true); + + builder + .stubSettings() + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) + .setBackgroundExecutorProvider(executorProvider) + .setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.FORCED_OFF); + InstantiatingGrpcChannelProvider channelProvider = + (InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider(); + InstantiatingGrpcChannelProvider.Builder channelProviderBuilder = channelProvider.toBuilder(); + channelProviderBuilder.setChannelPoolSettings(ChannelPoolSettings.staticallySized(poolSize)); + builder.stubSettings().setTransportChannelProvider(channelProviderBuilder.build()); + + BigtableDataClientFactory factory = BigtableDataClientFactory.create(builder.build()); + factory.createDefault(); + factory.createForAppProfile("other-appprofile"); + factory.createForInstance("other-project", "other-instance"); + // Make sure that only 1 instance is created by each provider // getCredentials was called twice, in patchCredentials and when creating the fixed credentials // in BigtableClientContext @@ -295,6 +387,7 @@ public void testCreateWithRefreshingChannel() throws Exception { long sleepTimeMs = 1000; Thread.sleep(sleepTimeMs); // Verify that all the channels are closed + // If we have DEFAULT, it will add one channel temporily assertThat(terminateAttributes).hasSize(poolSize); } 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 bfc94dea50..a46eab7005 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 @@ -549,11 +549,11 @@ public void testChannelPrimerConfigured() throws IOException { // TODO: remove the suppression once setRefreshingChannel can be removed @SuppressWarnings("deprecation") EnhancedBigtableStubSettings settings = - defaultSettings.toBuilder().setRefreshingChannel(true).build(); + defaultSettings.toBuilder().setRefreshingChannel(true).setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.DEFAULT).build(); try (EnhancedBigtableStub ignored = EnhancedBigtableStub.create(settings)) { // direct access checker ping - assertThat(fakeDataService.pingRequests).hasSize(1); + assertThat(fakeDataService.pingRequests).hasSize(2); } } From 4c7eb1e658206d562d41db52b8951841d2ac8636 Mon Sep 17 00:00:00 2001 From: Sushan Bhattarai Date: Tue, 31 Mar 2026 21:28:08 -0400 Subject: [PATCH 2/2] fix --- .../data/v2/stub/BigtableClientContext.java | 5 +- .../v2/BigtableDataClientFactoryTest.java | 53 +++++++++---------- .../v2/stub/EnhancedBigtableStubTest.java | 5 +- 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java index 696065d236..12cab879fc 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java @@ -175,8 +175,9 @@ public static BigtableClientContext create( break; case DEFAULT: default: - System.out.println("Using default direct access checker"); - directAccessChecker = new ClassicDirectAccessChecker(metrics.getDirectPathCompatibleTracer(), channelPrimer, backgroundExecutor); + directAccessChecker = + new ClassicDirectAccessChecker( + metrics.getDirectPathCompatibleTracer(), channelPrimer, backgroundExecutor); break; } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java index 6709174003..30a4853a61 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactoryTest.java @@ -286,7 +286,7 @@ public void testCreateWithRefreshingChannel() throws Exception { Mockito.verify(credentialsProvider, Mockito.times(2)).getCredentials(); Mockito.verify(executorProvider, Mockito.times(1)).getExecutor(); Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog(); - assertThat(warmedChannels).hasSize(poolSize+1); + assertThat(warmedChannels).hasSize(poolSize + 1); assertThat(warmedChannels.values()).doesNotContain(false); // Wait for all the connections to close asynchronously @@ -294,7 +294,7 @@ public void testCreateWithRefreshingChannel() throws Exception { long sleepTimeMs = 1000; Thread.sleep(sleepTimeMs); // Verify that all the channels are closed - assertThat(terminateAttributes).hasSize(poolSize+1); + assertThat(terminateAttributes).hasSize(poolSize + 1); } @Test @@ -303,19 +303,19 @@ public void testCreateWithRefreshingChannelWithDirectAccessByDefault() throws Ex // TODO: remove the suppression when setRefreshingChannel can be removed @SuppressWarnings("deprecation") BigtableDataSettings.Builder builder = - BigtableDataSettings.newBuilderForEmulator(server.getPort()) - .setProjectId(DEFAULT_PROJECT_ID) - .setInstanceId(DEFAULT_INSTANCE_ID) - .setAppProfileId(DEFAULT_APP_PROFILE_ID) - .setRefreshingChannel(true); + BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .setProjectId(DEFAULT_PROJECT_ID) + .setInstanceId(DEFAULT_INSTANCE_ID) + .setAppProfileId(DEFAULT_APP_PROFILE_ID) + .setRefreshingChannel(true); builder - .stubSettings() - .setCredentialsProvider(credentialsProvider) - .setStreamWatchdogProvider(watchdogProvider) - .setBackgroundExecutorProvider(executorProvider) - .setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.DEFAULT); + .stubSettings() + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) + .setBackgroundExecutorProvider(executorProvider) + .setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.DEFAULT); InstantiatingGrpcChannelProvider channelProvider = - (InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider(); + (InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider(); InstantiatingGrpcChannelProvider.Builder channelProviderBuilder = channelProvider.toBuilder(); channelProviderBuilder.setChannelPoolSettings(ChannelPoolSettings.staticallySized(poolSize)); builder.stubSettings().setTransportChannelProvider(channelProviderBuilder.build()); @@ -331,7 +331,7 @@ public void testCreateWithRefreshingChannelWithDirectAccessByDefault() throws Ex Mockito.verify(credentialsProvider, Mockito.times(2)).getCredentials(); Mockito.verify(executorProvider, Mockito.times(1)).getExecutor(); Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog(); - assertThat(warmedChannels).hasSize(poolSize+1); + assertThat(warmedChannels).hasSize(poolSize + 1); assertThat(warmedChannels.values()).doesNotContain(false); // Wait for all the connections to close asynchronously @@ -340,30 +340,29 @@ public void testCreateWithRefreshingChannelWithDirectAccessByDefault() throws Ex Thread.sleep(sleepTimeMs); // Verify that all the channels are closed // If we have DEFAULT, it will add one channel temporily - assertThat(terminateAttributes).hasSize(poolSize+1); + assertThat(terminateAttributes).hasSize(poolSize + 1); } - @Test public void testCreateWithRefreshingChannelDisableDirectAccess() throws Exception { int poolSize = 3; // TODO: remove the suppression when setRefreshingChannel can be removed @SuppressWarnings("deprecation") BigtableDataSettings.Builder builder = - BigtableDataSettings.newBuilderForEmulator(server.getPort()) - .setProjectId(DEFAULT_PROJECT_ID) - .setInstanceId(DEFAULT_INSTANCE_ID) - .setAppProfileId(DEFAULT_APP_PROFILE_ID) - .setRefreshingChannel(true); + BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .setProjectId(DEFAULT_PROJECT_ID) + .setInstanceId(DEFAULT_INSTANCE_ID) + .setAppProfileId(DEFAULT_APP_PROFILE_ID) + .setRefreshingChannel(true); builder - .stubSettings() - .setCredentialsProvider(credentialsProvider) - .setStreamWatchdogProvider(watchdogProvider) - .setBackgroundExecutorProvider(executorProvider) - .setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.FORCED_OFF); + .stubSettings() + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) + .setBackgroundExecutorProvider(executorProvider) + .setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.FORCED_OFF); InstantiatingGrpcChannelProvider channelProvider = - (InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider(); + (InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider(); InstantiatingGrpcChannelProvider.Builder channelProviderBuilder = channelProvider.toBuilder(); channelProviderBuilder.setChannelPoolSettings(ChannelPoolSettings.staticallySized(poolSize)); builder.stubSettings().setTransportChannelProvider(channelProviderBuilder.build()); 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 a46eab7005..462699bbd0 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 @@ -549,7 +549,10 @@ public void testChannelPrimerConfigured() throws IOException { // TODO: remove the suppression once setRefreshingChannel can be removed @SuppressWarnings("deprecation") EnhancedBigtableStubSettings settings = - defaultSettings.toBuilder().setRefreshingChannel(true).setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.DEFAULT).build(); + defaultSettings.toBuilder() + .setRefreshingChannel(true) + .setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.DEFAULT) + .build(); try (EnhancedBigtableStub ignored = EnhancedBigtableStub.create(settings)) { // direct access checker ping