Skip to content

Commit c1e2485

Browse files
sushanbsushanb
committed
wip
1 parent 8684bb4 commit c1e2485

12 files changed

Lines changed: 38 additions & 35 deletions

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/csm/metrics/ClientDpCompatGuage.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ public void recordSuccess(ClientInfo clientInfo, String ipPreference) {
6161
}
6262

6363
// TODO: replace reason with an enum
64-
public void recordFailure(ClientInfo clientInfo, DirectAccessInvestigator.FailureReason reason) {
64+
public void recordFailure(
65+
ClientInfo clientInfo, DirectAccessInvestigator.FailureReason reason) {
6566
instrument.set(
6667
1,
6768
getSchema()

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/csm/tracers/DirectPathCompatibleTracerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public class DirectPathCompatibleTracerImpl implements DirectPathCompatibleTrace
3030
public DirectPathCompatibleTracerImpl(
3131
ClientInfo clientInfo, MetricRegistry.RecorderRegistry recorder) {
3232
this.clientInfo = Preconditions.checkNotNull(clientInfo);
33-
this.recorder = Preconditions.checkNotNull(recorder);
33+
this.recorder = Preconditions.checkNotNull(recorder);
3434
}
3535

3636
@Override

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

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,21 @@
2323

2424
@InternalApi
2525
public class AlwaysEnabledDirectAccessChecker implements DirectAccessChecker {
26-
public static final AlwaysEnabledDirectAccessChecker INSTANCE =
27-
new AlwaysEnabledDirectAccessChecker();
26+
public static final AlwaysEnabledDirectAccessChecker INSTANCE =
27+
new AlwaysEnabledDirectAccessChecker();
2828

29-
private AlwaysEnabledDirectAccessChecker() {}
29+
private AlwaysEnabledDirectAccessChecker() {}
3030

31-
@Override
32-
public boolean check(Channel channel) {
33-
if (channel instanceof ManagedChannel) {
34-
((ManagedChannel) channel).shutdownNow();
35-
}
36-
return true;
31+
@Override
32+
public boolean check(Channel channel) {
33+
if (channel instanceof ManagedChannel) {
34+
((ManagedChannel) channel).shutdownNow();
3735
}
36+
return true;
37+
}
3838

39-
@Override
40-
public void investigateFailure(@Nullable Throwable originalError) {
41-
// No-op:
42-
}
39+
@Override
40+
public void investigateFailure(@Nullable Throwable originalError) {
41+
// No-op:
42+
}
4343
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import com.google.api.core.InternalApi;
1919
import io.grpc.Channel;
20+
import io.grpc.ManagedChannel;
2021
import javax.annotation.Nullable;
2122

2223
@InternalApi
@@ -27,6 +28,10 @@ private NoopDirectAccessChecker() {}
2728

2829
@Override
2930
public boolean check(Channel channel) {
31+
// We must shut down the temporary probe channel to prevent gRPC resource leaks!
32+
if (channel instanceof ManagedChannel) {
33+
((ManagedChannel) channel).shutdownNow();
34+
}
3035
// If it's disabled, it is never eligible.
3136
return false;
3237
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
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;
3736
import com.google.cloud.bigtable.data.v2.internal.dp.DirectAccessChecker;
3837
import com.google.cloud.bigtable.data.v2.internal.dp.NoopDirectAccessChecker;
3938
import com.google.cloud.bigtable.data.v2.stub.metrics.CustomOpenTelemetryMetricsProvider;
@@ -175,7 +174,7 @@ public static BigtableClientContext create(
175174
directAccessChecker = NoopDirectAccessChecker.INSTANCE;
176175
break;
177176
}
178-
177+
179178
BigtableTransportChannelProvider btTransportProvider =
180179
BigtableTransportChannelProvider.create(
181180
transportProvider.build(),

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,10 @@ public class EnhancedBigtableStubSettings extends StubSettings<EnhancedBigtableS
101101
// TODO change this to true when enabling directpath by default
102102
// For now, Only runs Direct Access Checker if user explicitly sets CBT_ENABLE_DIRECTPATH=true
103103
private static final DirectPathConfig DIRECT_PATH_CONFIG =
104-
Optional.ofNullable(System.getenv("CBT_ENABLE_DIRECTPATH")).map(
105-
Boolean::parseBoolean
106-
).map(b -> b ? DirectPathConfig.FORCED_ON : DirectPathConfig.FORCED_OFF).orElse(DirectPathConfig.DEFAULT);
104+
Optional.ofNullable(System.getenv("CBT_ENABLE_DIRECTPATH"))
105+
.map(Boolean::parseBoolean)
106+
.map(b -> b ? DirectPathConfig.FORCED_ON : DirectPathConfig.FORCED_OFF)
107+
.orElse(DirectPathConfig.DEFAULT);
107108

108109
// If true, disable the bound-token-by-default feature for DirectPath.
109110
private static final boolean DIRECT_PATH_BOUND_TOKEN_DISABLED =
@@ -149,6 +150,7 @@ public enum DirectPathConfig {
149150
FORCED_ON,
150151
FORCED_OFF,
151152
}
153+
152154
private final DirectPathConfig directPathConfig;
153155

154156
private EnhancedBigtableStubSettings(Builder builder) {
@@ -178,6 +180,7 @@ public String getProjectId() {
178180
return projectId;
179181
}
180182

183+
@InternalApi
181184
public DirectPathConfig getDirectPathConfig() {
182185
return DIRECT_PATH_CONFIG;
183186
}
@@ -280,7 +283,8 @@ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProvi
280283
}
281284

282285
/** Applies Direct Access traits to an existing builder. */
283-
public static InstantiatingGrpcChannelProvider.Builder applyDirectAccessTraits(
286+
@InternalApi
287+
public static InstantiatingGrpcChannelProvider.Builder applyDirectAccessTraitsInternal(
284288
InstantiatingGrpcChannelProvider.Builder builder) {
285289
builder
286290
.setAttemptDirectPathXds()
@@ -634,7 +638,8 @@ private Builder() {
634638
// TODO: flip the bit setDirectAccessRequested and setTrafficDirectorEnabled once we make
635639
// client compatible by default.
636640
boolean isDirectPathRequested =
637-
directPathConfig == DirectPathConfig.FORCED_ON || directPathConfig == DirectPathConfig.DEFAULT;
641+
directPathConfig == DirectPathConfig.FORCED_ON
642+
|| directPathConfig == DirectPathConfig.DEFAULT;
638643
featureFlags =
639644
FeatureFlags.newBuilder()
640645
.setReverseScans(true)

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ void onClose(Status status, Metadata trailers) {
153153
}
154154
}
155155

156-
@Nullable
157156
private static Util.IpProtocol extractIpProtocol(Attributes attributes) {
158157
SocketAddress remoteAddr = attributes.get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR);
159158
if (remoteAddr instanceof InetSocketAddress) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ public TransportChannelProvider withPoolSize(int size) {
164164
@Override
165165
public TransportChannel getTransportChannel() throws IOException {
166166
InstantiatingGrpcChannelProvider directAccessProvider =
167-
EnhancedBigtableStubSettings.applyDirectAccessTraits(delegate.toBuilder())
167+
EnhancedBigtableStubSettings.applyDirectAccessTraitsInternal(delegate.toBuilder())
168168
.setChannelPoolSettings(ChannelPoolSettings.staticallySized(1))
169169
.build();
170170

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ public interface ChannelPrimer {
2626
/**
2727
* @deprecated Use {@link #primeChannel(Channel)}
2828
*/
29-
@Deprecated
30-
default void primeChannel(ManagedChannel channel) {
29+
@Deprecated
30+
default void primeChannel(ManagedChannel channel) {
3131
primeChannel((Channel) channel);
3232
}
3333

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,17 +287,15 @@ public void testCreateWithRefreshingChannel() throws Exception {
287287
Mockito.verify(credentialsProvider, Mockito.times(2)).getCredentials();
288288
Mockito.verify(executorProvider, Mockito.times(1)).getExecutor();
289289
Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog();
290-
// +1 accounts for the temporary channel created by UnaryDirectAccessChecker
291-
int expectedChannelCount = poolSize + 1;
292-
assertThat(warmedChannels).hasSize(expectedChannelCount);
290+
assertThat(warmedChannels).hasSize(poolSize);
293291
assertThat(warmedChannels.values()).doesNotContain(false);
294292

295293
// Wait for all the connections to close asynchronously
296294
factory.close();
297295
long sleepTimeMs = 1000;
298296
Thread.sleep(sleepTimeMs);
299297
// Verify that all the channels are closed
300-
assertThat(terminateAttributes).hasSize(expectedChannelCount);
298+
assertThat(terminateAttributes).hasSize(poolSize);
301299
}
302300

303301
@Test

0 commit comments

Comments
 (0)