Skip to content
This repository was archived by the owner on May 8, 2026. It is now read-only.

Commit 1f22527

Browse files
sushanbsushanb
committed
feat(bigtable): enable directaccess by default
1 parent c2ccda1 commit 1f22527

4 files changed

Lines changed: 105 additions & 7 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.google.cloud.bigtable.data.v2.internal.csm.MetricsImpl;
3434
import com.google.cloud.bigtable.data.v2.internal.csm.attributes.ClientInfo;
3535
import com.google.cloud.bigtable.data.v2.internal.dp.AlwaysEnabledDirectAccessChecker;
36+
import com.google.cloud.bigtable.data.v2.internal.dp.ClassicDirectAccessChecker;
3637
import com.google.cloud.bigtable.data.v2.internal.dp.DirectAccessChecker;
3738
import com.google.cloud.bigtable.data.v2.internal.dp.NoopDirectAccessChecker;
3839
import com.google.cloud.bigtable.data.v2.stub.metrics.CustomOpenTelemetryMetricsProvider;
@@ -170,9 +171,13 @@ public static BigtableClientContext create(
170171
directAccessChecker = AlwaysEnabledDirectAccessChecker.INSTANCE;
171172
break;
172173
case FORCED_OFF:
173-
case DEFAULT:
174174
directAccessChecker = NoopDirectAccessChecker.INSTANCE;
175175
break;
176+
case DEFAULT:
177+
default:
178+
System.out.println("Using default direct access checker");
179+
directAccessChecker = new ClassicDirectAccessChecker(metrics.getDirectPathCompatibleTracer(), channelPrimer, backgroundExecutor);
180+
break;
176181
}
177182

