Skip to content

Commit 5cda5b1

Browse files
deps: upgrade java-bigtable to 2.49.0 and fix tests (#4462)
* deps: upgrade java-bigtable to 2.49.0 Change-Id: I801be8a764c212d7922634de61b41afaa3f2c3b6 * update metrics test to skipping trailers behavior Change-Id: Ib7f1e7d87b2ef177d6fba69f0d139f5291c3d38c * a bit more cleanup Change-Id: I23ef04f8ea555e2b6d2c53aa8b98a331c71960ae * more cleanup Change-Id: I002a750839ee0b324121c2098f93fbcd5385728b * try to fix test flakiness Change-Id: Iebf9d10d8194e7f0b2c85ee5e530aa84e6a3a895
1 parent 0826c2d commit 5cda5b1

File tree

14 files changed

+495
-302
lines changed

14 files changed

+495
-302
lines changed

.kokoro/build.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ javadoc)
5151
;;
5252
integration)
5353
# clean needed when running more than one IT profile
54-
mvn --no-transfer-progress clean verify -B ${INTEGRATION_TEST_ARGS} -Penable-integration-tests -DtrimStackTrace=false -Dclirr.skip=true -fae
54+
mvn --no-transfer-progress clean verify -B ${INTEGRATION_TEST_ARGS} -Penable-integration-tests -DtrimStackTrace=false -Dclirr.skip=true -DskipUnitTests=true
5555
RETURN_CODE=$?
5656
;;
5757
integration-migration)
@@ -60,7 +60,7 @@ integration-migration)
6060
RETURN_CODE=$?
6161
;;
6262
clirr)
63-
mvn --no-transfer-progress install -B -Denforcer.skip=true clirr:check
63+
mvn --no-transfer-progress install -B -Denforcer.skip=true -DskipTests clirr:check
6464
RETURN_CODE=$?
6565
;;
6666
*)

bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/wrappers/BigtableHBaseSettings.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package com.google.cloud.bigtable.hbase.wrappers;
1717

