From 083bb2cab27ec3d407f934090a528c06dfb67b5b Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 30 Jun 2023 13:36:46 -0400 Subject: [PATCH 1/5] fix: fix batcher metric labels --- .../clirr-ignored-differences.xml | 5 ++ .../data/v2/stub/EnhancedBigtableStub.java | 77 ++++++++++++++++--- ...igtableTracerBatchedStreamingCallable.java | 56 ++++++++++++++ .../BigtableTracerBatchedUnaryCallable.java | 55 ------------- .../BigtableTracerStreamingCallable.java | 2 +- .../metrics/BigtableTracerUnaryCallable.java | 2 +- .../metrics/BuiltinMetricsTracerTest.java | 6 ++ 7 files changed, 134 insertions(+), 69 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedStreamingCallable.java delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable.java diff --git a/google-cloud-bigtable/clirr-ignored-differences.xml b/google-cloud-bigtable/clirr-ignored-differences.xml index 1ca5867295..618a4611c0 100644 --- a/google-cloud-bigtable/clirr-ignored-differences.xml +++ b/google-cloud-bigtable/clirr-ignored-differences.xml @@ -145,4 +145,9 @@ com/google/cloud/bigtable/data/v2/stub/readrows/RowMerger * + + + 8001 + com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable + 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 c46539cddf..982449deb1 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 @@ -87,7 +87,7 @@ import com.google.cloud.bigtable.data.v2.stub.changestream.GenerateInitialChangeStreamPartitionsUserCallable; import com.google.cloud.bigtable.data.v2.stub.changestream.ReadChangeStreamResumptionStrategy; import com.google.cloud.bigtable.data.v2.stub.changestream.ReadChangeStreamUserCallable; -import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerBatchedUnaryCallable; +import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerBatchedStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTracerFactory; @@ -496,11 +496,65 @@ public Map extract(ReadRowsRequest readRowsRequest) { */ private UnaryCallable> createBulkReadRowsCallable( RowAdapter rowAdapter) { - ServerStreamingCallable readRowsCallable = - createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter); + ServerStreamingCallSettings readRowsSettings = settings.readRowsSettings(); + + ServerStreamingCallable base = + GrpcRawCallableFactory.createServerStreamingCallable( + GrpcCallSettings.newBuilder() + .setMethodDescriptor(BigtableGrpc.getReadRowsMethod()) + .setParamsExtractor( + new RequestParamsExtractor() { + @Override + public Map extract(ReadRowsRequest readRowsRequest) { + return ImmutableMap.of( + "table_name", readRowsRequest.getTableName(), + "app_profile_id", readRowsRequest.getAppProfileId()); + } + }) + .build(), + readRowsSettings.getRetryableCodes()); + + ServerStreamingCallable withStatsHeaders = + new StatsHeadersServerStreamingCallable<>(base); + + // Sometimes ReadRows connections are disconnected via an RST frame. This error is transient and + // should be treated similar to UNAVAILABLE. However, this exception has an INTERNAL error code + // which by default is not retryable. Convert the exception so it can be retried in the client. + ServerStreamingCallable convertException = + new ConvertExceptionCallable<>(withStatsHeaders); + + ServerStreamingCallable merging = + new RowMergingCallable<>(convertException, rowAdapter); + + // Copy settings for the middle ReadRowsRequest -> RowT callable (as opposed to the inner + // ReadRowsRequest -> ReadRowsResponse callable). + ServerStreamingCallSettings innerSettings = + ServerStreamingCallSettings.newBuilder() + .setResumptionStrategy(new ReadRowsResumptionStrategy<>(rowAdapter)) + .setRetryableCodes(readRowsSettings.getRetryableCodes()) + .setRetrySettings(readRowsSettings.getRetrySettings()) + .setIdleTimeout(readRowsSettings.getIdleTimeout()) + .build(); + + ServerStreamingCallable watched = + Callables.watched(merging, innerSettings, clientContext); + + ServerStreamingCallable withBigtableTracer = + new BigtableTracerBatchedStreamingCallable<>(watched); + + // Retry logic is split into 2 parts to workaround a rare edge case described in + // ReadRowsRetryCompletedCallable + ServerStreamingCallable retrying1 = + new ReadRowsRetryCompletedCallable<>(withBigtableTracer); + + ServerStreamingCallable retrying2 = + Callables.retrying(retrying1, innerSettings, clientContext); + + ServerStreamingCallable filterMarkerCallable = + new FilterMarkerRowsCallable<>(retrying2, rowAdapter); ServerStreamingCallable readRowsUserCallable = - new ReadRowsUserCallable<>(readRowsCallable, requestContext); + new ReadRowsUserCallable<>(filterMarkerCallable, requestContext); SpanName span = getSpanName("ReadRows"); @@ -509,11 +563,8 @@ private UnaryCallable> createBulkReadRowsCallable( UnaryCallable> tracedBatcher = new TracedBatcherUnaryCallable<>(readRowsUserCallable.all()); - UnaryCallable> withBigtableTracer = - new BigtableTracerBatchedUnaryCallable<>(tracedBatcher); - UnaryCallable> traced = - new TracedUnaryCallable<>(withBigtableTracer, clientContext.getTracerFactory(), span); + new TracedUnaryCallable<>(tracedBatcher, clientContext.getTracerFactory(), span); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -641,10 +692,9 @@ private UnaryCallable createBulkMutateRowsCallable() { UnaryCallable tracedBatcherUnaryCallable = new TracedBatcherUnaryCallable<>(userFacing); - UnaryCallable withBigtableTracer = - new BigtableTracerBatchedUnaryCallable<>(tracedBatcherUnaryCallable); UnaryCallable traced = - new TracedUnaryCallable<>(withBigtableTracer, clientContext.getTracerFactory(), spanName); + new TracedUnaryCallable<>( + tracedBatcherUnaryCallable, clientContext.getTracerFactory(), spanName); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -747,6 +797,9 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { ServerStreamingCallable convertException = new ConvertExceptionCallable<>(callable); + ServerStreamingCallable withBigtableTracer = + new BigtableTracerBatchedStreamingCallable<>(convertException); + RetryAlgorithm retryAlgorithm = new RetryAlgorithm<>( new ApiResultRetryAlgorithm(), @@ -757,7 +810,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { return new MutateRowsRetryingCallable( clientContext.getDefaultCallContext(), - convertException, + withBigtableTracer, retryingExecutor, settings.bulkMutateRowsSettings().getRetryableCodes()); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedStreamingCallable.java new file mode 100644 index 0000000000..05dfdb2219 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedStreamingCallable.java @@ -0,0 +1,56 @@ +/* + * 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.metrics; + +import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcResponseMetadata; +import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.ResponseObserver; +import com.google.api.gax.rpc.ServerStreamingCallable; +import javax.annotation.Nonnull; + +/** + * This callable will do everything described in {@link BigtableTracerStreamingCallable} except that + * it won't inject a {@link BigtableGrpcStreamTracer}. For batching calls, we only want to calculate + * the total time client is blocked because of flow control. + */ +@InternalApi +public class BigtableTracerBatchedStreamingCallable + extends BigtableTracerStreamingCallable { + + private ServerStreamingCallable innerCallable; + + public BigtableTracerBatchedStreamingCallable( + @Nonnull ServerStreamingCallable innerCallable) { + super(innerCallable); + this.innerCallable = innerCallable; + } + + @Override + public void call( + RequestT request, ResponseObserver responseObserver, ApiCallContext context) { + final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); + // tracer should always be an instance of bigtable tracer + if (context.getTracer() instanceof BigtableTracer) { + BigtableTracerResponseObserver innerObserver = + new BigtableTracerResponseObserver( + responseObserver, (BigtableTracer) context.getTracer(), responseMetadata); + innerCallable.call(request, innerObserver, responseMetadata.addHandlers(context)); + } else { + innerCallable.call(request, responseObserver, context); + } + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable.java deleted file mode 100644 index 06722aaea5..0000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * 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.metrics; - -import com.google.api.core.ApiFuture; -import com.google.api.core.ApiFutures; -import com.google.api.core.InternalApi; -import com.google.api.gax.grpc.GrpcResponseMetadata; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.common.util.concurrent.MoreExecutors; -import javax.annotation.Nonnull; - -/** - * This callable will do everything described in {@link BigtableTracerUnaryCallable} except that it - * won't inject a {@link BigtableGrpcStreamTracer}. For batching calls, we only want to calculate - * the total time client is blocked because of flow control. - */ -@InternalApi -public class BigtableTracerBatchedUnaryCallable - extends BigtableTracerUnaryCallable { - - private UnaryCallable innerCallable; - - public BigtableTracerBatchedUnaryCallable( - @Nonnull UnaryCallable innerCallable) { - super(innerCallable); - this.innerCallable = innerCallable; - } - - @Override - public ApiFuture futureCall(RequestT request, ApiCallContext context) { - final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); - BigtableTracerUnaryCallback callback = - new BigtableTracerUnaryCallback( - (BigtableTracer) context.getTracer(), responseMetadata); - ApiFuture future = - innerCallable.futureCall(request, responseMetadata.addHandlers(context)); - ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor()); - return future; - } -} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 167cd0dc2e..5b7a9befa6 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -72,7 +72,7 @@ public void call( } } - private class BigtableTracerResponseObserver extends SafeResponseObserver { + class BigtableTracerResponseObserver extends SafeResponseObserver { private final BigtableTracer tracer; private final ResponseObserver outerObserver; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java index e5ec7b806b..7dfca8b753 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java @@ -70,7 +70,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) } } - class BigtableTracerUnaryCallback implements ApiFutureCallback { + private class BigtableTracerUnaryCallback implements ApiFutureCallback { private final BigtableTracer tracer; private final GrpcResponseMetadata responseMetadata; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index 3bc283a7f7..dee110efaf 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -489,6 +489,12 @@ public void testBatchBlockingLatencies() throws InterruptedException { // Blocking latency should be around server latency. assertThat(throttledTime.getAllValues().get(1)).isAtLeast(SERVER_LATENCY - 10); assertThat(throttledTime.getAllValues().get(2)).isAtLeast(SERVER_LATENCY - 10); + + verify(statsRecorderWrapper, timeout(100).times(expectedNumRequests)) + .recordAttempt(status.capture(), tableId.capture(), zone.capture(), cluster.capture()); + + assertThat(zone.getAllValues()).containsExactly(ZONE, ZONE, ZONE); + assertThat(cluster.getAllValues()).containsExactly(CLUSTER, CLUSTER, CLUSTER); } } From eddf98918badf13f17ec08a424c2b11b45c8e201 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 6 Jul 2023 15:51:26 -0400 Subject: [PATCH 2/5] update --- .../data/v2/stub/EnhancedBigtableStub.java | 85 +++++-------------- 1 file changed, 23 insertions(+), 62 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 982449deb1..9144389aad 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 @@ -335,7 +335,7 @@ public EnhancedBigtableStub(EnhancedBigtableStubSettings settings, ClientContext @BetaApi("This surface is stable yet it might be removed in the future.") public ServerStreamingCallable createReadRowsRawCallable( RowAdapter rowAdapter) { - return createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter) + return createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter, false) .withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -356,7 +356,7 @@ public ServerStreamingCallable createReadRowsRawCa public ServerStreamingCallable createReadRowsCallable( RowAdapter rowAdapter) { ServerStreamingCallable readRowsCallable = - createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter); + createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter, false); ServerStreamingCallable readRowsUserCallable = new ReadRowsUserCallable<>(readRowsCallable, requestContext); @@ -391,7 +391,8 @@ public UnaryCallable createReadRowCallable(RowAdapter .setRetrySettings(settings.readRowSettings().getRetrySettings()) .setIdleTimeout(settings.readRowSettings().getRetrySettings().getTotalTimeout()) .build(), - rowAdapter); + rowAdapter, + false); ReadRowsUserCallable readRowCallable = new ReadRowsUserCallable<>(readRowsCallable, requestContext); @@ -421,7 +422,9 @@ public UnaryCallable createReadRowCallable(RowAdapter *

NOTE: the caller is responsible for adding tracing & metrics. */ private ServerStreamingCallable createReadRowsBaseCallable( - ServerStreamingCallSettings readRowsSettings, RowAdapter rowAdapter) { + ServerStreamingCallSettings readRowsSettings, + RowAdapter rowAdapter, + boolean batch) { ServerStreamingCallable base = GrpcRawCallableFactory.createServerStreamingCallable( @@ -464,8 +467,13 @@ public Map extract(ReadRowsRequest readRowsRequest) { ServerStreamingCallable watched = Callables.watched(merging, innerSettings, clientContext); - ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(watched); + ServerStreamingCallable withBigtableTracer; + + if (batch) { + withBigtableTracer = new BigtableTracerBatchedStreamingCallable<>(watched); + } else { + withBigtableTracer = new BigtableTracerStreamingCallable<>(watched); + } // Retry logic is split into 2 parts to workaround a rare edge case described in // ReadRowsRetryCompletedCallable @@ -496,65 +504,18 @@ public Map extract(ReadRowsRequest readRowsRequest) { */ private UnaryCallable> createBulkReadRowsCallable( RowAdapter rowAdapter) { - ServerStreamingCallSettings readRowsSettings = settings.readRowsSettings(); - - ServerStreamingCallable base = - GrpcRawCallableFactory.createServerStreamingCallable( - GrpcCallSettings.newBuilder() - .setMethodDescriptor(BigtableGrpc.getReadRowsMethod()) - .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract(ReadRowsRequest readRowsRequest) { - return ImmutableMap.of( - "table_name", readRowsRequest.getTableName(), - "app_profile_id", readRowsRequest.getAppProfileId()); - } - }) + ServerStreamingCallable baseCallable = + createReadRowsBaseCallable( + ServerStreamingCallSettings.newBuilder() + .setRetryableCodes(settings.readRowSettings().getRetryableCodes()) + .setRetrySettings(settings.readRowSettings().getRetrySettings()) + .setIdleTimeout(settings.readRowSettings().getRetrySettings().getTotalTimeout()) .build(), - readRowsSettings.getRetryableCodes()); - - ServerStreamingCallable withStatsHeaders = - new StatsHeadersServerStreamingCallable<>(base); - - // Sometimes ReadRows connections are disconnected via an RST frame. This error is transient and - // should be treated similar to UNAVAILABLE. However, this exception has an INTERNAL error code - // which by default is not retryable. Convert the exception so it can be retried in the client. - ServerStreamingCallable convertException = - new ConvertExceptionCallable<>(withStatsHeaders); - - ServerStreamingCallable merging = - new RowMergingCallable<>(convertException, rowAdapter); - - // Copy settings for the middle ReadRowsRequest -> RowT callable (as opposed to the inner - // ReadRowsRequest -> ReadRowsResponse callable). - ServerStreamingCallSettings innerSettings = - ServerStreamingCallSettings.newBuilder() - .setResumptionStrategy(new ReadRowsResumptionStrategy<>(rowAdapter)) - .setRetryableCodes(readRowsSettings.getRetryableCodes()) - .setRetrySettings(readRowsSettings.getRetrySettings()) - .setIdleTimeout(readRowsSettings.getIdleTimeout()) - .build(); - - ServerStreamingCallable watched = - Callables.watched(merging, innerSettings, clientContext); - - ServerStreamingCallable withBigtableTracer = - new BigtableTracerBatchedStreamingCallable<>(watched); - - // Retry logic is split into 2 parts to workaround a rare edge case described in - // ReadRowsRetryCompletedCallable - ServerStreamingCallable retrying1 = - new ReadRowsRetryCompletedCallable<>(withBigtableTracer); - - ServerStreamingCallable retrying2 = - Callables.retrying(retrying1, innerSettings, clientContext); - - ServerStreamingCallable filterMarkerCallable = - new FilterMarkerRowsCallable<>(retrying2, rowAdapter); + rowAdapter, + true); ServerStreamingCallable readRowsUserCallable = - new ReadRowsUserCallable<>(filterMarkerCallable, requestContext); + new ReadRowsUserCallable<>(baseCallable, requestContext); SpanName span = getSpanName("ReadRows"); From 0114330cd304820f2e69ff93160cabb20da0f260 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 6 Jul 2023 16:38:24 -0400 Subject: [PATCH 3/5] update --- .../data/v2/stub/EnhancedBigtableStub.java | 26 +++------ ...igtableTracerBatchedStreamingCallable.java | 56 ------------------- .../metrics/BuiltinMetricsTracerTest.java | 2 - 3 files changed, 8 insertions(+), 76 deletions(-) delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedStreamingCallable.java 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 9144389aad..8b9b2a5291 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 @@ -87,7 +87,6 @@ import com.google.cloud.bigtable.data.v2.stub.changestream.GenerateInitialChangeStreamPartitionsUserCallable; import com.google.cloud.bigtable.data.v2.stub.changestream.ReadChangeStreamResumptionStrategy; import com.google.cloud.bigtable.data.v2.stub.changestream.ReadChangeStreamUserCallable; -import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerBatchedStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTracerFactory; @@ -335,7 +334,7 @@ public EnhancedBigtableStub(EnhancedBigtableStubSettings settings, ClientContext @BetaApi("This surface is stable yet it might be removed in the future.") public ServerStreamingCallable createReadRowsRawCallable( RowAdapter rowAdapter) { - return createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter, false) + return createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter) .withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -356,7 +355,7 @@ public ServerStreamingCallable createReadRowsRawCa public ServerStreamingCallable createReadRowsCallable( RowAdapter rowAdapter) { ServerStreamingCallable readRowsCallable = - createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter, false); + createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter); ServerStreamingCallable readRowsUserCallable = new ReadRowsUserCallable<>(readRowsCallable, requestContext); @@ -391,8 +390,7 @@ public UnaryCallable createReadRowCallable(RowAdapter .setRetrySettings(settings.readRowSettings().getRetrySettings()) .setIdleTimeout(settings.readRowSettings().getRetrySettings().getTotalTimeout()) .build(), - rowAdapter, - false); + rowAdapter); ReadRowsUserCallable readRowCallable = new ReadRowsUserCallable<>(readRowsCallable, requestContext); @@ -422,9 +420,7 @@ public UnaryCallable createReadRowCallable(RowAdapter *

NOTE: the caller is responsible for adding tracing & metrics. */ private ServerStreamingCallable createReadRowsBaseCallable( - ServerStreamingCallSettings readRowsSettings, - RowAdapter rowAdapter, - boolean batch) { + ServerStreamingCallSettings readRowsSettings, RowAdapter rowAdapter) { ServerStreamingCallable base = GrpcRawCallableFactory.createServerStreamingCallable( @@ -467,13 +463,8 @@ public Map extract(ReadRowsRequest readRowsRequest) { ServerStreamingCallable watched = Callables.watched(merging, innerSettings, clientContext); - ServerStreamingCallable withBigtableTracer; - - if (batch) { - withBigtableTracer = new BigtableTracerBatchedStreamingCallable<>(watched); - } else { - withBigtableTracer = new BigtableTracerStreamingCallable<>(watched); - } + ServerStreamingCallable withBigtableTracer = + new BigtableTracerStreamingCallable<>(watched); // Retry logic is split into 2 parts to workaround a rare edge case described in // ReadRowsRetryCompletedCallable @@ -511,8 +502,7 @@ private UnaryCallable> createBulkReadRowsCallable( .setRetrySettings(settings.readRowSettings().getRetrySettings()) .setIdleTimeout(settings.readRowSettings().getRetrySettings().getTotalTimeout()) .build(), - rowAdapter, - true); + rowAdapter); ServerStreamingCallable readRowsUserCallable = new ReadRowsUserCallable<>(baseCallable, requestContext); @@ -759,7 +749,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { new ConvertExceptionCallable<>(callable); ServerStreamingCallable withBigtableTracer = - new BigtableTracerBatchedStreamingCallable<>(convertException); + new BigtableTracerStreamingCallable<>(convertException); RetryAlgorithm retryAlgorithm = new RetryAlgorithm<>( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedStreamingCallable.java deleted file mode 100644 index 05dfdb2219..0000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedStreamingCallable.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * 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.metrics; - -import com.google.api.core.InternalApi; -import com.google.api.gax.grpc.GrpcResponseMetadata; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.ResponseObserver; -import com.google.api.gax.rpc.ServerStreamingCallable; -import javax.annotation.Nonnull; - -/** - * This callable will do everything described in {@link BigtableTracerStreamingCallable} except that - * it won't inject a {@link BigtableGrpcStreamTracer}. For batching calls, we only want to calculate - * the total time client is blocked because of flow control. - */ -@InternalApi -public class BigtableTracerBatchedStreamingCallable - extends BigtableTracerStreamingCallable { - - private ServerStreamingCallable innerCallable; - - public BigtableTracerBatchedStreamingCallable( - @Nonnull ServerStreamingCallable innerCallable) { - super(innerCallable); - this.innerCallable = innerCallable; - } - - @Override - public void call( - RequestT request, ResponseObserver responseObserver, ApiCallContext context) { - final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); - // tracer should always be an instance of bigtable tracer - if (context.getTracer() instanceof BigtableTracer) { - BigtableTracerResponseObserver innerObserver = - new BigtableTracerResponseObserver( - responseObserver, (BigtableTracer) context.getTracer(), responseMetadata); - innerCallable.call(request, innerObserver, responseMetadata.addHandlers(context)); - } else { - innerCallable.call(request, responseObserver, context); - } - } -} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index dee110efaf..2331ac8538 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -483,8 +483,6 @@ public void testBatchBlockingLatencies() throws InterruptedException { verify(statsRecorderWrapper, timeout(1000).times(expectedNumRequests)) .putClientBlockingLatencies(throttledTime.capture()); - // Adding the first 2 elements should not get throttled since the batch is empty - assertThat(throttledTime.getAllValues().get(0)).isEqualTo(0); // After the first request is sent, batcher will block on add because of the server latency. // Blocking latency should be around server latency. assertThat(throttledTime.getAllValues().get(1)).isAtLeast(SERVER_LATENCY - 10); From 2f7bc8bbd7ac02ee4077366ac5d543ea01e0e1d9 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 6 Jul 2023 16:43:52 -0400 Subject: [PATCH 4/5] make methods private --- google-cloud-bigtable/clirr-ignored-differences.xml | 2 +- .../bigtable/data/v2/stub/EnhancedBigtableStub.java | 12 +++--------- .../metrics/BigtableTracerStreamingCallable.java | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/google-cloud-bigtable/clirr-ignored-differences.xml b/google-cloud-bigtable/clirr-ignored-differences.xml index 618a4611c0..4bb4684c38 100644 --- a/google-cloud-bigtable/clirr-ignored-differences.xml +++ b/google-cloud-bigtable/clirr-ignored-differences.xml @@ -145,7 +145,7 @@ com/google/cloud/bigtable/data/v2/stub/readrows/RowMerger * - + 8001 com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable 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 8b9b2a5291..474c140392 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 @@ -495,17 +495,11 @@ public Map extract(ReadRowsRequest readRowsRequest) { */ private UnaryCallable> createBulkReadRowsCallable( RowAdapter rowAdapter) { - ServerStreamingCallable baseCallable = - createReadRowsBaseCallable( - ServerStreamingCallSettings.newBuilder() - .setRetryableCodes(settings.readRowSettings().getRetryableCodes()) - .setRetrySettings(settings.readRowSettings().getRetrySettings()) - .setIdleTimeout(settings.readRowSettings().getRetrySettings().getTotalTimeout()) - .build(), - rowAdapter); + ServerStreamingCallable readRowsCallable = + createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter); ServerStreamingCallable readRowsUserCallable = - new ReadRowsUserCallable<>(baseCallable, requestContext); + new ReadRowsUserCallable<>(readRowsCallable, requestContext); SpanName span = getSpanName("ReadRows"); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 5b7a9befa6..167cd0dc2e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -72,7 +72,7 @@ public void call( } } - class BigtableTracerResponseObserver extends SafeResponseObserver { + private class BigtableTracerResponseObserver extends SafeResponseObserver { private final BigtableTracer tracer; private final ResponseObserver outerObserver; From cfe11dc5ad10cbf80efe3c3e6b2b0ffb53fc1a25 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 28 Jul 2023 16:17:57 +0000 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-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 c556fc94e8..2c9428effc 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.18.0') +implementation platform('com.google.cloud:libraries-bom:26.20.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.24.1' +implementation 'com.google.cloud:google-cloud-bigtable:2.25.1' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-bigtable" % "2.24.1" +libraryDependencies += "com.google.cloud" % "google-cloud-bigtable" % "2.25.1" ``` @@ -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.24.1 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-bigtable/2.25.1 [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