178183
BigtableTransportChannelProvider btTransportProvider =

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ public String getProjectId() {
182182

183183
@InternalApi
184184
public DirectPathConfig getDirectPathConfig() {
185-
return DIRECT_PATH_CONFIG;
185+
return this.directPathConfig;
186186
}
187187

188188
/** Returns the target instance id. */
@@ -637,7 +637,7 @@ private Builder() {
637637

638638
// TODO: flip the bit setDirectAccessRequested and setTrafficDirectorEnabled once we make
639639
// client compatible by default.
640-
boolean isDirectPathRequested = directPathConfig == DirectPathConfig.FORCED_ON;
640+
boolean isDirectPathRequested = directPathConfig != DirectPathConfig.FORCED_OFF;
641641
featureFlags =
642642
FeatureFlags.newBuilder()
643643
.setReverseScans(true)

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

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,7 @@ public void testCreateWithRefreshingChannel() throws Exception {
268268
.stubSettings()
269269
.setCredentialsProvider(credentialsProvider)
270270
.setStreamWatchdogProvider(watchdogProvider)
271-
.setBackgroundExecutorProvider(executorProvider)
272-
.setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.FORCED_ON);
271+
.setBackgroundExecutorProvider(executorProvider);
273272
InstantiatingGrpcChannelProvider channelProvider =
274273
(InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider();
275274
InstantiatingGrpcChannelProvider.Builder channelProviderBuilder = channelProvider.toBuilder();
@@ -281,6 +280,99 @@ public void testCreateWithRefreshingChannel() throws Exception {
281280
factory.createForAppProfile("other-appprofile");
282281
factory.createForInstance("other-project", "other-instance");
283282

283+
// Make sure that only 1 instance is created by each provider
284+
// getCredentials was called twice, in patchCredentials and when creating the fixed credentials
285+
// in BigtableClientContext
286+
Mockito.verify(credentialsProvider, Mockito.times(2)).getCredentials();
287+
Mockito.verify(executorProvider, Mockito.times(1)).getExecutor();
288+
Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog();
289+
assertThat(warmedChannels).hasSize(poolSize+1);
290+
assertThat(warmedChannels.values()).doesNotContain(false);
291+
292+
// Wait for all the connections to close asynchronously
293+
factory.close();
294+
long sleepTimeMs = 1000;
295+
Thread.sleep(sleepTimeMs);
296+
// Verify that all the channels are closed
297+
assertThat(terminateAttributes).hasSize(poolSize+1);
298+
}
299+
300+
@Test
301+
public void testCreateWithRefreshingChannelWithDirectAccessByDefault() throws Exception {
302+
int poolSize = 3;
303+
// TODO: remove the suppression when setRefreshingChannel can be removed
304+
@SuppressWarnings("deprecation")
305+
BigtableDataSettings.Builder builder =
306+
BigtableDataSettings.newBuilderForEmulator(server.getPort())
307+
.setProjectId(DEFAULT_PROJECT_ID)
308+
.setInstanceId(DEFAULT_INSTANCE_ID)
309+
.setAppProfileId(DEFAULT_APP_PROFILE_ID)
310+
.setRefreshingChannel(true);
311+
builder
312+
.stubSettings()
313+
.setCredentialsProvider(credentialsProvider)
314+
.setStreamWatchdogProvider(watchdogProvider)
315+
.setBackgroundExecutorProvider(executorProvider)
316+
.setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.DEFAULT);
317+
InstantiatingGrpcChannelProvider channelProvider =
318+
(InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider();
319+
InstantiatingGrpcChannelProvider.Builder channelProviderBuilder = channelProvider.toBuilder();
320+
channelProviderBuilder.setChannelPoolSettings(ChannelPoolSettings.staticallySized(poolSize));
321+
builder.stubSettings().setTransportChannelProvider(channelProviderBuilder.build());
322+
323+
BigtableDataClientFactory factory = BigtableDataClientFactory.create(builder.build());
324+
factory.createDefault();
325+
factory.createForAppProfile("other-appprofile");
326+
factory.createForInstance("other-project", "other-instance");
327+
328+
// Make sure that only 1 instance is created by each provider
329+
// getCredentials was called twice, in patchCredentials and when creating the fixed credentials
330+
// in BigtableClientContext
331+
Mockito.verify(credentialsProvider, Mockito.times(2)).getCredentials();
332+
Mockito.verify(executorProvider, Mockito.times(1)).getExecutor();
333+
Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog();
334+
assertThat(warmedChannels).hasSize(poolSize+1);
335+
assertThat(warmedChannels.values()).doesNotContain(false);
336+
337+
// Wait for all the connections to close asynchronously
338+
factory.close();
339+
long sleepTimeMs = 1000;
340+
Thread.sleep(sleepTimeMs);
341+
// Verify that all the channels are closed
342+
// If we have DEFAULT, it will add one channel temporily
343+
assertThat(terminateAttributes).hasSize(poolSize+1);
344+
}
345+
346+
347+
@Test
348+
public void testCreateWithRefreshingChannelDisableDirectAccess() throws Exception {
349+
int poolSize = 3;
350+
// TODO: remove the suppression when setRefreshingChannel can be removed
351+
@SuppressWarnings("deprecation")
352+
BigtableDataSettings.Builder builder =
353+
BigtableDataSettings.newBuilderForEmulator(server.getPort())
354+
.setProjectId(DEFAULT_PROJECT_ID)
355+
.setInstanceId(DEFAULT_INSTANCE_ID)
356+
.setAppProfileId(DEFAULT_APP_PROFILE_ID)
357+
.setRefreshingChannel(true);
358+
359+
builder
360+
.stubSettings()
361+
.setCredentialsProvider(credentialsProvider)
362+
.setStreamWatchdogProvider(watchdogProvider)
363+
.setBackgroundExecutorProvider(executorProvider)
364+
.setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.FORCED_OFF);
365+
InstantiatingGrpcChannelProvider channelProvider =
366+
(InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider();
367+
InstantiatingGrpcChannelProvider.Builder channelProviderBuilder = channelProvider.toBuilder();
368+
channelProviderBuilder.setChannelPoolSettings(ChannelPoolSettings.staticallySized(poolSize));
369+
builder.stubSettings().setTransportChannelProvider(channelProviderBuilder.build());
370+
371+
BigtableDataClientFactory factory = BigtableDataClientFactory.create(builder.build());
372+
factory.createDefault();
373+
factory.createForAppProfile("other-appprofile");
374+
factory.createForInstance("other-project", "other-instance");
375+
284376
// Make sure that only 1 instance is created by each provider
285377
// getCredentials was called twice, in patchCredentials and when creating the fixed credentials
286378
// in BigtableClientContext
@@ -295,6 +387,7 @@ public void testCreateWithRefreshingChannel() throws Exception {
295387
long sleepTimeMs = 1000;
296388
Thread.sleep(sleepTimeMs);
297389
// Verify that all the channels are closed
390+
// If we have DEFAULT, it will add one channel temporily
298391
assertThat(terminateAttributes).hasSize(poolSize);
299392
}
300393

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,11 +549,11 @@ public void testChannelPrimerConfigured() throws IOException {
549549
// TODO: remove the suppression once setRefreshingChannel can be removed
550550
@SuppressWarnings("deprecation")
551551
EnhancedBigtableStubSettings settings =
552-
defaultSettings.toBuilder().setRefreshingChannel(true).build();
552+
defaultSettings.toBuilder().setRefreshingChannel(true).setDirectPathConfig(EnhancedBigtableStubSettings.DirectPathConfig.DEFAULT).build();
553553

554554
try (EnhancedBigtableStub ignored = EnhancedBigtableStub.create(settings)) {
555555
// direct access checker ping
556-
assertThat(fakeDataService.pingRequests).hasSize(1);
556+
assertThat(fakeDataService.pingRequests).hasSize(2);
557557
}
558558
}
559559

0 commit comments

Comments
 (0)