18+
import static com.google.cloud.bigtable.hbase.BigtableOptionsFactory.APP_PROFILE_ID_KEY;
1819
import static com.google.cloud.bigtable.hbase.BigtableOptionsFactory.INSTANCE_ID_KEY;
1920
import static com.google.cloud.bigtable.hbase.BigtableOptionsFactory.PROJECT_ID_KEY;
2021
import static com.google.common.base.Strings.isNullOrEmpty;
@@ -38,6 +39,7 @@ public abstract class BigtableHBaseSettings {
3839
private final Configuration configuration;
3940
private final String projectId;
4041
private final String instanceId;
42+
private final String appProfileId;
4143
private final int ttlSecondsForBackup;
4244

4345
@VisibleForTesting
@@ -47,7 +49,7 @@ public abstract class BigtableHBaseSettings {
4749
"bulk.mutation.close.timeout.milliseconds";
4850

4951
// Must be non-negative. Set to 0 to disable timeout.
50-
private final long bulkMutationCloseTimeoutMilliseconds;
52+
private final long bulkMutationCloseTimeoutMilliseconds;;
5153

5254
public static BigtableHBaseSettings create(Configuration configuration) throws IOException {
5355
return BigtableHBaseVeneerSettings.create(configuration);
@@ -57,6 +59,8 @@ public BigtableHBaseSettings(Configuration configuration) {
5759
this.configuration = new Configuration(configuration);
5860
this.projectId = getRequiredValue(PROJECT_ID_KEY, "Project ID");
5961
this.instanceId = getRequiredValue(INSTANCE_ID_KEY, "Instance ID");
62+
this.appProfileId = configuration.get(APP_PROFILE_ID_KEY);
63+
6064
this.ttlSecondsForBackup =
6165
configuration.getInt(
6266
BigtableOptionsFactory.BIGTABLE_SNAPSHOT_DEFAULT_TTL_SECS_KEY,
@@ -79,6 +83,10 @@ public String getInstanceId() {
7983
return instanceId;
8084
}
8185

86+
public String getAppProfileId() {
87+
return appProfileId;
88+
}
89+
8290
public int getTtlSecondsForBackup() {
8391
return ttlSecondsForBackup;
8492
}

bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/wrappers/veneer/BigtableHBaseVeneerSettings.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import com.google.api.gax.batching.FlowControlSettings;
6565
import com.google.api.gax.core.FixedCredentialsProvider;
6666
import com.google.api.gax.core.NoCredentialsProvider;
67+
import com.google.api.gax.grpc.ChannelPoolSettings;
6768
import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
6869
import com.google.api.gax.rpc.FixedHeaderProvider;
6970
import com.google.api.gax.rpc.ServerStreamingCallSettings;
@@ -301,6 +302,7 @@ private BigtableDataSettings buildBigtableDataSettings(ClientOperationTimeouts c
301302
String host = emulatorEndpoint.get().substring(0, split);
302303
int port = Integer.parseInt(emulatorEndpoint.get().substring(split + 1));
303304
dataBuilder = BigtableDataSettings.newBuilderForEmulator(host, port);
305+
configureConnection(dataBuilder.stubSettings(), BIGTABLE_HOST_KEY, emulatorEndpoint.get());
304306
} else {
305307
String endpoint =
306308
BigtableDataSettings.newBuilder()
@@ -500,7 +502,8 @@ public ManagedChannelBuilder apply(ManagedChannelBuilder channelBuilder) {
500502
if (endpointKey.equals(BIGTABLE_HOST_KEY)) {
501503
String channelCount = configuration.get(BIGTABLE_DATA_CHANNEL_COUNT_KEY);
502504
if (!Strings.isNullOrEmpty(channelCount)) {
503-
channelProvider.setPoolSize(Integer.parseInt(channelCount));
505+
channelProvider.setChannelPoolSettings(
506+
ChannelPoolSettings.staticallySized(Integer.parseInt(channelCount)));
504507
}
505508
}
506509

@@ -511,14 +514,11 @@ public ManagedChannelBuilder apply(ManagedChannelBuilder channelBuilder) {
511514
channelProvider.getChannelConfigurator();
512515

513516
channelProvider.setChannelConfigurator(
514-
new ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder>() {
515-
@Override
516-
public ManagedChannelBuilder apply(ManagedChannelBuilder channelBuilder) {
517-
if (prevConfigurator != null) {
518-
channelBuilder = prevConfigurator.apply(channelBuilder);
519-
}
520-
return channelBuilder.executor(null);
517+
channelBuilder -> {
518+
if (prevConfigurator != null) {
519+
channelBuilder = prevConfigurator.apply(channelBuilder);
521520
}
521+
return channelBuilder.executor(null);
522522
});
523523
}
524524

bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/wrappers/veneer/BulkMutationVeneerApi.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ public synchronized void close() throws IOException {
8383
} catch (InterruptedException | ExecutionException e) {
8484
throw new IOException("Could not close the bulk mutation Batcher", e);
8585
} catch (TimeoutException e) {
86+
bulkMutateBatcher.cancelOutstanding();
8687
throw new IOException("Cloud not close the bulk mutation Batcher, timed out in close()", e);
8788
}
8889
}

bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/wrappers/veneer/SharedDataClientWrapperFactory.java

Lines changed: 32 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,12 @@
1515
*/
1616
package com.google.cloud.bigtable.hbase.wrappers.veneer;
1717

18-
import com.google.api.gax.core.BackgroundResource;
1918
import com.google.api.gax.core.CredentialsProvider;
20-
import com.google.api.gax.core.FixedCredentialsProvider;
21-
import com.google.api.gax.core.FixedExecutorProvider;
19+
import com.google.api.gax.core.NoCredentialsProvider;
2220
import com.google.api.gax.rpc.ClientContext;
23-
import com.google.api.gax.rpc.FixedHeaderProvider;
24-
import com.google.api.gax.rpc.FixedTransportChannelProvider;
25-
import com.google.api.gax.rpc.FixedWatchdogProvider;
2621
import com.google.cloud.bigtable.data.v2.BigtableDataClient;
22+
import com.google.cloud.bigtable.data.v2.BigtableDataClientFactory;
2723
import com.google.cloud.bigtable.data.v2.BigtableDataSettings;
28-
import com.google.cloud.bigtable.data.v2.BigtableDataSettings.Builder;
2924
import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings;
3025
import com.google.cloud.bigtable.hbase.wrappers.DataClientWrapper;
3126
import com.google.cloud.bigtable.metrics.BigtableClientMetrics;
@@ -41,7 +36,7 @@
4136
* <p>This class is meant to support channel pool caching feature.
4237
*/
4338
class SharedDataClientWrapperFactory {
44-
private final Map<Key, ClientContext> cachedContexts = new HashMap<>();
39+
private final Map<Key, BigtableDataClientFactory> cachedContexts = new HashMap<>();
4540
private final Map<Key, Integer> refCounts = new HashMap<>();
4641

4742
private final Map<Key, Integer> channelPoolSizes = new HashMap<>();
@@ -52,13 +47,14 @@ synchronized DataClientWrapper createDataClient(BigtableHBaseVeneerSettings sett
5247
Key key = Key.createFromSettings(settings.getDataSettings());
5348

5449
// Get or create ClientContext that will contained the shared resources
55-
ClientContext sharedCtx = cachedContexts.get(key);
50+
BigtableDataClientFactory sharedCtx = cachedContexts.get(key);
51+
5652
if (sharedCtx == null) {
57-
EnhancedBigtableStubSettings stubSettings = settings.getDataSettings().getStubSettings();
58-
sharedCtx = ClientContext.create(stubSettings);
53+
sharedCtx = BigtableDataClientFactory.create(settings.getDataSettings());
5954
cachedContexts.put(key, sharedCtx);
6055
refCounts.put(key, 0);
61-
int channelPoolSize = BigtableVeneerApi.getChannelPoolSize(stubSettings);
56+
int channelPoolSize =
57+
BigtableVeneerApi.getChannelPoolSize(settings.getDataSettings().getStubSettings());
6258
for (int i = 0; i < channelPoolSize; i++) {
6359
BigtableClientMetrics.counter(MetricLevel.Info, "grpc.channel.active").inc();
6460
}
@@ -68,25 +64,17 @@ synchronized DataClientWrapper createDataClient(BigtableHBaseVeneerSettings sett
6864
refCounts.put(key, refCounts.get(key) + 1);
6965

7066
try {
71-
// Patch settings to use shared resources
72-
Builder builder = settings.getDataSettings().toBuilder();
73-
builder
74-
.stubSettings()
75-
.setRefreshingChannel(false)
76-
.setTransportChannelProvider(
77-
FixedTransportChannelProvider.create(sharedCtx.getTransportChannel()))
78-
.setCredentialsProvider(FixedCredentialsProvider.create(sharedCtx.getCredentials()))
79-
.setExecutorProvider(FixedExecutorProvider.create(sharedCtx.getExecutor()))
80-
.setStreamWatchdogProvider(FixedWatchdogProvider.create(sharedCtx.getStreamWatchdog()))
81-
.setHeaderProvider(FixedHeaderProvider.create(sharedCtx.getHeaders()))
82-
.setClock(sharedCtx.getClock());
83-
84-
BigtableDataSettings data = builder.build();
67+
final BigtableDataClient client;
68+
if (settings.getAppProfileId() == null) {
69+
client = sharedCtx.createForInstance(settings.getProjectId(), settings.getInstanceId());
70+
} else {
71+
client =
72+
sharedCtx.createForInstance(
73+
settings.getProjectId(), settings.getInstanceId(), settings.getAppProfileId());
74+
}
8575
// Create a reference counted client wrapper
8676
return new SharedDataClientWrapper(
87-
this,
88-
key,
89-
new DataClientVeneerApi(BigtableDataClient.create(data), settings.getClientTimeouts()));
77+
this, key, new DataClientVeneerApi(client, settings.getClientTimeouts()));
9078
} catch (IOException | RuntimeException e) {
9179
release(key);
9280
throw e;
@@ -101,13 +89,15 @@ synchronized void release(Key key) {
10189
}
10290

10391
refCounts.remove(key);
104-
ClientContext clientContext = cachedContexts.remove(key);
92+
BigtableDataClientFactory clientContext = cachedContexts.remove(key);
10593
for (int i = 0; i < channelPoolSizes.get(key); i++) {
10694
BigtableClientMetrics.counter(MetricLevel.Info, "grpc.channel.active").dec();
10795
}
10896
channelPoolSizes.remove(key);
109-
for (BackgroundResource resource : clientContext.getBackgroundResources()) {
110-
resource.shutdown();
97+
try {
98+
clientContext.close();
99+
} catch (Exception e) {
100+
throw new RuntimeException(e);
111101
}
112102
}
113103

@@ -119,23 +109,24 @@ synchronized void release(Key key) {
119109
* compatible with a ClientContext required by {@link BigtableDataSettings}.
120110
*/
121111
static final class Key {
112+
private static final NoCredentialsProvider NO_CREDENTIALS_PROVIDER_INSTANCE =
113+
NoCredentialsProvider.create();
122114
private final String endpoint;
123-
private final Map<String, String> headers;
124115
private final CredentialsProvider credentialsProvider;
125116

126117
static Key createFromSettings(BigtableDataSettings settings) {
127118
EnhancedBigtableStubSettings stubSettings = settings.getStubSettings();
119+
CredentialsProvider effectiveCredProvider = stubSettings.getCredentialsProvider();
120+
// NoCredentialsProvider doesnt implement equals, but all instances are equivalent
121+
if (effectiveCredProvider instanceof NoCredentialsProvider) {
122+
effectiveCredProvider = NO_CREDENTIALS_PROVIDER_INSTANCE;
123+
}
128124

129-
return new Key(
130-
stubSettings.getEndpoint(),
131-
stubSettings.getHeaderProvider().getHeaders(),
132-
stubSettings.getCredentialsProvider());
125+
return new Key(stubSettings.getEndpoint(), effectiveCredProvider);
133126
}
134127

135-
private Key(
136-
String endpoint, Map<String, String> headers, CredentialsProvider credentialsProvider) {
128+
private Key(String endpoint, CredentialsProvider credentialsProvider) {
137129
this.endpoint = endpoint;
138-
this.headers = headers;
139130
this.credentialsProvider = credentialsProvider;
140131
}
141132

@@ -149,13 +140,12 @@ public boolean equals(Object o) {
149140
}
150141
Key key = (Key) o;
151142
return Objects.equal(endpoint, key.endpoint)
152-
&& Objects.equal(headers, key.headers)
153143
&& Objects.equal(credentialsProvider, key.credentialsProvider);
154144
}
155145

156146
@Override
157147
public int hashCode() {
158-
return Objects.hashCode(endpoint, headers, credentialsProvider);
148+
return Objects.hashCode(endpoint, credentialsProvider);
159149
}
160150
}
161151
}

0 commit comments

Comments
 (0)