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

Commit d8cf0e8

Browse files
sushanbsushanb
committed
fix
1 parent cc306bc commit d8cf0e8

9 files changed

Lines changed: 97 additions & 53 deletions

File tree

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/dp/ClassicDirectAccessChecker.java‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.util.function.Supplier;
2929
import java.util.logging.Level;
3030
import java.util.logging.Logger;
31-
import javax.annotation.Nullable;
3231

3332
/**
3433
* Evaluates whether a given channel has Direct Access (DirectPath) routing by executing a RPC and
@@ -37,27 +36,25 @@
3736
@InternalApi
3837
public class ClassicDirectAccessChecker implements DirectAccessChecker {
3938
private static final Logger LOG = Logger.getLogger(ClassicDirectAccessChecker.class.getName());
39+
private final DirectPathCompatibleTracer tracer;
4040
private final ChannelPrimer channelPrimer;
4141

42-
private ClassicDirectAccessChecker(ChannelPrimer channelPrimer) {
42+
public ClassicDirectAccessChecker(
43+
DirectPathCompatibleTracer tracer, ChannelPrimer channelPrimer) {
44+
this.tracer = tracer;
4345
this.channelPrimer = channelPrimer;
4446
}
4547

46-
public static ClassicDirectAccessChecker create(ChannelPrimer channelPrimer) {
47-
return new ClassicDirectAccessChecker(channelPrimer);
48-
}
49-
5048
@VisibleForTesting
5149
MetadataExtractorInterceptor createInterceptor() {
5250
return new MetadataExtractorInterceptor();
5351
}
5452

5553
@Override
56-
public boolean check(
57-
Supplier<ManagedChannel> channelSupplier, @Nullable DirectPathCompatibleTracer tracer) {
54+
public boolean check(Supplier<ManagedChannel> supplier) {
5855
ManagedChannel channel = null;
5956
try {
60-
channel = channelSupplier.get();
57+
channel = supplier.get();
6158
MetadataExtractorInterceptor interceptor = createInterceptor();
6259
Channel interceptedChannel = ClientInterceptors.intercept(channel, interceptor);
6360
channelPrimer.primeChannel(interceptedChannel);

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/dp/DirectAccessChecker.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,8 @@
1616
package com.google.cloud.bigtable.data.v2.internal.dp;
1717

1818
import com.google.api.core.InternalApi;
19-
import com.google.cloud.bigtable.data.v2.internal.csm.tracers.DirectPathCompatibleTracer;
2019
import io.grpc.ManagedChannel;
2120
import java.util.function.Supplier;
22-
import javax.annotation.Nullable;
2321

2422
@InternalApi
2523
/* Evaluates whether a given channel supports Direct Access. */
@@ -30,5 +28,5 @@ public interface DirectAccessChecker {
3028
* @param supplier A supplier to create maybe direct access channel
3129
* @return true if the channel is eligible for Direct Access
3230
*/
33-
boolean check(Supplier<ManagedChannel> supplier, @Nullable DirectPathCompatibleTracer tracer);
31+
boolean check(Supplier<ManagedChannel> supplier);
3432
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,14 @@ public static BigtableClientContext create(
160160
credentials,
161161
builder.getHeaderProvider().getHeaders());
162162
}
163-
164163
BigtableTransportChannelProvider btTransportProvider =
165164
BigtableTransportChannelProvider.create(
166165
transportProvider.build(),
167166
channelPrimer,
168167
metrics.getChannelPoolMetricsTracer(),
169168
backgroundExecutor,
170-
metrics.getDirectPathCompatibleTracer());
169+
metrics.getDirectPathCompatibleTracer(),
170+
builder.isDirectPathEnabledByDefault());
171171

172172
builder.setTransportChannelProvider(btTransportProvider);
173173
}

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

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ public class EnhancedBigtableStubSettings extends StubSettings<EnhancedBigtableS
101101
private static final boolean DIRECT_PATH_ENABLED =
102102
Boolean.parseBoolean(System.getenv("CBT_ENABLE_DIRECTPATH"));
103103

104+
;
105+
104106
// If true, disable the bound-token-by-default feature for DirectPath.
105107
private static final boolean DIRECT_PATH_BOUND_TOKEN_DISABLED =
106108
Boolean.parseBoolean(System.getenv("CBT_DISABLE_DIRECTPATH_BOUND_TOKEN"));
@@ -138,10 +140,13 @@ public class EnhancedBigtableStubSettings extends StubSettings<EnhancedBigtableS
138140
@Nullable private final String metricsEndpoint;
139141
private final boolean areInternalMetricsEnabled;
140142
private final String jwtAudience;
143+
private boolean enableDirectPathByDefault;
141144

