From ac7363d8356ea982918524e4d21ad6ac75753b49 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Fri, 2 Feb 2024 14:02:08 -0500 Subject: [PATCH 01/23] Create the backbone of counting errors per connection each minute. --- .gitignore | 1 + .../bigtable/stats/StatsRecorderWrapper.java | 30 +++++++- .../cloud/bigtable/stats/StatsWrapper.java | 4 + .../stub/ConnectionErrorCountInterceptor.java | 73 +++++++++++++++++++ .../stub/CountErrorsPerInterceptorTask.java | 48 ++++++++++++ .../data/v2/stub/EnhancedBigtableStub.java | 64 ++++++++++++---- 6 files changed, 203 insertions(+), 17 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java diff --git a/.gitignore b/.gitignore index dbde6a740b..4a670f8c53 100644 --- a/.gitignore +++ b/.gitignore @@ -42,3 +42,4 @@ api_key artman-genfiles .flattened-pom.xml +dependency-reduced-pom.xml diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java index 6bf0988b91..bafde228cc 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java @@ -32,16 +32,17 @@ @InternalApi("For internal use only") public class StatsRecorderWrapper { - private final OperationType operationType; + private OperationType operationType; private final Tagger tagger; private final StatsRecorder statsRecorder; private final TagContext parentContext; - private final SpanName spanName; + private SpanName spanName; private final Map statsAttributes; private MeasureMap attemptMeasureMap; private MeasureMap operationMeasureMap; + private MeasureMap perConnectionErrorCountMeasureMap; public StatsRecorderWrapper( OperationType operationType, @@ -59,6 +60,14 @@ public StatsRecorderWrapper( this.operationMeasureMap = statsRecorder.newMeasureMap(); } + public StatsRecorderWrapper(Map statsAttributes, StatsRecorder statsRecorder) { + this.tagger = Tags.getTagger(); + this.statsRecorder = statsRecorder; + this.parentContext = tagger.getCurrentTagContext(); + this.statsAttributes = statsAttributes; + this.perConnectionErrorCountMeasureMap = statsRecorder.newMeasureMap(); + } + public void recordOperation(String status, String tableId, String zone, String cluster) { TagContextBuilder tagCtx = newTagContextBuilder(tableId, zone, cluster) @@ -87,6 +96,15 @@ public void recordAttempt(String status, String tableId, String zone, String clu attemptMeasureMap = statsRecorder.newMeasureMap(); } + public void putAndRecordPerConnectionErrorCount(long errorCount) { + perConnectionErrorCountMeasureMap.put( + BuiltinMeasureConstants.PER_CONNECTION_ERROR_COUNT, errorCount); + + perConnectionErrorCountMeasureMap.record( + newTagContextBuilderForPerConnectionErrorCount().build()); + perConnectionErrorCountMeasureMap = statsRecorder.newMeasureMap(); + } + public void putOperationLatencies(long operationLatency) { operationMeasureMap.put(BuiltinMeasureConstants.OPERATION_LATENCIES, operationLatency); } @@ -132,4 +150,12 @@ private TagContextBuilder newTagContextBuilder(String tableId, String zone, Stri } return tagContextBuilder; } + + private TagContextBuilder newTagContextBuilderForPerConnectionErrorCount() { + TagContextBuilder tagContextBuilder = tagger.toBuilder(parentContext); + for (Map.Entry entry : statsAttributes.entrySet()) { + tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagValue.create(entry.getValue())); + } + return tagContextBuilder; + } } diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java index 401a1cf975..0d5d735c43 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java @@ -40,6 +40,10 @@ public static StatsRecorderWrapper createRecorder( operationType, spanName, statsAttributes, Stats.getStatsRecorder()); } + public static StatsRecorderWrapper createRecorder(Map statsAttributes) { + return new StatsRecorderWrapper(statsAttributes, Stats.getStatsRecorder()); + } + // This is used in integration tests to get the tag value strings from view manager because Stats // is relocated to com.google.bigtable.veneer.repackaged.io.opencensus. @InternalApi("Visible for testing") diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java new file mode 100644 index 0000000000..e5c8bed13d --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java @@ -0,0 +1,73 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ClientInterceptor; +import io.grpc.ForwardingClientCall; +import io.grpc.ForwardingClientCallListener; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; +import io.grpc.Status; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; + +/** An interceptor which counts the number of failed responses for a channel. */ +class ConnectionErrorCountInterceptor implements ClientInterceptor { + private final AtomicInteger numOfErrors; + private final AtomicInteger numOfSuccesses; + + public ConnectionErrorCountInterceptor(Set interceptors) { + numOfErrors = new AtomicInteger(0); + numOfSuccesses = new AtomicInteger(0); + interceptors.add(this); + } + + @Override + public ClientCall interceptCall( + MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { + return new ForwardingClientCall.SimpleForwardingClientCall( + channel.newCall(methodDescriptor, callOptions)) { + @Override + public void start(Listener responseListener, Metadata headers) { + super.start( + new ForwardingClientCallListener.SimpleForwardingClientCallListener( + responseListener) { + @Override + public void onClose(Status status, Metadata trailers) { + if (status.isOk()) { + numOfSuccesses.getAndIncrement(); + } else { + numOfErrors.getAndIncrement(); + } + super.onClose(status, trailers); + } + }, + headers); + } + }; + } + + public int getAndResetNumOfErrors() { + return numOfErrors.getAndSet(0); + } + + public int getAndResetNumOfSuccesses() { + return numOfSuccesses.getAndSet(0); + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java new file mode 100644 index 0000000000..5b941dd23d --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java @@ -0,0 +1,48 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import com.google.cloud.bigtable.stats.StatsRecorderWrapper; +import com.google.cloud.bigtable.stats.StatsWrapper; +import com.google.common.collect.ImmutableMap; +import java.util.Set; + +class CountErrorsPerInterceptorTask implements Runnable { + private final Set interceptors; + private final StatsRecorderWrapper statsRecorderWrapper; + + public CountErrorsPerInterceptorTask( + Set interceptors, + ImmutableMap builtinAttributes) { + this.interceptors = interceptors; + this.statsRecorderWrapper = StatsWrapper.createRecorder(builtinAttributes); + } + + @Override + public void run() { + for (ConnectionErrorCountInterceptor interceptor : interceptors) { + int errors = interceptor.getAndResetNumOfErrors(); + int successes = interceptor.getAndResetNumOfSuccesses(); + // We avoid keeping track of inactive connections (i.e., without any failed or successful + // requests). + if (errors > 0 || successes > 0) { + // TODO: add a metric to also keep track of the number of successful requests per each + // connection. + this.statsRecorderWrapper.putAndRecordPerConnectionErrorCount(errors); + } + } + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 9fb906c2d1..d98b43364a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -126,9 +126,9 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; -import java.util.Collections; -import java.util.List; -import java.util.Map; +import java.util.*; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -149,6 +149,7 @@ public class EnhancedBigtableStub implements AutoCloseable { private static final String CLIENT_NAME = "Bigtable"; private static final long FLOW_CONTROL_ADJUSTING_INTERVAL_MS = TimeUnit.SECONDS.toMillis(20); + private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; private final EnhancedBigtableStubSettings settings; private final ClientContext clientContext; @@ -204,11 +205,7 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set ? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder() : null; - if (builder.getEnableRoutingCookie() && transportProvider != null) { - // TODO: this also need to be added to BigtableClientFactory - // patch cookies interceptor - transportProvider.setInterceptorProvider(() -> ImmutableList.of(new CookiesInterceptor())); - } + setInterceptors(transportProvider, builder); // Inject channel priming if (settings.isRefreshingChannel()) { @@ -236,6 +233,39 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set return ClientContext.create(builder.build()); } + private static void setInterceptors( + InstantiatingGrpcChannelProvider.Builder transportProvider, + EnhancedBigtableStubSettings.Builder settings) { + Set interceptors = + setupConnectionErrorCountInterceptors(settings); + if (transportProvider != null) { + if (settings.getEnableRoutingCookie()) { + // TODO: this also need to be added to BigtableClientFactory + transportProvider.setInterceptorProvider( + () -> + ImmutableList.of( + new CookiesInterceptor(), new ConnectionErrorCountInterceptor(interceptors))); + } else { + transportProvider.setInterceptorProvider( + () -> ImmutableList.of(new ConnectionErrorCountInterceptor(interceptors))); + } + } + } + + private static Set setupConnectionErrorCountInterceptors( + EnhancedBigtableStubSettings.Builder settings) { + Set interceptors = + Collections.newSetFromMap(new WeakHashMap<>()); + ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); + ImmutableMap builtinAttributes = createBuiltinAttributes(settings); + scheduler.scheduleAtFixedRate( + new CountErrorsPerInterceptorTask(interceptors, builtinAttributes), + 0, + PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, + TimeUnit.SECONDS); + return interceptors; + } + public static ApiTracerFactory createBigtableTracerFactory( EnhancedBigtableStubSettings settings) { return createBigtableTracerFactory(settings, Tags.getTagger(), Stats.getStatsRecorder()); @@ -254,13 +284,7 @@ public static ApiTracerFactory createBigtableTracerFactory( .put(RpcMeasureConstants.BIGTABLE_INSTANCE_ID, TagValue.create(instanceId)) .put(RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, TagValue.create(appProfileId)) .build(); - ImmutableMap builtinAttributes = - ImmutableMap.builder() - .put("project_id", projectId) - .put("instance", instanceId) - .put("app_profile", appProfileId) - .put("client_name", "bigtable-java/" + Version.VERSION) - .build(); + ImmutableMap builtinAttributes = createBuiltinAttributes(settings.toBuilder()); return new CompositeTracerFactory( ImmutableList.of( @@ -283,6 +307,16 @@ public static ApiTracerFactory createBigtableTracerFactory( settings.getTracerFactory())); } + private static ImmutableMap createBuiltinAttributes( + EnhancedBigtableStubSettings.Builder settings) { + return ImmutableMap.builder() + .put("project", settings.getProjectId()) + .put("instance", settings.getInstanceId()) + .put("app_profile", settings.getAppProfileId()) + .put("client_name", "bigtable-java/" + Version.VERSION) + .build(); + } + private static void patchCredentials(EnhancedBigtableStubSettings.Builder settings) throws IOException { int i = settings.getEndpoint().lastIndexOf(":"); From 1f26796187780796640c6e2660e47996ab2fda63 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Mon, 5 Feb 2024 13:10:14 -0500 Subject: [PATCH 02/23] Clean up creating new interceptors and StatsRecorderWrapper ctor. --- .../bigtable/stats/StatsRecorderWrapper.java | 11 ++----- .../cloud/bigtable/stats/StatsWrapper.java | 4 --- .../stub/ConnectionErrorCountInterceptor.java | 4 +-- .../stub/CountErrorsPerInterceptorTask.java | 2 +- .../data/v2/stub/EnhancedBigtableStub.java | 32 ++++++++++--------- 5 files changed, 21 insertions(+), 32 deletions(-) diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java index bafde228cc..65ebe1e0fe 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java @@ -32,12 +32,12 @@ @InternalApi("For internal use only") public class StatsRecorderWrapper { - private OperationType operationType; + private final OperationType operationType; private final Tagger tagger; private final StatsRecorder statsRecorder; private final TagContext parentContext; - private SpanName spanName; + private final SpanName spanName; private final Map statsAttributes; private MeasureMap attemptMeasureMap; @@ -58,13 +58,6 @@ public StatsRecorderWrapper( this.attemptMeasureMap = statsRecorder.newMeasureMap(); this.operationMeasureMap = statsRecorder.newMeasureMap(); - } - - public StatsRecorderWrapper(Map statsAttributes, StatsRecorder statsRecorder) { - this.tagger = Tags.getTagger(); - this.statsRecorder = statsRecorder; - this.parentContext = tagger.getCurrentTagContext(); - this.statsAttributes = statsAttributes; this.perConnectionErrorCountMeasureMap = statsRecorder.newMeasureMap(); } diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java index 0d5d735c43..401a1cf975 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java @@ -40,10 +40,6 @@ public static StatsRecorderWrapper createRecorder( operationType, spanName, statsAttributes, Stats.getStatsRecorder()); } - public static StatsRecorderWrapper createRecorder(Map statsAttributes) { - return new StatsRecorderWrapper(statsAttributes, Stats.getStatsRecorder()); - } - // This is used in integration tests to get the tag value strings from view manager because Stats // is relocated to com.google.bigtable.veneer.repackaged.io.opencensus. @InternalApi("Visible for testing") diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java index e5c8bed13d..8e73340de1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java @@ -24,7 +24,6 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Status; -import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; /** An interceptor which counts the number of failed responses for a channel. */ @@ -32,10 +31,9 @@ class ConnectionErrorCountInterceptor implements ClientInterceptor { private final AtomicInteger numOfErrors; private final AtomicInteger numOfSuccesses; - public ConnectionErrorCountInterceptor(Set interceptors) { + public ConnectionErrorCountInterceptor() { numOfErrors = new AtomicInteger(0); numOfSuccesses = new AtomicInteger(0); - interceptors.add(this); } @Override diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java index 5b941dd23d..73b36ba9cd 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java @@ -28,7 +28,7 @@ public CountErrorsPerInterceptorTask( Set interceptors, ImmutableMap builtinAttributes) { this.interceptors = interceptors; - this.statsRecorderWrapper = StatsWrapper.createRecorder(builtinAttributes); + this.statsRecorderWrapper = StatsWrapper.createRecorder(null, null, builtinAttributes); } @Override diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index d98b43364a..f646fd3ff6 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -117,6 +117,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.protobuf.ByteString; +import io.grpc.ClientInterceptor; import io.opencensus.stats.Stats; import io.opencensus.stats.StatsRecorder; import io.opencensus.tags.TagKey; @@ -205,7 +206,9 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set ? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder() : null; - setInterceptors(transportProvider, builder); + if (transportProvider != null) { + transportProvider.setInterceptorProvider(() -> getInterceptors(builder)); + } // Inject channel priming if (settings.isRefreshingChannel()) { @@ -233,23 +236,22 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set return ClientContext.create(builder.build()); } - private static void setInterceptors( - InstantiatingGrpcChannelProvider.Builder transportProvider, + private static ImmutableList getInterceptors( EnhancedBigtableStubSettings.Builder settings) { - Set interceptors = + Set connectionErrorCountInterceptors = setupConnectionErrorCountInterceptors(settings); - if (transportProvider != null) { - if (settings.getEnableRoutingCookie()) { - // TODO: this also need to be added to BigtableClientFactory - transportProvider.setInterceptorProvider( - () -> - ImmutableList.of( - new CookiesInterceptor(), new ConnectionErrorCountInterceptor(interceptors))); - } else { - transportProvider.setInterceptorProvider( - () -> ImmutableList.of(new ConnectionErrorCountInterceptor(interceptors))); - } + ConnectionErrorCountInterceptor connectionErrorCountInterceptor = + new ConnectionErrorCountInterceptor(); + connectionErrorCountInterceptors.add(connectionErrorCountInterceptor); + ImmutableList.Builder builder = + ImmutableList.builder().add(connectionErrorCountInterceptor); + + if (settings.getEnableRoutingCookie()) { + // TODO: this also need to be added to BigtableClientFactory + builder.add(new CookiesInterceptor()); } + + return builder.build(); } private static Set setupConnectionErrorCountInterceptors( From a82f3323bdd5fbccfb4e28dac5bba704d2d165ce Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Mon, 5 Feb 2024 16:01:47 -0500 Subject: [PATCH 03/23] Rename setting background task and fix imports. --- .../data/v2/stub/EnhancedBigtableStub.java | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index f646fd3ff6..5a16376b5a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -127,7 +127,11 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; -import java.util.*; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.WeakHashMap; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -206,8 +210,13 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set ? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder() : null; + Set connectionErrorCountInterceptors = + Collections.newSetFromMap(new WeakHashMap<>()); + setupConnectionErrorCountTask(builder, connectionErrorCountInterceptors); + if (transportProvider != null) { - transportProvider.setInterceptorProvider(() -> getInterceptors(builder)); + transportProvider.setInterceptorProvider( + () -> getInterceptors(builder, connectionErrorCountInterceptors)); } // Inject channel priming @@ -237,9 +246,8 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set } private static ImmutableList getInterceptors( - EnhancedBigtableStubSettings.Builder settings) { - Set connectionErrorCountInterceptors = - setupConnectionErrorCountInterceptors(settings); + EnhancedBigtableStubSettings.Builder settings, + Set connectionErrorCountInterceptors) { ConnectionErrorCountInterceptor connectionErrorCountInterceptor = new ConnectionErrorCountInterceptor(); connectionErrorCountInterceptors.add(connectionErrorCountInterceptor); @@ -254,10 +262,9 @@ private static ImmutableList getInterceptors( return builder.build(); } - private static Set setupConnectionErrorCountInterceptors( - EnhancedBigtableStubSettings.Builder settings) { - Set interceptors = - Collections.newSetFromMap(new WeakHashMap<>()); + private static void setupConnectionErrorCountTask( + EnhancedBigtableStubSettings.Builder settings, + Set interceptors) { ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); ImmutableMap builtinAttributes = createBuiltinAttributes(settings); scheduler.scheduleAtFixedRate( @@ -265,7 +272,6 @@ private static Set setupConnectionErrorCountInt 0, PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, TimeUnit.SECONDS); - return interceptors; } public static ApiTracerFactory createBigtableTracerFactory( From 0c0ae0074b95cea9dca1b8095f923cfa34b37fe2 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Tue, 6 Feb 2024 10:20:57 -0500 Subject: [PATCH 04/23] Temporarily skip exporting per connection metrics to fix test failures. --- .../bigtable/stats/BigtableCreateTimeSeriesExporter.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BigtableCreateTimeSeriesExporter.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BigtableCreateTimeSeriesExporter.java index 325a07a0c5..dca52f1c81 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BigtableCreateTimeSeriesExporter.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BigtableCreateTimeSeriesExporter.java @@ -51,6 +51,10 @@ public void export(Collection metrics) { if (!metric.getMetricDescriptor().getName().contains("bigtable")) { continue; } + // TODO: temporarily skip exporting per connection metrics. + if (metric.getMetricDescriptor().getName().contains("per_connection_error_count")) { + continue; + } projectToTimeSeries = metric.getTimeSeriesList().stream() From 2a094497b505d2e0158cb8a52bb6c2a097e7b3b1 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Tue, 6 Feb 2024 16:35:48 -0500 Subject: [PATCH 05/23] Temporarily share the tests for debugging purposes --- .../stub/ConnectionErrorCountInterceptor.java | 2 +- .../stub/CountErrorsPerInterceptorTask.java | 8 +- .../data/v2/stub/EnhancedBigtableStub.java | 6 +- .../v2/stub/EnhancedBigtableStubSettings.java | 23 ++ .../v2/stub/ErrorCountPerConnectionTest.java | 203 ++++++++++++++++++ 5 files changed, 239 insertions(+), 3 deletions(-) create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java index 8e73340de1..48187d9e13 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java @@ -32,7 +32,7 @@ class ConnectionErrorCountInterceptor implements ClientInterceptor { private final AtomicInteger numOfSuccesses; public ConnectionErrorCountInterceptor() { - numOfErrors = new AtomicInteger(0); + numOfErrors = new AtomicInteger(36); numOfSuccesses = new AtomicInteger(0); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java index 73b36ba9cd..c798c2e4b3 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java @@ -22,7 +22,11 @@ class CountErrorsPerInterceptorTask implements Runnable { private final Set interceptors; - private final StatsRecorderWrapper statsRecorderWrapper; + private StatsRecorderWrapper statsRecorderWrapper; + + public void setStatsRecorderWrapper(StatsRecorderWrapper statsRecorderWrapper) { + this.statsRecorderWrapper = statsRecorderWrapper; + } public CountErrorsPerInterceptorTask( Set interceptors, @@ -36,11 +40,13 @@ public void run() { for (ConnectionErrorCountInterceptor interceptor : interceptors) { int errors = interceptor.getAndResetNumOfErrors(); int successes = interceptor.getAndResetNumOfSuccesses(); + System.out.println("reza running run()" + errors + ", " + successes); // We avoid keeping track of inactive connections (i.e., without any failed or successful // requests). if (errors > 0 || successes > 0) { // TODO: add a metric to also keep track of the number of successful requests per each // connection. + System.out.println("reza gonna call putAndRecordPerConnectionErrorCount w/ " + errors); this.statsRecorderWrapper.putAndRecordPerConnectionErrorCount(errors); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 5a16376b5a..6a7b724865 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -182,6 +182,7 @@ public class EnhancedBigtableStub implements AutoCloseable { public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) throws IOException { + System.out.println("reza in create()"); settings = settings.toBuilder().setTracerFactory(createBigtableTracerFactory(settings)).build(); ClientContext clientContext = createClientContext(settings); @@ -197,6 +198,7 @@ public static EnhancedBigtableStub createWithClientContext( public static ClientContext createClientContext(EnhancedBigtableStubSettings settings) throws IOException { + System.out.println("reza in createClientContext"); EnhancedBigtableStubSettings.Builder builder = settings.toBuilder(); // TODO: this implementation is on the cusp of unwieldy, if we end up adding more features @@ -265,7 +267,9 @@ private static ImmutableList getInterceptors( private static void setupConnectionErrorCountTask( EnhancedBigtableStubSettings.Builder settings, Set interceptors) { - ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); +// ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); +// ScheduledExecutorService scheduler = settings.getBackgroundExecutorProvider().getExecutor(); + ScheduledExecutorService scheduler = settings.getMyExecutorProvider().getExecutor(); ImmutableMap builtinAttributes = createBuiltinAttributes(settings); scheduler.scheduleAtFixedRate( new CountErrorsPerInterceptorTask(interceptors, builtinAttributes), 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 44e4752cd5..f227d3ca49 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 @@ -22,7 +22,9 @@ import com.google.api.gax.batching.FlowControlSettings; import com.google.api.gax.batching.FlowController; import com.google.api.gax.batching.FlowController.LimitExceededBehavior; +import com.google.api.gax.core.ExecutorProvider; import com.google.api.gax.core.GoogleCredentialsProvider; +import com.google.api.gax.core.InstantiatingExecutorProvider; import com.google.api.gax.grpc.ChannelPoolSettings; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.retrying.RetrySettings; @@ -212,6 +214,7 @@ public class EnhancedBigtableStubSettings extends StubSettings jwtAudienceMapping; private final boolean enableRoutingCookie; private final boolean enableRetryInfo; + private final ExecutorProvider myExecutorProvider; private final ServerStreamingCallSettings readRowsSettings; private final UnaryCallSettings readRowSettings; @@ -255,6 +258,8 @@ private EnhancedBigtableStubSettings(Builder builder) { jwtAudienceMapping = builder.jwtAudienceMapping; enableRoutingCookie = builder.enableRoutingCookie; enableRetryInfo = builder.enableRetryInfo; + System.out.println("reza setting good provider"); + myExecutorProvider = builder.getMyExecutorProvider(); // Per method settings. readRowsSettings = builder.readRowsSettings.build(); @@ -334,6 +339,10 @@ public boolean getEnableRetryInfo() { return enableRetryInfo; } + public ExecutorProvider getMyExecutorProvider() { + return myExecutorProvider; + } + /** Returns a builder for the default ChannelProvider for this service. */ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() { return BigtableStubSettings.defaultGrpcTransportProviderBuilder() @@ -635,6 +644,7 @@ public static class Builder extends StubSettings.Builder pingAndWarmSettings; private FeatureFlags.Builder featureFlags; + private ExecutorProvider myExecutorProvider; /** * Initializes a new Builder with sane defaults for all settings. @@ -652,6 +662,8 @@ private Builder() { setCredentialsProvider(defaultCredentialsProviderBuilder().build()); this.enableRoutingCookie = true; this.enableRetryInfo = true; + System.out.println("reza setting myExecutor1"); + myExecutorProvider = InstantiatingExecutorProvider.newBuilder().build(); // Defaults provider BigtableStubSettings.Builder baseDefaults = BigtableStubSettings.newBuilder(); @@ -772,6 +784,8 @@ private Builder(EnhancedBigtableStubSettings settings) { jwtAudienceMapping = settings.jwtAudienceMapping; enableRoutingCookie = settings.enableRoutingCookie; enableRetryInfo = settings.enableRetryInfo; + System.out.println("reza setting myExecutorProvider 2"); + myExecutorProvider = settings.getMyExecutorProvider(); // Per method settings. readRowsSettings = settings.readRowsSettings.toBuilder(); @@ -949,6 +963,15 @@ public Builder setEnableRetryInfo(boolean enableRetryInfo) { return this; } + public Builder setMyExecutorProvider(ExecutorProvider myExecutorProvider) { + this.myExecutorProvider = myExecutorProvider; + return this; + } + + public ExecutorProvider getMyExecutorProvider() { + return myExecutorProvider; + } + /** * Gets if RetryInfo is enabled. If true, client bases retry decision and back off time on * server returned RetryInfo value. Otherwise, client uses {@link RetrySettings}. diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java new file mode 100644 index 0000000000..d5300771b8 --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java @@ -0,0 +1,203 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import static com.google.cloud.bigtable.data.v2.MetadataSubject.assertThat; +import static com.google.common.truth.Truth.assertThat; + +import com.google.api.gax.core.FixedExecutorProvider; +import com.google.api.gax.rpc.ServerStream; +import com.google.bigtable.v2.*; +import com.google.cloud.bigtable.data.v2.BigtableDataClient; +import com.google.cloud.bigtable.data.v2.BigtableDataSettings; +import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; +import com.google.cloud.bigtable.data.v2.models.*; +import com.google.cloud.bigtable.data.v2.models.Row; +import com.google.cloud.bigtable.stats.StatsRecorderWrapper; +import io.grpc.*; +import io.grpc.stub.StreamObserver; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; + +@RunWith(JUnit4.class) +public class ErrorCountPerConnectionTest { + private Server server; + private final FakeService fakeService = new FakeService(); + private BigtableDataSettings.Builder settings; + private BigtableDataClient client; + private EnhancedBigtableStub stub; + private ArgumentCaptor runnableCaptor; + private StatsRecorderWrapper statsRecorderWrapper; + private final List serverMetadata = new ArrayList<>(); + + private final Set methods = new HashSet<>(); + + @Before + public void setup() throws Exception { +// ServerInterceptor serverInterceptor = +// new ServerInterceptor() { +// @Override +// public ServerCall.Listener interceptCall( +// ServerCall serverCall, +// Metadata metadata, +// ServerCallHandler serverCallHandler) { +// serverMetadata.add(metadata); +// // if (metadata.containsKey(ROUTING_COOKIE_1)) { +// // methods.add(serverCall.getMethodDescriptor().getBareMethodName()); +// // } +// return serverCallHandler.startCall( +// new ForwardingServerCall.SimpleForwardingServerCall(serverCall) { +// @Override +// public void sendHeaders(Metadata responseHeaders) { +// // responseHeaders.put(ROUTING_COOKIE_HEADER, +// // testHeaderCookie); +// // responseHeaders.put(ROUTING_COOKIE_1, +// // routingCookie1Header); +// super.sendHeaders(responseHeaders); +// } +// }, +// metadata); +// } +// }; + +// server = FakeServiceBuilder.create(fakeService).intercept(serverInterceptor).start(); + server = FakeServiceBuilder.create(fakeService).start(); + +// new + System.out.println("reza start"); + ScheduledExecutorService executors = Mockito.mock(ScheduledExecutorService.class); + EnhancedBigtableStubSettings.Builder builder = + BigtableDataSettings.newBuilderForEmulator(server.getPort()).stubSettings() +// NOTE: when replacing this line with the one below, the stub creation hangs. +// .setBackgroundExecutorProvider(FixedExecutorProvider.create(executors)) + .setMyExecutorProvider(FixedExecutorProvider.create(executors)) + .setProjectId("fake-project") + .setInstanceId("fake-instance"); + runnableCaptor = ArgumentCaptor.forClass(Runnable.class); + System.out.println("rez1 " + executors + " 2 = " + builder); + Mockito.when(executors.scheduleAtFixedRate(runnableCaptor.capture(), anyLong(), anyLong(), any())).thenReturn(null); + stub = EnhancedBigtableStub.create(builder.build()); + + statsRecorderWrapper = Mockito.mock(StatsRecorderWrapper.class); + List runnables = runnableCaptor.getAllValues(); + for (Runnable runnable : runnables) { + if (runnable instanceof CountErrorsPerInterceptorTask) { + System.out.println("REZA iterating over runnable."); + ((CountErrorsPerInterceptorTask) runnable).setStatsRecorderWrapper(statsRecorderWrapper); + } + } +// end new + +// BigtableDataSettings.Builder settings = +// BigtableDataSettings.newBuilderForEmulator(server.getPort()) +// .setProjectId("fake-project") +// .setInstanceId("fake-instance"); +// +// this.settings = settings; +// +// client = BigtableDataClient.create(settings.build()); + } + + @After + public void tearDown() throws Exception { +// if (client != null) { +// client.close(); +// } + if (server != null) { + server.shutdown(); + } + } + + @Test + public void testReadRows() { + System.out.println("rezaar"); + Query query = Query.create("fake-table"); + + ServerStream responses = stub.readRowsCallable().call(query); + for (Row row : responses) { + System.out.println("row = " + row); + } +// System.out.println("reza hasNext = " + stub.readRowsCallable().call(query).iterator().hasNext()); + ArgumentCaptor longCaptor = ArgumentCaptor.forClass(long.class); + statsRecorderWrapper.putAndRecordPerConnectionErrorCount(longCaptor.capture()); + Mockito.doNothing().when(statsRecorderWrapper).putAndRecordPerConnectionErrorCount(longCaptor.capture()); + for (Runnable runnable : runnableCaptor.getAllValues()) { + if (runnable instanceof CountErrorsPerInterceptorTask) { + System.out.println("REZA iterating over runnable Second time."); + runnable.run(); + } + } + for (long longVal : longCaptor.getAllValues()) { + System.out.println("REZA got long = " + longVal); + } + +// client.readRows(Query.create("fake-table")).iterator().hasNext(); +// +// assertThat(fakeService.count.get()).isGreaterThan(1); +// assertThat(serverMetadata).hasSize(fakeService.count.get()); +// +// Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); +// +// // assertThat(lastMetadata) +// // .containsAtLeast( +// // ROUTING_COOKIE_1.name(), +// // "readRows", +// // ROUTING_COOKIE_2.name(), +// // testCookie, +// // ROUTING_COOKIE_HEADER.name(), +// // testHeaderCookie); +// // assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); +// +// serverMetadata.clear(); + } + + static class FakeService extends BigtableGrpc.BigtableImplBase { + + private boolean returnCookie = true; + private final AtomicInteger count = new AtomicInteger(); + + @Override + public void readRows( + ReadRowsRequest request, StreamObserver responseObserver) { + if (count.getAndIncrement() > 3) { + System.out.println("reza in server = " + count.get()); + Metadata trailers = new Metadata(); + // maybePopulateCookie(trailers, "readRows"); + responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); + StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + responseObserver.onError(exception); +// responseObserver.onCompleted(); + return; + } + System.out.println("reza in server = " + count.get()); + responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + } +} From b98607361991ea831cb70de2c2fcc2be1049eb6c Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Tue, 6 Feb 2024 17:28:37 -0500 Subject: [PATCH 06/23] Temporarily add the test for debugging. --- .../stub/ConnectionErrorCountInterceptor.java | 3 +- .../stub/CountErrorsPerInterceptorTask.java | 2 +- .../data/v2/stub/EnhancedBigtableStub.java | 7 +++- .../v2/stub/ErrorCountPerConnectionTest.java | 38 +++++++++++-------- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java index 48187d9e13..c76496071b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java @@ -32,7 +32,7 @@ class ConnectionErrorCountInterceptor implements ClientInterceptor { private final AtomicInteger numOfSuccesses; public ConnectionErrorCountInterceptor() { - numOfErrors = new AtomicInteger(36); + numOfErrors = new AtomicInteger(0); numOfSuccesses = new AtomicInteger(0); } @@ -48,6 +48,7 @@ public void start(Listener responseListener, Metadata headers) { responseListener) { @Override public void onClose(Status status, Metadata trailers) { + System.out.println("reza is in onClose w/ status = " + status.isOk()); if (status.isOk()) { numOfSuccesses.getAndIncrement(); } else { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java index c798c2e4b3..7e23dc3a4a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java @@ -40,7 +40,7 @@ public void run() { for (ConnectionErrorCountInterceptor interceptor : interceptors) { int errors = interceptor.getAndResetNumOfErrors(); int successes = interceptor.getAndResetNumOfSuccesses(); - System.out.println("reza running run()" + errors + ", " + successes); + System.out.println("reza running run(), e = " + errors + ", s = " + successes); // We avoid keeping track of inactive connections (i.e., without any failed or successful // requests). if (errors > 0 || successes > 0) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 6a7b724865..842c8af116 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -268,9 +268,12 @@ private static void setupConnectionErrorCountTask( EnhancedBigtableStubSettings.Builder settings, Set interceptors) { // ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); -// ScheduledExecutorService scheduler = settings.getBackgroundExecutorProvider().getExecutor(); - ScheduledExecutorService scheduler = settings.getMyExecutorProvider().getExecutor(); +// NOTE: when replacing this line with the one below, the stub creation hangs. + ScheduledExecutorService scheduler = settings.getBackgroundExecutorProvider().getExecutor(); +// ScheduledExecutorService scheduler = settings.getMyExecutorProvider().getExecutor(); + System.out.println("reza using ScheduledExecutorService = " + scheduler); ImmutableMap builtinAttributes = createBuiltinAttributes(settings); +// scheduler.scheduleWithFixedDelay() scheduler.scheduleAtFixedRate( new CountErrorsPerInterceptorTask(interceptors, builtinAttributes), 0, diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java index d5300771b8..098e52cde2 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java @@ -90,13 +90,13 @@ public void setup() throws Exception { server = FakeServiceBuilder.create(fakeService).start(); // new - System.out.println("reza start"); + System.out.println("rezaa start"); ScheduledExecutorService executors = Mockito.mock(ScheduledExecutorService.class); EnhancedBigtableStubSettings.Builder builder = BigtableDataSettings.newBuilderForEmulator(server.getPort()).stubSettings() // NOTE: when replacing this line with the one below, the stub creation hangs. -// .setBackgroundExecutorProvider(FixedExecutorProvider.create(executors)) - .setMyExecutorProvider(FixedExecutorProvider.create(executors)) + .setBackgroundExecutorProvider(FixedExecutorProvider.create(executors)) +// .setMyExecutorProvider(FixedExecutorProvider.create(executors)) .setProjectId("fake-project") .setInstanceId("fake-instance"); runnableCaptor = ArgumentCaptor.forClass(Runnable.class); @@ -136,12 +136,18 @@ public void tearDown() throws Exception { @Test public void testReadRows() { - System.out.println("rezaar"); + System.out.println("rezaaaar"); Query query = Query.create("fake-table"); - ServerStream responses = stub.readRowsCallable().call(query); - for (Row row : responses) { - System.out.println("row = " + row); + for (int i = 0; i < 4; i++) { + try { + ServerStream responses = stub.readRowsCallable().call(query); + for (Row row : responses) { + System.out.println("row = " + row); + } + } catch (Exception e) { + System.out.println("reza got exception = " + e.getMessage()); + } } // System.out.println("reza hasNext = " + stub.readRowsCallable().call(query).iterator().hasNext()); ArgumentCaptor longCaptor = ArgumentCaptor.forClass(long.class); @@ -185,19 +191,21 @@ static class FakeService extends BigtableGrpc.BigtableImplBase { @Override public void readRows( ReadRowsRequest request, StreamObserver responseObserver) { - if (count.getAndIncrement() > 3) { - System.out.println("reza in server = " + count.get()); + count.getAndIncrement(); + if (count.get() > 2) { + System.out.println("reza in server bad = " + count.get()); Metadata trailers = new Metadata(); // maybePopulateCookie(trailers, "readRows"); responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); - StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE, trailers); + StatusRuntimeException exception = new StatusRuntimeException(Status.DEADLINE_EXCEEDED, trailers); responseObserver.onError(exception); -// responseObserver.onCompleted(); - return; + } else { + System.out.println("reza in server good = " + count.get()); + responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); + responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); + responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); + responseObserver.onCompleted(); } - System.out.println("reza in server = " + count.get()); - responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); - responseObserver.onCompleted(); } } } From c254251fce7677c52ee47ad5fe2492391a849f5a Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Tue, 6 Feb 2024 20:15:36 -0500 Subject: [PATCH 07/23] Remove the new ExecutorProvider and fix integration test failures. --- .../data/v2/stub/EnhancedBigtableStub.java | 3 +- .../v2/stub/EnhancedBigtableStubSettings.java | 23 -------- .../v2/stub/ErrorCountPerConnectionTest.java | 59 +------------------ 3 files changed, 3 insertions(+), 82 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 842c8af116..f212c10fde 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -270,7 +270,6 @@ private static void setupConnectionErrorCountTask( // ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); // NOTE: when replacing this line with the one below, the stub creation hangs. ScheduledExecutorService scheduler = settings.getBackgroundExecutorProvider().getExecutor(); -// ScheduledExecutorService scheduler = settings.getMyExecutorProvider().getExecutor(); System.out.println("reza using ScheduledExecutorService = " + scheduler); ImmutableMap builtinAttributes = createBuiltinAttributes(settings); // scheduler.scheduleWithFixedDelay() @@ -325,7 +324,7 @@ public static ApiTracerFactory createBigtableTracerFactory( private static ImmutableMap createBuiltinAttributes( EnhancedBigtableStubSettings.Builder settings) { return ImmutableMap.builder() - .put("project", settings.getProjectId()) + .put("project_id", settings.getProjectId()) .put("instance", settings.getInstanceId()) .put("app_profile", settings.getAppProfileId()) .put("client_name", "bigtable-java/" + Version.VERSION) 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 f227d3ca49..f840df3cc3 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 @@ -214,7 +214,6 @@ public class EnhancedBigtableStubSettings extends StubSettings jwtAudienceMapping; private final boolean enableRoutingCookie; private final boolean enableRetryInfo; - private final ExecutorProvider myExecutorProvider; private final ServerStreamingCallSettings readRowsSettings; private final UnaryCallSettings readRowSettings; @@ -258,8 +257,6 @@ private EnhancedBigtableStubSettings(Builder builder) { jwtAudienceMapping = builder.jwtAudienceMapping; enableRoutingCookie = builder.enableRoutingCookie; enableRetryInfo = builder.enableRetryInfo; - System.out.println("reza setting good provider"); - myExecutorProvider = builder.getMyExecutorProvider(); // Per method settings. readRowsSettings = builder.readRowsSettings.build(); @@ -339,10 +336,6 @@ public boolean getEnableRetryInfo() { return enableRetryInfo; } - public ExecutorProvider getMyExecutorProvider() { - return myExecutorProvider; - } - /** Returns a builder for the default ChannelProvider for this service. */ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() { return BigtableStubSettings.defaultGrpcTransportProviderBuilder() @@ -644,7 +637,6 @@ public static class Builder extends StubSettings.Builder pingAndWarmSettings; private FeatureFlags.Builder featureFlags; - private ExecutorProvider myExecutorProvider; /** * Initializes a new Builder with sane defaults for all settings. @@ -662,9 +654,6 @@ private Builder() { setCredentialsProvider(defaultCredentialsProviderBuilder().build()); this.enableRoutingCookie = true; this.enableRetryInfo = true; - System.out.println("reza setting myExecutor1"); - myExecutorProvider = InstantiatingExecutorProvider.newBuilder().build(); - // Defaults provider BigtableStubSettings.Builder baseDefaults = BigtableStubSettings.newBuilder(); @@ -784,9 +773,6 @@ private Builder(EnhancedBigtableStubSettings settings) { jwtAudienceMapping = settings.jwtAudienceMapping; enableRoutingCookie = settings.enableRoutingCookie; enableRetryInfo = settings.enableRetryInfo; - System.out.println("reza setting myExecutorProvider 2"); - myExecutorProvider = settings.getMyExecutorProvider(); - // Per method settings. readRowsSettings = settings.readRowsSettings.toBuilder(); readRowSettings = settings.readRowSettings.toBuilder(); @@ -963,15 +949,6 @@ public Builder setEnableRetryInfo(boolean enableRetryInfo) { return this; } - public Builder setMyExecutorProvider(ExecutorProvider myExecutorProvider) { - this.myExecutorProvider = myExecutorProvider; - return this; - } - - public ExecutorProvider getMyExecutorProvider() { - return myExecutorProvider; - } - /** * Gets if RetryInfo is enabled. If true, client bases retry decision and back off time on * server returned RetryInfo value. Otherwise, client uses {@link RetrySettings}. diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java index 098e52cde2..0a876e76f0 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java @@ -60,32 +60,6 @@ public class ErrorCountPerConnectionTest { @Before public void setup() throws Exception { -// ServerInterceptor serverInterceptor = -// new ServerInterceptor() { -// @Override -// public ServerCall.Listener interceptCall( -// ServerCall serverCall, -// Metadata metadata, -// ServerCallHandler serverCallHandler) { -// serverMetadata.add(metadata); -// // if (metadata.containsKey(ROUTING_COOKIE_1)) { -// // methods.add(serverCall.getMethodDescriptor().getBareMethodName()); -// // } -// return serverCallHandler.startCall( -// new ForwardingServerCall.SimpleForwardingServerCall(serverCall) { -// @Override -// public void sendHeaders(Metadata responseHeaders) { -// // responseHeaders.put(ROUTING_COOKIE_HEADER, -// // testHeaderCookie); -// // responseHeaders.put(ROUTING_COOKIE_1, -// // routingCookie1Header); -// super.sendHeaders(responseHeaders); -// } -// }, -// metadata); -// } -// }; - // server = FakeServiceBuilder.create(fakeService).intercept(serverInterceptor).start(); server = FakeServiceBuilder.create(fakeService).start(); @@ -96,7 +70,6 @@ public void setup() throws Exception { BigtableDataSettings.newBuilderForEmulator(server.getPort()).stubSettings() // NOTE: when replacing this line with the one below, the stub creation hangs. .setBackgroundExecutorProvider(FixedExecutorProvider.create(executors)) -// .setMyExecutorProvider(FixedExecutorProvider.create(executors)) .setProjectId("fake-project") .setInstanceId("fake-instance"); runnableCaptor = ArgumentCaptor.forClass(Runnable.class); @@ -113,15 +86,6 @@ public void setup() throws Exception { } } // end new - -// BigtableDataSettings.Builder settings = -// BigtableDataSettings.newBuilderForEmulator(server.getPort()) -// .setProjectId("fake-project") -// .setInstanceId("fake-instance"); -// -// this.settings = settings; -// -// client = BigtableDataClient.create(settings.build()); } @After @@ -139,7 +103,7 @@ public void testReadRows() { System.out.println("rezaaaar"); Query query = Query.create("fake-table"); - for (int i = 0; i < 4; i++) { + for (int i = 0; i < 6; i++) { try { ServerStream responses = stub.readRowsCallable().call(query); for (Row row : responses) { @@ -162,25 +126,6 @@ public void testReadRows() { for (long longVal : longCaptor.getAllValues()) { System.out.println("REZA got long = " + longVal); } - -// client.readRows(Query.create("fake-table")).iterator().hasNext(); -// -// assertThat(fakeService.count.get()).isGreaterThan(1); -// assertThat(serverMetadata).hasSize(fakeService.count.get()); -// -// Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); -// -// // assertThat(lastMetadata) -// // .containsAtLeast( -// // ROUTING_COOKIE_1.name(), -// // "readRows", -// // ROUTING_COOKIE_2.name(), -// // testCookie, -// // ROUTING_COOKIE_HEADER.name(), -// // testHeaderCookie); -// // assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); -// -// serverMetadata.clear(); } static class FakeService extends BigtableGrpc.BigtableImplBase { @@ -197,7 +142,7 @@ public void readRows( Metadata trailers = new Metadata(); // maybePopulateCookie(trailers, "readRows"); responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); - StatusRuntimeException exception = new StatusRuntimeException(Status.DEADLINE_EXCEEDED, trailers); + StatusRuntimeException exception = new StatusRuntimeException(Status.INTERNAL, trailers); responseObserver.onError(exception); } else { System.out.println("reza in server good = " + count.get()); From 7c149ed65d989398efd09fe73d8b4d1ccfe879e4 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Tue, 6 Feb 2024 21:27:09 -0500 Subject: [PATCH 08/23] Update unit tests to reflect the new setup. --- .../data/v2/BigtableDataClientFactoryTest.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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 edcda45938..2aec1a0c1c 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 @@ -177,10 +177,11 @@ public void testNewClientsShareTransportChannel() throws Exception { BigtableDataClient ignored2 = factory.createForInstance("project2", "instance2"); BigtableDataClient ignored3 = factory.createForInstance("project3", "instance3")) { - // Make sure that only 1 instance is created by each provider + // Make sure that the right number of instances is created by each provider Mockito.verify(transportChannelProvider, Mockito.times(1)).getTransportChannel(); Mockito.verify(credentialsProvider, Mockito.times(1)).getCredentials(); - Mockito.verify(executorProvider, Mockito.times(1)).getExecutor(); + // getExecutor() is called for setting Watchdog and per_connection_error_count metric. + Mockito.verify(executorProvider, Mockito.times(2)).getExecutor(); Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog(); } } @@ -266,9 +267,10 @@ public void testCreateWithRefreshingChannel() throws Exception { factory.createForAppProfile("other-appprofile"); factory.createForInstance("other-project", "other-instance"); - // Make sure that only 1 instance is created for all clients + // Make sure that the right number of instances is created for all clients Mockito.verify(credentialsProvider, Mockito.times(1)).getCredentials(); - Mockito.verify(executorProvider, Mockito.times(1)).getExecutor(); + // getExecutor() is called for setting Watchdog and per_connection_error_count metric. + Mockito.verify(executorProvider, Mockito.times(2)).getExecutor(); Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog(); // Make sure that the clients are sharing the same ChannelPool From 69e693f9ec65d9a9ecc560d367f1f8edbbb09ec8 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Wed, 7 Feb 2024 19:25:46 -0500 Subject: [PATCH 09/23] Clean up and add tests. --- .../stub/ConnectionErrorCountInterceptor.java | 1 - .../stub/CountErrorsPerInterceptorTask.java | 22 +- .../data/v2/stub/EnhancedBigtableStub.java | 10 +- .../v2/stub/EnhancedBigtableStubSettings.java | 2 - .../v2/stub/ErrorCountPerConnectionTest.java | 238 +++++++++++++----- 5 files changed, 184 insertions(+), 89 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java index c76496071b..8e73340de1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java @@ -48,7 +48,6 @@ public void start(Listener responseListener, Metadata headers) { responseListener) { @Override public void onClose(Status status, Metadata trailers) { - System.out.println("reza is in onClose w/ status = " + status.isOk()); if (status.isOk()) { numOfSuccesses.getAndIncrement(); } else { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java index 7e23dc3a4a..ff64518dc5 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java @@ -37,17 +37,17 @@ public CountErrorsPerInterceptorTask( @Override public void run() { - for (ConnectionErrorCountInterceptor interceptor : interceptors) { - int errors = interceptor.getAndResetNumOfErrors(); - int successes = interceptor.getAndResetNumOfSuccesses(); - System.out.println("reza running run(), e = " + errors + ", s = " + successes); - // We avoid keeping track of inactive connections (i.e., without any failed or successful - // requests). - if (errors > 0 || successes > 0) { - // TODO: add a metric to also keep track of the number of successful requests per each - // connection. - System.out.println("reza gonna call putAndRecordPerConnectionErrorCount w/ " + errors); - this.statsRecorderWrapper.putAndRecordPerConnectionErrorCount(errors); + synchronized (interceptors) { + for (ConnectionErrorCountInterceptor interceptor : interceptors) { + int errors = interceptor.getAndResetNumOfErrors(); + int successes = interceptor.getAndResetNumOfSuccesses(); + // We avoid keeping track of inactive connections (i.e., without any failed or successful + // requests). + if (errors > 0 || successes > 0) { + // TODO: add a metric to also keep track of the number of successful requests per each + // connection. + this.statsRecorderWrapper.putAndRecordPerConnectionErrorCount(errors); + } } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index f212c10fde..0b7a5574a3 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -132,7 +132,6 @@ import java.util.Map; import java.util.Set; import java.util.WeakHashMap; -import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; @@ -182,8 +181,6 @@ public class EnhancedBigtableStub implements AutoCloseable { public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) throws IOException { - System.out.println("reza in create()"); - settings = settings.toBuilder().setTracerFactory(createBigtableTracerFactory(settings)).build(); ClientContext clientContext = createClientContext(settings); @@ -198,7 +195,6 @@ public static EnhancedBigtableStub createWithClientContext( public static ClientContext createClientContext(EnhancedBigtableStubSettings settings) throws IOException { - System.out.println("reza in createClientContext"); EnhancedBigtableStubSettings.Builder builder = settings.toBuilder(); // TODO: this implementation is on the cusp of unwieldy, if we end up adding more features @@ -213,7 +209,7 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set : null; Set connectionErrorCountInterceptors = - Collections.newSetFromMap(new WeakHashMap<>()); + Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); setupConnectionErrorCountTask(builder, connectionErrorCountInterceptors); if (transportProvider != null) { @@ -267,12 +263,8 @@ private static ImmutableList getInterceptors( private static void setupConnectionErrorCountTask( EnhancedBigtableStubSettings.Builder settings, Set interceptors) { -// ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); -// NOTE: when replacing this line with the one below, the stub creation hangs. ScheduledExecutorService scheduler = settings.getBackgroundExecutorProvider().getExecutor(); - System.out.println("reza using ScheduledExecutorService = " + scheduler); ImmutableMap builtinAttributes = createBuiltinAttributes(settings); -// scheduler.scheduleWithFixedDelay() scheduler.scheduleAtFixedRate( new CountErrorsPerInterceptorTask(interceptors, builtinAttributes), 0, 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 f840df3cc3..9a5027c740 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 @@ -22,9 +22,7 @@ import com.google.api.gax.batching.FlowControlSettings; import com.google.api.gax.batching.FlowController; import com.google.api.gax.batching.FlowController.LimitExceededBehavior; -import com.google.api.gax.core.ExecutorProvider; import com.google.api.gax.core.GoogleCredentialsProvider; -import com.google.api.gax.core.InstantiatingExecutorProvider; import com.google.api.gax.grpc.ChannelPoolSettings; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.retrying.RetrySettings; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java index 0a876e76f0..18a9f00745 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 Google LLC + * Copyright 2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,24 +15,19 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import static com.google.cloud.bigtable.data.v2.MetadataSubject.assertThat; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; import com.google.api.gax.core.FixedExecutorProvider; -import com.google.api.gax.rpc.ServerStream; import com.google.bigtable.v2.*; -import com.google.cloud.bigtable.data.v2.BigtableDataClient; import com.google.cloud.bigtable.data.v2.BigtableDataSettings; import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; import com.google.cloud.bigtable.data.v2.models.*; -import com.google.cloud.bigtable.data.v2.models.Row; import com.google.cloud.bigtable.stats.StatsRecorderWrapper; import io.grpc.*; import io.grpc.stub.StreamObserver; -import java.util.ArrayList; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; @@ -42,114 +37,225 @@ import org.junit.runners.JUnit4; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyLong; @RunWith(JUnit4.class) public class ErrorCountPerConnectionTest { + private static final String SUCCESS_TABLE_NAME = "some-table"; + private static final String ERROR_TABLE_NAME = "nonexistent-table"; private Server server; private final FakeService fakeService = new FakeService(); - private BigtableDataSettings.Builder settings; - private BigtableDataClient client; - private EnhancedBigtableStub stub; + private EnhancedBigtableStubSettings.Builder builder; private ArgumentCaptor runnableCaptor; private StatsRecorderWrapper statsRecorderWrapper; - private final List serverMetadata = new ArrayList<>(); - - private final Set methods = new HashSet<>(); @Before public void setup() throws Exception { -// server = FakeServiceBuilder.create(fakeService).intercept(serverInterceptor).start(); server = FakeServiceBuilder.create(fakeService).start(); -// new - System.out.println("rezaa start"); ScheduledExecutorService executors = Mockito.mock(ScheduledExecutorService.class); - EnhancedBigtableStubSettings.Builder builder = - BigtableDataSettings.newBuilderForEmulator(server.getPort()).stubSettings() -// NOTE: when replacing this line with the one below, the stub creation hangs. - .setBackgroundExecutorProvider(FixedExecutorProvider.create(executors)) - .setProjectId("fake-project") - .setInstanceId("fake-instance"); + builder = + BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .stubSettings() + .setBackgroundExecutorProvider(FixedExecutorProvider.create(executors)) + .setProjectId("fake-project") + .setInstanceId("fake-instance"); runnableCaptor = ArgumentCaptor.forClass(Runnable.class); System.out.println("rez1 " + executors + " 2 = " + builder); - Mockito.when(executors.scheduleAtFixedRate(runnableCaptor.capture(), anyLong(), anyLong(), any())).thenReturn(null); - stub = EnhancedBigtableStub.create(builder.build()); + Mockito.when( + executors.scheduleAtFixedRate(runnableCaptor.capture(), anyLong(), anyLong(), any())) + .thenReturn(null); statsRecorderWrapper = Mockito.mock(StatsRecorderWrapper.class); - List runnables = runnableCaptor.getAllValues(); - for (Runnable runnable : runnables) { - if (runnable instanceof CountErrorsPerInterceptorTask) { - System.out.println("REZA iterating over runnable."); - ((CountErrorsPerInterceptorTask) runnable).setStatsRecorderWrapper(statsRecorderWrapper); - } - } -// end new } @After public void tearDown() throws Exception { -// if (client != null) { -// client.close(); -// } if (server != null) { server.shutdown(); } } @Test - public void testReadRows() { - System.out.println("rezaaaar"); - Query query = Query.create("fake-table"); + public void singleRead() throws Exception { + EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build()); + long errorCount = 0; - for (int i = 0; i < 6; i++) { + for (int i = 0; i < 20; i++) { + Query query; + if (i % 3 == 0) { + query = Query.create(ERROR_TABLE_NAME); + errorCount += 1; + } else { + query = Query.create(SUCCESS_TABLE_NAME); + } try { - ServerStream responses = stub.readRowsCallable().call(query); - for (Row row : responses) { - System.out.println("row = " + row); + stub.readRowsCallable().call(query).iterator().hasNext(); + } catch (Exception e) { + // noop + } + } + ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); + Mockito.doNothing() + .when(statsRecorderWrapper) + .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); + runInterceptorTasksAndAssertCount(1); + List allErrorCounts = errorCountCaptor.getAllValues(); + assertThat(allErrorCounts.size()).isEqualTo(1); + assertThat(allErrorCounts.get(0)).isEqualTo(errorCount); + } + + @Test + public void readWithTwoStubs() throws Exception { + EnhancedBigtableStub stub1 = EnhancedBigtableStub.create(builder.build()); + EnhancedBigtableStub stub2 = EnhancedBigtableStub.create(builder.build()); + long errorCount1 = 0; + long errorCount2 = 0; + + for (int i = 0; i < 20; i++) { + Query successQuery = Query.create(SUCCESS_TABLE_NAME); + Query errorQuery = Query.create(ERROR_TABLE_NAME); + try { + if (i % 3 == 0) { + errorCount2 += 1; + stub1.readRowsCallable().call(successQuery).iterator().hasNext(); + stub2.readRowsCallable().call(errorQuery).iterator().hasNext(); + } else { + errorCount1 += 1; + stub1.readRowsCallable().call(errorQuery).iterator().hasNext(); + stub2.readRowsCallable().call(successQuery).iterator().hasNext(); } } catch (Exception e) { - System.out.println("reza got exception = " + e.getMessage()); + // noop } } -// System.out.println("reza hasNext = " + stub.readRowsCallable().call(query).iterator().hasNext()); - ArgumentCaptor longCaptor = ArgumentCaptor.forClass(long.class); - statsRecorderWrapper.putAndRecordPerConnectionErrorCount(longCaptor.capture()); - Mockito.doNothing().when(statsRecorderWrapper).putAndRecordPerConnectionErrorCount(longCaptor.capture()); + ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); + Mockito.doNothing() + .when(statsRecorderWrapper) + .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); + runInterceptorTasksAndAssertCount(2); + + List allErrorCounts = errorCountCaptor.getAllValues(); + assertThat(allErrorCounts.size()).isEqualTo(2); + assertThat(allErrorCounts).containsExactly(errorCount1, errorCount2); + } + + @Test + public void readOverTwoPeriods() throws Exception { + EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build()); + long errorCount = 0; + + for (int i = 0; i < 20; i++) { + Query query; + if (i % 3 == 0) { + query = Query.create(ERROR_TABLE_NAME); + errorCount += 1; + } else { + query = Query.create(SUCCESS_TABLE_NAME); + } + try { + stub.readRowsCallable().call(query).iterator().hasNext(); + } catch (Exception e) { + // noop + } + } + ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); + Mockito.doNothing() + .when(statsRecorderWrapper) + .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); + runInterceptorTasksAndAssertCount(1); + List allErrorCounts = errorCountCaptor.getAllValues(); + assertThat(allErrorCounts.size()).isEqualTo(1); + assertThat(allErrorCounts.get(0)).isEqualTo(errorCount); + + errorCount = 0; + + for (int i = 0; i < 20; i++) { + Query query; + if (i % 3 == 0) { + query = Query.create(SUCCESS_TABLE_NAME); + } else { + query = Query.create(ERROR_TABLE_NAME); + errorCount += 1; + } + try { + stub.readRowsCallable().call(query).iterator().hasNext(); + } catch (Exception e) { + // noop + } + } + errorCountCaptor = ArgumentCaptor.forClass(long.class); + Mockito.doNothing() + .when(statsRecorderWrapper) + .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); + runInterceptorTasksAndAssertCount(1); + allErrorCounts = errorCountCaptor.getAllValues(); + assertThat(allErrorCounts.size()).isEqualTo(1); + assertThat(allErrorCounts.get(0)).isEqualTo(errorCount); + } + + @Test + public void ignoreInactiveConnection() throws Exception { + EnhancedBigtableStub.create(builder.build()); + + ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); + Mockito.doNothing() + .when(statsRecorderWrapper) + .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); + runInterceptorTasksAndAssertCount(1); + List allErrorCounts = errorCountCaptor.getAllValues(); + assertThat(allErrorCounts).isEmpty(); + } + + @Test + public void noFailedRequests() throws Exception { + EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build()); + + for (int i = 0; i < 20; i++) { + try { + stub.readRowsCallable().call(Query.create(SUCCESS_TABLE_NAME)).iterator().hasNext(); + } catch (Exception e) { + // noop + } + } + ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); + Mockito.doNothing() + .when(statsRecorderWrapper) + .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); + runInterceptorTasksAndAssertCount(1); + List allErrorCounts = errorCountCaptor.getAllValues(); + assertThat(allErrorCounts.size()).isEqualTo(1); + assertThat(allErrorCounts.get(0)).isEqualTo(0); + } + + private void runInterceptorTasksAndAssertCount(int expectNumOfTasks) { + int actualNumOfTasks = 0; for (Runnable runnable : runnableCaptor.getAllValues()) { if (runnable instanceof CountErrorsPerInterceptorTask) { - System.out.println("REZA iterating over runnable Second time."); + ((CountErrorsPerInterceptorTask) runnable).setStatsRecorderWrapper(statsRecorderWrapper); runnable.run(); + actualNumOfTasks++; } } - for (long longVal : longCaptor.getAllValues()) { - System.out.println("REZA got long = " + longVal); - } + assertThat(actualNumOfTasks).isEqualTo(expectNumOfTasks); } static class FakeService extends BigtableGrpc.BigtableImplBase { - private boolean returnCookie = true; private final AtomicInteger count = new AtomicInteger(); @Override public void readRows( ReadRowsRequest request, StreamObserver responseObserver) { count.getAndIncrement(); - if (count.get() > 2) { - System.out.println("reza in server bad = " + count.get()); - Metadata trailers = new Metadata(); - // maybePopulateCookie(trailers, "readRows"); - responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); - StatusRuntimeException exception = new StatusRuntimeException(Status.INTERNAL, trailers); - responseObserver.onError(exception); - } else { - System.out.println("reza in server good = " + count.get()); - responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); - responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); + if (request.getTableName().contains(SUCCESS_TABLE_NAME)) { responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); responseObserver.onCompleted(); + } else { + // Send a non-retriable error, since otherwise the client tries to use the mocked + // ScheduledExecutorService + // object for retyring, resulting in a hang. + StatusRuntimeException exception = new StatusRuntimeException(Status.INTERNAL); + responseObserver.onError(exception); } } } From 9f6e964cade61cdb671dffe7fd83cac72c34c066 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Thu, 8 Feb 2024 11:56:16 -0500 Subject: [PATCH 10/23] Clean comments and add a TODO. --- .../cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java | 2 ++ .../bigtable/data/v2/stub/ErrorCountPerConnectionTest.java | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 0b7a5574a3..c37c61a655 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -212,6 +212,8 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); setupConnectionErrorCountTask(builder, connectionErrorCountInterceptors); + // TODO: This currently overrides the InterceptorProvider if one has been set by the user, since + // the attribute is private w/o a getter. Consider finding a way around it. if (transportProvider != null) { transportProvider.setInterceptorProvider( () -> getInterceptors(builder, connectionErrorCountInterceptors)); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java index 18a9f00745..283438f305 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java @@ -252,8 +252,7 @@ public void readRows( responseObserver.onCompleted(); } else { // Send a non-retriable error, since otherwise the client tries to use the mocked - // ScheduledExecutorService - // object for retyring, resulting in a hang. + // ScheduledExecutorService object for retyring, resulting in a hang. StatusRuntimeException exception = new StatusRuntimeException(Status.INTERNAL); responseObserver.onError(exception); } From 92fd9ea38d190acf915efceae0e025b514e3374d Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Thu, 8 Feb 2024 14:46:07 -0500 Subject: [PATCH 11/23] Improve tests and comments. --- .../stub/CountErrorsPerInterceptorTask.java | 7 ++++ .../v2/stub/ErrorCountPerConnectionTest.java | 33 ++++++++++--------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java index ff64518dc5..18944bae80 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java @@ -17,13 +17,18 @@ import com.google.cloud.bigtable.stats.StatsRecorderWrapper; import com.google.cloud.bigtable.stats.StatsWrapper; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import java.util.Set; +/** + * A background task that goes through all channels and updates the errors_per_connection metric. + */ class CountErrorsPerInterceptorTask implements Runnable { private final Set interceptors; private StatsRecorderWrapper statsRecorderWrapper; + @VisibleForTesting public void setStatsRecorderWrapper(StatsRecorderWrapper statsRecorderWrapper) { this.statsRecorderWrapper = statsRecorderWrapper; } @@ -32,6 +37,8 @@ public CountErrorsPerInterceptorTask( Set interceptors, ImmutableMap builtinAttributes) { this.interceptors = interceptors; + // We only interact with the putAndRecordPerConnectionErrorCount method, so OperationType and + // SpanName won't matter. this.statsRecorderWrapper = StatsWrapper.createRecorder(null, null, builtinAttributes); } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java index 283438f305..fd0d01b498 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java @@ -20,6 +20,8 @@ import static org.mockito.ArgumentMatchers.anyLong; import com.google.api.gax.core.FixedExecutorProvider; +import com.google.api.gax.grpc.ChannelPoolSettings; +import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.bigtable.v2.*; import com.google.cloud.bigtable.data.v2.BigtableDataSettings; import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; @@ -60,7 +62,6 @@ public void setup() throws Exception { .setProjectId("fake-project") .setInstanceId("fake-instance"); runnableCaptor = ArgumentCaptor.forClass(Runnable.class); - System.out.println("rez1 " + executors + " 2 = " + builder); Mockito.when( executors.scheduleAtFixedRate(runnableCaptor.capture(), anyLong(), anyLong(), any())) .thenReturn(null); @@ -106,23 +107,22 @@ public void singleRead() throws Exception { @Test public void readWithTwoStubs() throws Exception { - EnhancedBigtableStub stub1 = EnhancedBigtableStub.create(builder.build()); - EnhancedBigtableStub stub2 = EnhancedBigtableStub.create(builder.build()); - long errorCount1 = 0; - long errorCount2 = 0; + EnhancedBigtableStubSettings.Builder builderWithTwoChannels = + builder.setTransportChannelProvider( + ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()) + .toBuilder() + .setChannelPoolSettings(ChannelPoolSettings.staticallySized(2)) + .build()); + EnhancedBigtableStub stub = EnhancedBigtableStub.create(builderWithTwoChannels.build()); + long totalErrorCount = 0; for (int i = 0; i < 20; i++) { - Query successQuery = Query.create(SUCCESS_TABLE_NAME); - Query errorQuery = Query.create(ERROR_TABLE_NAME); try { - if (i % 3 == 0) { - errorCount2 += 1; - stub1.readRowsCallable().call(successQuery).iterator().hasNext(); - stub2.readRowsCallable().call(errorQuery).iterator().hasNext(); + if (i < 10) { + totalErrorCount += 1; + stub.readRowsCallable().call(Query.create(ERROR_TABLE_NAME)).iterator().hasNext(); } else { - errorCount1 += 1; - stub1.readRowsCallable().call(errorQuery).iterator().hasNext(); - stub2.readRowsCallable().call(successQuery).iterator().hasNext(); + stub.readRowsCallable().call(Query.create(SUCCESS_TABLE_NAME)).iterator().hasNext(); } } catch (Exception e) { // noop @@ -132,11 +132,12 @@ public void readWithTwoStubs() throws Exception { Mockito.doNothing() .when(statsRecorderWrapper) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); - runInterceptorTasksAndAssertCount(2); + runInterceptorTasksAndAssertCount(1); List allErrorCounts = errorCountCaptor.getAllValues(); assertThat(allErrorCounts.size()).isEqualTo(2); - assertThat(allErrorCounts).containsExactly(errorCount1, errorCount2); + // Requests get assigned to channels using a Round Robin algorithm, so half to each. + assertThat(allErrorCounts).containsExactly(totalErrorCount / 2, totalErrorCount / 2); } @Test From cb29640033ecc4aed1efd5c73928410691e5648f Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Tue, 13 Feb 2024 14:08:43 -0500 Subject: [PATCH 12/23] Address comments and refactor by defining new classes. --- .../bigtable/stats/StatsRecorderWrapper.java | 19 ----- .../StatsRecorderWrapperForConnection.java | 58 +++++++++++++++ .../cloud/bigtable/stats/StatsWrapper.java | 5 ++ .../stub/ConnectionErrorCountInterceptor.java | 22 +++--- .../v2/stub/ConnectionMetricsTracker.java | 64 +++++++++++++++++ .../stub/CountErrorsPerInterceptorTask.java | 25 ++++--- .../data/v2/stub/EnhancedBigtableStub.java | 71 +++++++------------ .../v2/stub/ErrorCountPerConnectionTest.java | 21 +++--- 8 files changed, 188 insertions(+), 97 deletions(-) create mode 100644 google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperForConnection.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionMetricsTracker.java diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java index 65ebe1e0fe..6bf0988b91 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java @@ -42,7 +42,6 @@ public class StatsRecorderWrapper { private MeasureMap attemptMeasureMap; private MeasureMap operationMeasureMap; - private MeasureMap perConnectionErrorCountMeasureMap; public StatsRecorderWrapper( OperationType operationType, @@ -58,7 +57,6 @@ public StatsRecorderWrapper( this.attemptMeasureMap = statsRecorder.newMeasureMap(); this.operationMeasureMap = statsRecorder.newMeasureMap(); - this.perConnectionErrorCountMeasureMap = statsRecorder.newMeasureMap(); } public void recordOperation(String status, String tableId, String zone, String cluster) { @@ -89,15 +87,6 @@ public void recordAttempt(String status, String tableId, String zone, String clu attemptMeasureMap = statsRecorder.newMeasureMap(); } - public void putAndRecordPerConnectionErrorCount(long errorCount) { - perConnectionErrorCountMeasureMap.put( - BuiltinMeasureConstants.PER_CONNECTION_ERROR_COUNT, errorCount); - - perConnectionErrorCountMeasureMap.record( - newTagContextBuilderForPerConnectionErrorCount().build()); - perConnectionErrorCountMeasureMap = statsRecorder.newMeasureMap(); - } - public void putOperationLatencies(long operationLatency) { operationMeasureMap.put(BuiltinMeasureConstants.OPERATION_LATENCIES, operationLatency); } @@ -143,12 +132,4 @@ private TagContextBuilder newTagContextBuilder(String tableId, String zone, Stri } return tagContextBuilder; } - - private TagContextBuilder newTagContextBuilderForPerConnectionErrorCount() { - TagContextBuilder tagContextBuilder = tagger.toBuilder(parentContext); - for (Map.Entry entry : statsAttributes.entrySet()) { - tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagValue.create(entry.getValue())); - } - return tagContextBuilder; - } } diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperForConnection.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperForConnection.java new file mode 100644 index 0000000000..eb0758c722 --- /dev/null +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperForConnection.java @@ -0,0 +1,58 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.stats; + +import com.google.api.core.InternalApi; +import io.opencensus.stats.MeasureMap; +import io.opencensus.stats.StatsRecorder; +import io.opencensus.tags.TagContext; +import io.opencensus.tags.TagContextBuilder; +import io.opencensus.tags.TagKey; +import io.opencensus.tags.TagValue; +import io.opencensus.tags.Tagger; +import io.opencensus.tags.Tags; +import java.util.Map; + +/** A wrapper to record built-in metrics for connection metrics not tied to operations/RPCs. */ +@InternalApi("For internal use only") +public class StatsRecorderWrapperForConnection { + private final StatsRecorder statsRecorder; + private final TagContext tagContext; + private MeasureMap perConnectionErrorCountMeasureMap; + + public StatsRecorderWrapperForConnection( + Map statsAttributes, StatsRecorder statsRecorder) { + this.statsRecorder = statsRecorder; + + this.perConnectionErrorCountMeasureMap = statsRecorder.newMeasureMap(); + + Tagger tagger = Tags.getTagger(); + TagContextBuilder tagContextBuilder = tagger.toBuilder(tagger.getCurrentTagContext()); + for (Map.Entry entry : statsAttributes.entrySet()) { + tagContextBuilder.putLocal( + TagKey.create(entry.getKey()), TagValue.create(entry.getValue())); + } + this.tagContext = tagContextBuilder.build(); + } + + public void putAndRecordPerConnectionErrorCount(long errorCount) { + perConnectionErrorCountMeasureMap.put( + BuiltinMeasureConstants.PER_CONNECTION_ERROR_COUNT, errorCount); + + perConnectionErrorCountMeasureMap.record(tagContext); + perConnectionErrorCountMeasureMap = statsRecorder.newMeasureMap(); + } +} diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java index 401a1cf975..0a97b21d67 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java @@ -40,6 +40,11 @@ public static StatsRecorderWrapper createRecorder( operationType, spanName, statsAttributes, Stats.getStatsRecorder()); } + public static StatsRecorderWrapperForConnection createRecorderForConnection( + Map statsAttributes) { + return new StatsRecorderWrapperForConnection(statsAttributes, Stats.getStatsRecorder()); + } + // This is used in integration tests to get the tag value strings from view manager because Stats // is relocated to com.google.bigtable.veneer.repackaged.io.opencensus. @InternalApi("Visible for testing") diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java index 8e73340de1..d3e4929df9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java @@ -24,16 +24,16 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Status; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.LongAdder; /** An interceptor which counts the number of failed responses for a channel. */ class ConnectionErrorCountInterceptor implements ClientInterceptor { - private final AtomicInteger numOfErrors; - private final AtomicInteger numOfSuccesses; + private final LongAdder numOfErrors; + private final LongAdder numOfSuccesses; public ConnectionErrorCountInterceptor() { - numOfErrors = new AtomicInteger(0); - numOfSuccesses = new AtomicInteger(0); + numOfErrors = new LongAdder(); + numOfSuccesses = new LongAdder(); } @Override @@ -49,9 +49,9 @@ public void start(Listener responseListener, Metadata headers) { @Override public void onClose(Status status, Metadata trailers) { if (status.isOk()) { - numOfSuccesses.getAndIncrement(); + numOfSuccesses.increment(); } else { - numOfErrors.getAndIncrement(); + numOfErrors.increment(); } super.onClose(status, trailers); } @@ -61,11 +61,11 @@ public void onClose(Status status, Metadata trailers) { }; } - public int getAndResetNumOfErrors() { - return numOfErrors.getAndSet(0); + public long getAndResetNumOfErrors() { + return numOfErrors.sumThenReset(); } - public int getAndResetNumOfSuccesses() { - return numOfSuccesses.getAndSet(0); + public long getAndResetNumOfSuccesses() { + return numOfSuccesses.sumThenReset(); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionMetricsTracker.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionMetricsTracker.java new file mode 100644 index 0000000000..e1f9ff209c --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionMetricsTracker.java @@ -0,0 +1,64 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import com.google.common.collect.ImmutableMap; +import io.grpc.ClientInterceptor; +import java.util.Collections; +import java.util.Set; +import java.util.WeakHashMap; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +/* Sets up and starts the monitoring for per-connection metrics. */ +class ConnectionMetricsTracker { + private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; + private final EnhancedBigtableStubSettings.Builder builder; + private final ImmutableMap builtinAttributes; + private final Set connectionErrorCountInterceptors; + + private final Object interceptorsLock = new Object(); + + ConnectionMetricsTracker( + EnhancedBigtableStubSettings.Builder builder, + ImmutableMap builtinAttributes) { + this.builder = builder; + this.builtinAttributes = builtinAttributes; + connectionErrorCountInterceptors = + Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); + + startConnectionErrorCountTracker(); + } + + private void startConnectionErrorCountTracker() { + ScheduledExecutorService scheduler = builder.getBackgroundExecutorProvider().getExecutor(); + scheduler.scheduleAtFixedRate( + new CountErrorsPerInterceptorTask( + connectionErrorCountInterceptors, interceptorsLock, builtinAttributes), + 0, + PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, + TimeUnit.SECONDS); + } + + ClientInterceptor getInterceptor() { + ConnectionErrorCountInterceptor connectionErrorCountInterceptor = + new ConnectionErrorCountInterceptor(); + synchronized (interceptorsLock) { + connectionErrorCountInterceptors.add(connectionErrorCountInterceptor); + } + return connectionErrorCountInterceptor; + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java index 18944bae80..c9c7b5735b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java @@ -15,7 +15,7 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import com.google.cloud.bigtable.stats.StatsRecorderWrapper; +import com.google.cloud.bigtable.stats.StatsRecorderWrapperForConnection; import com.google.cloud.bigtable.stats.StatsWrapper; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; @@ -26,34 +26,39 @@ */ class CountErrorsPerInterceptorTask implements Runnable { private final Set interceptors; - private StatsRecorderWrapper statsRecorderWrapper; + private final Object interceptorsLock; + // This is not final so that it can be updated and mocked during testing. + private StatsRecorderWrapperForConnection statsRecorderWrapperForConnection; @VisibleForTesting - public void setStatsRecorderWrapper(StatsRecorderWrapper statsRecorderWrapper) { - this.statsRecorderWrapper = statsRecorderWrapper; + void setStatsRecorderWrapperForConnection( + StatsRecorderWrapperForConnection statsRecorderWrapperForConnection) { + this.statsRecorderWrapperForConnection = statsRecorderWrapperForConnection; } - public CountErrorsPerInterceptorTask( + CountErrorsPerInterceptorTask( Set interceptors, + Object interceptorsLock, ImmutableMap builtinAttributes) { this.interceptors = interceptors; + this.interceptorsLock = interceptorsLock; // We only interact with the putAndRecordPerConnectionErrorCount method, so OperationType and // SpanName won't matter. - this.statsRecorderWrapper = StatsWrapper.createRecorder(null, null, builtinAttributes); + this.statsRecorderWrapperForConnection = StatsWrapper.createRecorderForConnection(builtinAttributes); } @Override public void run() { - synchronized (interceptors) { + synchronized (interceptorsLock) { for (ConnectionErrorCountInterceptor interceptor : interceptors) { - int errors = interceptor.getAndResetNumOfErrors(); - int successes = interceptor.getAndResetNumOfSuccesses(); + long errors = interceptor.getAndResetNumOfErrors(); + long successes = interceptor.getAndResetNumOfSuccesses(); // We avoid keeping track of inactive connections (i.e., without any failed or successful // requests). if (errors > 0 || successes > 0) { // TODO: add a metric to also keep track of the number of successful requests per each // connection. - this.statsRecorderWrapper.putAndRecordPerConnectionErrorCount(errors); + this.statsRecorderWrapperForConnection.putAndRecordPerConnectionErrorCount(errors); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index c37c61a655..9a0b80eeda 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -15,6 +15,7 @@ */ package com.google.cloud.bigtable.data.v2.stub; +import com.google.api.core.ApiFunction; import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.api.gax.batching.Batcher; @@ -117,7 +118,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.protobuf.ByteString; -import io.grpc.ClientInterceptor; +import io.grpc.ManagedChannelBuilder; import io.opencensus.stats.Stats; import io.opencensus.stats.StatsRecorder; import io.opencensus.tags.TagKey; @@ -130,9 +131,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; -import java.util.WeakHashMap; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -153,8 +151,6 @@ public class EnhancedBigtableStub implements AutoCloseable { private static final String CLIENT_NAME = "Bigtable"; private static final long FLOW_CONTROL_ADJUSTING_INTERVAL_MS = TimeUnit.SECONDS.toMillis(20); - private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; - private final EnhancedBigtableStubSettings settings; private final ClientContext clientContext; @@ -208,15 +204,25 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set ? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder() : null; - Set connectionErrorCountInterceptors = - Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); - setupConnectionErrorCountTask(builder, connectionErrorCountInterceptors); - - // TODO: This currently overrides the InterceptorProvider if one has been set by the user, since - // the attribute is private w/o a getter. Consider finding a way around it. + ConnectionMetricsTracker connectionMetricsTracker = + new ConnectionMetricsTracker(builder, createBuiltinAttributes(builder)); if (transportProvider != null) { - transportProvider.setInterceptorProvider( - () -> getInterceptors(builder, connectionErrorCountInterceptors)); + ApiFunction oldChannelConfigurator = + transportProvider.getChannelConfigurator(); + transportProvider.setChannelConfigurator( + managedChannelBuilder -> { + if (settings.getEnableRoutingCookie()) { + // TODO: this also need to be added to BigtableClientFactory + managedChannelBuilder.intercept(new CookiesInterceptor()); + } + + managedChannelBuilder.intercept(connectionMetricsTracker.getInterceptor()); + + if (oldChannelConfigurator != null) { + managedChannelBuilder = oldChannelConfigurator.apply(managedChannelBuilder); + } + return managedChannelBuilder; + }); } // Inject channel priming @@ -245,35 +251,6 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set return ClientContext.create(builder.build()); } - private static ImmutableList getInterceptors( - EnhancedBigtableStubSettings.Builder settings, - Set connectionErrorCountInterceptors) { - ConnectionErrorCountInterceptor connectionErrorCountInterceptor = - new ConnectionErrorCountInterceptor(); - connectionErrorCountInterceptors.add(connectionErrorCountInterceptor); - ImmutableList.Builder builder = - ImmutableList.builder().add(connectionErrorCountInterceptor); - - if (settings.getEnableRoutingCookie()) { - // TODO: this also need to be added to BigtableClientFactory - builder.add(new CookiesInterceptor()); - } - - return builder.build(); - } - - private static void setupConnectionErrorCountTask( - EnhancedBigtableStubSettings.Builder settings, - Set interceptors) { - ScheduledExecutorService scheduler = settings.getBackgroundExecutorProvider().getExecutor(); - ImmutableMap builtinAttributes = createBuiltinAttributes(settings); - scheduler.scheduleAtFixedRate( - new CountErrorsPerInterceptorTask(interceptors, builtinAttributes), - 0, - PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, - TimeUnit.SECONDS); - } - public static ApiTracerFactory createBigtableTracerFactory( EnhancedBigtableStubSettings settings) { return createBigtableTracerFactory(settings, Tags.getTagger(), Stats.getStatsRecorder()); @@ -316,11 +293,11 @@ public static ApiTracerFactory createBigtableTracerFactory( } private static ImmutableMap createBuiltinAttributes( - EnhancedBigtableStubSettings.Builder settings) { + EnhancedBigtableStubSettings.Builder builder) { return ImmutableMap.builder() - .put("project_id", settings.getProjectId()) - .put("instance", settings.getInstanceId()) - .put("app_profile", settings.getAppProfileId()) + .put("project_id", builder.getProjectId()) + .put("instance", builder.getInstanceId()) + .put("app_profile", builder.getAppProfileId()) .put("client_name", "bigtable-java/" + Version.VERSION) .build(); } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java index fd0d01b498..c7c0b7e764 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java @@ -26,7 +26,7 @@ import com.google.cloud.bigtable.data.v2.BigtableDataSettings; import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; import com.google.cloud.bigtable.data.v2.models.*; -import com.google.cloud.bigtable.stats.StatsRecorderWrapper; +import com.google.cloud.bigtable.stats.StatsRecorderWrapperForConnection; import io.grpc.*; import io.grpc.stub.StreamObserver; import java.util.List; @@ -48,7 +48,7 @@ public class ErrorCountPerConnectionTest { private final FakeService fakeService = new FakeService(); private EnhancedBigtableStubSettings.Builder builder; private ArgumentCaptor runnableCaptor; - private StatsRecorderWrapper statsRecorderWrapper; + private StatsRecorderWrapperForConnection statsRecorderWrapperForConnection; @Before public void setup() throws Exception { @@ -66,7 +66,7 @@ public void setup() throws Exception { executors.scheduleAtFixedRate(runnableCaptor.capture(), anyLong(), anyLong(), any())) .thenReturn(null); - statsRecorderWrapper = Mockito.mock(StatsRecorderWrapper.class); + statsRecorderWrapperForConnection = Mockito.mock(StatsRecorderWrapperForConnection.class); } @After @@ -97,7 +97,7 @@ public void singleRead() throws Exception { } ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); Mockito.doNothing() - .when(statsRecorderWrapper) + .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); runInterceptorTasksAndAssertCount(1); List allErrorCounts = errorCountCaptor.getAllValues(); @@ -130,7 +130,7 @@ public void readWithTwoStubs() throws Exception { } ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); Mockito.doNothing() - .when(statsRecorderWrapper) + .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); runInterceptorTasksAndAssertCount(1); @@ -161,7 +161,7 @@ public void readOverTwoPeriods() throws Exception { } ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); Mockito.doNothing() - .when(statsRecorderWrapper) + .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); runInterceptorTasksAndAssertCount(1); List allErrorCounts = errorCountCaptor.getAllValues(); @@ -186,7 +186,7 @@ public void readOverTwoPeriods() throws Exception { } errorCountCaptor = ArgumentCaptor.forClass(long.class); Mockito.doNothing() - .when(statsRecorderWrapper) + .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); runInterceptorTasksAndAssertCount(1); allErrorCounts = errorCountCaptor.getAllValues(); @@ -200,7 +200,7 @@ public void ignoreInactiveConnection() throws Exception { ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); Mockito.doNothing() - .when(statsRecorderWrapper) + .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); runInterceptorTasksAndAssertCount(1); List allErrorCounts = errorCountCaptor.getAllValues(); @@ -220,7 +220,7 @@ public void noFailedRequests() throws Exception { } ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); Mockito.doNothing() - .when(statsRecorderWrapper) + .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); runInterceptorTasksAndAssertCount(1); List allErrorCounts = errorCountCaptor.getAllValues(); @@ -232,7 +232,8 @@ private void runInterceptorTasksAndAssertCount(int expectNumOfTasks) { int actualNumOfTasks = 0; for (Runnable runnable : runnableCaptor.getAllValues()) { if (runnable instanceof CountErrorsPerInterceptorTask) { - ((CountErrorsPerInterceptorTask) runnable).setStatsRecorderWrapper(statsRecorderWrapper); + ((CountErrorsPerInterceptorTask) runnable) + .setStatsRecorderWrapperForConnection(statsRecorderWrapperForConnection); runnable.run(); actualNumOfTasks++; } From d827cf3bf390f413ad7a26529d95d4ae4b27f91b Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Tue, 13 Feb 2024 15:44:34 -0500 Subject: [PATCH 13/23] Fix code formatting. --- .../bigtable/stats/StatsRecorderWrapperForConnection.java | 3 +-- .../bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperForConnection.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperForConnection.java index eb0758c722..3c335d28bc 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperForConnection.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperForConnection.java @@ -42,8 +42,7 @@ public StatsRecorderWrapperForConnection( Tagger tagger = Tags.getTagger(); TagContextBuilder tagContextBuilder = tagger.toBuilder(tagger.getCurrentTagContext()); for (Map.Entry entry : statsAttributes.entrySet()) { - tagContextBuilder.putLocal( - TagKey.create(entry.getKey()), TagValue.create(entry.getValue())); + tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagValue.create(entry.getValue())); } this.tagContext = tagContextBuilder.build(); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java index c9c7b5735b..d39711568b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java @@ -44,7 +44,8 @@ void setStatsRecorderWrapperForConnection( this.interceptorsLock = interceptorsLock; // We only interact with the putAndRecordPerConnectionErrorCount method, so OperationType and // SpanName won't matter. - this.statsRecorderWrapperForConnection = StatsWrapper.createRecorderForConnection(builtinAttributes); + this.statsRecorderWrapperForConnection = + StatsWrapper.createRecorderForConnection(builtinAttributes); } @Override From b5389f0ee48f787251ac98c828f8fae0a56a7e41 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Thu, 15 Feb 2024 18:00:34 -0500 Subject: [PATCH 14/23] Refactor classes and move to better packages. --- .../v2/stub/ConnectionMetricsTracker.java | 64 ------------------- .../data/v2/stub/EnhancedBigtableStub.java | 8 +-- .../ConnectionErrorCountInterceptor.java | 8 +-- ...ErrorCountPerConnectionMetricTracker.java} | 53 ++++++++++----- .../ErrorCountPerConnectionTest.java | 30 +++++---- 5 files changed, 61 insertions(+), 102 deletions(-) delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionMetricsTracker.java rename google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/{ => metrics}/ConnectionErrorCountInterceptor.java (92%) rename google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/{CountErrorsPerInterceptorTask.java => metrics/ErrorCountPerConnectionMetricTracker.java} (50%) rename google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/{ => metrics}/ErrorCountPerConnectionTest.java (91%) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionMetricsTracker.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionMetricsTracker.java deleted file mode 100644 index e1f9ff209c..0000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionMetricsTracker.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright 2024 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.cloud.bigtable.data.v2.stub; - -import com.google.common.collect.ImmutableMap; -import io.grpc.ClientInterceptor; -import java.util.Collections; -import java.util.Set; -import java.util.WeakHashMap; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; - -/* Sets up and starts the monitoring for per-connection metrics. */ -class ConnectionMetricsTracker { - private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; - private final EnhancedBigtableStubSettings.Builder builder; - private final ImmutableMap builtinAttributes; - private final Set connectionErrorCountInterceptors; - - private final Object interceptorsLock = new Object(); - - ConnectionMetricsTracker( - EnhancedBigtableStubSettings.Builder builder, - ImmutableMap builtinAttributes) { - this.builder = builder; - this.builtinAttributes = builtinAttributes; - connectionErrorCountInterceptors = - Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); - - startConnectionErrorCountTracker(); - } - - private void startConnectionErrorCountTracker() { - ScheduledExecutorService scheduler = builder.getBackgroundExecutorProvider().getExecutor(); - scheduler.scheduleAtFixedRate( - new CountErrorsPerInterceptorTask( - connectionErrorCountInterceptors, interceptorsLock, builtinAttributes), - 0, - PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, - TimeUnit.SECONDS); - } - - ClientInterceptor getInterceptor() { - ConnectionErrorCountInterceptor connectionErrorCountInterceptor = - new ConnectionErrorCountInterceptor(); - synchronized (interceptorsLock) { - connectionErrorCountInterceptors.add(connectionErrorCountInterceptor); - } - return connectionErrorCountInterceptor; - } -} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 9a0b80eeda..d40c16361e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -95,6 +95,7 @@ import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTracerFactory; import com.google.cloud.bigtable.data.v2.stub.metrics.CompositeTracerFactory; +import com.google.cloud.bigtable.data.v2.stub.metrics.ErrorCountPerConnectionMetricTracker; import com.google.cloud.bigtable.data.v2.stub.metrics.MetricsTracerFactory; import com.google.cloud.bigtable.data.v2.stub.metrics.RpcMeasureConstants; import com.google.cloud.bigtable.data.v2.stub.metrics.StatsHeadersServerStreamingCallable; @@ -204,19 +205,18 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set ? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder() : null; - ConnectionMetricsTracker connectionMetricsTracker = - new ConnectionMetricsTracker(builder, createBuiltinAttributes(builder)); + ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker = + new ErrorCountPerConnectionMetricTracker(builder, createBuiltinAttributes(builder)); if (transportProvider != null) { ApiFunction oldChannelConfigurator = transportProvider.getChannelConfigurator(); transportProvider.setChannelConfigurator( managedChannelBuilder -> { if (settings.getEnableRoutingCookie()) { - // TODO: this also need to be added to BigtableClientFactory managedChannelBuilder.intercept(new CookiesInterceptor()); } - managedChannelBuilder.intercept(connectionMetricsTracker.getInterceptor()); + managedChannelBuilder.intercept(errorCountPerConnectionMetricTracker.getInterceptor()); if (oldChannelConfigurator != null) { managedChannelBuilder = oldChannelConfigurator.apply(managedChannelBuilder); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java similarity index 92% rename from google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java rename to google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java index d3e4929df9..c209bdc0c8 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.google.cloud.bigtable.data.v2.stub; +package com.google.cloud.bigtable.data.v2.stub.metrics; import io.grpc.CallOptions; import io.grpc.Channel; @@ -31,7 +31,7 @@ class ConnectionErrorCountInterceptor implements ClientInterceptor { private final LongAdder numOfErrors; private final LongAdder numOfSuccesses; - public ConnectionErrorCountInterceptor() { + ConnectionErrorCountInterceptor() { numOfErrors = new LongAdder(); numOfSuccesses = new LongAdder(); } @@ -61,11 +61,11 @@ public void onClose(Status status, Metadata trailers) { }; } - public long getAndResetNumOfErrors() { + long getAndResetNumOfErrors() { return numOfErrors.sumThenReset(); } - public long getAndResetNumOfSuccesses() { + long getAndResetNumOfSuccesses() { return numOfSuccesses.sumThenReset(); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java similarity index 50% rename from google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java rename to google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java index d39711568b..87be9830e5 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CountErrorsPerInterceptorTask.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java @@ -13,20 +13,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.google.cloud.bigtable.data.v2.stub; +package com.google.cloud.bigtable.data.v2.stub.metrics; +import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; import com.google.cloud.bigtable.stats.StatsRecorderWrapperForConnection; import com.google.cloud.bigtable.stats.StatsWrapper; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; +import io.grpc.ClientInterceptor; +import java.util.Collections; import java.util.Set; +import java.util.WeakHashMap; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; -/** - * A background task that goes through all channels and updates the errors_per_connection metric. - */ -class CountErrorsPerInterceptorTask implements Runnable { - private final Set interceptors; - private final Object interceptorsLock; +/* Background task that goes through all connections and updates the errors_per_connection metric. */ +public class ErrorCountPerConnectionMetricTracker implements Runnable { + private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; + private final EnhancedBigtableStubSettings.Builder builder; + private final Set connectionErrorCountInterceptors; + private final Object interceptorsLock = new Object(); // This is not final so that it can be updated and mocked during testing. private StatsRecorderWrapperForConnection statsRecorderWrapperForConnection; @@ -36,22 +42,37 @@ void setStatsRecorderWrapperForConnection( this.statsRecorderWrapperForConnection = statsRecorderWrapperForConnection; } - CountErrorsPerInterceptorTask( - Set interceptors, - Object interceptorsLock, + public ErrorCountPerConnectionMetricTracker( + EnhancedBigtableStubSettings.Builder builder, ImmutableMap builtinAttributes) { - this.interceptors = interceptors; - this.interceptorsLock = interceptorsLock; - // We only interact with the putAndRecordPerConnectionErrorCount method, so OperationType and - // SpanName won't matter. + this.builder = builder; + connectionErrorCountInterceptors = + Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); + this.statsRecorderWrapperForConnection = StatsWrapper.createRecorderForConnection(builtinAttributes); + startConnectionErrorCountTracker(); + } + + private void startConnectionErrorCountTracker() { + ScheduledExecutorService scheduler = builder.getBackgroundExecutorProvider().getExecutor(); + scheduler.scheduleAtFixedRate( + this, 0, PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, TimeUnit.SECONDS); + } + + public ClientInterceptor getInterceptor() { + ConnectionErrorCountInterceptor connectionErrorCountInterceptor = + new ConnectionErrorCountInterceptor(); + synchronized (interceptorsLock) { + connectionErrorCountInterceptors.add(connectionErrorCountInterceptor); + } + return connectionErrorCountInterceptor; } @Override public void run() { synchronized (interceptorsLock) { - for (ConnectionErrorCountInterceptor interceptor : interceptors) { + for (ConnectionErrorCountInterceptor interceptor : connectionErrorCountInterceptors) { long errors = interceptor.getAndResetNumOfErrors(); long successes = interceptor.getAndResetNumOfSuccesses(); // We avoid keeping track of inactive connections (i.e., without any failed or successful @@ -59,7 +80,7 @@ public void run() { if (errors > 0 || successes > 0) { // TODO: add a metric to also keep track of the number of successful requests per each // connection. - this.statsRecorderWrapperForConnection.putAndRecordPerConnectionErrorCount(errors); + statsRecorderWrapperForConnection.putAndRecordPerConnectionErrorCount(errors); } } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java similarity index 91% rename from google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java rename to google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java index c7c0b7e764..fbcfc3617e 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.google.cloud.bigtable.data.v2.stub; +package com.google.cloud.bigtable.data.v2.stub.metrics; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -26,6 +26,8 @@ import com.google.cloud.bigtable.data.v2.BigtableDataSettings; import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; import com.google.cloud.bigtable.data.v2.models.*; +import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; +import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; import com.google.cloud.bigtable.stats.StatsRecorderWrapperForConnection; import io.grpc.*; import io.grpc.stub.StreamObserver; @@ -77,7 +79,7 @@ public void tearDown() throws Exception { } @Test - public void singleRead() throws Exception { + public void readWithOneChannel() throws Exception { EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build()); long errorCount = 0; @@ -99,14 +101,14 @@ public void singleRead() throws Exception { Mockito.doNothing() .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); - runInterceptorTasksAndAssertCount(1); + runInterceptorTasksAndAssertCount(); List allErrorCounts = errorCountCaptor.getAllValues(); assertThat(allErrorCounts.size()).isEqualTo(1); assertThat(allErrorCounts.get(0)).isEqualTo(errorCount); } @Test - public void readWithTwoStubs() throws Exception { + public void readWithTwoChannels() throws Exception { EnhancedBigtableStubSettings.Builder builderWithTwoChannels = builder.setTransportChannelProvider( ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()) @@ -132,7 +134,7 @@ public void readWithTwoStubs() throws Exception { Mockito.doNothing() .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); - runInterceptorTasksAndAssertCount(1); + runInterceptorTasksAndAssertCount(); List allErrorCounts = errorCountCaptor.getAllValues(); assertThat(allErrorCounts.size()).isEqualTo(2); @@ -163,7 +165,7 @@ public void readOverTwoPeriods() throws Exception { Mockito.doNothing() .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); - runInterceptorTasksAndAssertCount(1); + runInterceptorTasksAndAssertCount(); List allErrorCounts = errorCountCaptor.getAllValues(); assertThat(allErrorCounts.size()).isEqualTo(1); assertThat(allErrorCounts.get(0)).isEqualTo(errorCount); @@ -188,7 +190,7 @@ public void readOverTwoPeriods() throws Exception { Mockito.doNothing() .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); - runInterceptorTasksAndAssertCount(1); + runInterceptorTasksAndAssertCount(); allErrorCounts = errorCountCaptor.getAllValues(); assertThat(allErrorCounts.size()).isEqualTo(1); assertThat(allErrorCounts.get(0)).isEqualTo(errorCount); @@ -196,13 +198,13 @@ public void readOverTwoPeriods() throws Exception { @Test public void ignoreInactiveConnection() throws Exception { - EnhancedBigtableStub.create(builder.build()); + EnhancedBigtableStub stub = EnhancedBigtableStub.create(builder.build()); ArgumentCaptor errorCountCaptor = ArgumentCaptor.forClass(long.class); Mockito.doNothing() .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); - runInterceptorTasksAndAssertCount(1); + runInterceptorTasksAndAssertCount(); List allErrorCounts = errorCountCaptor.getAllValues(); assertThat(allErrorCounts).isEmpty(); } @@ -222,23 +224,23 @@ public void noFailedRequests() throws Exception { Mockito.doNothing() .when(statsRecorderWrapperForConnection) .putAndRecordPerConnectionErrorCount(errorCountCaptor.capture()); - runInterceptorTasksAndAssertCount(1); + runInterceptorTasksAndAssertCount(); List allErrorCounts = errorCountCaptor.getAllValues(); assertThat(allErrorCounts.size()).isEqualTo(1); assertThat(allErrorCounts.get(0)).isEqualTo(0); } - private void runInterceptorTasksAndAssertCount(int expectNumOfTasks) { + private void runInterceptorTasksAndAssertCount() { int actualNumOfTasks = 0; for (Runnable runnable : runnableCaptor.getAllValues()) { - if (runnable instanceof CountErrorsPerInterceptorTask) { - ((CountErrorsPerInterceptorTask) runnable) + if (runnable instanceof ErrorCountPerConnectionMetricTracker) { + ((ErrorCountPerConnectionMetricTracker) runnable) .setStatsRecorderWrapperForConnection(statsRecorderWrapperForConnection); runnable.run(); actualNumOfTasks++; } } - assertThat(actualNumOfTasks).isEqualTo(expectNumOfTasks); + assertThat(actualNumOfTasks).isEqualTo(1); } static class FakeService extends BigtableGrpc.BigtableImplBase { From 9248b148a323eb0a8276d1c81efbfeaf1de4b650 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Fri, 16 Feb 2024 10:44:25 -0500 Subject: [PATCH 15/23] Clean up classes and address comments. --- .../bigtable/data/v2/stub/EnhancedBigtableStub.java | 4 +++- .../metrics/ErrorCountPerConnectionMetricTracker.java | 11 +++++------ .../v2/stub/metrics/ErrorCountPerConnectionTest.java | 4 +++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index d40c16361e..1c4e7e1976 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -206,7 +206,9 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set : null; ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker = - new ErrorCountPerConnectionMetricTracker(builder, createBuiltinAttributes(builder)); + new ErrorCountPerConnectionMetricTracker( + builder.getBackgroundExecutorProvider().getExecutor(), + createBuiltinAttributes(builder)); if (transportProvider != null) { ApiFunction oldChannelConfigurator = transportProvider.getChannelConfigurator(); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java index 87be9830e5..18b674b51a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java @@ -15,7 +15,7 @@ */ package com.google.cloud.bigtable.data.v2.stub.metrics; -import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; +import com.google.api.core.InternalApi; import com.google.cloud.bigtable.stats.StatsRecorderWrapperForConnection; import com.google.cloud.bigtable.stats.StatsWrapper; import com.google.common.annotations.VisibleForTesting; @@ -28,9 +28,10 @@ import java.util.concurrent.TimeUnit; /* Background task that goes through all connections and updates the errors_per_connection metric. */ +@InternalApi("For internal use only") public class ErrorCountPerConnectionMetricTracker implements Runnable { private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; - private final EnhancedBigtableStubSettings.Builder builder; + private final ScheduledExecutorService scheduler; private final Set connectionErrorCountInterceptors; private final Object interceptorsLock = new Object(); // This is not final so that it can be updated and mocked during testing. @@ -43,9 +44,8 @@ void setStatsRecorderWrapperForConnection( } public ErrorCountPerConnectionMetricTracker( - EnhancedBigtableStubSettings.Builder builder, - ImmutableMap builtinAttributes) { - this.builder = builder; + ScheduledExecutorService scheduler, ImmutableMap builtinAttributes) { + this.scheduler = scheduler; connectionErrorCountInterceptors = Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); @@ -55,7 +55,6 @@ public ErrorCountPerConnectionMetricTracker( } private void startConnectionErrorCountTracker() { - ScheduledExecutorService scheduler = builder.getBackgroundExecutorProvider().getExecutor(); scheduler.scheduleAtFixedRate( this, 0, PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, TimeUnit.SECONDS); } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java index fbcfc3617e..79960c8225 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java @@ -29,7 +29,9 @@ import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; import com.google.cloud.bigtable.stats.StatsRecorderWrapperForConnection; -import io.grpc.*; +import io.grpc.Server; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import java.util.List; import java.util.concurrent.ScheduledExecutorService; From b5fcd12b1eb8cdfa4861482bdc578bb08245c277 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Fri, 16 Feb 2024 11:00:45 -0500 Subject: [PATCH 16/23] Update the scheduler object. --- .../bigtable/data/v2/stub/EnhancedBigtableStub.java | 9 +++++---- .../metrics/ErrorCountPerConnectionMetricTracker.java | 8 ++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 1c4e7e1976..54da33b658 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -206,9 +206,7 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set : null; ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker = - new ErrorCountPerConnectionMetricTracker( - builder.getBackgroundExecutorProvider().getExecutor(), - createBuiltinAttributes(builder)); + new ErrorCountPerConnectionMetricTracker(createBuiltinAttributes(builder)); if (transportProvider != null) { ApiFunction oldChannelConfigurator = transportProvider.getChannelConfigurator(); @@ -250,7 +248,10 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set builder.setTransportChannelProvider(transportProvider.build()); } - return ClientContext.create(builder.build()); + ClientContext clientContext = ClientContext.create(builder.build()); + errorCountPerConnectionMetricTracker.startConnectionErrorCountTracker( + clientContext.getExecutor()); + return clientContext; } public static ApiTracerFactory createBigtableTracerFactory( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java index 18b674b51a..cab3b0bbd0 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionMetricTracker.java @@ -31,7 +31,6 @@ @InternalApi("For internal use only") public class ErrorCountPerConnectionMetricTracker implements Runnable { private static final Integer PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS = 60; - private final ScheduledExecutorService scheduler; private final Set connectionErrorCountInterceptors; private final Object interceptorsLock = new Object(); // This is not final so that it can be updated and mocked during testing. @@ -43,18 +42,15 @@ void setStatsRecorderWrapperForConnection( this.statsRecorderWrapperForConnection = statsRecorderWrapperForConnection; } - public ErrorCountPerConnectionMetricTracker( - ScheduledExecutorService scheduler, ImmutableMap builtinAttributes) { - this.scheduler = scheduler; + public ErrorCountPerConnectionMetricTracker(ImmutableMap builtinAttributes) { connectionErrorCountInterceptors = Collections.synchronizedSet(Collections.newSetFromMap(new WeakHashMap<>())); this.statsRecorderWrapperForConnection = StatsWrapper.createRecorderForConnection(builtinAttributes); - startConnectionErrorCountTracker(); } - private void startConnectionErrorCountTracker() { + public void startConnectionErrorCountTracker(ScheduledExecutorService scheduler) { scheduler.scheduleAtFixedRate( this, 0, PER_CONNECTION_ERROR_COUNT_PERIOD_SECONDS, TimeUnit.SECONDS); } From 92e6fdf66e6325cddd2c881eed5aecaa90c4515a Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Fri, 16 Feb 2024 11:28:22 -0500 Subject: [PATCH 17/23] Apply cleanups. --- .../bigtable/data/v2/stub/EnhancedBigtableStub.java | 12 ++++++++---- .../v2/stub/metrics/ErrorCountPerConnectionTest.java | 4 ---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 54da33b658..577a3dc114 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -205,9 +205,9 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set ? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder() : null; - ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker = - new ErrorCountPerConnectionMetricTracker(createBuiltinAttributes(builder)); + ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker; if (transportProvider != null) { + errorCountPerConnectionMetricTracker = new ErrorCountPerConnectionMetricTracker(createBuiltinAttributes(builder)); ApiFunction oldChannelConfigurator = transportProvider.getChannelConfigurator(); transportProvider.setChannelConfigurator( @@ -223,6 +223,8 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set } return managedChannelBuilder; }); + } else { + errorCountPerConnectionMetricTracker = null; } // Inject channel priming @@ -249,8 +251,10 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set } ClientContext clientContext = ClientContext.create(builder.build()); - errorCountPerConnectionMetricTracker.startConnectionErrorCountTracker( - clientContext.getExecutor()); + if (errorCountPerConnectionMetricTracker != null) { + errorCountPerConnectionMetricTracker.startConnectionErrorCountTracker( + clientContext.getExecutor()); + } return clientContext; } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java index 79960c8225..67c682184c 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java @@ -246,13 +246,9 @@ private void runInterceptorTasksAndAssertCount() { } static class FakeService extends BigtableGrpc.BigtableImplBase { - - private final AtomicInteger count = new AtomicInteger(); - @Override public void readRows( ReadRowsRequest request, StreamObserver responseObserver) { - count.getAndIncrement(); if (request.getTableName().contains(SUCCESS_TABLE_NAME)) { responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); responseObserver.onCompleted(); From 96730e78fca7f39c54e2a5dc3399add79c11c6b8 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Fri, 16 Feb 2024 12:16:49 -0500 Subject: [PATCH 18/23] Fix unit tests and avoid hanging when getting error in close(). --- .../metrics/ConnectionErrorCountInterceptor.java | 14 +++++++++----- .../data/v2/BigtableDataClientFactoryTest.java | 10 ++++------ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java index c209bdc0c8..253ae4c215 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java @@ -48,12 +48,16 @@ public void start(Listener responseListener, Metadata headers) { responseListener) { @Override public void onClose(Status status, Metadata trailers) { - if (status.isOk()) { - numOfSuccesses.increment(); - } else { - numOfErrors.increment(); + try { + if (status.isOk()) { + numOfSuccesses.increment(); + } else { + numOfErrors.increment(); + } + } + finally { + super.onClose(status, trailers); } - super.onClose(status, trailers); } }, headers); 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 2aec1a0c1c..a35112b380 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 @@ -177,11 +177,10 @@ public void testNewClientsShareTransportChannel() throws Exception { BigtableDataClient ignored2 = factory.createForInstance("project2", "instance2"); BigtableDataClient ignored3 = factory.createForInstance("project3", "instance3")) { - // Make sure that the right number of instances is created by each provider + // Make sure that only 1 instance is created by each provider Mockito.verify(transportChannelProvider, Mockito.times(1)).getTransportChannel(); Mockito.verify(credentialsProvider, Mockito.times(1)).getCredentials(); - // getExecutor() is called for setting Watchdog and per_connection_error_count metric. - Mockito.verify(executorProvider, Mockito.times(2)).getExecutor(); + Mockito.verify(executorProvider, Mockito.times(1)).getExecutor(); Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog(); } } @@ -267,10 +266,9 @@ public void testCreateWithRefreshingChannel() throws Exception { factory.createForAppProfile("other-appprofile"); factory.createForInstance("other-project", "other-instance"); - // Make sure that the right number of instances is created for all clients + // Make sure that only 1 instance is created by each provider Mockito.verify(credentialsProvider, Mockito.times(1)).getCredentials(); - // getExecutor() is called for setting Watchdog and per_connection_error_count metric. - Mockito.verify(executorProvider, Mockito.times(2)).getExecutor(); + Mockito.verify(executorProvider, Mockito.times(1)).getExecutor(); Mockito.verify(watchdogProvider, Mockito.times(1)).getWatchdog(); // Make sure that the clients are sharing the same ChannelPool From fc67864d61c1c2392de772f2db46d7f3ced559b9 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Fri, 16 Feb 2024 12:19:29 -0500 Subject: [PATCH 19/23] Fix code formatting. --- .../cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java | 5 +++-- .../v2/stub/metrics/ConnectionErrorCountInterceptor.java | 3 +-- .../data/v2/stub/metrics/ErrorCountPerConnectionTest.java | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 577a3dc114..ef37dd4e48 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -207,7 +207,8 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set ErrorCountPerConnectionMetricTracker errorCountPerConnectionMetricTracker; if (transportProvider != null) { - errorCountPerConnectionMetricTracker = new ErrorCountPerConnectionMetricTracker(createBuiltinAttributes(builder)); + errorCountPerConnectionMetricTracker = + new ErrorCountPerConnectionMetricTracker(createBuiltinAttributes(builder)); ApiFunction oldChannelConfigurator = transportProvider.getChannelConfigurator(); transportProvider.setChannelConfigurator( @@ -253,7 +254,7 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set ClientContext clientContext = ClientContext.create(builder.build()); if (errorCountPerConnectionMetricTracker != null) { errorCountPerConnectionMetricTracker.startConnectionErrorCountTracker( - clientContext.getExecutor()); + clientContext.getExecutor()); } return clientContext; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java index 253ae4c215..e219578b11 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java @@ -54,8 +54,7 @@ public void onClose(Status status, Metadata trailers) { } else { numOfErrors.increment(); } - } - finally { + } finally { super.onClose(status, trailers); } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java index 67c682184c..a6670182b8 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java @@ -35,7 +35,6 @@ import io.grpc.stub.StreamObserver; import java.util.List; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; import org.junit.Before; import org.junit.Test; From 26e5a935fc42ad93ed579ff6e9a8a236c4030d29 Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Fri, 16 Feb 2024 13:09:54 -0500 Subject: [PATCH 20/23] Improve error handling in the close() method. --- .../ConnectionErrorCountInterceptor.java | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java index e219578b11..3745794662 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java @@ -24,10 +24,14 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Status; +import java.util.Arrays; import java.util.concurrent.atomic.LongAdder; +import java.util.logging.Logger; /** An interceptor which counts the number of failed responses for a channel. */ class ConnectionErrorCountInterceptor implements ClientInterceptor { + private static final Logger LOG = + Logger.getLogger(ConnectionErrorCountInterceptor.class.toString()); private final LongAdder numOfErrors; private final LongAdder numOfSuccesses; @@ -48,14 +52,25 @@ public void start(Listener responseListener, Metadata headers) { responseListener) { @Override public void onClose(Status status, Metadata trailers) { + // Connection accounting is non-critical, so we log the exception, but let normal + // processing proceed. try { - if (status.isOk()) { - numOfSuccesses.increment(); - } else { - numOfErrors.increment(); + handleOnCloseUnsafe(status); + } catch (Throwable t) { + if (t instanceof InterruptedException) { + Thread.currentThread().interrupt(); } - } finally { - super.onClose(status, trailers); + LOG.warning( + "Failed to record connection stats, " + Arrays.toString(t.getStackTrace())); + } + super.onClose(status, trailers); + } + + private void handleOnCloseUnsafe(Status status) { + if (status.isOk()) { + numOfSuccesses.increment(); + } else { + numOfErrors.increment(); } } }, From 26c165bb5cbb36dc0b884afc9baaa6a8236f988d Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Fri, 16 Feb 2024 13:18:32 -0500 Subject: [PATCH 21/23] Improve exception logging. --- .../data/v2/stub/metrics/ConnectionErrorCountInterceptor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java index 3745794662..6bd0b4139e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java @@ -26,6 +26,7 @@ import io.grpc.Status; import java.util.Arrays; import java.util.concurrent.atomic.LongAdder; +import java.util.logging.Level; import java.util.logging.Logger; /** An interceptor which counts the number of failed responses for a channel. */ @@ -60,8 +61,7 @@ public void onClose(Status status, Metadata trailers) { if (t instanceof InterruptedException) { Thread.currentThread().interrupt(); } - LOG.warning( - "Failed to record connection stats, " + Arrays.toString(t.getStackTrace())); + LOG.log(Level.WARNING, "Unexpected error while updating connection error stats", t); } super.onClose(status, trailers); } From a2e6d18d500ffb29c8f1b1a5b7de653a6e8691bd Mon Sep 17 00:00:00 2001 From: Reza Karegar Date: Fri, 16 Feb 2024 13:19:09 -0500 Subject: [PATCH 22/23] Fix code formatting. --- .../data/v2/stub/metrics/ConnectionErrorCountInterceptor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java index 6bd0b4139e..17fcf9018e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/ConnectionErrorCountInterceptor.java @@ -24,7 +24,6 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Status; -import java.util.Arrays; import java.util.concurrent.atomic.LongAdder; import java.util.logging.Level; import java.util.logging.Logger; @@ -61,7 +60,8 @@ public void onClose(Status status, Metadata trailers) { if (t instanceof InterruptedException) { Thread.currentThread().interrupt(); } - LOG.log(Level.WARNING, "Unexpected error while updating connection error stats", t); + LOG.log( + Level.WARNING, "Unexpected error while updating connection error stats", t); } super.onClose(status, trailers); } From a1d9c77f2711eab9d1665e463ce2999d3c459887 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 16 Feb 2024 20:07:03 +0000 Subject: [PATCH 23/23] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See /~https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 93c8269892..12590a0257 100644 --- a/README.md +++ b/README.md @@ -50,20 +50,20 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.31.0') +implementation platform('com.google.cloud:libraries-bom:26.32.0') implementation 'com.google.cloud:google-cloud-bigtable' ``` If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-bigtable:2.32.0' +implementation 'com.google.cloud:google-cloud-bigtable:2.33.0' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-bigtable" % "2.32.0" +libraryDependencies += "com.google.cloud" % "google-cloud-bigtable" % "2.33.0" ``` @@ -609,7 +609,7 @@ Java is a registered trademark of Oracle and/or its affiliates. [kokoro-badge-link-5]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-bigtable/java11.html [stability-image]: https://img.shields.io/badge/stability-stable-green [maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-bigtable.svg -[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-bigtable/2.32.0 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-bigtable/2.33.0 [authentication]: /~https://github.com/googleapis/google-cloud-java#authentication [auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes [predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles