From b7e5c815b428f7ef1576e92853aeb2588730162e Mon Sep 17 00:00:00 2001 From: "release-please[bot]" <55107282+release-please[bot]@users.noreply.github.com> Date: Tue, 29 Oct 2024 16:58:28 +0000 Subject: [PATCH 01/16] chore(main): release 2.46.1-SNAPSHOT (#2390) :robot: I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please). --- google-cloud-bigtable-bom/pom.xml | 16 ++++++++-------- google-cloud-bigtable-deps-bom/pom.xml | 2 +- google-cloud-bigtable-emulator-core/pom.xml | 4 ++-- google-cloud-bigtable-emulator/pom.xml | 10 +++++----- google-cloud-bigtable/pom.xml | 10 +++++----- .../java/com/google/cloud/bigtable/Version.java | 2 +- grpc-google-cloud-bigtable-admin-v2/pom.xml | 8 ++++---- grpc-google-cloud-bigtable-v2/pom.xml | 8 ++++---- pom.xml | 12 ++++++------ proto-google-cloud-bigtable-admin-v2/pom.xml | 8 ++++---- proto-google-cloud-bigtable-v2/pom.xml | 8 ++++---- samples/snapshot/pom.xml | 2 +- test-proxy/pom.xml | 4 ++-- versions.txt | 14 +++++++------- 14 files changed, 54 insertions(+), 54 deletions(-) diff --git a/google-cloud-bigtable-bom/pom.xml b/google-cloud-bigtable-bom/pom.xml index 395d38e3dd19..c5fe60b4abef 100644 --- a/google-cloud-bigtable-bom/pom.xml +++ b/google-cloud-bigtable-bom/pom.xml @@ -3,7 +3,7 @@ 4.0.0 com.google.cloud google-cloud-bigtable-bom - 2.46.0 + 2.46.1-SNAPSHOT pom com.google.cloud @@ -63,37 +63,37 @@ com.google.cloud google-cloud-bigtable - 2.46.0 + 2.46.1-SNAPSHOT com.google.cloud google-cloud-bigtable-emulator - 0.183.0 + 0.183.1-SNAPSHOT com.google.cloud google-cloud-bigtable-emulator-core - 0.183.0 + 0.183.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-bigtable-admin-v2 - 2.46.0 + 2.46.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-bigtable-v2 - 2.46.0 + 2.46.1-SNAPSHOT com.google.api.grpc proto-google-cloud-bigtable-admin-v2 - 2.46.0 + 2.46.1-SNAPSHOT com.google.api.grpc proto-google-cloud-bigtable-v2 - 2.46.0 + 2.46.1-SNAPSHOT diff --git a/google-cloud-bigtable-deps-bom/pom.xml b/google-cloud-bigtable-deps-bom/pom.xml index 8776779fe11b..be0e83e515d7 100644 --- a/google-cloud-bigtable-deps-bom/pom.xml +++ b/google-cloud-bigtable-deps-bom/pom.xml @@ -13,7 +13,7 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.0 + 2.46.1-SNAPSHOT pom diff --git a/google-cloud-bigtable-emulator-core/pom.xml b/google-cloud-bigtable-emulator-core/pom.xml index b1f5bc09c260..700cfaf3b8c6 100644 --- a/google-cloud-bigtable-emulator-core/pom.xml +++ b/google-cloud-bigtable-emulator-core/pom.xml @@ -7,11 +7,11 @@ google-cloud-bigtable-parent com.google.cloud - 2.46.0 + 2.46.1-SNAPSHOT google-cloud-bigtable-emulator-core - 0.183.0 + 0.183.1-SNAPSHOT A Java wrapper for the Cloud Bigtable emulator. diff --git a/google-cloud-bigtable-emulator/pom.xml b/google-cloud-bigtable-emulator/pom.xml index d4a5fec878c3..06583c372653 100644 --- a/google-cloud-bigtable-emulator/pom.xml +++ b/google-cloud-bigtable-emulator/pom.xml @@ -5,7 +5,7 @@ 4.0.0 google-cloud-bigtable-emulator - 0.183.0 + 0.183.1-SNAPSHOT Google Cloud Java - Bigtable Emulator https://github.com/googleapis/java-bigtable @@ -14,7 +14,7 @@ com.google.cloud google-cloud-bigtable-parent - 2.46.0 + 2.46.1-SNAPSHOT scm:git:git@github.com:googleapis/java-bigtable.git @@ -81,14 +81,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import com.google.cloud google-cloud-bigtable-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import @@ -99,7 +99,7 @@ com.google.cloud google-cloud-bigtable-emulator-core - 0.183.0 + 0.183.1-SNAPSHOT diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index bfa117899496..9b09f630936d 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -2,7 +2,7 @@ 4.0.0 google-cloud-bigtable - 2.46.0 + 2.46.1-SNAPSHOT jar Google Cloud Bigtable https://github.com/googleapis/java-bigtable @@ -12,11 +12,11 @@ com.google.cloud google-cloud-bigtable-parent - 2.46.0 + 2.46.1-SNAPSHOT - 2.46.0 + 2.46.1-SNAPSHOT google-cloud-bigtable @@ -52,14 +52,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import com.google.cloud google-cloud-bigtable-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/Version.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/Version.java index f214deff22ff..4ed97314cecf 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/Version.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/Version.java @@ -20,6 +20,6 @@ @InternalApi("For internal use only") public final class Version { // {x-version-update-start:google-cloud-bigtable:current} - public static String VERSION = "2.46.0"; + public static String VERSION = "2.46.1-SNAPSHOT"; // {x-version-update-end} } diff --git a/grpc-google-cloud-bigtable-admin-v2/pom.xml b/grpc-google-cloud-bigtable-admin-v2/pom.xml index 704b5444703b..dd5d1168d4c0 100644 --- a/grpc-google-cloud-bigtable-admin-v2/pom.xml +++ b/grpc-google-cloud-bigtable-admin-v2/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc grpc-google-cloud-bigtable-admin-v2 - 2.46.0 + 2.46.1-SNAPSHOT grpc-google-cloud-bigtable-admin-v2 GRPC library for grpc-google-cloud-bigtable-admin-v2 com.google.cloud google-cloud-bigtable-parent - 2.46.0 + 2.46.1-SNAPSHOT @@ -18,14 +18,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import com.google.cloud google-cloud-bigtable-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import diff --git a/grpc-google-cloud-bigtable-v2/pom.xml b/grpc-google-cloud-bigtable-v2/pom.xml index 7ff019c8db54..c415de9996c1 100644 --- a/grpc-google-cloud-bigtable-v2/pom.xml +++ b/grpc-google-cloud-bigtable-v2/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc grpc-google-cloud-bigtable-v2 - 2.46.0 + 2.46.1-SNAPSHOT grpc-google-cloud-bigtable-v2 GRPC library for grpc-google-cloud-bigtable-v2 com.google.cloud google-cloud-bigtable-parent - 2.46.0 + 2.46.1-SNAPSHOT @@ -18,14 +18,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import com.google.cloud google-cloud-bigtable-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import diff --git a/pom.xml b/pom.xml index f1cb892a0f91..15a243b8ca5d 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ google-cloud-bigtable-parent pom - 2.46.0 + 2.46.1-SNAPSHOT Google Cloud Bigtable Parent https://github.com/googleapis/java-bigtable @@ -153,27 +153,27 @@ com.google.api.grpc proto-google-cloud-bigtable-v2 - 2.46.0 + 2.46.1-SNAPSHOT com.google.api.grpc proto-google-cloud-bigtable-admin-v2 - 2.46.0 + 2.46.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-bigtable-v2 - 2.46.0 + 2.46.1-SNAPSHOT com.google.api.grpc grpc-google-cloud-bigtable-admin-v2 - 2.46.0 + 2.46.1-SNAPSHOT com.google.cloud google-cloud-bigtable - 2.46.0 + 2.46.1-SNAPSHOT diff --git a/proto-google-cloud-bigtable-admin-v2/pom.xml b/proto-google-cloud-bigtable-admin-v2/pom.xml index 8911842acd99..367c6fee2240 100644 --- a/proto-google-cloud-bigtable-admin-v2/pom.xml +++ b/proto-google-cloud-bigtable-admin-v2/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc proto-google-cloud-bigtable-admin-v2 - 2.46.0 + 2.46.1-SNAPSHOT proto-google-cloud-bigtable-admin-v2 PROTO library for proto-google-cloud-bigtable-admin-v2 com.google.cloud google-cloud-bigtable-parent - 2.46.0 + 2.46.1-SNAPSHOT @@ -18,14 +18,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import com.google.cloud google-cloud-bigtable-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import diff --git a/proto-google-cloud-bigtable-v2/pom.xml b/proto-google-cloud-bigtable-v2/pom.xml index f9bed75ef102..f53212594d59 100644 --- a/proto-google-cloud-bigtable-v2/pom.xml +++ b/proto-google-cloud-bigtable-v2/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc proto-google-cloud-bigtable-v2 - 2.46.0 + 2.46.1-SNAPSHOT proto-google-cloud-bigtable-v2 PROTO library for proto-google-cloud-bigtable-v2 com.google.cloud google-cloud-bigtable-parent - 2.46.0 + 2.46.1-SNAPSHOT @@ -18,14 +18,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import com.google.cloud google-cloud-bigtable-bom - 2.46.0 + 2.46.1-SNAPSHOT pom import diff --git a/samples/snapshot/pom.xml b/samples/snapshot/pom.xml index e92578396211..4187b8d32ea3 100644 --- a/samples/snapshot/pom.xml +++ b/samples/snapshot/pom.xml @@ -28,7 +28,7 @@ com.google.cloud google-cloud-bigtable - 2.46.0 + 2.46.1-SNAPSHOT diff --git a/test-proxy/pom.xml b/test-proxy/pom.xml index acc7936844f7..03f319f5c674 100644 --- a/test-proxy/pom.xml +++ b/test-proxy/pom.xml @@ -12,11 +12,11 @@ google-cloud-bigtable-parent com.google.cloud - 2.46.0 + 2.46.1-SNAPSHOT - 2.46.0 + 2.46.1-SNAPSHOT diff --git a/versions.txt b/versions.txt index 7af9d1297d03..e896e3222ef4 100644 --- a/versions.txt +++ b/versions.txt @@ -1,10 +1,10 @@ # Format: # module:released-version:current-version -google-cloud-bigtable:2.46.0:2.46.0 -grpc-google-cloud-bigtable-admin-v2:2.46.0:2.46.0 -grpc-google-cloud-bigtable-v2:2.46.0:2.46.0 -proto-google-cloud-bigtable-admin-v2:2.46.0:2.46.0 -proto-google-cloud-bigtable-v2:2.46.0:2.46.0 -google-cloud-bigtable-emulator:0.183.0:0.183.0 -google-cloud-bigtable-emulator-core:0.183.0:0.183.0 +google-cloud-bigtable:2.46.0:2.46.1-SNAPSHOT +grpc-google-cloud-bigtable-admin-v2:2.46.0:2.46.1-SNAPSHOT +grpc-google-cloud-bigtable-v2:2.46.0:2.46.1-SNAPSHOT +proto-google-cloud-bigtable-admin-v2:2.46.0:2.46.1-SNAPSHOT +proto-google-cloud-bigtable-v2:2.46.0:2.46.1-SNAPSHOT +google-cloud-bigtable-emulator:0.183.0:0.183.1-SNAPSHOT +google-cloud-bigtable-emulator-core:0.183.0:0.183.1-SNAPSHOT From ecf4d3ee31a6a69481bdef3cbc36d696b05ec624 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 29 Oct 2024 18:28:25 -0400 Subject: [PATCH 02/16] chore: add feature flags for DirectAccess (#2392) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I81d16773980bff6232ffe20839a9968718a361b1 Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Rollback plan is reviewed and LGTMed - [ ] All new data plane features have a completed end to end testing plan Fixes # ☕️ If you write sample code, please follow the [samples format]( https://togithub.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md). --- .../data/v2/stub/EnhancedBigtableStubSettings.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) 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 2a3d0ddba4e5..5e5dc64fd991 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 @@ -105,7 +105,9 @@ public class EnhancedBigtableStubSettings extends StubSettings IDEMPOTENT_RETRY_CODES = ImmutableSet.of(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE); @@ -376,10 +378,9 @@ public String getMetricsEndpoint() { /** Returns a builder for the default ChannelProvider for this service. */ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() { - Boolean isDirectpathEnabled = Boolean.parseBoolean(System.getenv(CBT_ENABLE_DIRECTPATH)); InstantiatingGrpcChannelProvider.Builder grpcTransportProviderBuilder = BigtableStubSettings.defaultGrpcTransportProviderBuilder(); - if (isDirectpathEnabled) { + if (DIRECT_PATH_ENABLED) { // Attempts direct access to CBT service over gRPC to improve throughput, // whether the attempt is allowed is totally controlled by service owner. grpcTransportProviderBuilder @@ -830,7 +831,11 @@ private Builder() { .setWaitTimeout(Duration.ofMinutes(5)); featureFlags = - FeatureFlags.newBuilder().setReverseScans(true).setLastScannedRowResponses(true); + FeatureFlags.newBuilder() + .setReverseScans(true) + .setLastScannedRowResponses(true) + .setDirectAccessRequested(DIRECT_PATH_ENABLED) + .setTrafficDirectorEnabled(DIRECT_PATH_ENABLED); } private Builder(EnhancedBigtableStubSettings settings) { From 81431d905b50cc64f3f731be10f73960dfa7eb80 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 4 Nov 2024 10:24:45 -0500 Subject: [PATCH 03/16] chore: improve logging in safe response observer (#2397) Log the extra error --- .../cloud/bigtable/data/v2/stub/SafeResponseObserver.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/SafeResponseObserver.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/SafeResponseObserver.java index 7c65bdf95a60..0133dd3c2ba7 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/SafeResponseObserver.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/SafeResponseObserver.java @@ -83,7 +83,7 @@ public final void onResponse(ResponseT response) { @Override public final void onError(Throwable throwable) { if (!isClosed.compareAndSet(false, true)) { - logException("Received error after the stream is closed"); + logException("Received error after the stream is closed", throwable); return; } @@ -113,6 +113,10 @@ private void logException(String message) { LOGGER.log(Level.WARNING, message, new IllegalStateException(message)); } + private void logException(String message, Throwable cause) { + LOGGER.log(Level.WARNING, message, new IllegalStateException(message, cause)); + } + protected abstract void onStartImpl(StreamController streamController); protected abstract void onResponseImpl(ResponseT response); From c3e92303adfaca9ab73a947070668dc83b64bf56 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 4 Nov 2024 10:30:26 -0500 Subject: [PATCH 04/16] chore(test): remove some warnings from tests (#2398) Make sure to close the ephemeral stubs to avoid polluting the logs with unclosed channel errors --- .../v2/stub/sql/ExecuteQueryCallableTest.java | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/sql/ExecuteQueryCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/sql/ExecuteQueryCallableTest.java index deedfbaba136..1ddac3372089 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/sql/ExecuteQueryCallableTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/sql/ExecuteQueryCallableTest.java @@ -124,15 +124,16 @@ public void testExecuteQueryRequestsIgnoreOverriddenMaxAttempts() throws IOExcep .stubSettings() .executeQuerySettings() .setRetrySettings(RetrySettings.newBuilder().setMaxAttempts(10).build()); - EnhancedBigtableStub overrideStub = - EnhancedBigtableStub.create(overrideSettings.build().getStubSettings()); - SqlServerStream stream = - overrideStub.executeQueryCallable().call(Statement.of("SELECT * FROM table")); - Iterator iterator = stream.rows().iterator(); + try (EnhancedBigtableStub overrideStub = + EnhancedBigtableStub.create(overrideSettings.build().getStubSettings())) { + SqlServerStream stream = + overrideStub.executeQueryCallable().call(Statement.of("SELECT * FROM table")); + Iterator iterator = stream.rows().iterator(); - assertThrows(UnavailableException.class, iterator::next).getCause(); - assertThat(fakeService.attempts).isEqualTo(1); + assertThrows(UnavailableException.class, iterator::next).getCause(); + assertThat(fakeService.attempts).isEqualTo(1); + } } @Test @@ -160,13 +161,15 @@ public void testExecuteQueryRequestsRespectDeadline() throws IOException { .setInitialRpcTimeout(Duration.ofMillis(10)) .setMaxRpcTimeout(Duration.ofMillis(10)) .build()); - EnhancedBigtableStub overrideDeadline = - EnhancedBigtableStub.create(overrideSettings.build().getStubSettings()); - SqlServerStream streamOverride = - overrideDeadline.executeQueryCallable().call(Statement.of("SELECT * FROM table")); - Iterator overrideIterator = streamOverride.rows().iterator(); - // We don't care about this but are reusing the fake service that tests retries - assertThrows(DeadlineExceededException.class, overrideIterator::next).getCause(); + + try (EnhancedBigtableStub overrideDeadline = + EnhancedBigtableStub.create(overrideSettings.build().getStubSettings())) { + SqlServerStream streamOverride = + overrideDeadline.executeQueryCallable().call(Statement.of("SELECT * FROM table")); + Iterator overrideIterator = streamOverride.rows().iterator(); + // We don't care about this but are reusing the fake service that tests retries + assertThrows(DeadlineExceededException.class, overrideIterator::next).getCause(); + } } private static class FakeService extends BigtableGrpc.BigtableImplBase { From 0ad0c95b2243c8eedf048533ca78668dd49fd90c Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 4 Nov 2024 10:32:46 -0500 Subject: [PATCH 05/16] chore(test): make sure to print the stacktrace in test logs (#2399) --- google-cloud-bigtable/src/test/resources/logging.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/test/resources/logging.properties b/google-cloud-bigtable/src/test/resources/logging.properties index 70319867bffa..e181b45a01e8 100644 --- a/google-cloud-bigtable/src/test/resources/logging.properties +++ b/google-cloud-bigtable/src/test/resources/logging.properties @@ -7,6 +7,6 @@ java.util.logging.ConsoleHandler.level = INFO java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter # time [level] loggerName: message -java.util.logging.SimpleFormatter.format=%1$tT [%4$-7s] %2$s: %5$s%n +java.util.logging.SimpleFormatter.format=%1$tT [%4$-7s] %2$s: %5$s %6$s%n From e7ffbda7eb4ad39c3c674fe69a8fd35c737bb748 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 4 Nov 2024 10:42:25 -0500 Subject: [PATCH 06/16] chore: remove unnecessary variable (#2400) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Ide37e9226983ffaa32d8d2cda03c1fd8d1ad7fff Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Rollback plan is reviewed and LGTMed - [ ] All new data plane features have a completed end to end testing plan Fixes # ☕️ If you write sample code, please follow the [samples format]( https://togithub.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md). --- .../v2/stub/metrics/BuiltinMetricsTracer.java | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index d4e27ff2ced3..14a112b27087 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -67,7 +67,6 @@ class BuiltinMetricsTracer extends BigtableTracer { // Stopwatch is not thread safe so this is a workaround to check if the stopwatch changes is // flushed to memory. private final Stopwatch serverLatencyTimer = Stopwatch.createUnstarted(); - private boolean serverLatencyTimerIsRunning = false; private final Object timerLock = new Object(); private boolean flowControlIsDisabled = false; @@ -163,9 +162,8 @@ public void attemptStarted(Object request, int attemptNumber) { } if (!flowControlIsDisabled) { synchronized (timerLock) { - if (!serverLatencyTimerIsRunning) { + if (!serverLatencyTimer.isRunning()) { serverLatencyTimer.start(); - serverLatencyTimerIsRunning = true; } } } @@ -198,9 +196,8 @@ public void onRequest(int requestCount) { // On request is only called when auto flow control is disabled. When auto flow control is // disabled, server latency is measured between onRequest and onResponse. synchronized (timerLock) { - if (!serverLatencyTimerIsRunning) { + if (!serverLatencyTimer.isRunning()) { serverLatencyTimer.start(); - serverLatencyTimerIsRunning = true; } } } @@ -219,10 +216,9 @@ public void responseReceived() { // latency is measured between afterResponse and responseReceived. // In all the cases, we want to stop the serverLatencyTimer here. synchronized (timerLock) { - if (serverLatencyTimerIsRunning) { + if (serverLatencyTimer.isRunning()) { totalServerLatencyNano.addAndGet(serverLatencyTimer.elapsed(TimeUnit.NANOSECONDS)); serverLatencyTimer.reset(); - serverLatencyTimerIsRunning = false; } } } @@ -235,9 +231,8 @@ public void afterResponse(long applicationLatency) { // received. If flow control is disabled but requestLeft is greater than 0, // also start the timer to count the time between afterResponse and responseReceived. synchronized (timerLock) { - if (!serverLatencyTimerIsRunning) { + if (!serverLatencyTimer.isRunning()) { serverLatencyTimer.start(); - serverLatencyTimerIsRunning = true; } } } @@ -324,11 +319,10 @@ private void recordAttemptCompletion(@Nullable Throwable status) { // If the attempt failed, the time spent in retry should be counted in application latency. // Stop the stopwatch and decrement requestLeft. synchronized (timerLock) { - if (serverLatencyTimerIsRunning) { + if (serverLatencyTimer.isRunning()) { requestLeft.decrementAndGet(); totalServerLatencyNano.addAndGet(serverLatencyTimer.elapsed(TimeUnit.NANOSECONDS)); serverLatencyTimer.reset(); - serverLatencyTimerIsRunning = false; } } From 3fa44fd605824b89898708c1ec65d2d69475cae9 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 4 Nov 2024 15:04:19 -0500 Subject: [PATCH 07/16] chore: simplify sample rows key callable chain (#2396) Previously there were 3 chain creators: 1. createSampleRowKeysBaseCallable 2. createSampleRowKeysWithRequestCallable 3. createSampleRowKeysCallable The primary reason for this is that SampleRowKeysWithRequest was introduced after createSampleRowKeysCallable because it supports authorized views. This pr simplifies the logic by moving everything into createSampleRowKeysWithRequestCallable and makes createSampleRowKeysCallable be a tiny shim to convert a String tableId into a SampleRowKeysRequest --- .../data/v2/stub/EnhancedBigtableStub.java | 89 ++++++------- .../data/v2/stub/SampleRowKeysCallable.java | 79 ----------- .../v2/stub/SampleRowKeysCallableTest.java | 125 ------------------ 3 files changed, 40 insertions(+), 253 deletions(-) delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/SampleRowKeysCallable.java delete mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SampleRowKeysCallableTest.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 91c63c2b85a8..3a9344268fcb 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 @@ -21,6 +21,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.INSTANCE_ID_KEY; import com.google.api.core.ApiFunction; +import com.google.api.core.ApiFuture; import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.api.gax.batching.Batcher; @@ -39,6 +40,7 @@ import com.google.api.gax.retrying.RetryAlgorithm; import com.google.api.gax.retrying.RetryingExecutorWithContext; import com.google.api.gax.retrying.ScheduledRetryingExecutor; +import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.Callables; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.RequestParamsExtractor; @@ -98,6 +100,7 @@ import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; import com.google.cloud.bigtable.data.v2.models.SampleRowKeysRequest; +import com.google.cloud.bigtable.data.v2.models.TableId; import com.google.cloud.bigtable.data.v2.models.TargetId; import com.google.cloud.bigtable.data.v2.models.sql.Statement; import com.google.cloud.bigtable.data.v2.stub.changestream.ChangeStreamRecordMergingCallable; @@ -194,7 +197,7 @@ public class EnhancedBigtableStub implements AutoCloseable { private final ServerStreamingCallable readRowsCallable; private final UnaryCallable readRowCallable; private final UnaryCallable> bulkReadRowsCallable; - private final UnaryCallable> sampleRowKeysCallable; + @Deprecated private final UnaryCallable> sampleRowKeysCallable; private final UnaryCallable> sampleRowKeysCallableWithRequest; private final UnaryCallable mutateRowCallable; @@ -698,11 +701,40 @@ private UnaryCallable> createBulkReadRowsCallable( } /** - * Helper function that should only be used by createSampleRowKeysCallable() and - * createSampleRowKeysWithRequestCallable(). + * Simple wrapper around {@link #createSampleRowKeysCallableWithRequest()} to provide backwards + * compatibility + * + * @deprecated + */ + @Deprecated + private UnaryCallable> createSampleRowKeysCallable() { + UnaryCallable> baseCallable = + createSampleRowKeysCallableWithRequest(); + return new UnaryCallable>() { + @Override + public ApiFuture> futureCall(String s, ApiCallContext apiCallContext) { + return baseCallable.futureCall(SampleRowKeysRequest.create(TableId.of(s)), apiCallContext); + } + }; + } + + /** + * Creates a callable chain to handle SampleRowKeys RPcs. The chain will: + * + *
    + *
  • Convert a {@link SampleRowKeysRequest} to a {@link + * com.google.bigtable.v2.SampleRowKeysRequest}. + *
  • Dispatch the request to the GAPIC's {@link BigtableStub#sampleRowKeysCallable()}. + *
  • Spool responses into a list. + *
  • Retry on failure. + *
  • Convert the responses into {@link KeyOffset}s. + *
  • Add tracing & metrics. + *
*/ - private UnaryCallable> - createSampleRowKeysBaseCallable() { + private UnaryCallable> + createSampleRowKeysCallableWithRequest() { + String methodName = "SampleRowKeys"; + ServerStreamingCallable base = GrpcRawCallableFactory.createServerStreamingCallable( @@ -745,51 +777,8 @@ public Map extract( UnaryCallable> retryable = withRetries(withBigtableTracer, settings.sampleRowKeysSettings()); - return retryable; - } - - /** - * Creates a callable chain to handle SampleRowKeys RPcs. The chain will: - * - *
    - *
  • Convert a table id to a {@link com.google.bigtable.v2.SampleRowKeysRequest}. - *
  • Dispatch the request to the GAPIC's {@link BigtableStub#sampleRowKeysCallable()}. - *
  • Spool responses into a list. - *
  • Retry on failure. - *
  • Convert the responses into {@link KeyOffset}s. - *
  • Add tracing & metrics. - *
- */ - private UnaryCallable> createSampleRowKeysCallable() { - String methodName = "SampleRowKeys"; - - UnaryCallable> - baseCallable = createSampleRowKeysBaseCallable(); - return createUserFacingUnaryCallable( - methodName, new SampleRowKeysCallable(baseCallable, requestContext)); - } - - /** - * Creates a callable chain to handle SampleRowKeys RPcs. The chain will: - * - *
    - *
  • Convert a {@link SampleRowKeysRequest} to a {@link - * com.google.bigtable.v2.SampleRowKeysRequest}. - *
  • Dispatch the request to the GAPIC's {@link BigtableStub#sampleRowKeysCallable()}. - *
  • Spool responses into a list. - *
  • Retry on failure. - *
  • Convert the responses into {@link KeyOffset}s. - *
  • Add tracing & metrics. - *
- */ - private UnaryCallable> - createSampleRowKeysCallableWithRequest() { - String methodName = "SampleRowKeys"; - - UnaryCallable> - baseCallable = createSampleRowKeysBaseCallable(); return createUserFacingUnaryCallable( - methodName, new SampleRowKeysCallableWithRequest(baseCallable, requestContext)); + methodName, new SampleRowKeysCallableWithRequest(retryable, requestContext)); } /** @@ -1470,6 +1459,8 @@ public UnaryCallable readRowCallable() { return readRowCallable; } + /** Deprecated, please use {@link #sampleRowKeysCallableWithRequest} */ + @Deprecated public UnaryCallable> sampleRowKeysCallable() { return sampleRowKeysCallable; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/SampleRowKeysCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/SampleRowKeysCallable.java deleted file mode 100644 index 7658e414922b..000000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/SampleRowKeysCallable.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright 2018 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.api.core.ApiFunction; -import com.google.api.core.ApiFuture; -import com.google.api.core.ApiFutures; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.bigtable.v2.SampleRowKeysRequest; -import com.google.bigtable.v2.SampleRowKeysResponse; -import com.google.cloud.bigtable.data.v2.internal.NameUtil; -import com.google.cloud.bigtable.data.v2.internal.RequestContext; -import com.google.cloud.bigtable.data.v2.models.KeyOffset; -import com.google.common.collect.ImmutableList; -import com.google.common.util.concurrent.MoreExecutors; -import java.util.List; - -/** Simple wrapper for SampleRowKeys to wrap the request and response protobufs. */ -class SampleRowKeysCallable extends UnaryCallable> { - private final RequestContext requestContext; - private final UnaryCallable> inner; - - SampleRowKeysCallable( - UnaryCallable> inner, - RequestContext requestContext) { - - this.requestContext = requestContext; - this.inner = inner; - } - - @Override - public ApiFuture> futureCall(String tableId, ApiCallContext context) { - String tableName = - NameUtil.formatTableName( - requestContext.getProjectId(), requestContext.getInstanceId(), tableId); - - SampleRowKeysRequest request = - SampleRowKeysRequest.newBuilder() - .setTableName(tableName) - .setAppProfileId(requestContext.getAppProfileId()) - .build(); - - ApiFuture> rawResponse = inner.futureCall(request, context); - - return ApiFutures.transform( - rawResponse, - new ApiFunction, List>() { - @Override - public List apply(List rawResponse) { - return convert(rawResponse); - } - }, - MoreExecutors.directExecutor()); - } - - private static List convert(List rawResponse) { - ImmutableList.Builder results = ImmutableList.builder(); - - for (SampleRowKeysResponse element : rawResponse) { - results.add(KeyOffset.create(element.getRowKey(), element.getOffsetBytes())); - } - - return results.build(); - } -} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SampleRowKeysCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SampleRowKeysCallableTest.java deleted file mode 100644 index 40a30d326393..000000000000 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SampleRowKeysCallableTest.java +++ /dev/null @@ -1,125 +0,0 @@ -/* - * Copyright 2018 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.common.truth.Truth.assertThat; - -import com.google.api.core.ApiFuture; -import com.google.api.core.SettableApiFuture; -import com.google.api.gax.grpc.GrpcStatusCode; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.NotFoundException; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.bigtable.v2.SampleRowKeysRequest; -import com.google.bigtable.v2.SampleRowKeysResponse; -import com.google.cloud.bigtable.data.v2.internal.NameUtil; -import com.google.cloud.bigtable.data.v2.internal.RequestContext; -import com.google.cloud.bigtable.data.v2.models.KeyOffset; -import com.google.common.collect.ImmutableList; -import com.google.protobuf.ByteString; -import io.grpc.Status.Code; -import java.util.List; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class SampleRowKeysCallableTest { - - private final RequestContext requestContext = - RequestContext.create("my-project", "my-instance", "my-profile"); - private FakeCallable inner; - private SampleRowKeysCallable callable; - - @Before - public void setUp() { - inner = new FakeCallable(); - callable = new SampleRowKeysCallable(inner, requestContext); - } - - @Test - public void requestIsCorrect() { - callable.futureCall("my-table"); - - assertThat(inner.request) - .isEqualTo( - SampleRowKeysRequest.newBuilder() - .setTableName( - NameUtil.formatTableName( - requestContext.getProjectId(), requestContext.getInstanceId(), "my-table")) - .setAppProfileId(requestContext.getAppProfileId()) - .build()); - } - - @Test - public void responseCorrectlyTransformed() throws Exception { - ApiFuture> result = callable.futureCall("my-table"); - - inner.response.set( - ImmutableList.of( - SampleRowKeysResponse.newBuilder() - .setRowKey(ByteString.copyFromUtf8("key1")) - .setOffsetBytes(100) - .build(), - SampleRowKeysResponse.newBuilder() - .setRowKey(ByteString.copyFromUtf8("")) - .setOffsetBytes(1000) - .build())); - - assertThat(result.get(1, TimeUnit.SECONDS)) - .isEqualTo( - ImmutableList.of( - KeyOffset.create(ByteString.copyFromUtf8("key1"), 100), - KeyOffset.create(ByteString.EMPTY, 1000))); - } - - @Test - public void errorIsPropagated() throws Exception { - ApiFuture> result = callable.futureCall("my-table"); - - Throwable expectedError = - new NotFoundException("fake error", null, GrpcStatusCode.of(Code.NOT_FOUND), false); - inner.response.setException(expectedError); - - Throwable actualError = null; - try { - result.get(1, TimeUnit.SECONDS); - } catch (ExecutionException e) { - actualError = e.getCause(); - } - - assertThat(actualError).isEqualTo(expectedError); - } - - static class FakeCallable - extends UnaryCallable> { - SampleRowKeysRequest request; - ApiCallContext callContext; - SettableApiFuture> response = SettableApiFuture.create(); - - @Override - public ApiFuture> futureCall( - SampleRowKeysRequest request, ApiCallContext context) { - this.request = request; - this.callContext = context; - - return response; - } - } -} From b40828c207cc1e2925bb5b23db1461bccb756f5b Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 4 Nov 2024 16:18:19 -0500 Subject: [PATCH 08/16] chore: refactor unary callable chains to avoid code duplication (#2394) This is primarily a code clean up change that extracts all of the unary chain building logic into a helper method and removes a significant amount of code duplication. --- .../v2/stub/CheckAndMutateRowCallable.java | 56 ----- .../data/v2/stub/EnhancedBigtableStub.java | 192 +++++++---------- .../data/v2/stub/MutateRowCallable.java | 56 ----- .../v2/stub/ReadModifyWriteRowCallable.java | 64 ------ .../stub/CheckAndMutateRowCallableTest.java | 123 ----------- .../v2/stub/EnhancedBigtableStubTest.java | 156 +++++++++++++- .../data/v2/stub/MutateRowCallableTest.java | 68 ------ .../stub/ReadModifyWriteRowCallableTest.java | 200 ------------------ 8 files changed, 234 insertions(+), 681 deletions(-) delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MutateRowCallable.java delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallable.java delete mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallableTest.java delete mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MutateRowCallableTest.java delete mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallableTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java deleted file mode 100644 index 549e10f44b95..000000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallable.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2018 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.api.core.ApiFunction; -import com.google.api.core.ApiFuture; -import com.google.api.core.ApiFutures; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.bigtable.v2.CheckAndMutateRowRequest; -import com.google.bigtable.v2.CheckAndMutateRowResponse; -import com.google.cloud.bigtable.data.v2.internal.RequestContext; -import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; -import com.google.common.util.concurrent.MoreExecutors; - -/** Simple wrapper for CheckAndMutateRow to wrap the request and response protobufs. */ -class CheckAndMutateRowCallable extends UnaryCallable { - private final UnaryCallable inner; - private final RequestContext requestContext; - - CheckAndMutateRowCallable( - UnaryCallable inner, - RequestContext requestContext) { - this.inner = inner; - this.requestContext = requestContext; - } - - @Override - public ApiFuture futureCall(ConditionalRowMutation request, ApiCallContext context) { - ApiFuture rawResponse = - inner.futureCall(request.toProto(requestContext), context); - - return ApiFutures.transform( - rawResponse, - new ApiFunction() { - @Override - public Boolean apply(CheckAndMutateRowResponse checkAndMutateRowResponse) { - return checkAndMutateRowResponse.getPredicateMatched(); - } - }, - MoreExecutors.directExecutor()); - } -} 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 3a9344268fcb..2b2ca38540af 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 @@ -22,6 +22,7 @@ import com.google.api.core.ApiFunction; import com.google.api.core.ApiFuture; +import com.google.api.core.ApiFutures; import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.api.gax.batching.Batcher; @@ -57,22 +58,17 @@ import com.google.auth.Credentials; import com.google.auth.oauth2.ServiceAccountJwtAccessCredentials; import com.google.bigtable.v2.BigtableGrpc; -import com.google.bigtable.v2.CheckAndMutateRowRequest; import com.google.bigtable.v2.CheckAndMutateRowResponse; import com.google.bigtable.v2.ExecuteQueryRequest; import com.google.bigtable.v2.ExecuteQueryResponse; import com.google.bigtable.v2.GenerateInitialChangeStreamPartitionsRequest; import com.google.bigtable.v2.GenerateInitialChangeStreamPartitionsResponse; -import com.google.bigtable.v2.MutateRowRequest; -import com.google.bigtable.v2.MutateRowResponse; import com.google.bigtable.v2.MutateRowsRequest; import com.google.bigtable.v2.MutateRowsResponse; import com.google.bigtable.v2.PingAndWarmRequest; import com.google.bigtable.v2.PingAndWarmResponse; import com.google.bigtable.v2.ReadChangeStreamRequest; import com.google.bigtable.v2.ReadChangeStreamResponse; -import com.google.bigtable.v2.ReadModifyWriteRowRequest; -import com.google.bigtable.v2.ReadModifyWriteRowResponse; import com.google.bigtable.v2.ReadRowsRequest; import com.google.bigtable.v2.ReadRowsResponse; import com.google.bigtable.v2.RowRange; @@ -144,8 +140,10 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.MoreExecutors; import com.google.protobuf.ByteString; import io.grpc.ManagedChannelBuilder; +import io.grpc.MethodDescriptor; import io.opencensus.stats.Stats; import io.opencensus.stats.StatsRecorder; import io.opencensus.tags.TagKey; @@ -162,6 +160,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.function.Function; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nonnull; @@ -790,42 +789,14 @@ public Map extract( * */ private UnaryCallable createMutateRowCallable() { - String methodName = "MutateRow"; - UnaryCallable base = - GrpcRawCallableFactory.createUnaryCallable( - GrpcCallSettings.newBuilder() - .setMethodDescriptor(BigtableGrpc.getMutateRowMethod()) - .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract(MutateRowRequest mutateRowRequest) { - String tableName = mutateRowRequest.getTableName(); - String authorizedViewName = mutateRowRequest.getAuthorizedViewName(); - if (tableName.isEmpty()) { - tableName = - NameUtil.extractTableNameFromAuthorizedViewName(authorizedViewName); - } - return ImmutableMap.of( - "table_name", - tableName, - "app_profile_id", - mutateRowRequest.getAppProfileId()); - } - }) - .build(), - settings.mutateRowSettings().getRetryableCodes()); - - UnaryCallable withStatsHeaders = - new StatsHeadersUnaryCallable<>(base); - - UnaryCallable withBigtableTracer = - new BigtableTracerUnaryCallable<>(withStatsHeaders); - - UnaryCallable retrying = - withRetries(withBigtableTracer, settings.mutateRowSettings()); - - return createUserFacingUnaryCallable( - methodName, new MutateRowCallable(retrying, requestContext)); + return createUnaryCallable( + BigtableGrpc.getMutateRowMethod(), + req -> + composeRequestParams( + req.getAppProfileId(), req.getTableName(), req.getAuthorizedViewName()), + settings.mutateRowSettings(), + req -> req.toProto(requestContext), + resp -> null); } /** @@ -1045,44 +1016,14 @@ public Batcher newBulkReadRowsBatcher( * */ private UnaryCallable createCheckAndMutateRowCallable() { - String methodName = "CheckAndMutateRow"; - UnaryCallable base = - GrpcRawCallableFactory.createUnaryCallable( - GrpcCallSettings.newBuilder() - .setMethodDescriptor(BigtableGrpc.getCheckAndMutateRowMethod()) - .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract( - CheckAndMutateRowRequest checkAndMutateRowRequest) { - String tableName = checkAndMutateRowRequest.getTableName(); - String authorizedViewName = - checkAndMutateRowRequest.getAuthorizedViewName(); - if (tableName.isEmpty()) { - tableName = - NameUtil.extractTableNameFromAuthorizedViewName(authorizedViewName); - } - return ImmutableMap.of( - "table_name", - tableName, - "app_profile_id", - checkAndMutateRowRequest.getAppProfileId()); - } - }) - .build(), - settings.checkAndMutateRowSettings().getRetryableCodes()); - - UnaryCallable withStatsHeaders = - new StatsHeadersUnaryCallable<>(base); - - UnaryCallable withBigtableTracer = - new BigtableTracerUnaryCallable<>(withStatsHeaders); - - UnaryCallable retrying = - withRetries(withBigtableTracer, settings.checkAndMutateRowSettings()); - - return createUserFacingUnaryCallable( - methodName, new CheckAndMutateRowCallable(retrying, requestContext)); + return createUnaryCallable( + BigtableGrpc.getCheckAndMutateRowMethod(), + req -> + composeRequestParams( + req.getAppProfileId(), req.getTableName(), req.getAuthorizedViewName()), + settings.checkAndMutateRowSettings(), + req -> req.toProto(requestContext), + CheckAndMutateRowResponse::getPredicateMatched); } /** @@ -1096,39 +1037,16 @@ public Map extract( * */ private UnaryCallable createReadModifyWriteRowCallable() { - UnaryCallable base = - GrpcRawCallableFactory.createUnaryCallable( - GrpcCallSettings.newBuilder() - .setMethodDescriptor(BigtableGrpc.getReadModifyWriteRowMethod()) - .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract(ReadModifyWriteRowRequest request) { - String tableName = request.getTableName(); - String authorizedViewName = request.getAuthorizedViewName(); - if (tableName.isEmpty()) { - tableName = - NameUtil.extractTableNameFromAuthorizedViewName(authorizedViewName); - } - return ImmutableMap.of( - "table_name", tableName, "app_profile_id", request.getAppProfileId()); - } - }) - .build(), - settings.readModifyWriteRowSettings().getRetryableCodes()); - - UnaryCallable withStatsHeaders = - new StatsHeadersUnaryCallable<>(base); - - String methodName = "ReadModifyWriteRow"; - UnaryCallable withBigtableTracer = - new BigtableTracerUnaryCallable<>(withStatsHeaders); - - UnaryCallable retrying = - withRetries(withBigtableTracer, settings.readModifyWriteRowSettings()); - - return createUserFacingUnaryCallable( - methodName, new ReadModifyWriteRowCallable(retrying, requestContext)); + DefaultRowAdapter rowAdapter = new DefaultRowAdapter(); + + return createUnaryCallable( + BigtableGrpc.getReadModifyWriteRowMethod(), + req -> + composeRequestParams( + req.getAppProfileId(), req.getTableName(), req.getAuthorizedViewName()), + settings.readModifyWriteRowSettings(), + req -> req.toProto(requestContext), + resp -> rowAdapter.createRowFromProto(resp.getRow())); } /** @@ -1393,6 +1311,56 @@ private UnaryCallable createUserFacin return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } + private Map composeRequestParams( + String appProfileId, String tableName, String authorizedViewName) { + if (tableName.isEmpty()) { + tableName = NameUtil.extractTableNameFromAuthorizedViewName(authorizedViewName); + } + return ImmutableMap.of("table_name", tableName, "app_profile_id", appProfileId); + } + + private UnaryCallable createUnaryCallable( + MethodDescriptor methodDescriptor, + RequestParamsExtractor headerParamsFn, + UnaryCallSettings callSettings, + Function requestTransformer, + Function responseTranformer) { + + UnaryCallable base = + GrpcRawCallableFactory.createUnaryCallable( + GrpcCallSettings.newBuilder() + .setMethodDescriptor(methodDescriptor) + .setParamsExtractor(headerParamsFn) + .build(), + callSettings.getRetryableCodes()); + + UnaryCallable withStatsHeaders = new StatsHeadersUnaryCallable<>(base); + + UnaryCallable withBigtableTracer = + new BigtableTracerUnaryCallable<>(withStatsHeaders); + + UnaryCallable retrying = withRetries(withBigtableTracer, callSettings); + + UnaryCallable transformed = + new UnaryCallable() { + @Override + public ApiFuture futureCall(ReqT reqT, ApiCallContext apiCallContext) { + ApiFuture f = + retrying.futureCall(requestTransformer.apply(reqT), apiCallContext); + return ApiFutures.transform( + f, responseTranformer::apply, MoreExecutors.directExecutor()); + } + }; + + UnaryCallable traced = + new TracedUnaryCallable<>( + transformed, + clientContext.getTracerFactory(), + getSpanName(methodDescriptor.getBareMethodName())); + + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + } + private UnaryCallable createPingAndWarmCallable() { UnaryCallable pingAndWarm = GrpcRawCallableFactory.createUnaryCallable( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MutateRowCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MutateRowCallable.java deleted file mode 100644 index 36f47c2d1f92..000000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/MutateRowCallable.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2018 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.api.core.ApiFunction; -import com.google.api.core.ApiFuture; -import com.google.api.core.ApiFutures; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.bigtable.v2.MutateRowRequest; -import com.google.bigtable.v2.MutateRowResponse; -import com.google.cloud.bigtable.data.v2.internal.RequestContext; -import com.google.cloud.bigtable.data.v2.models.RowMutation; -import com.google.common.util.concurrent.MoreExecutors; - -/** Simple wrapper for MutateRow to wrap the request and response protobufs. */ -class MutateRowCallable extends UnaryCallable { - private final UnaryCallable inner; - private final RequestContext requestContext; - - MutateRowCallable( - UnaryCallable inner, RequestContext requestContext) { - - this.inner = inner; - this.requestContext = requestContext; - } - - @Override - public ApiFuture futureCall(RowMutation request, ApiCallContext context) { - ApiFuture rawResponse = - inner.futureCall(request.toProto(requestContext), context); - - return ApiFutures.transform( - rawResponse, - new ApiFunction() { - @Override - public Void apply(MutateRowResponse mutateRowResponse) { - return null; - } - }, - MoreExecutors.directExecutor()); - } -} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallable.java deleted file mode 100644 index 09e133678e6a..000000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallable.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright 2018 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.api.core.ApiFunction; -import com.google.api.core.ApiFuture; -import com.google.api.core.ApiFutures; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.bigtable.v2.ReadModifyWriteRowRequest; -import com.google.bigtable.v2.ReadModifyWriteRowResponse; -import com.google.cloud.bigtable.data.v2.internal.RequestContext; -import com.google.cloud.bigtable.data.v2.models.DefaultRowAdapter; -import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; -import com.google.cloud.bigtable.data.v2.models.Row; -import com.google.common.util.concurrent.MoreExecutors; - -/** Simple wrapper for ReadModifyWriteRow to wrap the request and response protobufs. */ -class ReadModifyWriteRowCallable extends UnaryCallable { - private final UnaryCallable inner; - private final RequestContext requestContext; - private final DefaultRowAdapter rowAdapter; - - ReadModifyWriteRowCallable( - UnaryCallable inner, - RequestContext requestContext) { - this.inner = inner; - this.requestContext = requestContext; - this.rowAdapter = new DefaultRowAdapter(); - } - - @Override - public ApiFuture futureCall(ReadModifyWriteRow request, ApiCallContext context) { - ApiFuture rawResponse = - inner.futureCall(request.toProto(requestContext), context); - - return ApiFutures.transform( - rawResponse, - new ApiFunction() { - @Override - public Row apply(ReadModifyWriteRowResponse readModifyWriteRowResponse) { - return convertResponse(readModifyWriteRowResponse); - } - }, - MoreExecutors.directExecutor()); - } - - private Row convertResponse(ReadModifyWriteRowResponse response) { - return rowAdapter.createRowFromProto(response.getRow()); - } -} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallableTest.java deleted file mode 100644 index 5441f1d1f8ec..000000000000 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CheckAndMutateRowCallableTest.java +++ /dev/null @@ -1,123 +0,0 @@ -/* - * Copyright 2018 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.common.truth.Truth.assertThat; - -import com.google.api.core.ApiFuture; -import com.google.api.core.SettableApiFuture; -import com.google.api.gax.grpc.GrpcStatusCode; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.NotFoundException; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.bigtable.v2.CheckAndMutateRowRequest; -import com.google.bigtable.v2.CheckAndMutateRowResponse; -import com.google.bigtable.v2.Mutation.DeleteFromRow; -import com.google.cloud.bigtable.data.v2.internal.NameUtil; -import com.google.cloud.bigtable.data.v2.internal.RequestContext; -import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; -import com.google.cloud.bigtable.data.v2.models.Mutation; -import com.google.protobuf.ByteString; -import io.grpc.Status.Code; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class CheckAndMutateRowCallableTest { - - private final RequestContext requestContext = - RequestContext.create("my-project", "my-instance", "my-app-profile"); - private FakeCallable inner; - private CheckAndMutateRowCallable callable; - - @Before - public void setUp() { - inner = new FakeCallable(); - callable = new CheckAndMutateRowCallable(inner, requestContext); - } - - @Test - public void requestIsCorrect() { - callable.futureCall( - ConditionalRowMutation.create("my-table", "row-key").then(Mutation.create().deleteRow())); - - assertThat(inner.request) - .isEqualTo( - CheckAndMutateRowRequest.newBuilder() - .setTableName( - NameUtil.formatTableName( - requestContext.getProjectId(), requestContext.getInstanceId(), "my-table")) - .setRowKey(ByteString.copyFromUtf8("row-key")) - .setAppProfileId(requestContext.getAppProfileId()) - .addTrueMutations( - com.google.bigtable.v2.Mutation.newBuilder() - .setDeleteFromRow(DeleteFromRow.getDefaultInstance())) - .build()); - } - - @Test - public void responseCorrectlyTransformed() throws Exception { - ApiFuture result = - callable.futureCall( - ConditionalRowMutation.create("my-table", "row-key") - .then(Mutation.create().deleteRow())); - - inner.response.set(CheckAndMutateRowResponse.newBuilder().setPredicateMatched(true).build()); - - assertThat(result.get(1, TimeUnit.SECONDS)).isEqualTo(true); - } - - @Test - public void errorIsPropagated() throws Exception { - ApiFuture result = - callable.futureCall( - ConditionalRowMutation.create("my-table", "row-key") - .then(Mutation.create().deleteRow())); - - Throwable expectedError = - new NotFoundException("fake error", null, GrpcStatusCode.of(Code.NOT_FOUND), false); - inner.response.setException(expectedError); - - Throwable actualError = null; - try { - result.get(1, TimeUnit.SECONDS); - } catch (ExecutionException e) { - actualError = e.getCause(); - } - - assertThat(actualError).isEqualTo(expectedError); - } - - static class FakeCallable - extends UnaryCallable { - CheckAndMutateRowRequest request; - ApiCallContext callContext; - SettableApiFuture response = SettableApiFuture.create(); - - @Override - public ApiFuture futureCall( - CheckAndMutateRowRequest request, ApiCallContext context) { - this.request = request; - this.callContext = context; - - return response; - } - } -} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java index 50d086b7110d..495250fe1354 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java @@ -37,6 +37,7 @@ import com.google.api.gax.grpc.GaxGrpcProperties; import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcTransportChannel; +import com.google.api.gax.rpc.FailedPreconditionException; import com.google.api.gax.rpc.FixedTransportChannelProvider; import com.google.api.gax.rpc.InstantiatingWatchdogProvider; import com.google.api.gax.rpc.ServerStream; @@ -44,15 +45,21 @@ import com.google.api.gax.rpc.WatchdogTimeoutException; import com.google.auth.oauth2.ServiceAccountJwtAccessCredentials; import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.CheckAndMutateRowRequest; +import com.google.bigtable.v2.CheckAndMutateRowResponse; import com.google.bigtable.v2.ExecuteQueryRequest; import com.google.bigtable.v2.ExecuteQueryResponse; import com.google.bigtable.v2.FeatureFlags; +import com.google.bigtable.v2.MutateRowRequest; +import com.google.bigtable.v2.MutateRowResponse; import com.google.bigtable.v2.MutateRowsRequest; import com.google.bigtable.v2.MutateRowsResponse; import com.google.bigtable.v2.PingAndWarmRequest; import com.google.bigtable.v2.PingAndWarmResponse; import com.google.bigtable.v2.ReadChangeStreamRequest; import com.google.bigtable.v2.ReadChangeStreamResponse; +import com.google.bigtable.v2.ReadModifyWriteRowRequest; +import com.google.bigtable.v2.ReadModifyWriteRowResponse; import com.google.bigtable.v2.ReadRowsRequest; import com.google.bigtable.v2.ReadRowsResponse; import com.google.bigtable.v2.RowSet; @@ -62,7 +69,19 @@ import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; import com.google.cloud.bigtable.data.v2.internal.RequestContext; import com.google.cloud.bigtable.data.v2.internal.SqlRow; -import com.google.cloud.bigtable.data.v2.models.*; +import com.google.cloud.bigtable.data.v2.models.BulkMutation; +import com.google.cloud.bigtable.data.v2.models.ChangeStreamRecord; +import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; +import com.google.cloud.bigtable.data.v2.models.DefaultRowAdapter; +import com.google.cloud.bigtable.data.v2.models.Filters; +import com.google.cloud.bigtable.data.v2.models.Mutation; +import com.google.cloud.bigtable.data.v2.models.Query; +import com.google.cloud.bigtable.data.v2.models.ReadChangeStreamQuery; +import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; +import com.google.cloud.bigtable.data.v2.models.Row; +import com.google.cloud.bigtable.data.v2.models.RowMutation; +import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; +import com.google.cloud.bigtable.data.v2.models.TableId; import com.google.cloud.bigtable.data.v2.models.sql.ResultSetMetadata; import com.google.cloud.bigtable.data.v2.models.sql.Statement; import com.google.cloud.bigtable.data.v2.stub.sql.ExecuteQueryCallable; @@ -75,6 +94,7 @@ import com.google.protobuf.StringValue; import com.google.rpc.Code; import com.google.rpc.Status; +import io.grpc.CallOptions; import io.grpc.Context; import io.grpc.Deadline; import io.grpc.ManagedChannel; @@ -105,6 +125,7 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -118,8 +139,9 @@ public class EnhancedBigtableStubTest { private static final String PROJECT_ID = "fake-project"; private static final String INSTANCE_ID = "fake-instance"; + private static final String TABLE_ID = "fake-table"; private static final String TABLE_NAME = - NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, "fake-table"); + NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, TABLE_ID); private static final String APP_PROFILE_ID = "app-profile-id"; private static final String WAIT_TIME_TABLE_ID = "test-wait-timeout"; private static final String WAIT_TIME_QUERY = "test-wait-timeout"; @@ -269,6 +291,101 @@ public void testFeatureFlags() throws InterruptedException, IOException, Executi assertThat(featureFlags.getLastScannedRowResponses()).isTrue(); } + @Test + public void testCheckAndMutateRequestResponseConversion() + throws ExecutionException, InterruptedException { + ConditionalRowMutation req = + ConditionalRowMutation.create(TableId.of("my-table"), "my-key") + .condition(Filters.FILTERS.pass()) + .then(Mutation.create().deleteRow()); + + ApiFuture f = enhancedBigtableStub.checkAndMutateRowCallable().futureCall(req, null); + f.get(); + + CheckAndMutateRowRequest protoReq = + fakeDataService.checkAndMutateRowRequests.poll(1, TimeUnit.SECONDS); + assertThat(protoReq) + .isEqualTo(req.toProto(RequestContext.create(PROJECT_ID, INSTANCE_ID, APP_PROFILE_ID))); + assertThat(f.get()).isEqualTo(true); + } + + @Test + public void testRMWRequestResponseConversion() throws ExecutionException, InterruptedException { + ReadModifyWriteRow req = + ReadModifyWriteRow.create(TableId.of("my-table"), "my-key").append("f", "q", "v"); + + ApiFuture f = enhancedBigtableStub.readModifyWriteRowCallable().futureCall(req, null); + f.get(); + + ReadModifyWriteRowRequest protoReq = fakeDataService.rmwRequests.poll(1, TimeUnit.SECONDS); + assertThat(protoReq) + .isEqualTo(req.toProto(RequestContext.create(PROJECT_ID, INSTANCE_ID, APP_PROFILE_ID))); + assertThat(f.get().getKey()).isEqualTo(ByteString.copyFromUtf8("my-key")); + } + + @Test + public void testMutateRowRequestResponseConversion() + throws ExecutionException, InterruptedException { + RowMutation req = RowMutation.create(TableId.of("my-table"), "my-key").deleteRow(); + CallOptions.Key testKey = CallOptions.Key.create("test-key"); + + GrpcCallContext ctx = + GrpcCallContext.createDefault() + .withCallOptions(CallOptions.DEFAULT.withOption(testKey, "callopt-value")); + ApiFuture f = enhancedBigtableStub.mutateRowCallable().futureCall(req, ctx); + f.get(); + + MutateRowRequest protoReq = fakeDataService.mutateRowRequests.poll(1, TimeUnit.SECONDS); + assertThat(protoReq) + .isEqualTo(req.toProto(RequestContext.create(PROJECT_ID, INSTANCE_ID, APP_PROFILE_ID))); + assertThat(f.get()).isEqualTo(null); + } + + @Test + public void testMutateRowRequestParams() throws ExecutionException, InterruptedException { + RowMutation req = RowMutation.create(TableId.of(TABLE_ID), "my-key").deleteRow(); + + ApiFuture f = enhancedBigtableStub.mutateRowCallable().futureCall(req, null); + f.get(); + + Metadata reqMetadata = metadataInterceptor.headers.poll(1, TimeUnit.SECONDS); + + // RequestParamsExtractor + String reqParams = + reqMetadata.get(Key.of("x-goog-request-params", Metadata.ASCII_STRING_MARSHALLER)); + assertThat(reqParams).contains("table_name=" + TABLE_NAME.replace("/", "%2F")); + assertThat(reqParams).contains(String.format("app_profile_id=%s", APP_PROFILE_ID)); + + // StatsHeadersUnaryCallable + assertThat(reqMetadata.keys()).contains("bigtable-client-attempt-epoch-usec"); + + assertThat(f.get()).isEqualTo(null); + } + + @Test + public void testMutateRowErrorPropagation() { + AtomicInteger invocationCount = new AtomicInteger(); + Mockito.doAnswer( + invocationOnMock -> { + StreamObserver observer = invocationOnMock.getArgument(1); + if (invocationCount.getAndIncrement() == 0) { + observer.onError(io.grpc.Status.UNAVAILABLE.asRuntimeException()); + } else { + observer.onError(io.grpc.Status.FAILED_PRECONDITION.asRuntimeException()); + } + return null; + }) + .when(fakeDataService) + .mutateRow(Mockito.any(), Mockito.any(StreamObserver.class)); + + RowMutation req = RowMutation.create(TableId.of(TABLE_ID), "my-key").deleteRow(); + ApiFuture f = enhancedBigtableStub.mutateRowCallable().futureCall(req, null); + + ExecutionException e = assertThrows(ExecutionException.class, f::get); + assertThat(e.getCause()).isInstanceOf(FailedPreconditionException.class); + assertThat(invocationCount.get()).isEqualTo(2); + } + @Test public void testCreateReadRowsCallable() throws InterruptedException { ServerStreamingCallable streamingCallable = @@ -751,6 +868,10 @@ private static class FakeDataService extends BigtableGrpc.BigtableImplBase { Queues.newLinkedBlockingDeque(); final BlockingQueue pingRequests = Queues.newLinkedBlockingDeque(); final BlockingQueue executeQueryRequests = Queues.newLinkedBlockingDeque(); + final BlockingQueue mutateRowRequests = Queues.newLinkedBlockingDeque(); + final BlockingQueue checkAndMutateRowRequests = + Queues.newLinkedBlockingDeque(); + final BlockingQueue rmwRequests = Queues.newLinkedBlockingDeque(); @SuppressWarnings("unchecked") ReadRowsRequest popLastRequest() throws InterruptedException { @@ -761,6 +882,37 @@ ExecuteQueryRequest popLastExecuteQueryRequest() throws InterruptedException { return executeQueryRequests.poll(1, TimeUnit.SECONDS); } + @Override + public void mutateRow( + MutateRowRequest request, StreamObserver responseObserver) { + mutateRowRequests.add(request); + + responseObserver.onNext(MutateRowResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + + @Override + public void checkAndMutateRow( + CheckAndMutateRowRequest request, + StreamObserver responseObserver) { + checkAndMutateRowRequests.add(request); + responseObserver.onNext( + CheckAndMutateRowResponse.newBuilder().setPredicateMatched(true).build()); + responseObserver.onCompleted(); + } + + @Override + public void readModifyWriteRow( + ReadModifyWriteRowRequest request, + StreamObserver responseObserver) { + rmwRequests.add(request); + responseObserver.onNext( + ReadModifyWriteRowResponse.newBuilder() + .setRow(com.google.bigtable.v2.Row.newBuilder().setKey(request.getRowKey())) + .build()); + responseObserver.onCompleted(); + } + @Override public void mutateRows( MutateRowsRequest request, StreamObserver responseObserver) { diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MutateRowCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MutateRowCallableTest.java deleted file mode 100644 index 4792b668905e..000000000000 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/MutateRowCallableTest.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright 2018 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.api.core.SettableApiFuture; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.bigtable.v2.MutateRowRequest; -import com.google.bigtable.v2.MutateRowResponse; -import com.google.cloud.bigtable.data.v2.internal.RequestContext; -import com.google.cloud.bigtable.data.v2.models.RowMutation; -import com.google.common.primitives.Longs; -import com.google.common.truth.Truth; -import com.google.protobuf.ByteString; -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; - -@RunWith(JUnit4.class) -public class MutateRowCallableTest { - - private static final RequestContext REQUEST_CONTEXT = - RequestContext.create("fake-project", "fake-instance", "fake-profile"); - private UnaryCallable innerCallable; - private ArgumentCaptor innerMutation; - private SettableApiFuture innerResult; - - @SuppressWarnings("unchecked") - @Before - public void setUp() { - innerCallable = Mockito.mock(UnaryCallable.class); - innerMutation = ArgumentCaptor.forClass(MutateRowRequest.class); - innerResult = SettableApiFuture.create(); - Mockito.when(innerCallable.futureCall(innerMutation.capture(), Mockito.any())) - .thenReturn(innerResult); - } - - @Test - public void testRequestConversion() { - MutateRowCallable callable = new MutateRowCallable(innerCallable, REQUEST_CONTEXT); - RowMutation outerRequest = - RowMutation.create("fake-table", "fake-key") - .setCell("fake-family", "fake-qualifier", 1_000, "fake-value") - .addToCell("family-2", "qualifier", 1_000, 1234) - .mergeToCell( - "family-2", "qualifier2", 1_000, ByteString.copyFrom(Longs.toByteArray(1234L))); - - innerResult.set(MutateRowResponse.getDefaultInstance()); - callable.call(outerRequest); - - Truth.assertThat(innerMutation.getValue()).isEqualTo(outerRequest.toProto(REQUEST_CONTEXT)); - } -} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallableTest.java deleted file mode 100644 index 4a8f857d0530..000000000000 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallableTest.java +++ /dev/null @@ -1,200 +0,0 @@ -/* - * Copyright 2018 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.common.truth.Truth.assertThat; - -import com.google.api.core.ApiFuture; -import com.google.api.core.SettableApiFuture; -import com.google.api.gax.grpc.GrpcStatusCode; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.NotFoundException; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.bigtable.v2.Cell; -import com.google.bigtable.v2.Column; -import com.google.bigtable.v2.Family; -import com.google.bigtable.v2.ReadModifyWriteRowRequest; -import com.google.bigtable.v2.ReadModifyWriteRowResponse; -import com.google.bigtable.v2.ReadModifyWriteRule; -import com.google.cloud.bigtable.data.v2.internal.NameUtil; -import com.google.cloud.bigtable.data.v2.internal.RequestContext; -import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; -import com.google.cloud.bigtable.data.v2.models.Row; -import com.google.cloud.bigtable.data.v2.models.RowCell; -import com.google.common.collect.ImmutableList; -import com.google.protobuf.ByteString; -import io.grpc.Status.Code; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class ReadModifyWriteRowCallableTest { - private final RequestContext requestContext = - RequestContext.create("fake-project", "fake-instance", "fake-profile"); - private FakeCallable inner; - private ReadModifyWriteRowCallable callable; - - @Before - public void setUp() { - inner = new FakeCallable(); - callable = new ReadModifyWriteRowCallable(inner, requestContext); - } - - @Test - public void requestIsCorrect() { - callable.futureCall( - ReadModifyWriteRow.create("my-table", "my-key").append("my-family", "", "suffix")); - - assertThat(inner.request) - .isEqualTo( - ReadModifyWriteRowRequest.newBuilder() - .setTableName( - NameUtil.formatTableName( - requestContext.getProjectId(), requestContext.getInstanceId(), "my-table")) - .setAppProfileId(requestContext.getAppProfileId()) - .setRowKey(ByteString.copyFromUtf8("my-key")) - .addRules( - ReadModifyWriteRule.newBuilder() - .setFamilyName("my-family") - .setColumnQualifier(ByteString.EMPTY) - .setAppendValue(ByteString.copyFromUtf8("suffix"))) - .build()); - } - - @Test - public void responseCorrectlyTransformed() throws Exception { - ApiFuture result = - callable.futureCall( - ReadModifyWriteRow.create("my-table", "my-key").append("my-family", "col", "suffix")); - - inner.response.set( - ReadModifyWriteRowResponse.newBuilder() - .setRow( - com.google.bigtable.v2.Row.newBuilder() - .setKey(ByteString.copyFromUtf8("my-key")) - .addFamilies( - Family.newBuilder() - .setName("my-family") - .addColumns( - Column.newBuilder() - .setQualifier(ByteString.copyFromUtf8("col")) - .addCells( - Cell.newBuilder() - .setTimestampMicros(1_000) - .setValue(ByteString.copyFromUtf8("suffix")))))) - .build()); - - assertThat(result.get(1, TimeUnit.SECONDS)) - .isEqualTo( - Row.create( - ByteString.copyFromUtf8("my-key"), - ImmutableList.of( - RowCell.create( - "my-family", - ByteString.copyFromUtf8("col"), - 1_000, - ImmutableList.of(), - ByteString.copyFromUtf8("suffix"))))); - } - - @Test - public void responseSortsFamilies() throws Exception { - ByteString col = ByteString.copyFromUtf8("col1"); - ByteString value1 = ByteString.copyFromUtf8("value1"); - ByteString value2 = ByteString.copyFromUtf8("value2"); - - ApiFuture result = - callable.futureCall( - ReadModifyWriteRow.create("my-table", "my-key").append("my-family", "col", "suffix")); - - inner.response.set( - ReadModifyWriteRowResponse.newBuilder() - .setRow( - com.google.bigtable.v2.Row.newBuilder() - .setKey(ByteString.copyFromUtf8("my-key")) - // family2 is out of order - .addFamilies( - Family.newBuilder() - .setName("family2") - .addColumns( - Column.newBuilder() - .setQualifier(col) - .addCells( - Cell.newBuilder() - .setTimestampMicros(1_000) - .setValue(value2)))) - .addFamilies( - Family.newBuilder() - .setName("family1") - .addColumns( - Column.newBuilder() - .setQualifier(col) - .addCells( - Cell.newBuilder() - .setTimestampMicros(1_000) - .setValue(value1))) - .build())) - .build()); - - assertThat(result.get(1, TimeUnit.SECONDS)) - .isEqualTo( - Row.create( - ByteString.copyFromUtf8("my-key"), - ImmutableList.of( - RowCell.create("family1", col, 1_000, ImmutableList.of(), value1), - RowCell.create("family2", col, 1_000, ImmutableList.of(), value2)))); - } - - @Test - public void errorIsPropagated() throws Exception { - ApiFuture result = - callable.futureCall( - ReadModifyWriteRow.create("my-table", "my-key").append("my-family", "", "suffix")); - - Throwable expectedError = - new NotFoundException("fake error", null, GrpcStatusCode.of(Code.NOT_FOUND), false); - inner.response.setException(expectedError); - - Throwable actualError = null; - try { - result.get(1, TimeUnit.SECONDS); - } catch (ExecutionException e) { - actualError = e.getCause(); - } - - assertThat(actualError).isEqualTo(expectedError); - } - - static class FakeCallable - extends UnaryCallable { - ReadModifyWriteRowRequest request; - ApiCallContext callContext; - SettableApiFuture response = SettableApiFuture.create(); - - @Override - public ApiFuture futureCall( - ReadModifyWriteRowRequest request, ApiCallContext context) { - this.request = request; - this.callContext = context; - - return response; - } - } -} From bcf60c259f880d19b0a5661ad6c113c8b3c809ee Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Mon, 4 Nov 2024 17:26:23 -0500 Subject: [PATCH 09/16] chore: extract all table related request params extractors (#2402) --- .../data/v2/stub/EnhancedBigtableStub.java | 85 +++---------------- 1 file changed, 12 insertions(+), 73 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 2b2ca38540af..7dcc8dc318ad 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 @@ -603,22 +603,9 @@ private ServerStreamingCallable createReadRo GrpcCallSettings.newBuilder() .setMethodDescriptor(BigtableGrpc.getReadRowsMethod()) .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract(ReadRowsRequest readRowsRequest) { - String tableName = readRowsRequest.getTableName(); - String authorizedViewName = readRowsRequest.getAuthorizedViewName(); - if (tableName.isEmpty()) { - tableName = - NameUtil.extractTableNameFromAuthorizedViewName(authorizedViewName); - } - return ImmutableMap.of( - "table_name", - tableName, - "app_profile_id", - readRowsRequest.getAppProfileId()); - } - }) + r -> + composeRequestParams( + r.getAppProfileId(), r.getTableName(), r.getAuthorizedViewName())) .build(), readRowsSettings.getRetryableCodes()); @@ -742,25 +729,9 @@ public ApiFuture> futureCall(String s, ApiCallContext apiCallCon newBuilder() .setMethodDescriptor(BigtableGrpc.getSampleRowKeysMethod()) .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract( - com.google.bigtable.v2.SampleRowKeysRequest sampleRowKeysRequest) { - String tableName = sampleRowKeysRequest.getTableName(); - String authorizedViewName = - sampleRowKeysRequest.getAuthorizedViewName(); - if (tableName.isEmpty()) { - tableName = - NameUtil.extractTableNameFromAuthorizedViewName( - authorizedViewName); - } - return ImmutableMap.of( - "table_name", - tableName, - "app_profile_id", - sampleRowKeysRequest.getAppProfileId()); - } - }) + r -> + composeRequestParams( + r.getAppProfileId(), r.getTableName(), r.getAuthorizedViewName())) .build(), settings.sampleRowKeysSettings().getRetryableCodes()); @@ -823,22 +794,9 @@ private UnaryCallable createMutateRowsBas GrpcCallSettings.newBuilder() .setMethodDescriptor(BigtableGrpc.getMutateRowsMethod()) .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract(MutateRowsRequest mutateRowsRequest) { - String tableName = mutateRowsRequest.getTableName(); - String authorizedViewName = mutateRowsRequest.getAuthorizedViewName(); - if (tableName.isEmpty()) { - tableName = - NameUtil.extractTableNameFromAuthorizedViewName(authorizedViewName); - } - return ImmutableMap.of( - "table_name", - tableName, - "app_profile_id", - mutateRowsRequest.getAppProfileId()); - } - }) + r -> + composeRequestParams( + r.getAppProfileId(), r.getTableName(), r.getAuthorizedViewName())) .build(), settings.bulkMutateRowsSettings().getRetryableCodes()); @@ -1075,18 +1033,7 @@ private UnaryCallable createReadModifyWriteRowCallable( .setMethodDescriptor( BigtableGrpc.getGenerateInitialChangeStreamPartitionsMethod()) .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract( - GenerateInitialChangeStreamPartitionsRequest - generateInitialChangeStreamPartitionsRequest) { - return ImmutableMap.of( - "table_name", - generateInitialChangeStreamPartitionsRequest.getTableName(), - "app_profile_id", - generateInitialChangeStreamPartitionsRequest.getAppProfileId()); - } - }) + r -> composeRequestParams(r.getAppProfileId(), r.getTableName(), "")) .build(), settings.generateInitialChangeStreamPartitionsSettings().getRetryableCodes()); @@ -1155,15 +1102,7 @@ public Map extract( GrpcCallSettings.newBuilder() .setMethodDescriptor(BigtableGrpc.getReadChangeStreamMethod()) .setParamsExtractor( - new RequestParamsExtractor() { - @Override - public Map extract( - ReadChangeStreamRequest readChangeStreamRequest) { - return ImmutableMap.of( - "table_name", readChangeStreamRequest.getTableName(), - "app_profile_id", readChangeStreamRequest.getAppProfileId()); - } - }) + r -> composeRequestParams(r.getAppProfileId(), r.getTableName(), "")) .build(), settings.readChangeStreamSettings().getRetryableCodes()); @@ -1313,7 +1252,7 @@ private UnaryCallable createUserFacin private Map composeRequestParams( String appProfileId, String tableName, String authorizedViewName) { - if (tableName.isEmpty()) { + if (tableName.isEmpty() && !authorizedViewName.isEmpty()) { tableName = NameUtil.extractTableNameFromAuthorizedViewName(authorizedViewName); } return ImmutableMap.of("table_name", tableName, "app_profile_id", appProfileId); From 6685aa331f2010c645f2dbe50f2fe4f7a6ed8b8a Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 5 Nov 2024 16:48:04 -0500 Subject: [PATCH 10/16] chore: remodel unary callables as server streaming callables with an adapter at the end (#2403) * chore: remodel unary callables as server streaming callables with an adapter at the end Change-Id: I8708dff0e192d7647ef2cb361fc0992e1ddd2b24 * test + fixes Change-Id: Id4c56656a829f5f4c7ab1170f5f980cf3cc3760c * chore: generate libraries at Mon Nov 4 22:30:01 UTC 2024 * oops Change-Id: I1bd8c318b3272925cd6b81601d7b1d7c772a853f * more tests Change-Id: I1c45f2058cadc1acb9c6abd87222be9eb233778c * avoid multiple cancels Change-Id: I4e05efaac6ae60f5827c6d666c3c6f6cebebaa54 * chore: generate libraries at Tue Nov 5 00:23:44 UTC 2024 * fix fallback Change-Id: I654e70f0b34f5d4c3071ba3c2fed64ea183a865e * chore: generate libraries at Tue Nov 5 00:42:37 UTC 2024 * proper fallback Change-Id: Ic0106f3c6983edbb032aeba6e107e4324952397d * Use transforming callable Change-Id: I8d8474050e40cd819d3be2a5b251448f6eb8c94f * fix npe Change-Id: Ib589ca063369e26ef214eb89099e459981dafe83 * clean up logic Change-Id: I4504c47143000d97554a96469d5f3fd368d08ef1 * oops, messed up splitting commits, this should've been part of this pr not the next Change-Id: I16a35e19c50b7b7b855f4299cf41f0607b3e90bd * typo Change-Id: I8202e935975e1a55606265c502fe7573b8a4acb0 * disable watchdog for the new ReadRow callable chain Change-Id: I4522719a65f24d27fb9dccde031c3b1cc04042c2 --------- Co-authored-by: cloud-java-bot --- .../stub/BigtableUnaryOperationCallable.java | 189 ++++++++++++++++++ .../data/v2/stub/EnhancedBigtableStub.java | 135 +++++++++++-- .../v2/stub/EnhancedBigtableStubSettings.java | 6 + .../TransformingServerStreamingCallable.java | 72 +++++++ .../BigtableUnaryOperationCallableTest.java | 166 +++++++++++++++ ...ansformingServerStreamingCallableTest.java | 74 +++++++ 6 files changed, 620 insertions(+), 22 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallable.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallable.java create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallableTest.java create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallableTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallable.java new file mode 100644 index 000000000000..19f7a5224cf9 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallable.java @@ -0,0 +1,189 @@ +/* + * 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.api.core.AbstractApiFuture; +import com.google.api.core.ApiFuture; +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.InternalException; +import com.google.api.gax.rpc.ResponseObserver; +import com.google.api.gax.rpc.ServerStreamingCallable; +import com.google.api.gax.rpc.StreamController; +import com.google.api.gax.rpc.UnaryCallable; +import com.google.api.gax.tracing.ApiTracer; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.SpanName; +import com.google.common.base.Preconditions; +import io.grpc.Status; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.Nullable; + +/** + * Helper to convert a fake {@link ServerStreamingCallable} (ie only up to 1 response) into a {@link + * UnaryCallable}. It is intended to be the outermost callable of a chain. + * + *

Responsibilities: + * + *

    + *
  • Operation level metrics + *
  • Configuring the default call context + *
  • Converting the result to a future + */ +class BigtableUnaryOperationCallable extends UnaryCallable { + private static final Logger LOGGER = + Logger.getLogger(BigtableUnaryOperationCallable.class.getName()); + Logger logger = LOGGER; + + private final ServerStreamingCallable inner; + private final ApiCallContext defaultCallContext; + private final ApiTracerFactory tracerFactory; + private final SpanName spanName; + private final boolean allowNoResponse; + + public BigtableUnaryOperationCallable( + ServerStreamingCallable inner, + ApiCallContext defaultCallContext, + ApiTracerFactory tracerFactory, + SpanName spanName, + boolean allowNoResponse) { + this.inner = inner; + this.defaultCallContext = defaultCallContext; + this.tracerFactory = tracerFactory; + this.spanName = spanName; + this.allowNoResponse = allowNoResponse; + } + + @Override + public ApiFuture futureCall(ReqT req, ApiCallContext apiCallContext) { + apiCallContext = defaultCallContext.merge(apiCallContext); + + ApiTracer apiTracer = + tracerFactory.newTracer( + apiCallContext.getTracer(), spanName, ApiTracerFactory.OperationType.Unary); + + apiCallContext = apiCallContext.withTracer(apiTracer); + + UnaryFuture f = new UnaryFuture(apiTracer, allowNoResponse); + inner.call(req, f, apiCallContext); + return f; + } + + class UnaryFuture extends AbstractApiFuture implements ResponseObserver { + private final ApiTracer tracer; + private final boolean allowNoResponse; + + private StreamController controller; + private final AtomicBoolean upstreamCancelled = new AtomicBoolean(); + private boolean responseReceived; + private @Nullable RespT response; + + private UnaryFuture(ApiTracer tracer, boolean allowNoResponse) { + this.tracer = Preconditions.checkNotNull(tracer, "tracer can't be null"); + this.allowNoResponse = allowNoResponse; + this.responseReceived = false; + } + + @Override + public void onStart(StreamController controller) { + this.controller = controller; + controller.disableAutoInboundFlowControl(); + // Request 2 to detect protocol bugs + controller.request(2); + } + + /** + * Immediately cancel the future state and try to cancel the underlying operation. Will return + * false if the future is already resolved. + */ + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + if (super.cancel(mayInterruptIfRunning)) { + cancelUpstream(); + return true; + } + return false; + } + + private void cancelUpstream() { + if (upstreamCancelled.compareAndSet(false, true)) { + controller.cancel(); + } + } + + @Override + public void onResponse(RespT resp) { + tracer.responseReceived(); + + // happy path - buffer the only responsse + if (!responseReceived) { + responseReceived = true; + this.response = resp; + return; + } + + String msg = + String.format( + "Received multiple responses for a %s unary operation. Previous: %s, New: %s", + spanName, response, resp); + logger.log(Level.WARNING, msg); + + InternalException error = + new InternalException(msg, null, GrpcStatusCode.of(Status.Code.INTERNAL), false); + if (setException(error)) { + tracer.operationFailed(error); + } + + cancelUpstream(); + } + + @Override + public void onError(Throwable throwable) { + if (this.setException(throwable)) { + tracer.operationFailed(throwable); + } else if (isCancelled()) { + tracer.operationCancelled(); + } + // The future might've been resolved due to double response + } + + @Override + public void onComplete() { + if (allowNoResponse || responseReceived) { + if (set(response)) { + tracer.operationSucceeded(); + return; + } + } else { + String msg = spanName + " unary operation completed without a response message"; + InternalException e = + new InternalException(msg, null, GrpcStatusCode.of(Status.Code.INTERNAL), false); + + if (setException(e)) { + tracer.operationFailed(e); + return; + } + } + + // check cancellation race + if (isCancelled()) { + tracer.operationCancelled(); + } + } + } +} 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 7dcc8dc318ad..94c91fb72a87 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 @@ -41,6 +41,8 @@ import com.google.api.gax.retrying.RetryAlgorithm; import com.google.api.gax.retrying.RetryingExecutorWithContext; import com.google.api.gax.retrying.ScheduledRetryingExecutor; +import com.google.api.gax.retrying.SimpleStreamResumptionStrategy; +import com.google.api.gax.retrying.StreamResumptionStrategy; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.Callables; import com.google.api.gax.rpc.ClientContext; @@ -136,6 +138,7 @@ import com.google.cloud.bigtable.gaxx.retrying.ApiResultRetryAlgorithm; import com.google.cloud.bigtable.gaxx.retrying.RetryInfoRetryAlgorithm; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Functions; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -155,6 +158,7 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; +import java.time.Duration; import java.util.Collections; import java.util.List; import java.util.Map; @@ -559,27 +563,54 @@ public ServerStreamingCallable createReadRowsCallable( *
*/ public UnaryCallable createReadRowCallable(RowAdapter rowAdapter) { - ServerStreamingCallable readRowsCallable = - createReadRowsBaseCallable( - ServerStreamingCallSettings.newBuilder() - .setRetryableCodes(settings.readRowSettings().getRetryableCodes()) - .setRetrySettings(settings.readRowSettings().getRetrySettings()) - .setIdleTimeout(settings.readRowSettings().getRetrySettings().getTotalTimeout()) - .build(), - rowAdapter); - - ReadRowsUserCallable readRowCallable = - new ReadRowsUserCallable<>(readRowsCallable, requestContext); - - ReadRowsFirstCallable firstRow = new ReadRowsFirstCallable<>(readRowCallable); - - UnaryCallable traced = - new TracedUnaryCallable<>( - firstRow, clientContext.getTracerFactory(), getSpanName("ReadRow")); - - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + if (!EnhancedBigtableStubSettings.SKIP_TRAILERS) { + ServerStreamingCallable readRowsCallable = + createReadRowsBaseCallable( + ServerStreamingCallSettings.newBuilder() + .setRetryableCodes(settings.readRowSettings().getRetryableCodes()) + .setRetrySettings(settings.readRowSettings().getRetrySettings()) + .setIdleTimeout(settings.readRowSettings().getRetrySettings().getTotalTimeout()) + .build(), + rowAdapter); + + ReadRowsUserCallable readRowCallable = + new ReadRowsUserCallable<>(readRowsCallable, requestContext); + ReadRowsFirstCallable firstRow = new ReadRowsFirstCallable<>(readRowCallable); + UnaryCallable traced = + new TracedUnaryCallable<>( + firstRow, clientContext.getTracerFactory(), getSpanName("ReadRow")); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + } else { + ServerStreamingCallable readRowsCallable = + createReadRowsBaseCallable( + ServerStreamingCallSettings.newBuilder() + .setRetryableCodes(settings.readRowSettings().getRetryableCodes()) + .setRetrySettings(settings.readRowSettings().getRetrySettings()) + .setIdleTimeoutDuration(Duration.ZERO) + .setWaitTimeoutDuration(Duration.ZERO) + .build(), + rowAdapter, + new SimpleStreamResumptionStrategy<>()); + ServerStreamingCallable readRowCallable = + new TransformingServerStreamingCallable<>( + readRowsCallable, + (query) -> query.limit(1).toProto(requestContext), + Functions.identity()); + + return new BigtableUnaryOperationCallable<>( + readRowCallable, + clientContext.getDefaultCallContext(), + clientContext.getTracerFactory(), + getSpanName("ReadRow"), + /*allowNoResponses=*/ true); + } } + private ServerStreamingCallable createReadRowsBaseCallable( + ServerStreamingCallSettings readRowsSettings, RowAdapter rowAdapter) { + return createReadRowsBaseCallable( + readRowsSettings, rowAdapter, new ReadRowsResumptionStrategy(rowAdapter)); + } /** * Creates a callable chain to handle ReadRows RPCs. The chain will: * @@ -596,8 +627,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, + StreamResumptionStrategy resumptionStrategy) { ServerStreamingCallable base = GrpcRawCallableFactory.createServerStreamingCallable( GrpcCallSettings.newBuilder() @@ -625,7 +657,7 @@ private ServerStreamingCallable createReadRo // ReadRowsRequest -> ReadRowsResponse callable). ServerStreamingCallSettings innerSettings = ServerStreamingCallSettings.newBuilder() - .setResumptionStrategy(new ReadRowsResumptionStrategy<>(rowAdapter)) + .setResumptionStrategy(resumptionStrategy) .setRetryableCodes(readRowsSettings.getRetryableCodes()) .setRetrySettings(readRowsSettings.getRetrySettings()) .setIdleTimeout(readRowsSettings.getIdleTimeout()) @@ -1264,6 +1296,21 @@ private UnaryCallable createUnar UnaryCallSettings callSettings, Function requestTransformer, Function responseTranformer) { + if (EnhancedBigtableStubSettings.SKIP_TRAILERS) { + return createUnaryCallableNew( + methodDescriptor, headerParamsFn, callSettings, requestTransformer, responseTranformer); + } else { + return createUnaryCallableOld( + methodDescriptor, headerParamsFn, callSettings, requestTransformer, responseTranformer); + } + } + + private UnaryCallable createUnaryCallableOld( + MethodDescriptor methodDescriptor, + RequestParamsExtractor headerParamsFn, + UnaryCallSettings callSettings, + Function requestTransformer, + Function responseTranformer) { UnaryCallable base = GrpcRawCallableFactory.createUnaryCallable( @@ -1300,6 +1347,50 @@ public ApiFuture futureCall(ReqT reqT, ApiCallContext apiCallContext) { return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } + private UnaryCallable createUnaryCallableNew( + MethodDescriptor methodDescriptor, + RequestParamsExtractor headerParamsFn, + UnaryCallSettings callSettings, + Function requestTransformer, + Function responseTranformer) { + + ServerStreamingCallable base = + GrpcRawCallableFactory.createServerStreamingCallable( + GrpcCallSettings.newBuilder() + .setMethodDescriptor(methodDescriptor) + .setParamsExtractor(headerParamsFn) + .build(), + callSettings.getRetryableCodes()); + + base = new StatsHeadersServerStreamingCallable<>(base); + + base = new BigtableTracerStreamingCallable<>(base); + + base = withRetries(base, convertUnaryToServerStreamingSettings(callSettings)); + + ServerStreamingCallable transformed = + new TransformingServerStreamingCallable<>(base, requestTransformer, responseTranformer); + + return new BigtableUnaryOperationCallable<>( + transformed, + clientContext.getDefaultCallContext(), + clientContext.getTracerFactory(), + getSpanName(methodDescriptor.getBareMethodName()), + /* allowNoResponse= */ false); + } + + private static + ServerStreamingCallSettings convertUnaryToServerStreamingSettings( + UnaryCallSettings unarySettings) { + return ServerStreamingCallSettings.newBuilder() + .setResumptionStrategy(new SimpleStreamResumptionStrategy<>()) + .setRetryableCodes(unarySettings.getRetryableCodes()) + .setRetrySettings(unarySettings.getRetrySettings()) + .setIdleTimeoutDuration(Duration.ZERO) + .setWaitTimeoutDuration(Duration.ZERO) + .build(); + } + private UnaryCallable createPingAndWarmCallable() { UnaryCallable pingAndWarm = GrpcRawCallableFactory.createUnaryCallable( 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 5e5dc64fd991..863389166f34 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 @@ -62,6 +62,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.logging.Logger; import javax.annotation.Nonnull; @@ -108,6 +109,11 @@ public class EnhancedBigtableStubSettings extends StubSettings IDEMPOTENT_RETRY_CODES = ImmutableSet.of(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallable.java new file mode 100644 index 000000000000..576d8257d70e --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallable.java @@ -0,0 +1,72 @@ +/* + * 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.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.ResponseObserver; +import com.google.api.gax.rpc.ServerStreamingCallable; +import com.google.api.gax.rpc.StreamController; +import java.util.function.Function; + +/** Callable to help crossing api boundary lines between models and protos */ +class TransformingServerStreamingCallable + extends ServerStreamingCallable { + private final ServerStreamingCallable inner; + private final Function requestTransformer; + private final Function responseTransformer; + + public TransformingServerStreamingCallable( + ServerStreamingCallable inner, + Function requestTransformer, + Function responseTransformer) { + this.inner = inner; + this.requestTransformer = requestTransformer; + this.responseTransformer = responseTransformer; + } + + @Override + public void call( + OuterReqT outerReqT, + ResponseObserver outerObserver, + ApiCallContext apiCallContext) { + InnerReqT innerReq = requestTransformer.apply(outerReqT); + + inner.call( + innerReq, + new ResponseObserver() { + @Override + public void onStart(StreamController streamController) { + outerObserver.onStart(streamController); + } + + @Override + public void onResponse(InnerRespT innerResp) { + outerObserver.onResponse(responseTransformer.apply(innerResp)); + } + + @Override + public void onError(Throwable throwable) { + outerObserver.onError(throwable); + } + + @Override + public void onComplete() { + outerObserver.onComplete(); + } + }, + apiCallContext); + } +} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallableTest.java new file mode 100644 index 000000000000..b6f1a24b70fd --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallableTest.java @@ -0,0 +1,166 @@ +/* + * 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 static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import com.google.api.core.ApiFuture; +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.rpc.InternalException; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.SpanName; +import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracer; +import com.google.cloud.bigtable.gaxx.testing.FakeStreamingApi; +import com.google.cloud.bigtable.gaxx.testing.MockStreamingApi.MockServerStreamingCall; +import com.google.cloud.bigtable.gaxx.testing.MockStreamingApi.MockServerStreamingCallable; +import com.google.common.collect.ImmutableList; +import java.util.concurrent.ExecutionException; +import java.util.logging.Logger; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +@RunWith(JUnit4.class) +public class BigtableUnaryOperationCallableTest { + @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); + + @Mock private ApiTracerFactory tracerFactory; + @Mock private BigtableTracer tracer; + + @Before + public void setUp() throws Exception { + Mockito.when(tracerFactory.newTracer(Mockito.any(), Mockito.any(), Mockito.any())) + .thenReturn(tracer); + } + + @Test + public void testFutureResolve() throws Exception { + BigtableUnaryOperationCallable callable = + new BigtableUnaryOperationCallable<>( + new FakeStreamingApi.ServerStreamingStashCallable<>(ImmutableList.of("value")), + GrpcCallContext.createDefault(), + tracerFactory, + SpanName.of("Fake", "method"), + false); + + ApiFuture f = callable.futureCall("fake"); + assertThat(f.get()).isEqualTo("value"); + } + + @Test + public void testMultipleResponses() throws Exception { + MockServerStreamingCallable inner = new MockServerStreamingCallable<>(); + + BigtableUnaryOperationCallable callable = + new BigtableUnaryOperationCallable<>( + inner, + GrpcCallContext.createDefault(), + tracerFactory, + SpanName.of("Fake", "method"), + false); + callable.logger = Mockito.mock(Logger.class); + + ApiFuture f = callable.futureCall("fake"); + MockServerStreamingCall call = inner.popLastCall(); + call.getController().getObserver().onResponse("first"); + call.getController().getObserver().onResponse("second"); + + Throwable e = Assert.assertThrows(ExecutionException.class, f::get).getCause(); + assertThat(e).isInstanceOf(InternalException.class); + assertThat(e) + .hasMessageThat() + .contains( + "Received multiple responses for a Fake.method unary operation. Previous: first, New: second"); + + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); + verify(callable.logger).log(Mockito.any(), msgCaptor.capture()); + assertThat(msgCaptor.getValue()) + .isEqualTo( + "Received multiple responses for a Fake.method unary operation. Previous: first, New: second"); + + assertThat(call.getController().isCancelled()).isTrue(); + } + + @Test + public void testCancel() { + MockServerStreamingCallable inner = new MockServerStreamingCallable<>(); + BigtableUnaryOperationCallable callable = + new BigtableUnaryOperationCallable<>( + inner, + GrpcCallContext.createDefault(), + tracerFactory, + SpanName.of("Fake", "method"), + false); + ApiFuture f = callable.futureCall("req"); + f.cancel(true); + + MockServerStreamingCall call = inner.popLastCall(); + assertThat(call.getController().isCancelled()).isTrue(); + } + + @Test + public void testMissingResponse() { + MockServerStreamingCallable inner = new MockServerStreamingCallable<>(); + BigtableUnaryOperationCallable callable = + new BigtableUnaryOperationCallable<>( + inner, + GrpcCallContext.createDefault(), + tracerFactory, + SpanName.of("Fake", "method"), + false); + ApiFuture f = callable.futureCall("req"); + MockServerStreamingCall call = inner.popLastCall(); + call.getController().getObserver().onComplete(); + + Throwable cause = Assert.assertThrows(ExecutionException.class, f::get).getCause(); + assertThat(cause) + .hasMessageThat() + .isEqualTo("Fake.method unary operation completed without a response message"); + } + + @Test + public void testTracing() throws Exception { + MockServerStreamingCallable inner = new MockServerStreamingCallable<>(); + BigtableUnaryOperationCallable callable = + new BigtableUnaryOperationCallable<>( + inner, + GrpcCallContext.createDefault(), + tracerFactory, + SpanName.of("Fake", "method"), + false); + ApiFuture f = callable.futureCall("req"); + MockServerStreamingCall call = inner.popLastCall(); + call.getController().getObserver().onResponse("value"); + call.getController().getObserver().onComplete(); + + f.get(); + verify(tracer).responseReceived(); + verify(tracer).operationSucceeded(); + + // afterResponse is the responsibility of BigtableTracerStreamingCallable + verify(tracer, never()).afterResponse(Mockito.anyLong()); + } +} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallableTest.java new file mode 100644 index 000000000000..856d732f5c9f --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallableTest.java @@ -0,0 +1,74 @@ +/* + * 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 static com.google.common.truth.Truth.assertThat; + +import com.google.cloud.bigtable.gaxx.testing.MockStreamingApi.MockResponseObserver; +import com.google.cloud.bigtable.gaxx.testing.MockStreamingApi.MockServerStreamingCall; +import com.google.cloud.bigtable.gaxx.testing.MockStreamingApi.MockServerStreamingCallable; +import com.google.common.base.Functions; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class TransformingServerStreamingCallableTest { + @Test + public void testReqTransform() { + MockServerStreamingCallable inner = new MockServerStreamingCallable<>(); + TransformingServerStreamingCallable xform = + new TransformingServerStreamingCallable<>(inner, Object::toString, Functions.identity()); + + MockResponseObserver responseObserver = new MockResponseObserver<>(true); + xform.call(37, responseObserver); + + MockServerStreamingCall call = inner.popLastCall(); + assertThat(call.getRequest()).isEqualTo("37"); + } + + @Test + public void testRespTransform() { + MockServerStreamingCallable inner = new MockServerStreamingCallable<>(); + TransformingServerStreamingCallable xform = + new TransformingServerStreamingCallable<>(inner, Functions.identity(), Integer::parseInt); + + MockResponseObserver outerObserver = new MockResponseObserver<>(true); + xform.call("req", outerObserver); + + MockServerStreamingCall call = inner.popLastCall(); + call.getController().getObserver().onResponse("37"); + + assertThat(outerObserver.popNextResponse()).isEqualTo(37); + } + + @Test + public void testError() { + MockServerStreamingCallable inner = new MockServerStreamingCallable<>(); + TransformingServerStreamingCallable xform = + new TransformingServerStreamingCallable<>( + inner, Functions.identity(), Functions.identity()); + + MockResponseObserver outerObserver = new MockResponseObserver<>(true); + xform.call("req", outerObserver); + + MockServerStreamingCall call = inner.popLastCall(); + RuntimeException e = new RuntimeException("fake error"); + call.getController().getObserver().onError(e); + + assertThat(outerObserver.getFinalError()).isEqualTo(e); + } +} From 75d4105e0376dbe5810d6b96d71daa74b85e68ce Mon Sep 17 00:00:00 2001 From: Derek Yau <134542470+djyau@users.noreply.github.com> Date: Wed, 6 Nov 2024 18:00:22 -0500 Subject: [PATCH 11/16] feat: add internal "deadline remaining" client side metric #2341 (#2370) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Rollback plan is reviewed and LGTMed - [ ] All new data plane features have a completed end to end testing plan Fixes # ☕️ If you write sample code, please follow the [samples format]( https://togithub.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md). --- .../data/v2/stub/EnhancedBigtableStub.java | 89 ++++++++++++++++--- .../BigtableCloudMonitoringExporter.java | 4 +- .../data/v2/stub/metrics/BigtableTracer.java | 14 +++ .../BigtableTracerStreamingCallable.java | 7 ++ .../metrics/BigtableTracerUnaryCallable.java | 7 ++ .../stub/metrics/BuiltinMetricsConstants.java | 11 +++ .../v2/stub/metrics/BuiltinMetricsTracer.java | 34 +++++++ .../metrics/BuiltinMetricsTracerFactory.java | 10 +++ .../data/v2/stub/metrics/CompositeTracer.java | 7 ++ .../metrics/BuiltinMetricsTracerTest.java | 62 +++++++++++++ 10 files changed, 232 insertions(+), 13 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 94c91fb72a87..53ebca291923 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 @@ -105,6 +105,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.BigtableTracer; 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; @@ -545,7 +546,12 @@ public ServerStreamingCallable createReadRowsCallable( new TracedServerStreamingCallable<>( readRowsUserCallable, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.readRowsSettings().getRetrySettings().getTotalTimeout())); } /** @@ -579,7 +585,12 @@ public UnaryCallable createReadRowCallable(RowAdapter UnaryCallable traced = new TracedUnaryCallable<>( firstRow, clientContext.getTracerFactory(), getSpanName("ReadRow")); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.readRowSettings().getRetrySettings().getTotalTimeout())); } else { ServerStreamingCallable readRowsCallable = createReadRowsBaseCallable( @@ -599,7 +610,11 @@ public UnaryCallable createReadRowCallable(RowAdapter return new BigtableUnaryOperationCallable<>( readRowCallable, - clientContext.getDefaultCallContext(), + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.readRowSettings().getRetrySettings().getTotalTimeout()), clientContext.getTracerFactory(), getSpanName("ReadRow"), /*allowNoResponses=*/ true); @@ -715,7 +730,12 @@ private UnaryCallable> createBulkReadRowsCallable( UnaryCallable> traced = new TracedUnaryCallable<>(tracedBatcher, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.bulkReadRowsSettings().getRetrySettings().getTotalTimeout())); } /** @@ -780,7 +800,14 @@ public ApiFuture> futureCall(String s, ApiCallContext apiCallCon retryable = withRetries(withBigtableTracer, settings.sampleRowKeysSettings()); return createUserFacingUnaryCallable( - methodName, new SampleRowKeysCallableWithRequest(retryable, requestContext)); + methodName, + new SampleRowKeysCallableWithRequest(retryable, requestContext) + .withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.sampleRowKeysSettings().getRetrySettings().getTotalTimeout()))); } /** @@ -903,7 +930,12 @@ private UnaryCallable createMutateRowsBas new TracedUnaryCallable<>( tracedBatcherUnaryCallable, clientContext.getTracerFactory(), spanName); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.bulkMutateRowsSettings().getRetrySettings().getTotalTimeout())); } /** @@ -1108,7 +1140,15 @@ private UnaryCallable createReadModifyWriteRowCallable( ServerStreamingCallable traced = new TracedServerStreamingCallable<>(retrying, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings + .generateInitialChangeStreamPartitionsSettings() + .getRetrySettings() + .getTotalTimeout())); } /** @@ -1180,7 +1220,12 @@ private UnaryCallable createReadModifyWriteRowCallable( new TracedServerStreamingCallable<>( readChangeStreamUserCallable, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.readChangeStreamSettings().getRetrySettings().getTotalTimeout())); } /** @@ -1266,7 +1311,13 @@ public Map extract(ExecuteQueryRequest executeQueryRequest) { new TracedServerStreamingCallable<>(retries, clientContext.getTracerFactory(), span); return new ExecuteQueryCallable( - traced.withDefaultCallContext(clientContext.getDefaultCallContext()), requestContext); + traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.executeQuerySettings().getRetrySettings().getTotalTimeout())), + requestContext); } /** @@ -1344,7 +1395,12 @@ public ApiFuture futureCall(ReqT reqT, ApiCallContext apiCallContext) { clientContext.getTracerFactory(), getSpanName(methodDescriptor.getBareMethodName())); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + callSettings.getRetrySettings().getTotalTimeout())); } private UnaryCallable createUnaryCallableNew( @@ -1373,7 +1429,11 @@ private UnaryCallable createUnar return new BigtableUnaryOperationCallable<>( transformed, - clientContext.getDefaultCallContext(), + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + callSettings.getRetrySettings().getTotalTimeout()), clientContext.getTracerFactory(), getSpanName(methodDescriptor.getBareMethodName()), /* allowNoResponse= */ false); @@ -1407,7 +1467,12 @@ public Map extract(PingAndWarmRequest request) { }) .build(), Collections.emptySet()); - return pingAndWarm.withDefaultCallContext(clientContext.getDefaultCallContext()); + return pingAndWarm.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.pingAndWarmSettings().getRetrySettings().getTotalTimeout())); } private UnaryCallable withRetries( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index fd54313e8d74..8aa53fa19861 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -23,6 +23,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME; @@ -115,7 +116,8 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter { CLIENT_BLOCKING_LATENCIES_NAME, APPLICATION_BLOCKING_LATENCIES_NAME, RETRY_COUNT_NAME, - CONNECTIVITY_ERROR_COUNT_NAME) + CONNECTIVITY_ERROR_COUNT_NAME, + REMAINING_DEADLINE_NAME) .stream() .map(m -> METER_NAME + m) .collect(ImmutableList.toImmutableList()); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index d0e307d5109c..ad77b207b398 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -16,10 +16,12 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.BaseApiTracer; import javax.annotation.Nullable; +import org.threeten.bp.Duration; /** * A Bigtable specific {@link ApiTracer} that includes additional contexts. This class is a base @@ -30,6 +32,10 @@ public class BigtableTracer extends BaseApiTracer { private volatile int attempt = 0; + @InternalApi("for internal use only") + public static final ApiCallContext.Key OPERATION_TIMEOUT_KEY = + ApiCallContext.Key.create("OPERATION_TIMEOUT"); + @Override public void attemptStarted(int attemptNumber) { this.attempt = attemptNumber; @@ -93,4 +99,12 @@ public void grpcChannelQueuedLatencies(long queuedTimeMs) { public void grpcMessageSent() { // noop } + + /** + * Record the operation timeout from user settings for calculating remaining deadline. This will + * be called in BuiltinMetricsTracer. + */ + public void setOperationTimeout(Duration operationTimeout) { + // noop + } } 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 167cd0dc2e1a..b977a0a2c724 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 @@ -16,6 +16,7 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ResponseObserver; @@ -26,6 +27,7 @@ import com.google.common.base.Stopwatch; import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; +import org.threeten.bp.Duration; /** * This callable will @@ -62,6 +64,11 @@ public void call( BigtableTracerResponseObserver innerObserver = new BigtableTracerResponseObserver<>( responseObserver, (BigtableTracer) context.getTracer(), responseMetadata); + GrpcCallContext callContext = (GrpcCallContext) context; + Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY); + if (deadline != null) { + ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); + } innerCallable.call( request, innerObserver, 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 7dfca8b753b7..1f000c4639ea 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 @@ -19,12 +19,14 @@ import com.google.api.core.ApiFutureCallback; import com.google.api.core.ApiFutures; import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcCallContext; 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.base.Preconditions; import com.google.common.util.concurrent.MoreExecutors; import javax.annotation.Nonnull; +import org.threeten.bp.Duration; /** * This callable will: @@ -58,6 +60,11 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) BigtableTracerUnaryCallback callback = new BigtableTracerUnaryCallback( (BigtableTracer) context.getTracer(), responseMetadata); + GrpcCallContext callContext = (GrpcCallContext) context; + Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY); + if (deadline != null) { + ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); + } ApiFuture future = innerCallable.futureCall( request, diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java index d85300828bab..62ac0f11535c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java @@ -58,6 +58,7 @@ public class BuiltinMetricsConstants { static final String SERVER_LATENCIES_NAME = "server_latencies"; static final String FIRST_RESPONSE_LATENCIES_NAME = "first_response_latencies"; static final String APPLICATION_BLOCKING_LATENCIES_NAME = "application_latencies"; + static final String REMAINING_DEADLINE_NAME = "remaining_deadline"; static final String CLIENT_BLOCKING_LATENCIES_NAME = "throttling_latencies"; static final String PER_CONNECTION_ERROR_COUNT_NAME = "per_connection_error_count"; @@ -214,6 +215,16 @@ public static Map getAllViews() { ImmutableSet.builder() .add(BIGTABLE_PROJECT_ID_KEY, INSTANCE_ID_KEY, APP_PROFILE_KEY, CLIENT_NAME_KEY) .build()); + defineView( + views, + REMAINING_DEADLINE_NAME, + AGGREGATION_WITH_MILLIS_HISTOGRAM, + InstrumentType.HISTOGRAM, + "ms", + ImmutableSet.builder() + .addAll(COMMON_ATTRIBUTES) + .add(STREAMING_KEY, STATUS_KEY) + .build()); return views.build(); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 14a112b27087..07583bde52c1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -37,6 +37,8 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.logging.Level; +import java.util.logging.Logger; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -46,6 +48,8 @@ */ class BuiltinMetricsTracer extends BigtableTracer { + private static final Logger logger = Logger.getLogger(BuiltinMetricsTracer.class.getName()); + private static final String NAME = "java-bigtable/" + Version.VERSION; private final OperationType operationType; private final SpanName spanName; @@ -85,6 +89,9 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; private final AtomicLong grpcMessageSentDelay = new AtomicLong(0); + private Duration operationTimeout = Duration.ofMillis(0); + private long remainingOperationTimeout = 0; + // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them // to milliseconds and use DoubleHistogram. This should minimize the chance of a data @@ -95,6 +102,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; + private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; @@ -108,6 +116,7 @@ class BuiltinMetricsTracer extends BigtableTracer { DoubleHistogram firstResponseLatenciesHistogram, DoubleHistogram clientBlockingLatenciesHistogram, DoubleHistogram applicationBlockingLatenciesHistogram, + DoubleHistogram deadlineHistogram, LongCounter connectivityErrorCounter, LongCounter retryCounter) { this.operationType = operationType; @@ -120,6 +129,7 @@ class BuiltinMetricsTracer extends BigtableTracer { this.firstResponseLatenciesHistogram = firstResponseLatenciesHistogram; this.clientBlockingLatenciesHistogram = clientBlockingLatenciesHistogram; this.applicationBlockingLatenciesHistogram = applicationBlockingLatenciesHistogram; + this.remainingDeadlineHistogram = deadlineHistogram; this.connectivityErrorCounter = connectivityErrorCounter; this.retryCounter = retryCounter; } @@ -167,6 +177,11 @@ public void attemptStarted(Object request, int attemptNumber) { } } } + // OperationTimeout is only set after the first attempt. + if (attemptCount > 1) { + remainingOperationTimeout = + operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS); + } } @Override @@ -266,6 +281,14 @@ public void grpcMessageSent() { grpcMessageSentDelay.set(attemptTimer.elapsed(TimeUnit.NANOSECONDS)); } + /* + This is called by BigtableTracerCallables that sets operation timeout from user settings. + */ + @Override + public void setOperationTimeout(Duration operationTimeout) { + this.operationTimeout = operationTimeout; + } + @Override public void disableFlowControl() { flowControlIsDisabled = true; @@ -355,6 +378,17 @@ private void recordAttemptCompletion(@Nullable Throwable status) { attemptLatenciesHistogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); + if (attemptCount <= 1) { + remainingDeadlineHistogram.record(operationTimeout.toMillis(), attributes); + } else if (remainingOperationTimeout >= 0) { + remainingDeadlineHistogram.record(remainingOperationTimeout, attributes); + } else if (operationTimeout.toMillis() != 0) { + // If the operationTimeout is set but remaining deadline is < 0, log a warning. This should + // never happen. + logger.log( + Level.WARNING, "The remaining deadline was less than 0: " + remainingOperationTimeout); + } + if (serverLatencies != null) { serverLatenciesHistogram.record(serverLatencies, attributes); connectivityErrorCounter.add(0, attributes); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index f0ac656978cf..18d3a3ace98e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -22,6 +22,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.FIRST_RESPONSE_LATENCIES_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME; @@ -55,6 +56,7 @@ public class BuiltinMetricsTracerFactory extends BaseApiTracerFactory { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; + private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; @@ -108,6 +110,13 @@ public static BuiltinMetricsTracerFactory create( "The latency of the client application consuming available response data.") .setUnit(MILLISECOND) .build(); + remainingDeadlineHistogram = + meter + .histogramBuilder(REMAINING_DEADLINE_NAME) + .setDescription( + "The remaining deadline when the request is sent to grpc. This will either be the operation timeout, or the remaining deadline from operation timeout after retries and back offs.") + .setUnit(MILLISECOND) + .build(); connectivityErrorCounter = meter .counterBuilder(CONNECTIVITY_ERROR_COUNT_NAME) @@ -135,6 +144,7 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op firstResponseLatenciesHistogram, clientBlockingLatenciesHistogram, applicationBlockingLatenciesHistogram, + remainingDeadlineHistogram, connectivityErrorCounter, retryCounter); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index d89aa90c6b8c..6135b2347f75 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -232,4 +232,11 @@ public void grpcMessageSent() { tracer.grpcMessageSent(); } } + + @Override + public void setOperationTimeout(Duration operationTimeout) { + for (BigtableTracer tracer : bigtableTracers) { + tracer.setOperationTimeout(operationTimeout); + } + } } 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 cec518e2c378..e902f0271f41 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 @@ -24,6 +24,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.FIRST_RESPONSE_LATENCIES_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METHOD_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.STATUS_KEY; @@ -90,11 +91,13 @@ import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; import io.opentelemetry.sdk.metrics.View; +import io.opentelemetry.sdk.metrics.data.HistogramPointData; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import java.io.IOException; import java.net.SocketAddress; import java.nio.charset.Charset; +import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; @@ -103,6 +106,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; import javax.annotation.Nullable; import org.junit.After; import org.junit.Assert; @@ -213,6 +217,17 @@ public void sendHeaders(Metadata headers) { .retrySettings() .setInitialRetryDelayDuration(java.time.Duration.ofMillis(200)); + stubSettingsBuilder + .readRowsSettings() + .retrySettings() + .setTotalTimeoutDuration(Duration.ofMillis(9000)) + .setMaxRpcTimeoutDuration(Duration.ofMillis(6000)) + .setRpcTimeoutMultiplier(1) + .setInitialRpcTimeoutDuration(Duration.ofMillis(6000)) + .setInitialRetryDelayDuration(Duration.ofMillis(10)) + .setRetryDelayMultiplier(1) + .setMaxRetryDelayDuration(Duration.ofMillis(10)); + stubSettingsBuilder .bulkMutateRowsSettings() .setBatchingSettings( @@ -750,6 +765,53 @@ public void testPermanentFailure() { verifyAttributes(opLatency, expected); } + @Test + public void testRemainingDeadline() { + stub.readRowsCallable().all().call(Query.create(TABLE)); + MetricData deadlineMetric = getMetricData(metricReader, REMAINING_DEADLINE_NAME); + + Attributes retryAttributes = + baseAttributes + .toBuilder() + .put(STATUS_KEY, "UNAVAILABLE") + .put(TABLE_ID_KEY, TABLE) + .put(METHOD_KEY, "Bigtable.ReadRows") + .put(ZONE_ID_KEY, "global") + .put(CLUSTER_ID_KEY, "unspecified") + .put(STREAMING_KEY, true) + .put(CLIENT_NAME_KEY, CLIENT_NAME) + .build(); + HistogramPointData retryHistogramPointData = + deadlineMetric.getHistogramData().getPoints().stream() + .filter(pd -> pd.getAttributes().equals(retryAttributes)) + .collect(Collectors.toList()) + .get(0); + + double retryRemainingDeadline = retryHistogramPointData.getSum(); + // The retry remaining deadline should be equivalent to the original timeout. + assertThat(retryRemainingDeadline).isEqualTo(9000); + + Attributes okAttributes = + baseAttributes + .toBuilder() + .put(STATUS_KEY, "OK") + .put(TABLE_ID_KEY, TABLE) + .put(ZONE_ID_KEY, ZONE) + .put(CLUSTER_ID_KEY, CLUSTER) + .put(METHOD_KEY, "Bigtable.ReadRows") + .put(STREAMING_KEY, true) + .put(CLIENT_NAME_KEY, CLIENT_NAME) + .build(); + HistogramPointData okHistogramPointData = + deadlineMetric.getHistogramData().getPoints().stream() + .filter(pd -> pd.getAttributes().equals(okAttributes)) + .collect(Collectors.toList()) + .get(0); + + double okRemainingDeadline = okHistogramPointData.getSum(); + assertThat(okRemainingDeadline).isWithin(200).of(8500); + } + private static class FakeService extends BigtableGrpc.BigtableImplBase { static List createFakeResponse() { From cf58f260fd7d3cb0dee4fee8e2d43367db6eadb1 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 7 Nov 2024 07:38:22 -0500 Subject: [PATCH 12/16] feat: add an experimental feature to skip waiting for trailers for unary ops (#2404) * feat: add an experimental feature to skip waiting for trailers for unary ops This is off by default and can be enabled using an environment variable. When enabled, BigtableUnaryOperationCallable will resolve the user visible future immediately when a response is available and will tell metrics to freeze all timers. Metrics will still wait for the trailers in the background for necessary metadata to publish the frozen timer values. Change-Id: I2101ff375de711693720af4fd2e9535aa5355f9d * more testing Change-Id: Ifc95aa89c080ee8395d43adce1172f11354c306e * cosmetics Change-Id: I679aeac3ec7475757ce769f4c64ede1130b35ebd * comment Change-Id: Ia535905f4fed6f30854c05ceb300af39877ca4a1 * fix test Change-Id: I77664e40c9fd2d52b609f5063386b158cbc1e81e --- .../stub/BigtableUnaryOperationCallable.java | 84 +++--- .../data/v2/stub/EnhancedBigtableStub.java | 4 +- .../v2/stub/EnhancedBigtableStubSettings.java | 16 +- .../data/v2/stub/metrics/BigtableTracer.java | 7 + .../v2/stub/metrics/BuiltinMetricsTracer.java | 31 ++- .../data/v2/stub/metrics/CompositeTracer.java | 7 + .../data/v2/stub/metrics/MetricsTracer.java | 7 +- .../BigtableUnaryOperationCallableTest.java | 10 +- .../EnhancedBigtableStubSettingsTest.java | 1 + .../data/v2/stub/SkipTrailersTest.java | 249 ++++++++++++++++++ 10 files changed, 370 insertions(+), 46 deletions(-) create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SkipTrailersTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallable.java index 19f7a5224cf9..78d507665e19 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallable.java @@ -24,15 +24,16 @@ import com.google.api.gax.rpc.ServerStreamingCallable; import com.google.api.gax.rpc.StreamController; import com.google.api.gax.rpc.UnaryCallable; -import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.ApiTracerFactory; import com.google.api.gax.tracing.SpanName; +import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracer; import com.google.common.base.Preconditions; +import com.google.common.util.concurrent.Futures; import io.grpc.Status; +import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; -import javax.annotation.Nullable; /** * Helper to convert a fake {@link ServerStreamingCallable} (ie only up to 1 response) into a {@link @@ -73,9 +74,10 @@ public BigtableUnaryOperationCallable( public ApiFuture futureCall(ReqT req, ApiCallContext apiCallContext) { apiCallContext = defaultCallContext.merge(apiCallContext); - ApiTracer apiTracer = - tracerFactory.newTracer( - apiCallContext.getTracer(), spanName, ApiTracerFactory.OperationType.Unary); + BigtableTracer apiTracer = + (BigtableTracer) + tracerFactory.newTracer( + apiCallContext.getTracer(), spanName, ApiTracerFactory.OperationType.Unary); apiCallContext = apiCallContext.withTracer(apiTracer); @@ -85,18 +87,15 @@ public ApiFuture futureCall(ReqT req, ApiCallContext apiCallContext) { } class UnaryFuture extends AbstractApiFuture implements ResponseObserver { - private final ApiTracer tracer; + private final BigtableTracer tracer; private final boolean allowNoResponse; private StreamController controller; private final AtomicBoolean upstreamCancelled = new AtomicBoolean(); - private boolean responseReceived; - private @Nullable RespT response; - private UnaryFuture(ApiTracer tracer, boolean allowNoResponse) { + private UnaryFuture(BigtableTracer tracer, boolean allowNoResponse) { this.tracer = Preconditions.checkNotNull(tracer, "tracer can't be null"); this.allowNoResponse = allowNoResponse; - this.responseReceived = false; } @Override @@ -130,23 +129,39 @@ private void cancelUpstream() { public void onResponse(RespT resp) { tracer.responseReceived(); - // happy path - buffer the only responsse - if (!responseReceived) { - responseReceived = true; - this.response = resp; + if (set(resp)) { + tracer.operationFinishEarly(); return; } - String msg = - String.format( - "Received multiple responses for a %s unary operation. Previous: %s, New: %s", - spanName, response, resp); - logger.log(Level.WARNING, msg); + // At this point we are guaranteed that the future has been resolved. However we need to check + // why. + // We know it's not because it was resolved with the current response. Moreover, since the + // future + // is resolved, our only means to flag the error is to log. + // So there are 3 possibilities: + // 1. user cancelled the future + // 2. this is an extra response and the previous one resolved the future + // 3. we got a response after the rpc failed (this should never happen and would be a bad bug) - InternalException error = - new InternalException(msg, null, GrpcStatusCode.of(Status.Code.INTERNAL), false); - if (setException(error)) { - tracer.operationFailed(error); + if (isCancelled()) { + return; + } + + try { + RespT prev = Futures.getDone(this); + String msg = + String.format( + "Received response after future is resolved for a %s unary operation. previous: %s, New response: %s", + spanName, prev, resp); + logger.log(Level.WARNING, msg); + } catch (ExecutionException e) { + // Should never happen + String msg = + String.format( + "Received response after future resolved as a failure for a %s unary operation. New response: %s", + spanName, resp); + logger.log(Level.WARNING, msg, e.getCause()); } cancelUpstream(); @@ -158,18 +173,24 @@ public void onError(Throwable throwable) { tracer.operationFailed(throwable); } else if (isCancelled()) { tracer.operationCancelled(); + } else { + // At this point the has been resolved, so we ignore the error + tracer.operationSucceeded(); } - // The future might've been resolved due to double response } @Override public void onComplete() { - if (allowNoResponse || responseReceived) { - if (set(response)) { - tracer.operationSucceeded(); - return; - } - } else { + if (allowNoResponse && set(null)) { + tracer.operationSucceeded(); + return; + + // Under normal circumstances the future wouldve been resolved in onResponse or via + // set(null) if it expected for + // the rpc to not have a response. So if aren't done, the only reason is that we didn't get + // a response + // but were expecting one + } else if (!isDone()) { String msg = spanName + " unary operation completed without a response message"; InternalException e = new InternalException(msg, null, GrpcStatusCode.of(Status.Code.INTERNAL), false); @@ -183,7 +204,10 @@ public void onComplete() { // check cancellation race if (isCancelled()) { tracer.operationCancelled(); + return; } + + tracer.operationSucceeded(); } } } 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 53ebca291923..266041a543f2 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 @@ -569,7 +569,7 @@ public ServerStreamingCallable createReadRowsCallable( * */ public UnaryCallable createReadRowCallable(RowAdapter rowAdapter) { - if (!EnhancedBigtableStubSettings.SKIP_TRAILERS) { + if (!settings.getEnableSkipTrailers()) { ServerStreamingCallable readRowsCallable = createReadRowsBaseCallable( ServerStreamingCallSettings.newBuilder() @@ -1347,7 +1347,7 @@ private UnaryCallable createUnar UnaryCallSettings callSettings, Function requestTransformer, Function responseTranformer) { - if (EnhancedBigtableStubSettings.SKIP_TRAILERS) { + if (settings.getEnableSkipTrailers()) { return createUnaryCallableNew( methodDescriptor, headerParamsFn, callSettings, requestTransformer, responseTranformer); } else { 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 863389166f34..1425e7b3628a 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 @@ -109,7 +109,7 @@ public class EnhancedBigtableStubSettings extends StubSettings jwtAudienceMapping; private final boolean enableRoutingCookie; private final boolean enableRetryInfo; + private final boolean enableSkipTrailers; private final ServerStreamingCallSettings readRowsSettings; private final UnaryCallSettings readRowSettings; @@ -287,6 +288,7 @@ private EnhancedBigtableStubSettings(Builder builder) { jwtAudienceMapping = builder.jwtAudienceMapping; enableRoutingCookie = builder.enableRoutingCookie; enableRetryInfo = builder.enableRetryInfo; + enableSkipTrailers = builder.enableSkipTrailers; metricsProvider = builder.metricsProvider; metricsEndpoint = builder.metricsEndpoint; @@ -373,6 +375,10 @@ public boolean getEnableRetryInfo() { return enableRetryInfo; } + boolean getEnableSkipTrailers() { + return enableSkipTrailers; + } + /** * Gets the Google Cloud Monitoring endpoint for publishing client side metrics. If it's null, * client will publish metrics to the default monitoring endpoint. @@ -683,6 +689,7 @@ public static class Builder extends StubSettings.Builder jwtAudienceMapping; private boolean enableRoutingCookie; private boolean enableRetryInfo; + private boolean enableSkipTrailers; private final ServerStreamingCallSettings.Builder readRowsSettings; private final UnaryCallSettings.Builder readRowSettings; @@ -721,6 +728,7 @@ private Builder() { setCredentialsProvider(defaultCredentialsProviderBuilder().build()); this.enableRoutingCookie = true; this.enableRetryInfo = true; + this.enableSkipTrailers = SKIP_TRAILERS; metricsProvider = DefaultMetricsProvider.INSTANCE; // Defaults provider @@ -1085,6 +1093,11 @@ public boolean getEnableRetryInfo() { return enableRetryInfo; } + Builder setEnableSkipTrailers(boolean enabled) { + this.enableSkipTrailers = enabled; + return this; + } + /** Returns the builder for the settings used for calls to readRows. */ public ServerStreamingCallSettings.Builder readRowsSettings() { return readRowsSettings; @@ -1212,6 +1225,7 @@ public String toString() { .add("jwtAudienceMapping", jwtAudienceMapping) .add("enableRoutingCookie", enableRoutingCookie) .add("enableRetryInfo", enableRetryInfo) + .add("enableSkipTrailers", enableSkipTrailers) .add("readRowsSettings", readRowsSettings) .add("readRowSettings", readRowSettings) .add("sampleRowKeysSettings", sampleRowKeysSettings) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index ad77b207b398..fb6a84a88d57 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -58,6 +58,13 @@ public void afterResponse(long applicationLatency) { // noop } + /** + * Used by BigtableUnaryOperationCallable to signal that the user visible portion of the RPC is + * complete and that metrics should freeze the timers and then publish the frozen values when the + * internal portion of the operation completes. + */ + public void operationFinishEarly() {} + /** * Get the attempt number of the current call. Attempt number for the current call is passed in * and should be recorded in {@link #attemptStarted(int)}. With the getter we can access it from diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 07583bde52c1..bd53d6d4d8dc 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -55,6 +55,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private final SpanName spanName; // Operation level metrics + private final AtomicBoolean operationFinishedEarly = new AtomicBoolean(); private final AtomicBoolean opFinished = new AtomicBoolean(); private final Stopwatch operationTimer = Stopwatch.createStarted(); private final Stopwatch firstResponsePerOpTimer = Stopwatch.createStarted(); @@ -142,6 +143,13 @@ public void close() {} }; } + @Override + public void operationFinishEarly() { + operationFinishedEarly.set(true); + attemptTimer.stop(); + operationTimer.stop(); + } + @Override public void operationSucceeded() { recordOperationCompletion(null); @@ -207,6 +215,11 @@ public void attemptPermanentFailure(Throwable throwable) { @Override public void onRequest(int requestCount) { requestLeft.accumulateAndGet(requestCount, IntMath::saturatedAdd); + + if (operationFinishedEarly.get()) { + return; + } + if (flowControlIsDisabled) { // On request is only called when auto flow control is disabled. When auto flow control is // disabled, server latency is measured between onRequest and onResponse. @@ -220,6 +233,10 @@ public void onRequest(int requestCount) { @Override public void responseReceived() { + if (operationFinishedEarly.get()) { + return; + } + if (firstResponsePerOpTimer.isRunning()) { firstResponsePerOpTimer.stop(); } @@ -241,6 +258,9 @@ public void responseReceived() { @Override public void afterResponse(long applicationLatency) { if (!flowControlIsDisabled || requestLeft.decrementAndGet() > 0) { + if (operationFinishedEarly.get()) { + return; + } // When auto flow control is enabled, request will never be called, so server latency is // measured between after the last response is processed and before the next response is // received. If flow control is disabled but requestLeft is greater than 0, @@ -295,10 +315,14 @@ public void disableFlowControl() { } private void recordOperationCompletion(@Nullable Throwable status) { + if (operationFinishedEarly.get()) { + status = null; // force an ok + } + if (!opFinished.compareAndSet(false, true)) { return; } - operationTimer.stop(); + long operationLatencyNano = operationTimer.elapsed(TimeUnit.NANOSECONDS); boolean isStreaming = operationType == OperationType.ServerStreaming; String statusStr = Util.extractStatus(status); @@ -317,8 +341,6 @@ private void recordOperationCompletion(@Nullable Throwable status) { .put(STATUS_KEY, statusStr) .build(); - long operationLatencyNano = operationTimer.elapsed(TimeUnit.NANOSECONDS); - // Only record when retry count is greater than 0 so the retry // graph will be less confusing if (attemptCount > 1) { @@ -339,6 +361,9 @@ private void recordOperationCompletion(@Nullable Throwable status) { } private void recordAttemptCompletion(@Nullable Throwable status) { + if (operationFinishedEarly.get()) { + status = null; // force an ok + } // If the attempt failed, the time spent in retry should be counted in application latency. // Stop the stopwatch and decrement requestLeft. synchronized (timerLock) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index 6135b2347f75..2cee944aa4e6 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -62,6 +62,13 @@ public void close() { }; } + @Override + public void operationFinishEarly() { + for (BigtableTracer tracer : bigtableTracers) { + tracer.operationFinishEarly(); + } + } + @Override public void operationSucceeded() { for (ApiTracer child : children) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java index 0ffabe260609..a2c5bdac1f37 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java @@ -84,6 +84,12 @@ public void close() {} }; } + @Override + public void operationFinishEarly() { + attemptTimer.stop(); + operationTimer.stop(); + } + @Override public void operationSucceeded() { recordOperationCompletion(null); @@ -103,7 +109,6 @@ private void recordOperationCompletion(@Nullable Throwable throwable) { if (!opFinished.compareAndSet(false, true)) { return; } - operationTimer.stop(); long elapsed = operationTimer.elapsed(TimeUnit.MILLISECONDS); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallableTest.java index b6f1a24b70fd..0b11ce3219ee 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallableTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/BigtableUnaryOperationCallableTest.java @@ -21,7 +21,6 @@ import com.google.api.core.ApiFuture; import com.google.api.gax.grpc.GrpcCallContext; -import com.google.api.gax.rpc.InternalException; import com.google.api.gax.tracing.ApiTracerFactory; import com.google.api.gax.tracing.SpanName; import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracer; @@ -88,18 +87,11 @@ public void testMultipleResponses() throws Exception { call.getController().getObserver().onResponse("first"); call.getController().getObserver().onResponse("second"); - Throwable e = Assert.assertThrows(ExecutionException.class, f::get).getCause(); - assertThat(e).isInstanceOf(InternalException.class); - assertThat(e) - .hasMessageThat() - .contains( - "Received multiple responses for a Fake.method unary operation. Previous: first, New: second"); - ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); verify(callable.logger).log(Mockito.any(), msgCaptor.capture()); assertThat(msgCaptor.getValue()) .isEqualTo( - "Received multiple responses for a Fake.method unary operation. Previous: first, New: second"); + "Received response after future is resolved for a Fake.method unary operation. previous: first, New response: second"); assertThat(call.getController().isCancelled()).isTrue(); } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java index 5280abe1fd3c..fdc6b5717e0e 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java @@ -961,6 +961,7 @@ public void enableRetryInfoFalseValueTest() throws IOException { "jwtAudienceMapping", "enableRoutingCookie", "enableRetryInfo", + "enableSkipTrailers", "readRowsSettings", "readRowSettings", "sampleRowKeysSettings", diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SkipTrailersTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SkipTrailersTest.java new file mode 100644 index 000000000000..07ac7deee4bb --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SkipTrailersTest.java @@ -0,0 +1,249 @@ +/* + * 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 static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.api.core.ApiFuture; +import com.google.api.gax.core.NoCredentialsProvider; +import com.google.api.gax.tracing.ApiTracer; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.auto.value.AutoValue; +import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.CheckAndMutateRowResponse; +import com.google.bigtable.v2.MutateRowResponse; +import com.google.bigtable.v2.PingAndWarmResponse; +import com.google.bigtable.v2.ReadModifyWriteRowResponse; +import com.google.bigtable.v2.ReadRowsResponse; +import com.google.bigtable.v2.Row; +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.ConditionalRowMutation; +import com.google.cloud.bigtable.data.v2.models.Filters; +import com.google.cloud.bigtable.data.v2.models.Mutation; +import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; +import com.google.cloud.bigtable.data.v2.models.RowMutation; +import com.google.cloud.bigtable.data.v2.models.TableId; +import com.google.cloud.bigtable.data.v2.models.TargetId; +import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracer; +import com.google.common.base.Preconditions; +import com.google.common.base.Supplier; +import com.google.common.collect.ImmutableList; +import com.google.protobuf.ByteString; +import com.google.protobuf.BytesValue; +import com.google.protobuf.GeneratedMessageV3; +import com.google.protobuf.StringValue; +import io.grpc.BindableService; +import io.grpc.MethodDescriptor; +import io.grpc.Server; +import io.grpc.ServerServiceDefinition; +import io.grpc.stub.ServerCalls; +import io.grpc.stub.StreamObserver; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.LinkedBlockingDeque; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.exceptions.verification.WantedButNotInvoked; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +@RunWith(JUnit4.class) +public class SkipTrailersTest { + @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); + + private static final String PROJECT_ID = "fake-project"; + private static final String INSTANCE_ID = "fake-instance"; + private static final TargetId TABLE_ID = TableId.of("fake-table"); + + private HackedBigtableService hackedService; + private Server server; + + @Mock private ApiTracerFactory tracerFactory; + @Mock private BigtableTracer tracer; + + private BigtableDataClient client; + + @Before + public void setUp() throws Exception { + hackedService = new HackedBigtableService(); + server = FakeServiceBuilder.create(hackedService).start(); + + when(tracerFactory.newTracer(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(tracer); + when(tracer.inScope()).thenReturn(Mockito.mock(ApiTracer.Scope.class)); + + BigtableDataSettings.Builder clientBuilder = + BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .setProjectId(PROJECT_ID) + .setInstanceId(INSTANCE_ID) + .setCredentialsProvider(NoCredentialsProvider.create()); + clientBuilder.stubSettings().setEnableSkipTrailers(true).setTracerFactory(tracerFactory); + + client = BigtableDataClient.create(clientBuilder.build()); + } + + @After + public void tearDown() throws Exception { + client.close(); + server.shutdown(); + } + + @Test + public void testReadRow() throws InterruptedException, ExecutionException { + ReadRowsResponse fakeResponse = + ReadRowsResponse.newBuilder() + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("fake-key")) + .setFamilyName(StringValue.newBuilder().setValue("cf")) + .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) + .setTimestampMicros(0) + .setValue(ByteString.copyFromUtf8("value")) + .setCommitRow(true)) + .build(); + test(() -> client.readRowAsync(TABLE_ID, "fake-key"), fakeResponse); + } + + @Test + public void testMutateRow() throws ExecutionException, InterruptedException { + test( + () -> client.mutateRowAsync(RowMutation.create(TABLE_ID, "fake-key")), + MutateRowResponse.getDefaultInstance()); + } + + @Test + public void testCheckAndMutateRow() throws ExecutionException, InterruptedException { + ConditionalRowMutation req = + ConditionalRowMutation.create(TABLE_ID, "fake-key") + .condition(Filters.FILTERS.pass()) + .then(Mutation.create().deleteRow()); + test(() -> client.checkAndMutateRowAsync(req), CheckAndMutateRowResponse.getDefaultInstance()); + } + + @Test + public void testRMW() throws ExecutionException, InterruptedException { + ReadModifyWriteRow req = ReadModifyWriteRow.create(TABLE_ID, "fake-key").append("cf", "q", "A"); + test( + () -> client.readModifyWriteRowAsync(req), + ReadModifyWriteRowResponse.newBuilder().setRow(Row.getDefaultInstance()).build()); + } + + private void test(Supplier> invoker, T fakeResponse) + throws InterruptedException, ExecutionException { + ApiFuture future = invoker.get(); + + // Wait for the call to start on the server + @SuppressWarnings("unchecked") + ServerRpc rpc = (ServerRpc) hackedService.rpcs.poll(10, TimeUnit.SECONDS); + Preconditions.checkNotNull( + rpc, "Timed out waiting for the call to be received by the mock server"); + + // Send the only row + rpc.getResponseStream().onNext(fakeResponse); + + // Ensure that the future resolves and does not throw an error + try { + future.get(1, TimeUnit.MINUTES); + } catch (TimeoutException e) { + Assert.fail("timed out waiting for the trailer optimization future to resolve"); + } + + verify(tracer, times(1)).operationFinishEarly(); + verify(tracer, never()).operationSucceeded(); + + // clean up + rpc.getResponseStream().onCompleted(); + + // Ensure that the tracer is invoked after the internal operation is complete + // Since we dont have a way to know exactly when this happens, we poll + for (int i = 10; i > 0; i--) { + try { + verify(tracer, times(1)).operationSucceeded(); + break; + } catch (WantedButNotInvoked e) { + if (i > 1) { + Thread.sleep(100); + } else { + throw e; + } + } + } + } + + /** + * Hack the srvice definition to allow grpc server to simulate delayed trailers. This will augment + * the bigtable service definition to promote unary rpcs to server streaming + */ + class HackedBigtableService implements BindableService { + private final LinkedBlockingDeque> rpcs = new LinkedBlockingDeque<>(); + + @Override + public ServerServiceDefinition bindService() { + ServerServiceDefinition.Builder builder = + ServerServiceDefinition.builder(BigtableGrpc.SERVICE_NAME) + .addMethod( + BigtableGrpc.getPingAndWarmMethod(), + ServerCalls.asyncUnaryCall( + (ignored, observer) -> { + observer.onNext(PingAndWarmResponse.getDefaultInstance()); + observer.onCompleted(); + })) + .addMethod( + BigtableGrpc.getReadRowsMethod(), + ServerCalls.asyncServerStreamingCall( + (req, observer) -> rpcs.add(ServerRpc.create(req, observer)))); + ImmutableList> + unaryDescriptors = + ImmutableList.of( + BigtableGrpc.getMutateRowMethod(), + BigtableGrpc.getCheckAndMutateRowMethod(), + BigtableGrpc.getReadModifyWriteRowMethod()); + + for (MethodDescriptor desc : + unaryDescriptors) { + builder.addMethod( + desc.toBuilder().setType(MethodDescriptor.MethodType.SERVER_STREAMING).build(), + ServerCalls.asyncServerStreamingCall( + (req, observer) -> rpcs.add(ServerRpc.create(req, observer)))); + } + return builder.build(); + } + } + + @AutoValue + abstract static class ServerRpc { + abstract ReqT getRequest(); + + abstract StreamObserver getResponseStream(); + + static ServerRpc create(ReqT req, StreamObserver resp) { + // return new AutoValue__(req, resp); + return new AutoValue_SkipTrailersTest_ServerRpc<>(req, resp); + } + } +} From a11d56f6855286acb454edb11378a6b20f42fd74 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 7 Nov 2024 15:18:41 -0500 Subject: [PATCH 13/16] chore: make server streaming transformer safer (#2405) Change-Id: I0fed7ea426a4b775a7fe5e661ed8e67f1faa53eb --- .../v2/stub/TransformingServerStreamingCallable.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallable.java index 576d8257d70e..29b104965ed8 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/TransformingServerStreamingCallable.java @@ -46,24 +46,24 @@ public void call( inner.call( innerReq, - new ResponseObserver() { + new SafeResponseObserver(outerObserver) { @Override - public void onStart(StreamController streamController) { + public void onStartImpl(StreamController streamController) { outerObserver.onStart(streamController); } @Override - public void onResponse(InnerRespT innerResp) { + public void onResponseImpl(InnerRespT innerResp) { outerObserver.onResponse(responseTransformer.apply(innerResp)); } @Override - public void onError(Throwable throwable) { + public void onErrorImpl(Throwable throwable) { outerObserver.onError(throwable); } @Override - public void onComplete() { + public void onCompleteImpl() { outerObserver.onComplete(); } }, From 9796d57b60d928d3390e4ad311d5704dcbe808ec Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 13 Nov 2024 10:52:21 -0500 Subject: [PATCH 14/16] fix: simplify remaining deadline metric impl (#2410) * fix: clean up remaining deadline metric * make naming consistent with gax * add a comment and skip record 0 * add comment * update calculation and test * fix test --- .../data/v2/stub/EnhancedBigtableStub.java | 57 +++++-------------- .../data/v2/stub/metrics/BigtableTracer.java | 14 ++--- .../BigtableTracerStreamingCallable.java | 12 ++-- .../metrics/BigtableTracerUnaryCallable.java | 9 +-- .../v2/stub/metrics/BuiltinMetricsTracer.java | 41 +++++++------ .../data/v2/stub/metrics/CompositeTracer.java | 4 +- .../metrics/BuiltinMetricsTracerTest.java | 7 ++- 7 files changed, 50 insertions(+), 94 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 266041a543f2..da0831304c39 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 @@ -105,7 +105,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.BigtableTracer; 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; @@ -549,9 +548,7 @@ public ServerStreamingCallable createReadRowsCallable( return traced.withDefaultCallContext( clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout())); + .withRetrySettings(settings.readRowsSettings().getRetrySettings())); } /** @@ -588,9 +585,7 @@ public UnaryCallable createReadRowCallable(RowAdapter return traced.withDefaultCallContext( clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.readRowSettings().getRetrySettings().getTotalTimeout())); + .withRetrySettings(settings.readRowSettings().getRetrySettings())); } else { ServerStreamingCallable readRowsCallable = createReadRowsBaseCallable( @@ -612,9 +607,7 @@ public UnaryCallable createReadRowCallable(RowAdapter readRowCallable, clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.readRowSettings().getRetrySettings().getTotalTimeout()), + .withRetrySettings(settings.readRowSettings().getRetrySettings()), clientContext.getTracerFactory(), getSpanName("ReadRow"), /*allowNoResponses=*/ true); @@ -733,9 +726,7 @@ private UnaryCallable> createBulkReadRowsCallable( return traced.withDefaultCallContext( clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.bulkReadRowsSettings().getRetrySettings().getTotalTimeout())); + .withRetrySettings(settings.readRowsSettings().getRetrySettings())); } /** @@ -805,9 +796,7 @@ public ApiFuture> futureCall(String s, ApiCallContext apiCallCon .withDefaultCallContext( clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.sampleRowKeysSettings().getRetrySettings().getTotalTimeout()))); + .withRetrySettings(settings.sampleRowKeysSettings().getRetrySettings()))); } /** @@ -933,9 +922,7 @@ private UnaryCallable createMutateRowsBas return traced.withDefaultCallContext( clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.bulkMutateRowsSettings().getRetrySettings().getTotalTimeout())); + .withRetrySettings(settings.bulkMutateRowsSettings().getRetrySettings())); } /** @@ -1143,12 +1130,8 @@ private UnaryCallable createReadModifyWriteRowCallable( return traced.withDefaultCallContext( clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings - .generateInitialChangeStreamPartitionsSettings() - .getRetrySettings() - .getTotalTimeout())); + .withRetrySettings( + settings.generateInitialChangeStreamPartitionsSettings().getRetrySettings())); } /** @@ -1223,9 +1206,7 @@ private UnaryCallable createReadModifyWriteRowCallable( return traced.withDefaultCallContext( clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.readChangeStreamSettings().getRetrySettings().getTotalTimeout())); + .withRetrySettings(settings.readChangeStreamSettings().getRetrySettings())); } /** @@ -1314,9 +1295,7 @@ public Map extract(ExecuteQueryRequest executeQueryRequest) { traced.withDefaultCallContext( clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.executeQuerySettings().getRetrySettings().getTotalTimeout())), + .withRetrySettings(settings.executeQuerySettings().getRetrySettings())), requestContext); } @@ -1396,11 +1375,7 @@ public ApiFuture futureCall(ReqT reqT, ApiCallContext apiCallContext) { getSpanName(methodDescriptor.getBareMethodName())); return traced.withDefaultCallContext( - clientContext - .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - callSettings.getRetrySettings().getTotalTimeout())); + clientContext.getDefaultCallContext().withRetrySettings(callSettings.getRetrySettings())); } private UnaryCallable createUnaryCallableNew( @@ -1429,11 +1404,7 @@ private UnaryCallable createUnar return new BigtableUnaryOperationCallable<>( transformed, - clientContext - .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - callSettings.getRetrySettings().getTotalTimeout()), + clientContext.getDefaultCallContext().withRetrySettings(callSettings.getRetrySettings()), clientContext.getTracerFactory(), getSpanName(methodDescriptor.getBareMethodName()), /* allowNoResponse= */ false); @@ -1470,9 +1441,7 @@ public Map extract(PingAndWarmRequest request) { return pingAndWarm.withDefaultCallContext( clientContext .getDefaultCallContext() - .withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.pingAndWarmSettings().getRetrySettings().getTotalTimeout())); + .withRetrySettings(settings.pingAndWarmSettings().getRetrySettings())); } private UnaryCallable withRetries( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index fb6a84a88d57..587475151216 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -16,12 +16,11 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.BetaApi; -import com.google.api.core.InternalApi; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.BaseApiTracer; +import java.time.Duration; import javax.annotation.Nullable; -import org.threeten.bp.Duration; /** * A Bigtable specific {@link ApiTracer} that includes additional contexts. This class is a base @@ -32,10 +31,6 @@ public class BigtableTracer extends BaseApiTracer { private volatile int attempt = 0; - @InternalApi("for internal use only") - public static final ApiCallContext.Key OPERATION_TIMEOUT_KEY = - ApiCallContext.Key.create("OPERATION_TIMEOUT"); - @Override public void attemptStarted(int attemptNumber) { this.attempt = attemptNumber; @@ -108,10 +103,11 @@ public void grpcMessageSent() { } /** - * Record the operation timeout from user settings for calculating remaining deadline. This will - * be called in BuiltinMetricsTracer. + * Record the operation timeout from user settings for calculating remaining deadline. Currently, + * it's called in BuiltinMetricsTracer on attempt start from {@link BigtableTracerUnaryCallable} + * and {@link BigtableTracerStreamingCallable}. */ - public void setOperationTimeout(Duration operationTimeout) { + public void setTotalTimeoutDuration(Duration totalTimeoutDuration) { // noop } } 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 b977a0a2c724..13b832b8b17a 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 @@ -16,7 +16,6 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.InternalApi; -import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ResponseObserver; @@ -27,7 +26,6 @@ import com.google.common.base.Stopwatch; import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; -import org.threeten.bp.Duration; /** * This callable will @@ -61,13 +59,11 @@ public void call( final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); // tracer should always be an instance of bigtable tracer if (context.getTracer() instanceof BigtableTracer) { + BigtableTracer tracer = (BigtableTracer) context.getTracer(); BigtableTracerResponseObserver innerObserver = - new BigtableTracerResponseObserver<>( - responseObserver, (BigtableTracer) context.getTracer(), responseMetadata); - GrpcCallContext callContext = (GrpcCallContext) context; - Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY); - if (deadline != null) { - ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); + new BigtableTracerResponseObserver<>(responseObserver, tracer, responseMetadata); + if (context.getRetrySettings() != null) { + tracer.setTotalTimeoutDuration(context.getRetrySettings().getTotalTimeoutDuration()); } innerCallable.call( request, 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 1f000c4639ea..37ba74bfdb71 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 @@ -19,14 +19,12 @@ import com.google.api.core.ApiFutureCallback; import com.google.api.core.ApiFutures; import com.google.api.core.InternalApi; -import com.google.api.gax.grpc.GrpcCallContext; 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.base.Preconditions; import com.google.common.util.concurrent.MoreExecutors; import javax.annotation.Nonnull; -import org.threeten.bp.Duration; /** * This callable will: @@ -56,14 +54,13 @@ public BigtableTracerUnaryCallable(@Nonnull UnaryCallable i public ApiFuture futureCall(RequestT request, ApiCallContext context) { // tracer should always be an instance of BigtableTracer if (context.getTracer() instanceof BigtableTracer) { + BigtableTracer tracer = (BigtableTracer) context.getTracer(); final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); BigtableTracerUnaryCallback callback = new BigtableTracerUnaryCallback( (BigtableTracer) context.getTracer(), responseMetadata); - GrpcCallContext callContext = (GrpcCallContext) context; - Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY); - if (deadline != null) { - ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); + if (context.getRetrySettings() != null) { + tracer.setTotalTimeoutDuration(context.getRetrySettings().getTotalTimeoutDuration()); } ApiFuture future = innerCallable.futureCall( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index bd53d6d4d8dc..e639ea562774 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -29,6 +29,7 @@ import com.google.cloud.bigtable.Version; import com.google.common.base.Stopwatch; import com.google.common.math.IntMath; +import io.grpc.Deadline; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.LongCounter; @@ -37,7 +38,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -90,8 +90,8 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; private final AtomicLong grpcMessageSentDelay = new AtomicLong(0); - private Duration operationTimeout = Duration.ofMillis(0); - private long remainingOperationTimeout = 0; + private Deadline operationDeadline = null; + private volatile long remainingDeadlineAtAttemptStart = 0; // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them @@ -175,6 +175,9 @@ public void attemptStarted(Object request, int attemptNumber) { this.attempt = attemptNumber; attemptCount++; attemptTimer = Stopwatch.createStarted(); + if (operationDeadline != null) { + remainingDeadlineAtAttemptStart = operationDeadline.timeRemaining(TimeUnit.MILLISECONDS); + } if (request != null) { this.tableId = Util.extractTableId(request); } @@ -185,11 +188,6 @@ public void attemptStarted(Object request, int attemptNumber) { } } } - // OperationTimeout is only set after the first attempt. - if (attemptCount > 1) { - remainingOperationTimeout = - operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS); - } } @Override @@ -301,12 +299,16 @@ public void grpcMessageSent() { grpcMessageSentDelay.set(attemptTimer.elapsed(TimeUnit.NANOSECONDS)); } - /* - This is called by BigtableTracerCallables that sets operation timeout from user settings. - */ @Override - public void setOperationTimeout(Duration operationTimeout) { - this.operationTimeout = operationTimeout; + public void setTotalTimeoutDuration(java.time.Duration totalTimeoutDuration) { + // This method is called by BigtableTracerStreamingCallable and + // BigtableTracerUnaryCallable which is called per attempt. We only set + // the operationDeadline on the first attempt and when totalTimeout is set. + if (operationDeadline == null && !totalTimeoutDuration.isZero()) { + this.operationDeadline = + Deadline.after(totalTimeoutDuration.toMillis(), TimeUnit.MILLISECONDS); + this.remainingDeadlineAtAttemptStart = totalTimeoutDuration.toMillis(); + } } @Override @@ -403,15 +405,10 @@ private void recordAttemptCompletion(@Nullable Throwable status) { attemptLatenciesHistogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); - if (attemptCount <= 1) { - remainingDeadlineHistogram.record(operationTimeout.toMillis(), attributes); - } else if (remainingOperationTimeout >= 0) { - remainingDeadlineHistogram.record(remainingOperationTimeout, attributes); - } else if (operationTimeout.toMillis() != 0) { - // If the operationTimeout is set but remaining deadline is < 0, log a warning. This should - // never happen. - logger.log( - Level.WARNING, "The remaining deadline was less than 0: " + remainingOperationTimeout); + // When operationDeadline is set, it's possible that the deadline is passed by the time we send + // a new attempt. In this case we'll record 0. + if (operationDeadline != null) { + remainingDeadlineHistogram.record(Math.max(0, remainingDeadlineAtAttemptStart), attributes); } if (serverLatencies != null) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index 2cee944aa4e6..7882c82d9326 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -241,9 +241,9 @@ public void grpcMessageSent() { } @Override - public void setOperationTimeout(Duration operationTimeout) { + public void setTotalTimeoutDuration(java.time.Duration totalTimeoutDuration) { for (BigtableTracer tracer : bigtableTracers) { - tracer.setOperationTimeout(operationTimeout); + tracer.setTotalTimeoutDuration(totalTimeoutDuration); } } } 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 e902f0271f41..6842f2c88cc6 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 @@ -134,7 +134,6 @@ public class BuiltinMetricsTracerTest { private static final long APPLICATION_LATENCY = 200; private static final long SLEEP_VARIABILITY = 15; private static final String CLIENT_NAME = "java-bigtable/" + Version.VERSION; - private static final long CHANNEL_BLOCKING_LATENCY = 200; @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); @@ -221,7 +220,7 @@ public void sendHeaders(Metadata headers) { .readRowsSettings() .retrySettings() .setTotalTimeoutDuration(Duration.ofMillis(9000)) - .setMaxRpcTimeoutDuration(Duration.ofMillis(6000)) + .setMaxRpcTimeoutDuration(Duration.ofMillis(9000)) .setRpcTimeoutMultiplier(1) .setInitialRpcTimeoutDuration(Duration.ofMillis(6000)) .setInitialRetryDelayDuration(Duration.ofMillis(10)) @@ -809,7 +808,9 @@ public void testRemainingDeadline() { .get(0); double okRemainingDeadline = okHistogramPointData.getSum(); - assertThat(okRemainingDeadline).isWithin(200).of(8500); + // first attempt latency + retry delay + double expected = 9000 - SERVER_LATENCY - CHANNEL_BLOCKING_LATENCY - 10; + assertThat(okRemainingDeadline).isIn(Range.closed(expected - 500, expected + 10)); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From a94df4a9224b1d44b9ad40ade6c9939699ec81f8 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 13 Nov 2024 14:44:20 -0500 Subject: [PATCH 15/16] chore: deflake trailer skipping test (#2411) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I43324a0916e61e67ff3b7019189a9c54afae38ba Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Rollback plan is reviewed and LGTMed - [ ] All new data plane features have a completed end to end testing plan Fixes # ☕️ If you write sample code, please follow the [samples format]( https://togithub.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md). --- .../data/v2/stub/SkipTrailersTest.java | 58 +++++++++++++++---- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SkipTrailersTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SkipTrailersTest.java index 07ac7deee4bb..935516d8314c 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SkipTrailersTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SkipTrailersTest.java @@ -15,14 +15,11 @@ */ package com.google.cloud.bigtable.data.v2.stub; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; +import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.when; import com.google.api.core.ApiFuture; import com.google.api.gax.core.NoCredentialsProvider; -import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.ApiTracerFactory; import com.google.auto.value.AutoValue; import com.google.bigtable.v2.BigtableGrpc; @@ -43,6 +40,7 @@ import com.google.cloud.bigtable.data.v2.models.TableId; import com.google.cloud.bigtable.data.v2.models.TargetId; import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracer; +import com.google.cloud.bigtable.data.v2.stub.metrics.NoopMetricsProvider; import com.google.common.base.Preconditions; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; @@ -56,10 +54,13 @@ import io.grpc.ServerServiceDefinition; import io.grpc.stub.ServerCalls; import io.grpc.stub.StreamObserver; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -69,7 +70,6 @@ import org.junit.runners.JUnit4; import org.mockito.Mock; import org.mockito.Mockito; -import org.mockito.exceptions.verification.WantedButNotInvoked; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -85,7 +85,7 @@ public class SkipTrailersTest { private Server server; @Mock private ApiTracerFactory tracerFactory; - @Mock private BigtableTracer tracer; + private FakeTracer tracer = new FakeTracer(); private BigtableDataClient client; @@ -95,12 +95,12 @@ public void setUp() throws Exception { server = FakeServiceBuilder.create(hackedService).start(); when(tracerFactory.newTracer(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(tracer); - when(tracer.inScope()).thenReturn(Mockito.mock(ApiTracer.Scope.class)); BigtableDataSettings.Builder clientBuilder = BigtableDataSettings.newBuilderForEmulator(server.getPort()) .setProjectId(PROJECT_ID) .setInstanceId(INSTANCE_ID) + .setMetricsProvider(NoopMetricsProvider.INSTANCE) .setCredentialsProvider(NoCredentialsProvider.create()); clientBuilder.stubSettings().setEnableSkipTrailers(true).setTracerFactory(tracerFactory); @@ -159,7 +159,7 @@ private void test(Supplier> invoker, T fakeResponse) // Wait for the call to start on the server @SuppressWarnings("unchecked") - ServerRpc rpc = (ServerRpc) hackedService.rpcs.poll(10, TimeUnit.SECONDS); + ServerRpc rpc = (ServerRpc) hackedService.rpcs.poll(30, TimeUnit.SECONDS); Preconditions.checkNotNull( rpc, "Timed out waiting for the call to be received by the mock server"); @@ -173,8 +173,21 @@ private void test(Supplier> invoker, T fakeResponse) Assert.fail("timed out waiting for the trailer optimization future to resolve"); } - verify(tracer, times(1)).operationFinishEarly(); - verify(tracer, never()).operationSucceeded(); + // The tracer will be notified in parallel to the future being resolved + // This normal and expected, but requires the test to wait a bit + for (int i = 10; i > 0; i--) { + try { + assertThat(tracer.getCallCount("operationFinishEarly")).isEqualTo(1); + break; + } catch (AssertionError e) { + if (i > 1) { + Thread.sleep(100); + } else { + throw e; + } + } + } + assertThat(tracer.getCallCount("operationSucceeded")).isEqualTo(0); // clean up rpc.getResponseStream().onCompleted(); @@ -183,9 +196,9 @@ private void test(Supplier> invoker, T fakeResponse) // Since we dont have a way to know exactly when this happens, we poll for (int i = 10; i > 0; i--) { try { - verify(tracer, times(1)).operationSucceeded(); + assertThat(tracer.getCallCount("operationSucceeded")).isEqualTo(1); break; - } catch (WantedButNotInvoked e) { + } catch (AssertionError e) { if (i > 1) { Thread.sleep(100); } else { @@ -195,6 +208,27 @@ private void test(Supplier> invoker, T fakeResponse) } } + static class FakeTracer extends BigtableTracer { + ConcurrentHashMap callCounts = new ConcurrentHashMap<>(); + + @Override + public void operationFinishEarly() { + record("operationFinishEarly"); + } + + @Override + public void operationSucceeded() { + record("operationSucceeded"); + } + + private void record(String op) { + callCounts.computeIfAbsent(op, (ignored) -> new AtomicInteger()).getAndIncrement(); + } + + private int getCallCount(String op) { + return Optional.ofNullable(callCounts.get(op)).map(AtomicInteger::get).orElse(0); + } + } /** * Hack the srvice definition to allow grpc server to simulate delayed trailers. This will augment * the bigtable service definition to promote unary rpcs to server streaming From 160391e49865589922e911b0839a7015e01ba076 Mon Sep 17 00:00:00 2001 From: "release-please[bot]" <55107282+release-please[bot]@users.noreply.github.com> Date: Wed, 13 Nov 2024 17:09:41 -0500 Subject: [PATCH 16/16] chore(main): release 2.47.0 (#2407) * chore(main): release 2.47.0 * chore: generate libraries at Wed Nov 13 15:53:21 UTC 2024 --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: cloud-java-bot --- CHANGELOG.md | 13 +++++++++++++ README.md | 6 +++--- google-cloud-bigtable-bom/pom.xml | 16 ++++++++-------- google-cloud-bigtable-deps-bom/pom.xml | 2 +- google-cloud-bigtable-emulator-core/pom.xml | 4 ++-- google-cloud-bigtable-emulator/pom.xml | 10 +++++----- google-cloud-bigtable/pom.xml | 10 +++++----- .../java/com/google/cloud/bigtable/Version.java | 2 +- grpc-google-cloud-bigtable-admin-v2/pom.xml | 8 ++++---- grpc-google-cloud-bigtable-v2/pom.xml | 8 ++++---- pom.xml | 12 ++++++------ proto-google-cloud-bigtable-admin-v2/pom.xml | 8 ++++---- proto-google-cloud-bigtable-v2/pom.xml | 8 ++++---- samples/snapshot/pom.xml | 2 +- test-proxy/pom.xml | 4 ++-- versions.txt | 14 +++++++------- 16 files changed, 70 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6b1fbdc3502..59f59906fb74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## [2.47.0](https://github.com/googleapis/java-bigtable/compare/v2.46.0...v2.47.0) (2024-11-13) + + +### Features + +* Add an experimental feature to skip waiting for trailers for unary ops ([#2404](https://github.com/googleapis/java-bigtable/issues/2404)) ([cf58f26](https://github.com/googleapis/java-bigtable/commit/cf58f260fd7d3cb0dee4fee8e2d43367db6eadb1)) +* Add internal "deadline remaining" client side metric [#2341](https://github.com/googleapis/java-bigtable/issues/2341) ([#2370](https://github.com/googleapis/java-bigtable/issues/2370)) ([75d4105](https://github.com/googleapis/java-bigtable/commit/75d4105e0376dbe5810d6b96d71daa74b85e68ce)) + + +### Bug Fixes + +* Simplify remaining deadline metric impl ([#2410](https://github.com/googleapis/java-bigtable/issues/2410)) ([9796d57](https://github.com/googleapis/java-bigtable/commit/9796d57b60d928d3390e4ad311d5704dcbe808ec)) + ## [2.46.0](https://github.com/googleapis/java-bigtable/compare/v2.45.1...v2.46.0) (2024-10-29) diff --git a/README.md b/README.md index 380b7b99301f..d0d9504f42c9 100644 --- a/README.md +++ b/README.md @@ -56,13 +56,13 @@ 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.46.0' +implementation 'com.google.cloud:google-cloud-bigtable:2.47.0' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-bigtable" % "2.46.0" +libraryDependencies += "com.google.cloud" % "google-cloud-bigtable" % "2.47.0" ``` ## Authentication @@ -543,7 +543,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.46.0 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-bigtable/2.47.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 diff --git a/google-cloud-bigtable-bom/pom.xml b/google-cloud-bigtable-bom/pom.xml index c5fe60b4abef..10524685ce97 100644 --- a/google-cloud-bigtable-bom/pom.xml +++ b/google-cloud-bigtable-bom/pom.xml @@ -3,7 +3,7 @@ 4.0.0 com.google.cloud google-cloud-bigtable-bom - 2.46.1-SNAPSHOT + 2.47.0 pom com.google.cloud @@ -63,37 +63,37 @@ com.google.cloud google-cloud-bigtable - 2.46.1-SNAPSHOT + 2.47.0 com.google.cloud google-cloud-bigtable-emulator - 0.183.1-SNAPSHOT + 0.184.0 com.google.cloud google-cloud-bigtable-emulator-core - 0.183.1-SNAPSHOT + 0.184.0 com.google.api.grpc grpc-google-cloud-bigtable-admin-v2 - 2.46.1-SNAPSHOT + 2.47.0 com.google.api.grpc grpc-google-cloud-bigtable-v2 - 2.46.1-SNAPSHOT + 2.47.0 com.google.api.grpc proto-google-cloud-bigtable-admin-v2 - 2.46.1-SNAPSHOT + 2.47.0 com.google.api.grpc proto-google-cloud-bigtable-v2 - 2.46.1-SNAPSHOT + 2.47.0 diff --git a/google-cloud-bigtable-deps-bom/pom.xml b/google-cloud-bigtable-deps-bom/pom.xml index be0e83e515d7..f99911c8e014 100644 --- a/google-cloud-bigtable-deps-bom/pom.xml +++ b/google-cloud-bigtable-deps-bom/pom.xml @@ -13,7 +13,7 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.1-SNAPSHOT + 2.47.0 pom diff --git a/google-cloud-bigtable-emulator-core/pom.xml b/google-cloud-bigtable-emulator-core/pom.xml index 700cfaf3b8c6..723a47c45086 100644 --- a/google-cloud-bigtable-emulator-core/pom.xml +++ b/google-cloud-bigtable-emulator-core/pom.xml @@ -7,11 +7,11 @@ google-cloud-bigtable-parent com.google.cloud - 2.46.1-SNAPSHOT + 2.47.0 google-cloud-bigtable-emulator-core - 0.183.1-SNAPSHOT + 0.184.0 A Java wrapper for the Cloud Bigtable emulator. diff --git a/google-cloud-bigtable-emulator/pom.xml b/google-cloud-bigtable-emulator/pom.xml index 06583c372653..a63a7864ec94 100644 --- a/google-cloud-bigtable-emulator/pom.xml +++ b/google-cloud-bigtable-emulator/pom.xml @@ -5,7 +5,7 @@ 4.0.0 google-cloud-bigtable-emulator - 0.183.1-SNAPSHOT + 0.184.0 Google Cloud Java - Bigtable Emulator https://github.com/googleapis/java-bigtable @@ -14,7 +14,7 @@ com.google.cloud google-cloud-bigtable-parent - 2.46.1-SNAPSHOT + 2.47.0 scm:git:git@github.com:googleapis/java-bigtable.git @@ -81,14 +81,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import com.google.cloud google-cloud-bigtable-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import @@ -99,7 +99,7 @@ com.google.cloud google-cloud-bigtable-emulator-core - 0.183.1-SNAPSHOT + 0.184.0 diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index 9b09f630936d..fa4758ba7d34 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -2,7 +2,7 @@ 4.0.0 google-cloud-bigtable - 2.46.1-SNAPSHOT + 2.47.0 jar Google Cloud Bigtable https://github.com/googleapis/java-bigtable @@ -12,11 +12,11 @@ com.google.cloud google-cloud-bigtable-parent - 2.46.1-SNAPSHOT + 2.47.0 - 2.46.1-SNAPSHOT + 2.47.0 google-cloud-bigtable @@ -52,14 +52,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import com.google.cloud google-cloud-bigtable-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/Version.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/Version.java index 4ed97314cecf..151defac368b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/Version.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/Version.java @@ -20,6 +20,6 @@ @InternalApi("For internal use only") public final class Version { // {x-version-update-start:google-cloud-bigtable:current} - public static String VERSION = "2.46.1-SNAPSHOT"; + public static String VERSION = "2.47.0"; // {x-version-update-end} } diff --git a/grpc-google-cloud-bigtable-admin-v2/pom.xml b/grpc-google-cloud-bigtable-admin-v2/pom.xml index dd5d1168d4c0..8e9b29bc6d52 100644 --- a/grpc-google-cloud-bigtable-admin-v2/pom.xml +++ b/grpc-google-cloud-bigtable-admin-v2/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc grpc-google-cloud-bigtable-admin-v2 - 2.46.1-SNAPSHOT + 2.47.0 grpc-google-cloud-bigtable-admin-v2 GRPC library for grpc-google-cloud-bigtable-admin-v2 com.google.cloud google-cloud-bigtable-parent - 2.46.1-SNAPSHOT + 2.47.0 @@ -18,14 +18,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import com.google.cloud google-cloud-bigtable-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import diff --git a/grpc-google-cloud-bigtable-v2/pom.xml b/grpc-google-cloud-bigtable-v2/pom.xml index c415de9996c1..840597f9f33b 100644 --- a/grpc-google-cloud-bigtable-v2/pom.xml +++ b/grpc-google-cloud-bigtable-v2/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc grpc-google-cloud-bigtable-v2 - 2.46.1-SNAPSHOT + 2.47.0 grpc-google-cloud-bigtable-v2 GRPC library for grpc-google-cloud-bigtable-v2 com.google.cloud google-cloud-bigtable-parent - 2.46.1-SNAPSHOT + 2.47.0 @@ -18,14 +18,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import com.google.cloud google-cloud-bigtable-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import diff --git a/pom.xml b/pom.xml index 15a243b8ca5d..2d159811d533 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ google-cloud-bigtable-parent pom - 2.46.1-SNAPSHOT + 2.47.0 Google Cloud Bigtable Parent https://github.com/googleapis/java-bigtable @@ -153,27 +153,27 @@ com.google.api.grpc proto-google-cloud-bigtable-v2 - 2.46.1-SNAPSHOT + 2.47.0 com.google.api.grpc proto-google-cloud-bigtable-admin-v2 - 2.46.1-SNAPSHOT + 2.47.0 com.google.api.grpc grpc-google-cloud-bigtable-v2 - 2.46.1-SNAPSHOT + 2.47.0 com.google.api.grpc grpc-google-cloud-bigtable-admin-v2 - 2.46.1-SNAPSHOT + 2.47.0 com.google.cloud google-cloud-bigtable - 2.46.1-SNAPSHOT + 2.47.0 diff --git a/proto-google-cloud-bigtable-admin-v2/pom.xml b/proto-google-cloud-bigtable-admin-v2/pom.xml index 367c6fee2240..710f791a3959 100644 --- a/proto-google-cloud-bigtable-admin-v2/pom.xml +++ b/proto-google-cloud-bigtable-admin-v2/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc proto-google-cloud-bigtable-admin-v2 - 2.46.1-SNAPSHOT + 2.47.0 proto-google-cloud-bigtable-admin-v2 PROTO library for proto-google-cloud-bigtable-admin-v2 com.google.cloud google-cloud-bigtable-parent - 2.46.1-SNAPSHOT + 2.47.0 @@ -18,14 +18,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import com.google.cloud google-cloud-bigtable-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import diff --git a/proto-google-cloud-bigtable-v2/pom.xml b/proto-google-cloud-bigtable-v2/pom.xml index f53212594d59..d01b119e6871 100644 --- a/proto-google-cloud-bigtable-v2/pom.xml +++ b/proto-google-cloud-bigtable-v2/pom.xml @@ -4,13 +4,13 @@ 4.0.0 com.google.api.grpc proto-google-cloud-bigtable-v2 - 2.46.1-SNAPSHOT + 2.47.0 proto-google-cloud-bigtable-v2 PROTO library for proto-google-cloud-bigtable-v2 com.google.cloud google-cloud-bigtable-parent - 2.46.1-SNAPSHOT + 2.47.0 @@ -18,14 +18,14 @@ com.google.cloud google-cloud-bigtable-deps-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import com.google.cloud google-cloud-bigtable-bom - 2.46.1-SNAPSHOT + 2.47.0 pom import diff --git a/samples/snapshot/pom.xml b/samples/snapshot/pom.xml index 4187b8d32ea3..c7f689326964 100644 --- a/samples/snapshot/pom.xml +++ b/samples/snapshot/pom.xml @@ -28,7 +28,7 @@ com.google.cloud google-cloud-bigtable - 2.46.1-SNAPSHOT + 2.47.0 diff --git a/test-proxy/pom.xml b/test-proxy/pom.xml index 03f319f5c674..9b4e46e0b01f 100644 --- a/test-proxy/pom.xml +++ b/test-proxy/pom.xml @@ -12,11 +12,11 @@ google-cloud-bigtable-parent com.google.cloud - 2.46.1-SNAPSHOT + 2.47.0 - 2.46.1-SNAPSHOT + 2.47.0 diff --git a/versions.txt b/versions.txt index e896e3222ef4..5b2ff5f0cbff 100644 --- a/versions.txt +++ b/versions.txt @@ -1,10 +1,10 @@ # Format: # module:released-version:current-version -google-cloud-bigtable:2.46.0:2.46.1-SNAPSHOT -grpc-google-cloud-bigtable-admin-v2:2.46.0:2.46.1-SNAPSHOT -grpc-google-cloud-bigtable-v2:2.46.0:2.46.1-SNAPSHOT -proto-google-cloud-bigtable-admin-v2:2.46.0:2.46.1-SNAPSHOT -proto-google-cloud-bigtable-v2:2.46.0:2.46.1-SNAPSHOT -google-cloud-bigtable-emulator:0.183.0:0.183.1-SNAPSHOT -google-cloud-bigtable-emulator-core:0.183.0:0.183.1-SNAPSHOT +google-cloud-bigtable:2.47.0:2.47.0 +grpc-google-cloud-bigtable-admin-v2:2.47.0:2.47.0 +grpc-google-cloud-bigtable-v2:2.47.0:2.47.0 +proto-google-cloud-bigtable-admin-v2:2.47.0:2.47.0 +proto-google-cloud-bigtable-v2:2.47.0:2.47.0 +google-cloud-bigtable-emulator:0.184.0:0.184.0 +google-cloud-bigtable-emulator-core:0.184.0:0.184.0