142145
private EnhancedBigtableStubSettings(Builder builder) {
143146
super(builder);
144147

148+
enableDirectPathByDefault = builder.enableDirectPathByDefault;
149+
145150
projectId = builder.projectId;
146151
instanceId = builder.instanceId;
147152
appProfileId = builder.appProfileId;
@@ -165,6 +170,10 @@ public String getProjectId() {
165170
return projectId;
166171
}
167172

173+
public boolean isDirectPathEnabledByDefault() {
174+
return enableDirectPathByDefault;
175+
}
176+
168177
/** Returns the target instance id. */
169178
public String getInstanceId() {
170179
return instanceId;
@@ -592,7 +601,7 @@ public Builder toBuilder() {
592601

593602
/** Builder for BigtableDataSettings. */
594603
public static class Builder extends StubSettings.Builder<EnhancedBigtableStubSettings, Builder> {
595-
604+
private boolean enableDirectPathByDefault;
596605
private String projectId;
597606
private String instanceId;
598607
private String appProfileId;
@@ -639,12 +648,16 @@ private Builder() {
639648
// is fine as GFE/CFE sends with gslb target type
640649
// TODO: flip the bit setDirectAccessRequested and setTrafficDirectorEnabled once we make
641650
// client compatible by default.
651+
652+
this.enableDirectPathByDefault =
653+
Boolean.parseBoolean(System.getenv("CBT_ENABLE_DIRECTPATH_BY_DEFAULT"));
654+
642655
featureFlags =
643656
FeatureFlags.newBuilder()
644657
.setReverseScans(true)
645658
.setLastScannedRowResponses(true)
646-
.setDirectAccessRequested(DIRECT_PATH_ENABLED)
647-
.setTrafficDirectorEnabled(DIRECT_PATH_ENABLED)
659+
.setDirectAccessRequested(DIRECT_PATH_ENABLED || enableDirectPathByDefault)
660+
.setTrafficDirectorEnabled(DIRECT_PATH_ENABLED || enableDirectPathByDefault)
648661
.setPeerInfo(true);
649662
}
650663

@@ -658,6 +671,7 @@ private Builder(EnhancedBigtableStubSettings settings) {
658671
metricsEndpoint = settings.getMetricsEndpoint();
659672
areInternalMetricsEnabled = settings.areInternalMetricsEnabled;
660673
jwtAudience = settings.jwtAudience;
674+
this.enableDirectPathByDefault = settings.isDirectPathEnabledByDefault();
661675

662676
this.perOpSettings = new ClientOperationSettings.Builder(settings.perOpSettings);
663677

@@ -743,6 +757,15 @@ public Builder setRefreshingChannel(boolean isRefreshingChannel) {
743757
return this;
744758
}
745759

760+
public Builder setEnableDirectPathByDefault(boolean enableDirectPathByDefault) {
761+
this.enableDirectPathByDefault = enableDirectPathByDefault;
762+
return this;
763+
}
764+
765+
public boolean isDirectPathEnabledByDefault() {
766+
return enableDirectPathByDefault;
767+
}
768+
746769
/**
747770
* @deprecated This field is ignored. If {@link #isRefreshingChannel()} is enabled, warm up
748771
* requests will be sent to all table ids of the instance.
@@ -1024,6 +1047,7 @@ public String toString() {
10241047
.add("metricsEndpoint", metricsEndpoint)
10251048
.add("areInternalMetricsEnabled", areInternalMetricsEnabled)
10261049
.add("jwtAudience", jwtAudience)
1050+
.add("enableDirectPathByDefault", enableDirectPathByDefault)
10271051
.add("parent", super.toString())
10281052
.toString();
10291053
}

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableTransportChannelProvider.java‎

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -52,18 +52,21 @@ public final class BigtableTransportChannelProvider implements TransportChannelP
5252
@Nullable private final ChannelPoolMetricsTracer channelPoolMetricsTracer;
5353
@Nullable private final ScheduledExecutorService backgroundExecutor;
5454
DirectPathCompatibleTracer directPathCompatibleTracer;
55+
private final boolean enableDirectAccessChecker;
5556

5657
private BigtableTransportChannelProvider(
5758
InstantiatingGrpcChannelProvider instantiatingGrpcChannelProvider,
5859
ChannelPrimer channelPrimer,
5960
@Nullable ChannelPoolMetricsTracer channelPoolMetricsTracer,
6061
@Nullable ScheduledExecutorService backgroundExecutor,
61-
DirectPathCompatibleTracer directPathCompatibleTracer) {
62+
DirectPathCompatibleTracer directPathCompatibleTracer,
63+
boolean enableDirectAccessChecker) {
6264
delegate = Preconditions.checkNotNull(instantiatingGrpcChannelProvider);
6365
this.channelPrimer = channelPrimer;
6466
this.channelPoolMetricsTracer = channelPoolMetricsTracer;
6567
this.backgroundExecutor = backgroundExecutor;
6668
this.directPathCompatibleTracer = directPathCompatibleTracer;
69+
this.enableDirectAccessChecker = enableDirectAccessChecker;
6770
}
6871

6972
@Override
@@ -94,7 +97,8 @@ public BigtableTransportChannelProvider withExecutor(Executor executor) {
9497
channelPrimer,
9598
channelPoolMetricsTracer,
9699
backgroundExecutor,
97-
directPathCompatibleTracer);
100+
directPathCompatibleTracer,
101+
enableDirectAccessChecker);
98102
}
99103

100104
@Override
@@ -111,7 +115,8 @@ public TransportChannelProvider withBackgroundExecutor(ScheduledExecutorService
111115
channelPrimer,
112116
channelPoolMetricsTracer,
113117
executor,
114-
directPathCompatibleTracer);
118+
directPathCompatibleTracer,
119+
enableDirectAccessChecker);
115120
}
116121

117122
@Override
@@ -128,7 +133,8 @@ public BigtableTransportChannelProvider withHeaders(Map<String, String> headers)
128133
channelPrimer,
129134
channelPoolMetricsTracer,
130135
backgroundExecutor,
131-
directPathCompatibleTracer);
136+
directPathCompatibleTracer,
137+
enableDirectAccessChecker);
132138
}
133139

134140
@Override
@@ -145,7 +151,8 @@ public TransportChannelProvider withEndpoint(String endpoint) {
145151
channelPrimer,
146152
channelPoolMetricsTracer,
147153
backgroundExecutor,
148-
directPathCompatibleTracer);
154+
directPathCompatibleTracer,
155+
enableDirectAccessChecker);
149156
}
150157

151158
@Deprecated
@@ -164,7 +171,8 @@ public TransportChannelProvider withPoolSize(int size) {
164171
channelPrimer,
165172
channelPoolMetricsTracer,
166173
backgroundExecutor,
167-
directPathCompatibleTracer);
174+
directPathCompatibleTracer,
175+
enableDirectAccessChecker);
168176
}
169177

170178
// We need this for direct access checker.
@@ -177,25 +185,27 @@ public TransportChannel getTransportChannel() throws IOException {
177185
.setChannelPoolSettings(ChannelPoolSettings.staticallySized(1))
178186
.build();
179187

180-
Supplier<ManagedChannel> maybeDirectAccessChannelFactory =
181-
() -> {
182-
try {
183-
GrpcTransportChannel channel =
184-
(GrpcTransportChannel) directAccessProvider.getTransportChannel();
185-
return (ManagedChannel) channel.getChannel();
186-
} catch (IOException e) {
187-
throw new java.io.UncheckedIOException(e);
188-
}
189-
};
190-
191-
DirectAccessChecker directAccessChecker = ClassicDirectAccessChecker.create(channelPrimer);
188+
DirectAccessChecker directAccessChecker =
189+
new ClassicDirectAccessChecker(directPathCompatibleTracer, channelPrimer);
192190
boolean isDirectAccessEligible = false;
193191

194-
try {
195-
isDirectAccessEligible =
196-
directAccessChecker.check(maybeDirectAccessChannelFactory, directPathCompatibleTracer);
197-
} catch (Exception e) {
198-
LOG.log(Level.FINE, "Client is not direct access eligible, using standard transport.", e);
192+
if (enableDirectAccessChecker) {
193+
Supplier<ManagedChannel> maybeDirectAccessChannelSupplier =
194+
() -> {
195+
try {
196+
GrpcTransportChannel channel =
197+
(GrpcTransportChannel) directAccessProvider.getTransportChannel();
198+
return (ManagedChannel) channel.getChannel();
199+
} catch (IOException e) {
200+
throw new java.io.UncheckedIOException(e);
201+
}
202+
};
203+
204+
try {
205+
isDirectAccessEligible = directAccessChecker.check(maybeDirectAccessChannelSupplier);
206+
} catch (Exception e) {
207+
LOG.log(Level.FINE, "Client is not direct access eligible, using standard transport.", e);
208+
}
199209
}
200210

201211
InstantiatingGrpcChannelProvider selectedProvider;
@@ -263,7 +273,8 @@ public TransportChannelProvider withCredentials(Credentials credentials) {
263273
channelPrimer,
264274
channelPoolMetricsTracer,
265275
backgroundExecutor,
266-
directPathCompatibleTracer);
276+
directPathCompatibleTracer,
277+
enableDirectAccessChecker);
267278
}
268279

269280
/** Creates a BigtableTransportChannelProvider. */
@@ -272,12 +283,14 @@ public static BigtableTransportChannelProvider create(
272283
ChannelPrimer channelPrimer,
273284
ChannelPoolMetricsTracer outstandingRpcsMetricTracker,
274285
ScheduledExecutorService backgroundExecutor,
275-
DirectPathCompatibleTracer directPathCompatibleTracer) {
286+
DirectPathCompatibleTracer directPathCompatibleTracer,
287+
boolean enableDirectAccessChecker) {
276288
return new BigtableTransportChannelProvider(
277289
instantiatingGrpcChannelProvider,
278290
channelPrimer,
279291
outstandingRpcsMetricTracker,
280292
backgroundExecutor,
281-
directPathCompatibleTracer);
293+
directPathCompatibleTracer,
294+
enableDirectAccessChecker);
282295
}
283296
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ public void testCreateWithRefreshingChannel() throws Exception {
267267
.stubSettings()
268268
.setCredentialsProvider(credentialsProvider)
269269
.setStreamWatchdogProvider(watchdogProvider)
270-
.setBackgroundExecutorProvider(executorProvider);
270+
.setBackgroundExecutorProvider(executorProvider)
271+
.setEnableDirectPathByDefault(true);
271272
InstantiatingGrpcChannelProvider channelProvider =
272273
(InstantiatingGrpcChannelProvider) builder.stubSettings().getTransportChannelProvider();
273274
InstantiatingGrpcChannelProvider.Builder channelProviderBuilder = channelProvider.toBuilder();

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/dp/ClassicDirectAccessCheckerTest.java‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,9 @@ public class ClassicDirectAccessCheckerTest {
5151

5252
@Before
5353
public void setUp() throws Exception {
54-
// 1. Create a SPY of the checker so we can override just one method
55-
checker = spy(ClassicDirectAccessChecker.create(mockChannelPrimer));
56-
57-
// 2. Tell the spy to return our mock interceptor instead of calling new()
54+
checker = spy(new ClassicDirectAccessChecker(mockTracer, mockChannelPrimer));
5855
doReturn(mockInterceptor).when(checker).createInterceptor();
5956

60-
// 3. Setup standard mocks
6157
when(mockChannelFactory.get()).thenReturn(mockChannel);
6258
when(mockInterceptor.getSidebandData()).thenReturn(mockSidebandData);
6359
}
@@ -72,7 +68,7 @@ public void testEligibleForDirectAccess() {
7268
when(mockSidebandData.getIpProtocol())
7369
.thenReturn(MetadataExtractorInterceptor.SidebandData.IpProtocol.IPV6);
7470

75-
boolean isEligible = checker.check(mockChannelFactory, mockTracer);
71+
boolean isEligible = checker.check(mockChannelFactory);
7672

7773
assertThat(isEligible).isTrue();
7874
verify(mockChannelPrimer).primeChannel(any(Channel.class));
@@ -88,7 +84,7 @@ public void testNotEligibleProxiedRouting() {
8884
.build();
8985
when(mockSidebandData.getPeerInfo()).thenReturn(peerInfo);
9086

91-
boolean isEligible = checker.check(mockChannelFactory, mockTracer);
87+
boolean isEligible = checker.check(mockChannelFactory);
9288

9389
assertThat(isEligible).isFalse();
9490
verifyNoInteractions(mockTracer);
@@ -100,7 +96,7 @@ public void testMissingSidebandData() {
10096
// Override the Before setup to return null for this specific test
10197
when(mockInterceptor.getSidebandData()).thenReturn(null);
10298

103-
boolean isEligible = checker.check(mockChannelFactory, mockTracer);
99+
boolean isEligible = checker.check(mockChannelFactory);
104100

105101
assertThat(isEligible).isFalse();
106102
verifyNoInteractions(mockTracer);
@@ -113,7 +109,7 @@ public void testExceptionSafetyAndCleanup() {
113109
.when(mockChannelPrimer)
114110
.primeChannel(any(Channel.class));
115111

116-
boolean isEligible = checker.check(mockChannelFactory, mockTracer);
112+
boolean isEligible = checker.check(mockChannelFactory);
117113

118114
assertThat(isEligible).isFalse();
119115
verifyNoInteractions(mockTracer);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,7 @@ public void isRefreshingChannelFalseValueTest() {
905905
"metricsEndpoint",
906906
"areInternalMetricsEnabled",
907907
"jwtAudience",
908+
"enableDirectPathByDefault",
908909
};
909910

910911
@Test

0 commit comments

Comments
 (0)