From 11c5a2435bda554cf73389ee55a080a65d4d9922 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Sat, 16 Dec 2023 14:42:26 -0500 Subject: [PATCH 01/13] feat: add a flag to add / remove routing cookie from callable chain --- .../data/v2/stub/EnhancedBigtableStub.java | 52 ++++++++++++------ .../v2/stub/EnhancedBigtableStubSettings.java | 31 +++++++++++ .../data/v2/stub/CookiesHolderTest.java | 53 +++++++++++++++++- .../EnhancedBigtableStubSettingsTest.java | 55 +++++++++++++++++-- 4 files changed, 167 insertions(+), 24 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 705b3027ed..15b16e32f0 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 @@ -185,11 +185,13 @@ public static EnhancedBigtableStubSettings finalizeSettings( // workaround JWT audience issues patchCredentials(builder); - // patch cookies interceptor - InstantiatingGrpcChannelProvider.Builder transportProvider = null; - if (builder.getTransportChannelProvider() instanceof InstantiatingGrpcChannelProvider) { - transportProvider = - ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder(); + InstantiatingGrpcChannelProvider.Builder transportProvider = + builder.getTransportChannelProvider() instanceof InstantiatingGrpcChannelProvider + ? ((InstantiatingGrpcChannelProvider) builder.getTransportChannelProvider()).toBuilder() + : null; + + if (builder.getEnableRoutingCookie() && transportProvider != null) { + // patch cookies interceptor transportProvider.setInterceptorProvider(() -> ImmutableList.of(new CookiesInterceptor())); } @@ -371,9 +373,12 @@ public ServerStreamingCallable createReadRowsCallable( new TracedServerStreamingCallable<>( readRowsUserCallable, clientContext.getTracerFactory(), span); - // CookieHolder needs to be injected to the CallOptions outside of retries, otherwise retry - // attempts won't see a CookieHolder. - ServerStreamingCallable withCookie = new CookiesServerStreamingCallable<>(traced); + ServerStreamingCallable withCookie = traced; + if (settings.getEnableRoutingCookie()) { + // CookieHolder needs to be injected to the CallOptions outside of retries, otherwise retry + // attempts won't see a CookieHolder. + withCookie = new CookiesServerStreamingCallable<>(traced); + } return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -411,7 +416,10 @@ public UnaryCallable createReadRowCallable(RowAdapter new TracedUnaryCallable<>( firstRow, clientContext.getTracerFactory(), getSpanName("ReadRow")); - UnaryCallable withCookie = new CookiesUnaryCallable<>(traced); + UnaryCallable withCookie = traced; + if (settings.getEnableRoutingCookie()) { + withCookie = new CookiesUnaryCallable<>(traced); + } return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -654,7 +662,10 @@ private UnaryCallable createBulkMutateRowsCallable() { new TracedUnaryCallable<>( tracedBatcherUnaryCallable, clientContext.getTracerFactory(), spanName); - UnaryCallable withCookie = new CookiesUnaryCallable<>(traced); + UnaryCallable withCookie = traced; + if (settings.getEnableRoutingCookie()) { + withCookie = new CookiesUnaryCallable<>(traced); + } return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -938,8 +949,10 @@ public Map extract( ServerStreamingCallable traced = new TracedServerStreamingCallable<>(retrying, clientContext.getTracerFactory(), span); - ServerStreamingCallable withCookie = - new CookiesServerStreamingCallable<>(traced); + ServerStreamingCallable withCookie = traced; + if (settings.getEnableRoutingCookie()) { + withCookie = new CookiesServerStreamingCallable<>(traced); + } return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -1021,8 +1034,10 @@ public Map extract( new TracedServerStreamingCallable<>( readChangeStreamUserCallable, clientContext.getTracerFactory(), span); - ServerStreamingCallable withCookie = - new CookiesServerStreamingCallable<>(traced); + ServerStreamingCallable withCookie = traced; + if (settings.getEnableRoutingCookie()) { + withCookie = new CookiesServerStreamingCallable<>(traced); + } return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -1037,9 +1052,12 @@ private UnaryCallable createUserFacin UnaryCallable traced = new TracedUnaryCallable<>(inner, clientContext.getTracerFactory(), getSpanName(methodName)); - // CookieHolder needs to be injected to the CallOptions outside of retries, otherwise retry - // attempts won't see a CookieHolder. - UnaryCallable withCookie = new CookiesUnaryCallable<>(traced); + UnaryCallable withCookie = traced; + if (settings.getEnableRoutingCookie()) { + // CookieHolder needs to be injected to the CallOptions outside of retries, otherwise retry + // attempts won't see a CookieHolder. + withCookie = new CookiesUnaryCallable<>(traced); + } return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); } 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 cffd9c85df..d099311ffd 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 @@ -211,6 +211,7 @@ public class EnhancedBigtableStubSettings extends StubSettings primedTableIds; private final Map jwtAudienceMapping; + private final boolean enableRoutingCookie; private final ServerStreamingCallSettings readRowsSettings; private final UnaryCallSettings readRowSettings; @@ -252,6 +253,7 @@ private EnhancedBigtableStubSettings(Builder builder) { isRefreshingChannel = builder.isRefreshingChannel; primedTableIds = builder.primedTableIds; jwtAudienceMapping = builder.jwtAudienceMapping; + enableRoutingCookie = builder.enableRoutingCookie; // Per method settings. readRowsSettings = builder.readRowsSettings.build(); @@ -313,6 +315,14 @@ public Map getJwtAudienceMapping() { return jwtAudienceMapping; } + /** + * Gets if routing cookie is enabled. If true, client will retry a request with extra metadata + * server sent back. + */ + public boolean getEnableRoutingCookie() { + return enableRoutingCookie; + } + /** Returns a builder for the default ChannelProvider for this service. */ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() { return BigtableStubSettings.defaultGrpcTransportProviderBuilder() @@ -595,6 +605,7 @@ public static class Builder extends StubSettings.Builder primedTableIds; private Map jwtAudienceMapping; + private boolean enableRoutingCookie; private final ServerStreamingCallSettings.Builder readRowsSettings; private final UnaryCallSettings.Builder readRowSettings; @@ -627,6 +638,7 @@ private Builder() { primedTableIds = ImmutableList.of(); jwtAudienceMapping = DEFAULT_JWT_AUDIENCE_MAPPING; setCredentialsProvider(defaultCredentialsProviderBuilder().build()); + this.enableRoutingCookie = true; // Defaults provider BigtableStubSettings.Builder baseDefaults = BigtableStubSettings.newBuilder(); @@ -745,6 +757,7 @@ private Builder(EnhancedBigtableStubSettings settings) { isRefreshingChannel = settings.isRefreshingChannel; primedTableIds = settings.primedTableIds; jwtAudienceMapping = settings.jwtAudienceMapping; + enableRoutingCookie = settings.enableRoutingCookie; // Per method settings. readRowsSettings = settings.readRowsSettings.toBuilder(); @@ -893,6 +906,23 @@ public Map getJwtAudienceMapping() { return jwtAudienceMapping; } + /** + * Sets if routing cookie is enabled. If true, client will retry a request with extra metadata + * server sent back. + */ + public Builder setEnableRoutingCookie(boolean enableRoutingCookie) { + this.enableRoutingCookie = enableRoutingCookie; + return this; + } + + /** + * Gets if routing cookie is enabled. If true, client will retry a request with extra metadata + * server sent back. + */ + public boolean getEnableRoutingCookie() { + return enableRoutingCookie; + } + /** Returns the builder for the settings used for calls to readRows. */ public ServerStreamingCallSettings.Builder readRowsSettings() { return readRowsSettings; @@ -1019,6 +1049,7 @@ public String toString() { .add("isRefreshingChannel", isRefreshingChannel) .add("primedTableIds", primedTableIds) .add("jwtAudienceMapping", jwtAudienceMapping) + .add("enableRoutingCookie", enableRoutingCookie) .add("readRowsSettings", readRowsSettings) .add("readRowSettings", readRowSettings) .add("sampleRowKeysSettings", sampleRowKeysSettings) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index 5dac053523..a52bd84592 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -58,6 +58,7 @@ import io.grpc.Status; import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; +import java.io.IOException; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -83,6 +84,7 @@ public class CookiesHolderTest { private Server server; private final FakeService fakeService = new FakeService(); + private BigtableDataSettings.Builder settings; private BigtableDataClient client; private final List serverMetadata = new ArrayList<>(); @@ -138,6 +140,8 @@ public ServerCall.Listener interceptCall( .build()) .setRetryableCodes(StatusCode.Code.UNAVAILABLE); + this.settings = settings; + client = BigtableDataClient.create(settings.build()); } @@ -379,7 +383,7 @@ public void sendHeaders(Metadata headers) { } @Test - public void testAllMethodsAreCalled() throws InterruptedException { + public void testAllMethodsAreCalled() { // This test ensures that all methods respect the retry cookie except for the ones that are // explicitly added to the methods list. It requires that any newly method is exercised in this // test. This is enforced by introspecting grpc method descriptors. @@ -422,6 +426,53 @@ public void testAllMethodsAreCalled() throws InterruptedException { assertThat(methods).containsExactlyElementsIn(expected); } + @Test + public void testDisableRoutingCookie() throws IOException { + // This test disables routing cookie in the client settings and ensures that none of the routing + // cookie + // is added. + settings.stubSettings().setEnableRoutingCookie(false); + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + client.readRows(Query.create("fake-table")).iterator().hasNext(); + assertThat(fakeService.count.get()).isEqualTo(2); + fakeService.count.set(0); + + client.mutateRow(RowMutation.create("fake-table", "key").setCell("cf", "q", "v")); + assertThat(fakeService.count.get()).isEqualTo(2); + fakeService.count.set(0); + + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("key").setCell("cf", "q", "v"))); + assertThat(fakeService.count.get()).isEqualTo(2); + fakeService.count.set(0); + + client.sampleRowKeys("fake-table"); + assertThat(fakeService.count.get()).isEqualTo(2); + fakeService.count.set(0); + + client.checkAndMutateRow( + ConditionalRowMutation.create("fake-table", "key") + .then(Mutation.create().setCell("cf", "q", "v"))); + assertThat(fakeService.count.get()).isEqualTo(2); + fakeService.count.set(0); + + client.readModifyWriteRow( + ReadModifyWriteRow.create("fake-table", "key").append("cf", "q", "v")); + assertThat(fakeService.count.get()).isEqualTo(2); + fakeService.count.set(0); + + client.generateInitialChangeStreamPartitions("fake-table").iterator().hasNext(); + assertThat(fakeService.count.get()).isEqualTo(2); + fakeService.count.set(0); + + client.readChangeStream(ReadChangeStreamQuery.create("fake-table")).iterator().hasNext(); + assertThat(fakeService.count.get()).isEqualTo(2); + + assertThat(methods).isEmpty(); + } + } + static class FakeService extends BigtableGrpc.BigtableImplBase { private boolean returnCookie = true; 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 fbd6442e0c..2c05ca4ee8 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 @@ -77,6 +77,7 @@ public void settingsAreNotLostTest() { CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); WatchdogProvider watchdogProvider = Mockito.mock(WatchdogProvider.class); Duration watchdogInterval = Duration.ofSeconds(12); + boolean enableRoutingCookie = false; EnhancedBigtableStubSettings.Builder builder = EnhancedBigtableStubSettings.newBuilder() @@ -87,7 +88,8 @@ public void settingsAreNotLostTest() { .setEndpoint(endpoint) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .setStreamWatchdogCheckInterval(watchdogInterval); + .setStreamWatchdogCheckInterval(watchdogInterval) + .setEnableRoutingCookie(enableRoutingCookie); verifyBuilder( builder, @@ -98,7 +100,8 @@ public void settingsAreNotLostTest() { endpoint, credentialsProvider, watchdogProvider, - watchdogInterval); + watchdogInterval, + enableRoutingCookie); verifySettings( builder.build(), projectId, @@ -108,7 +111,8 @@ public void settingsAreNotLostTest() { endpoint, credentialsProvider, watchdogProvider, - watchdogInterval); + watchdogInterval, + enableRoutingCookie); verifyBuilder( builder.build().toBuilder(), projectId, @@ -118,7 +122,8 @@ public void settingsAreNotLostTest() { endpoint, credentialsProvider, watchdogProvider, - watchdogInterval); + watchdogInterval, + enableRoutingCookie); } private void verifyBuilder( @@ -130,7 +135,8 @@ private void verifyBuilder( String endpoint, CredentialsProvider credentialsProvider, WatchdogProvider watchdogProvider, - Duration watchdogInterval) { + Duration watchdogInterval, + boolean enableRoutingCookie) { assertThat(builder.getProjectId()).isEqualTo(projectId); assertThat(builder.getInstanceId()).isEqualTo(instanceId); assertThat(builder.getAppProfileId()).isEqualTo(appProfileId); @@ -139,6 +145,7 @@ private void verifyBuilder( assertThat(builder.getCredentialsProvider()).isEqualTo(credentialsProvider); assertThat(builder.getStreamWatchdogProvider()).isSameInstanceAs(watchdogProvider); assertThat(builder.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); + assertThat(builder.getEnableRoutingCookie()).isEqualTo(enableRoutingCookie); } private void verifySettings( @@ -150,7 +157,8 @@ private void verifySettings( String endpoint, CredentialsProvider credentialsProvider, WatchdogProvider watchdogProvider, - Duration watchdogInterval) { + Duration watchdogInterval, + boolean enableRoutingCookie) { assertThat(settings.getProjectId()).isEqualTo(projectId); assertThat(settings.getInstanceId()).isEqualTo(instanceId); assertThat(settings.getAppProfileId()).isEqualTo(appProfileId); @@ -159,6 +167,7 @@ private void verifySettings( assertThat(settings.getCredentialsProvider()).isEqualTo(credentialsProvider); assertThat(settings.getStreamWatchdogProvider()).isSameInstanceAs(watchdogProvider); assertThat(settings.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); + assertThat(settings.getEnableRoutingCookie()).isEqualTo(enableRoutingCookie); } @Test @@ -781,6 +790,39 @@ public void isRefreshingChannelFalseValueTest() { assertThat(builder.build().toBuilder().isRefreshingChannel()).isFalse(); } + @Test + public void routingCookieIsEnabled() throws IOException { + String dummyProjectId = "my-project"; + String dummyInstanceId = "my-instance"; + CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); + Mockito.when(credentialsProvider.getCredentials()).thenReturn(new FakeCredentials()); + EnhancedBigtableStubSettings.Builder builder = + EnhancedBigtableStubSettings.newBuilder() + .setProjectId(dummyProjectId) + .setInstanceId(dummyInstanceId) + .setCredentialsProvider(credentialsProvider); + assertThat(builder.getEnableRoutingCookie()).isTrue(); + assertThat(builder.build().getEnableRoutingCookie()).isTrue(); + assertThat(builder.build().toBuilder().getEnableRoutingCookie()).isTrue(); + } + + @Test + public void routingCookieFalseValueSet() throws IOException { + String dummyProjectId = "my-project"; + String dummyInstanceId = "my-instance"; + CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); + Mockito.when(credentialsProvider.getCredentials()).thenReturn(new FakeCredentials()); + EnhancedBigtableStubSettings.Builder builder = + EnhancedBigtableStubSettings.newBuilder() + .setProjectId(dummyProjectId) + .setInstanceId(dummyInstanceId) + .setEnableRoutingCookie(false) + .setCredentialsProvider(credentialsProvider); + assertThat(builder.getEnableRoutingCookie()).isFalse(); + assertThat(builder.build().getEnableRoutingCookie()).isFalse(); + assertThat(builder.build().toBuilder().getEnableRoutingCookie()).isFalse(); + } + static final String[] SETTINGS_LIST = { "projectId", "instanceId", @@ -788,6 +830,7 @@ public void isRefreshingChannelFalseValueTest() { "isRefreshingChannel", "primedTableIds", "jwtAudienceMapping", + "enableRoutingCookie", "readRowsSettings", "readRowSettings", "sampleRowKeysSettings", From cd0996d5a8c74e7a85ec635e835988f58cc8fc7b Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 18 Dec 2023 11:22:22 -0500 Subject: [PATCH 02/13] refactor --- .../data/v2/stub/EnhancedBigtableStub.java | 86 +++++++++---------- .../data/v2/stub/CookiesHolderTest.java | 73 ++++++++++++++++ .../v2/stub/EnhancedBigtableStubTest.java | 1 + 3 files changed, 116 insertions(+), 44 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 15b16e32f0..1fb5bca9a9 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 @@ -37,6 +37,7 @@ import com.google.api.gax.rpc.RequestParamsExtractor; import com.google.api.gax.rpc.ServerStreamingCallSettings; import com.google.api.gax.rpc.ServerStreamingCallable; +import com.google.api.gax.rpc.UnaryCallSettings; import com.google.api.gax.rpc.UnaryCallable; import com.google.api.gax.tracing.OpencensusTracerFactory; import com.google.api.gax.tracing.SpanName; @@ -373,14 +374,7 @@ public ServerStreamingCallable createReadRowsCallable( new TracedServerStreamingCallable<>( readRowsUserCallable, clientContext.getTracerFactory(), span); - ServerStreamingCallable withCookie = traced; - if (settings.getEnableRoutingCookie()) { - // CookieHolder needs to be injected to the CallOptions outside of retries, otherwise retry - // attempts won't see a CookieHolder. - withCookie = new CookiesServerStreamingCallable<>(traced); - } - - return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -416,12 +410,7 @@ public UnaryCallable createReadRowCallable(RowAdapter new TracedUnaryCallable<>( firstRow, clientContext.getTracerFactory(), getSpanName("ReadRow")); - UnaryCallable withCookie = traced; - if (settings.getEnableRoutingCookie()) { - withCookie = new CookiesUnaryCallable<>(traced); - } - - return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -493,7 +482,7 @@ public Map extract(ReadRowsRequest readRowsRequest) { new ReadRowsRetryCompletedCallable<>(withBigtableTracer); ServerStreamingCallable retrying2 = - Callables.retrying(retrying1, innerSettings, clientContext); + withRetries(retrying1, innerSettings); return new FilterMarkerRowsCallable<>(retrying2, rowAdapter); } @@ -576,7 +565,7 @@ public Map extract( new BigtableTracerUnaryCallable<>(withStatsHeaders); UnaryCallable> retryable = - Callables.retrying(withBigtableTracer, settings.sampleRowKeysSettings(), clientContext); + withRetries(withBigtableTracer, settings.sampleRowKeysSettings()); return createUserFacingUnaryCallable( methodName, new SampleRowKeysCallable(retryable, requestContext)); @@ -615,7 +604,7 @@ public Map extract(MutateRowRequest mutateRowRequest) { new BigtableTracerUnaryCallable<>(withStatsHeaders); UnaryCallable retrying = - Callables.retrying(withBigtableTracer, settings.mutateRowSettings(), clientContext); + withRetries(withBigtableTracer, settings.mutateRowSettings()); return createUserFacingUnaryCallable( methodName, new MutateRowCallable(retrying, requestContext)); @@ -639,11 +628,17 @@ public Map extract(MutateRowRequest mutateRowRequest) { private UnaryCallable createBulkMutateRowsCallable() { UnaryCallable baseCallable = createMutateRowsBaseCallable(); + UnaryCallable withCookie = baseCallable; + + if (settings.getEnableRoutingCookie()) { + withCookie = new CookiesUnaryCallable<>(baseCallable); + } + UnaryCallable flowControlCallable = null; if (settings.bulkMutateRowsSettings().isLatencyBasedThrottlingEnabled()) { flowControlCallable = new DynamicFlowControlCallable( - baseCallable, + withCookie, bulkMutationFlowController, bulkMutationDynamicFlowControlStats, settings.bulkMutateRowsSettings().getTargetRpcLatencyMs(), @@ -651,7 +646,7 @@ private UnaryCallable createBulkMutateRowsCallable() { } UnaryCallable userFacing = new BulkMutateRowsUserFacingCallable( - flowControlCallable != null ? flowControlCallable : baseCallable, requestContext); + flowControlCallable != null ? flowControlCallable : withCookie, requestContext); SpanName spanName = getSpanName("MutateRows"); @@ -662,12 +657,7 @@ private UnaryCallable createBulkMutateRowsCallable() { new TracedUnaryCallable<>( tracedBatcherUnaryCallable, clientContext.getTracerFactory(), spanName); - UnaryCallable withCookie = traced; - if (settings.getEnableRoutingCookie()) { - withCookie = new CookiesUnaryCallable<>(traced); - } - - return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -821,7 +811,7 @@ public Map extract( new BigtableTracerUnaryCallable<>(withStatsHeaders); UnaryCallable retrying = - Callables.retrying(withBigtableTracer, settings.checkAndMutateRowSettings(), clientContext); + withRetries(withBigtableTracer, settings.checkAndMutateRowSettings()); return createUserFacingUnaryCallable( methodName, new CheckAndMutateRowCallable(retrying, requestContext)); @@ -862,8 +852,7 @@ public Map extract(ReadModifyWriteRowRequest request) { new BigtableTracerUnaryCallable<>(withStatsHeaders); UnaryCallable retrying = - Callables.retrying( - withBigtableTracer, settings.readModifyWriteRowSettings(), clientContext); + withRetries(withBigtableTracer, settings.readModifyWriteRowSettings()); return createUserFacingUnaryCallable( methodName, new ReadModifyWriteRowCallable(retrying, requestContext)); @@ -943,18 +932,13 @@ public Map extract( new BigtableTracerStreamingCallable<>(watched); ServerStreamingCallable retrying = - Callables.retrying(withBigtableTracer, innerSettings, clientContext); + withRetries(withBigtableTracer, innerSettings); SpanName span = getSpanName("GenerateInitialChangeStreamPartitions"); ServerStreamingCallable traced = new TracedServerStreamingCallable<>(retrying, clientContext.getTracerFactory(), span); - ServerStreamingCallable withCookie = traced; - if (settings.getEnableRoutingCookie()) { - withCookie = new CookiesServerStreamingCallable<>(traced); - } - - return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** @@ -1023,7 +1007,7 @@ public Map extract( new BigtableTracerStreamingCallable<>(watched); ServerStreamingCallable readChangeStreamCallable = - Callables.retrying(withBigtableTracer, innerSettings, clientContext); + withRetries(withBigtableTracer, innerSettings); ServerStreamingCallable readChangeStreamUserCallable = @@ -1052,14 +1036,7 @@ private UnaryCallable createUserFacin UnaryCallable traced = new TracedUnaryCallable<>(inner, clientContext.getTracerFactory(), getSpanName(methodName)); - UnaryCallable withCookie = traced; - if (settings.getEnableRoutingCookie()) { - // CookieHolder needs to be injected to the CallOptions outside of retries, otherwise retry - // attempts won't see a CookieHolder. - withCookie = new CookiesUnaryCallable<>(traced); - } - - return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } private UnaryCallable createPingAndWarmCallable() { @@ -1080,6 +1057,27 @@ public Map extract(PingAndWarmRequest request) { Collections.emptySet()); return pingAndWarm.withDefaultCallContext(clientContext.getDefaultCallContext()); } + + private UnaryCallable withRetries( + UnaryCallable innerCallable, UnaryCallSettings unaryCallSettings) { + UnaryCallable retrying = + Callables.retrying(innerCallable, unaryCallSettings, clientContext); + if (settings.getEnableRoutingCookie()) { + return new CookiesUnaryCallable<>(retrying); + } + return retrying; + } + + private ServerStreamingCallable withRetries( + ServerStreamingCallable innerCallable, + ServerStreamingCallSettings serverStreamingCallSettings) { + ServerStreamingCallable retrying = + Callables.retrying(innerCallable, serverStreamingCallSettings, clientContext); + if (settings.getEnableRoutingCookie()) { + return new CookiesServerStreamingCallable<>(retrying); + } + return retrying; + } // // diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index a52bd84592..b9fa2c4a9d 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -240,6 +240,43 @@ public void testSampleRowKeys() { serverMetadata.clear(); } + @Test + public void testReadChangeStream() { + client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + assertThat(lastMetadata) + .containsAtLeast( + ROUTING_COOKIE_1.name(), "readChangeStream", ROUTING_COOKIE_2.name(), testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + + serverMetadata.clear(); + } + + @Test + public void testGenerateInitialChangeStreamParition() { + client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + assertThat(lastMetadata) + .containsAtLeast( + ROUTING_COOKIE_1.name(), + "generateInitialChangeStreamPartitions", + ROUTING_COOKIE_2.name(), + testCookie); + assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); + + serverMetadata.clear(); + } + @Test public void testNoCookieSucceedReadRows() { fakeService.returnCookie = false; @@ -326,6 +363,42 @@ public void testNoCookieSucceedSampleRowKeys() { serverMetadata.clear(); } + @Test + public void testNoCookieSucceedReadChangeStream() { + fakeService.returnCookie = false; + + client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + assertThat(lastMetadata) + .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); + + serverMetadata.clear(); + + serverMetadata.clear(); + } + + @Test + public void testNoCookieSucceedGenerateInitialChangeStreamParition() { + fakeService.returnCookie = false; + + client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); + + assertThat(fakeService.count.get()).isGreaterThan(1); + assertThat(serverMetadata).hasSize(fakeService.count.get()); + + Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + + assertThat(lastMetadata) + .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); + + serverMetadata.clear(); + } + @Test public void testCookiesInHeaders() throws Exception { // Send 2 cookies in the headers, with routingCookieKey and ROUTING_COOKIE_2. ROUTING_COOKIE_2 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 e36eb1a8a9..eacf145bcb 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 @@ -549,6 +549,7 @@ public void testBulkMutationFlowControlFeatureFlagIsNotSet() throws Exception { FeatureFlags featureFlags = FeatureFlags.parseFrom(decodedFlags); assertThat(featureFlags.getMutateRowsRateLimit()).isFalse(); assertThat(featureFlags.getMutateRowsRateLimit2()).isFalse(); + stub.close(); } @Test From 82f5cd75d081eda1acf9c7f7ac45ebd5a3f619ba Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 18 Dec 2023 11:49:07 -0500 Subject: [PATCH 03/13] mark as beta api --- .../bigtable/data/v2/stub/EnhancedBigtableStubSettings.java | 4 ++++ 1 file changed, 4 insertions(+) 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 d099311ffd..64f44bb52f 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 @@ -15,6 +15,7 @@ */ package com.google.cloud.bigtable.data.v2.stub; +import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.api.gax.batching.BatchingCallSettings; import com.google.api.gax.batching.BatchingSettings; @@ -319,6 +320,7 @@ public Map getJwtAudienceMapping() { * Gets if routing cookie is enabled. If true, client will retry a request with extra metadata * server sent back. */ + @BetaApi("Routing cookie is not currently stable and may change in the future") public boolean getEnableRoutingCookie() { return enableRoutingCookie; } @@ -910,6 +912,7 @@ public Map getJwtAudienceMapping() { * Sets if routing cookie is enabled. If true, client will retry a request with extra metadata * server sent back. */ + @BetaApi("Routing cookie is not currently stable and may change in the future") public Builder setEnableRoutingCookie(boolean enableRoutingCookie) { this.enableRoutingCookie = enableRoutingCookie; return this; @@ -919,6 +922,7 @@ public Builder setEnableRoutingCookie(boolean enableRoutingCookie) { * Gets if routing cookie is enabled. If true, client will retry a request with extra metadata * server sent back. */ + @BetaApi("Routing cookie is not currently stable and may change in the future") public boolean getEnableRoutingCookie() { return enableRoutingCookie; } From 2b700b01645a0e8e5fd3024d5e65136b877b9695 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 18 Dec 2023 15:24:31 -0500 Subject: [PATCH 04/13] add a test for initial metadata --- .../bigtable/data/v2/stub/CookiesHolder.java | 8 +-- .../data/v2/stub/EnhancedBigtableStub.java | 7 +-- .../data/v2/stub/CookiesHolderTest.java | 61 ++++++++++++++++--- 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java index 7d7ca6a029..7a153cfd5f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolder.java @@ -55,14 +55,14 @@ Metadata injectCookiesInRequestHeaders(Metadata headers) { * COOKIE_KEY_PREFIX to cookies. Values in trailers will override the value set in initial * metadata for the same keys. */ - void extractCookiesFromMetadata(@Nullable Metadata trailers) { - if (trailers == null) { + void extractCookiesFromMetadata(@Nullable Metadata metadata) { + if (metadata == null) { return; } - for (String key : trailers.keys()) { + for (String key : metadata.keys()) { if (key.startsWith(COOKIE_KEY_PREFIX)) { Metadata.Key metadataKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER); - String value = trailers.get(metadataKey); + String value = metadata.get(metadataKey); cookies.put(metadataKey, value); } } 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 1fb5bca9a9..3b441972dc 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 @@ -1018,12 +1018,7 @@ public Map extract( new TracedServerStreamingCallable<>( readChangeStreamUserCallable, clientContext.getTracerFactory(), span); - ServerStreamingCallable withCookie = traced; - if (settings.getEnableRoutingCookie()) { - withCookie = new CookiesServerStreamingCallable<>(traced); - } - - return withCookie.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } /** diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java index b9fa2c4a9d..5058e4e19e 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/CookiesHolderTest.java @@ -78,9 +78,14 @@ public class CookiesHolderTest { Metadata.Key.of("x-goog-cbt-cookie-routing", Metadata.ASCII_STRING_MARSHALLER); private static final Metadata.Key ROUTING_COOKIE_2 = Metadata.Key.of("x-goog-cbt-cookie-random", Metadata.ASCII_STRING_MARSHALLER); + private static final Metadata.Key ROUTING_COOKIE_HEADER = + Metadata.Key.of("x-goog-cbt-cookie-header", Metadata.ASCII_STRING_MARSHALLER); private static final Metadata.Key BAD_KEY = Metadata.Key.of("x-goog-cbt-not-cookie", Metadata.ASCII_STRING_MARSHALLER); + + private static final String testHeaderCookie = "header-cookie"; private static final String testCookie = "test-routing-cookie"; + private static final String routingCookie1Header = "should-be-overridden"; private Server server; private final FakeService fakeService = new FakeService(); @@ -103,7 +108,16 @@ public ServerCall.Listener interceptCall( if (metadata.containsKey(ROUTING_COOKIE_1)) { methods.add(serverCall.getMethodDescriptor().getBareMethodName()); } - return serverCallHandler.startCall(serverCall, metadata); + return serverCallHandler.startCall( + new ForwardingServerCall.SimpleForwardingServerCall(serverCall) { + @Override + public void sendHeaders(Metadata responseHeaders) { + responseHeaders.put(ROUTING_COOKIE_HEADER, testHeaderCookie); + responseHeaders.put(ROUTING_COOKIE_1, routingCookie1Header); + super.sendHeaders(responseHeaders); + } + }, + metadata); } }; @@ -157,6 +171,7 @@ public void tearDown() throws Exception { @Test public void testReadRows() { + // Only server stream calls can return an initial metadata before the first response client.readRows(Query.create("fake-table")).iterator().hasNext(); assertThat(fakeService.count.get()).isGreaterThan(1); @@ -165,7 +180,13 @@ public void testReadRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); assertThat(lastMetadata) - .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); + .containsAtLeast( + ROUTING_COOKIE_1.name(), + "readRows", + ROUTING_COOKIE_2.name(), + testCookie, + ROUTING_COOKIE_HEADER.name(), + testHeaderCookie); assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); @@ -181,7 +202,13 @@ public void testReadRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); assertThat(lastMetadata) - .containsAtLeast(ROUTING_COOKIE_1.name(), "readRows", ROUTING_COOKIE_2.name(), testCookie); + .containsAtLeast( + ROUTING_COOKIE_1.name(), + "readRows", + ROUTING_COOKIE_2.name(), + testCookie, + ROUTING_COOKIE_HEADER.name(), + testHeaderCookie); assertThat(lastMetadata).doesNotContainKeys(BAD_KEY.name()); serverMetadata.clear(); @@ -249,6 +276,9 @@ public void testReadChangeStream() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + // TODO: read change stream should have ROUTING_COOKIE_HEADER but the sequence is different from + // ReadRows. + // Debug the test failure. assertThat(lastMetadata) .containsAtLeast( ROUTING_COOKIE_1.name(), "readChangeStream", ROUTING_COOKIE_2.name(), testCookie); @@ -266,6 +296,9 @@ public void testGenerateInitialChangeStreamParition() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); + // TODO: read change stream should have ROUTING_COOKIE_HEADER but the sequence is different from + // ReadRows. + // Debug the test failure. assertThat(lastMetadata) .containsAtLeast( ROUTING_COOKIE_1.name(), @@ -288,7 +321,9 @@ public void testNoCookieSucceedReadRows() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name()); + assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_2.name()); + // Should contain initial metadata + assertThat(lastMetadata).containsAtLeast(ROUTING_COOKIE_1.name(), routingCookie1Header); serverMetadata.clear(); } @@ -304,8 +339,8 @@ public void testNoCookieSucceedReadRow() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) - .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); + assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_2.name(), BAD_KEY.name()); + assertThat(lastMetadata).containsAtLeast(ROUTING_COOKIE_1.name(), routingCookie1Header); serverMetadata.clear(); } @@ -374,8 +409,11 @@ public void testNoCookieSucceedReadChangeStream() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) - .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); + assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_2.name(), BAD_KEY.name()); + // TODO: read change stream should have ROUTING_COOKIE_HEADER but the sequence is different from + // ReadRows. + // Debug the test failure. + // assertThat(lastMetadata).containsAtLeast(ROUTING_COOKIE_1.name(), routingCookie1Header); serverMetadata.clear(); @@ -393,8 +431,11 @@ public void testNoCookieSucceedGenerateInitialChangeStreamParition() { Metadata lastMetadata = serverMetadata.get(fakeService.count.get() - 1); - assertThat(lastMetadata) - .doesNotContainKeys(ROUTING_COOKIE_1.name(), ROUTING_COOKIE_2.name(), BAD_KEY.name()); + assertThat(lastMetadata).doesNotContainKeys(ROUTING_COOKIE_2.name(), BAD_KEY.name()); + // TODO: read change stream should have ROUTING_COOKIE_HEADER but the sequence is different from + // ReadRows. + // Debug the test failure. + // assertThat(lastMetadata).containsAtLeast(ROUTING_COOKIE_1.name(), routingCookie1Header); serverMetadata.clear(); } From bec1f11766cacfeaab380ede2efc2258363ecc5a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 21 Nov 2023 15:43:14 -0500 Subject: [PATCH 05/13] feat: handle retry info so client respect the delay server sets --- .../mutaterows/MutateRowsAttemptCallable.java | 4 +- .../retrying/ApiResultRetryAlgorithm.java | 62 ++- .../bigtable/gaxx/retrying/Callables.java | 2 +- .../gaxx/retrying/UnaryRetryAlgorithm.java | 51 +++ .../bigtable/data/v2/stub/RetryInfoTest.java | 377 ++++++++++++++++++ 5 files changed, 487 insertions(+), 9 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/UnaryRetryAlgorithm.java create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java index b049219a95..b6184bca44 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java @@ -31,6 +31,7 @@ import com.google.bigtable.v2.MutateRowsResponse.Entry; import com.google.cloud.bigtable.data.v2.models.MutateRowsException; import com.google.cloud.bigtable.data.v2.models.MutateRowsException.FailedMutation; +import com.google.cloud.bigtable.gaxx.retrying.ApiResultRetryAlgorithm; import com.google.cloud.bigtable.gaxx.retrying.NonCancellableFuture; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -235,7 +236,8 @@ private void handleAttemptError(Throwable rpcError) { FailedMutation failedMutation = FailedMutation.create(origIndex, entryError); allFailures.add(failedMutation); - if (!failedMutation.getError().isRetryable()) { + if (ApiResultRetryAlgorithm.extractRetryDelay(failedMutation.getError()) == null + && !failedMutation.getError().isRetryable()) { permanentFailures.add(failedMutation); } else { // Schedule the mutation entry for the next RPC by adding it to the request builder and diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiResultRetryAlgorithm.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiResultRetryAlgorithm.java index d71a044235..91d893e3f3 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiResultRetryAlgorithm.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiResultRetryAlgorithm.java @@ -18,17 +18,46 @@ import com.google.api.core.InternalApi; import com.google.api.gax.retrying.BasicResultRetryAlgorithm; import com.google.api.gax.retrying.RetryingContext; +import com.google.api.gax.retrying.TimedAttemptSettings; import com.google.api.gax.rpc.ApiException; +import com.google.protobuf.util.Durations; +import com.google.rpc.RetryInfo; +import io.grpc.Metadata; +import io.grpc.Status; +import io.grpc.protobuf.ProtoUtils; +import org.threeten.bp.Duration; -/** For internal use, public for technical reasons. */ +/** + * For internal use, public for technical reasons. This retry algorithm checks the metadata of an + * exception for additional error details. If the metadata has a RetryInfo field, use the retry + * delay to set the wait time between attempts. + */ @InternalApi public class ApiResultRetryAlgorithm extends BasicResultRetryAlgorithm { + private static final Metadata.Key KEY_RETRY_INFO = + ProtoUtils.keyForProto(RetryInfo.getDefaultInstance()); + + @Override + public TimedAttemptSettings createNextAttempt( + Throwable prevThrowable, ResponseT prevResponse, TimedAttemptSettings prevSettings) { + Duration retryDelay = extractRetryDelay(prevThrowable); + if (retryDelay != null) { + return prevSettings + .toBuilder() + .setRandomizedRetryDelay(retryDelay) + .setAttemptCount(prevSettings.getAttemptCount() + 1) + .build(); + } + return null; + } + /** Returns true if previousThrowable is an {@link ApiException} that is retryable. */ @Override public boolean shouldRetry(Throwable previousThrowable, ResponseT previousResponse) { - return (previousThrowable instanceof ApiException) - && ((ApiException) previousThrowable).isRetryable(); + return (extractRetryDelay(previousThrowable) != null) + || (previousThrowable instanceof ApiException + && ((ApiException) previousThrowable).isRetryable()); } /** @@ -43,11 +72,30 @@ public boolean shouldRetry( if (context.getRetryableCodes() != null) { // Ignore the isRetryable() value of the throwable if the RetryingContext has a specific list // of codes that should be retried. - return (previousThrowable instanceof ApiException) - && context - .getRetryableCodes() - .contains(((ApiException) previousThrowable).getStatusCode().getCode()); + return extractRetryDelay(previousThrowable) != null + || ((previousThrowable instanceof ApiException) + && context + .getRetryableCodes() + .contains(((ApiException) previousThrowable).getStatusCode().getCode())); } return shouldRetry(previousThrowable, previousResponse); } + + public static Duration extractRetryDelay(Throwable throwable) { + if (throwable == null) { + return null; + } + Metadata trailers = Status.trailersFromThrowable(throwable); + if (trailers == null) { + return null; + } + RetryInfo retryInfo = trailers.get(KEY_RETRY_INFO); + if (retryInfo == null) { + return null; + } + if (!retryInfo.hasRetryDelay()) { + return null; + } + return Duration.ofMillis(Durations.toMillis(retryInfo.getRetryDelay())); + } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java index e62d371ac3..5d2f9ed28d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java @@ -58,7 +58,7 @@ public static UnaryCallable retrying( } RetryAlgorithm retryAlgorithm = - new RetryAlgorithm<>( + new UnaryRetryAlgorithm<>( new ApiResultRetryAlgorithm(), new ExponentialRetryAlgorithm(settings.getRetrySettings(), clientContext.getClock())); ScheduledRetryingExecutor retryingExecutor = diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/UnaryRetryAlgorithm.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/UnaryRetryAlgorithm.java new file mode 100644 index 0000000000..0966e16881 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/UnaryRetryAlgorithm.java @@ -0,0 +1,51 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.gaxx.retrying; + +import com.google.api.core.InternalApi; +import com.google.api.gax.retrying.ResultRetryAlgorithmWithContext; +import com.google.api.gax.retrying.RetryAlgorithm; +import com.google.api.gax.retrying.RetryingContext; +import com.google.api.gax.retrying.TimedAttemptSettings; +import com.google.api.gax.retrying.TimedRetryAlgorithmWithContext; +import java.util.concurrent.CancellationException; + +/** + * Retry algorithm for unary calls. It'll use the result from result algorithm first and only fall + * back to timedAlgorithm if resultAlgorithm#shouldRetry is false. + */ +@InternalApi +public class UnaryRetryAlgorithm extends RetryAlgorithm { + + public UnaryRetryAlgorithm( + ResultRetryAlgorithmWithContext resultAlgorithm, + TimedRetryAlgorithmWithContext timedAlgorithm) { + super(resultAlgorithm, timedAlgorithm); + } + + @Override + public boolean shouldRetry( + RetryingContext context, + Throwable previousThrowable, + ResponseT previousResponse, + TimedAttemptSettings nextAttemptSettings) + throws CancellationException { + if (getResultAlgorithm().shouldRetry(previousThrowable, previousResponse)) { + return true; + } + return super.shouldRetry(context, previousThrowable, previousResponse, nextAttemptSettings); + } +} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java new file mode 100644 index 0000000000..b62bd6be20 --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java @@ -0,0 +1,377 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.api.gax.core.NoCredentialsProvider; +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.grpc.GrpcTransportChannel; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.FixedTransportChannelProvider; +import com.google.api.gax.rpc.InternalException; +import com.google.api.gax.rpc.UnavailableException; +import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.CheckAndMutateRowRequest; +import com.google.bigtable.v2.CheckAndMutateRowResponse; +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.ReadModifyWriteRowRequest; +import com.google.bigtable.v2.ReadModifyWriteRowResponse; +import com.google.bigtable.v2.ReadRowsRequest; +import com.google.bigtable.v2.ReadRowsResponse; +import com.google.bigtable.v2.SampleRowKeysRequest; +import com.google.bigtable.v2.SampleRowKeysResponse; +import com.google.cloud.bigtable.data.v2.BigtableDataClient; +import com.google.cloud.bigtable.data.v2.BigtableDataSettings; +import com.google.cloud.bigtable.data.v2.models.BulkMutation; +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.Query; +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.RowMutationEntry; +import com.google.common.base.Stopwatch; +import com.google.common.collect.Queues; +import com.google.protobuf.Duration; +import com.google.rpc.RetryInfo; +import io.grpc.Metadata; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; +import io.grpc.protobuf.ProtoUtils; +import io.grpc.stub.StreamObserver; +import io.grpc.testing.GrpcServerRule; +import java.io.IOException; +import java.util.Queue; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class RetryInfoTest { + + @Rule public GrpcServerRule serverRule = new GrpcServerRule(); + + private static final Metadata.Key RETRY_INFO_KEY = + ProtoUtils.keyForProto(RetryInfo.getDefaultInstance()); + + private FakeBigtableService service; + private BigtableDataClient client; + + private AtomicInteger attemptCounter = new AtomicInteger(); + private Duration delay = Duration.newBuilder().setSeconds(1).setNanos(0).build(); + + @Before + public void setUp() throws IOException { + service = new FakeBigtableService(); + serverRule.getServiceRegistry().addService(service); + + BigtableDataSettings.Builder settings = + BigtableDataSettings.newBuilder() + .setProjectId("fake-project") + .setInstanceId("fake-instance") + .setCredentialsProvider(NoCredentialsProvider.create()); + + settings + .stubSettings() + .setTransportChannelProvider( + FixedTransportChannelProvider.create( + GrpcTransportChannel.create(serverRule.getChannel()))) + // channel priming doesn't work with FixedTransportChannelProvider. Disable it for the test + .setRefreshingChannel(false) + .build(); + + this.client = BigtableDataClient.create(settings.build()); + } + + @Test + public void testReadRow() { + createRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readRow("table", "row"); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testReadRowNonRetryableErrorWithRetryInfo() { + createNonRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readRow("table", "row"); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testReadRows() { + createRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readRows(Query.create("table")).iterator().hasNext(); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testReadRowsNonRetraybleErrorWithRetryInfo() { + createNonRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readRows(Query.create("table")).iterator().hasNext(); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testMutateRows() { + createRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testMutateRowsNonRetryableErrorWithRetryInfo() { + createNonRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testMutateRow() { + createRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(1)); + } + + @Test + public void testMutateRowNonRetryableErrorWithRetryInfo() { + createNonRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(1)); + } + + @Test + public void testSampleRowKeys() { + createRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + + client.sampleRowKeys("table"); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testSampleRowKeysNonRetryableErrorWithRetryInfo() { + createNonRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + + client.sampleRowKeys("table"); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testCheckAndMutateRow() { + createRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.checkAndMutateRow( + ConditionalRowMutation.create("table", "key") + .condition(Filters.FILTERS.value().regex("old-value")) + .then(Mutation.create().setCell("cf", "q", "v"))); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testReadModifyWrite() { + createRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readModifyWriteRow(ReadModifyWriteRow.create("table", "row").append("cf", "q", "v")); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + private void createRetryableExceptionWithDelay(Duration delay) { + Metadata trailers = new Metadata(); + RetryInfo retryInfo = RetryInfo.newBuilder().setRetryDelay(delay).build(); + trailers.put(RETRY_INFO_KEY, retryInfo); + + ApiException exception = + new UnavailableException( + new StatusRuntimeException(Status.UNAVAILABLE, trailers), + GrpcStatusCode.of(Status.Code.UNAVAILABLE), + true); + + service.expectations.add(exception); + } + + private void createNonRetryableExceptionWithDelay(Duration delay) { + Metadata trailers = new Metadata(); + RetryInfo retryInfo = + RetryInfo.newBuilder() + .setRetryDelay(Duration.newBuilder().setSeconds(1).setNanos(0).build()) + .build(); + trailers.put(RETRY_INFO_KEY, retryInfo); + + ApiException exception = + new InternalException( + new StatusRuntimeException(Status.INTERNAL, trailers), + GrpcStatusCode.of(Status.Code.INTERNAL), + false); + + service.expectations.add(exception); + } + + private class FakeBigtableService extends BigtableGrpc.BigtableImplBase { + Queue expectations = Queues.newArrayDeque(); + + @Override + public void readRows( + ReadRowsRequest request, StreamObserver responseObserver) { + attemptCounter.incrementAndGet(); + if (expectations.isEmpty()) { + responseObserver.onNext(ReadRowsResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } else { + Exception expectedRpc = expectations.poll(); + responseObserver.onError(expectedRpc); + } + } + + @Override + public void mutateRow( + MutateRowRequest request, StreamObserver responseObserver) { + attemptCounter.incrementAndGet(); + if (expectations.isEmpty()) { + responseObserver.onNext(MutateRowResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } else { + Exception expectedRpc = expectations.poll(); + responseObserver.onError(expectedRpc); + } + } + + @Override + public void mutateRows( + MutateRowsRequest request, StreamObserver responseObserver) { + attemptCounter.incrementAndGet(); + if (expectations.isEmpty()) { + MutateRowsResponse.Builder builder = MutateRowsResponse.newBuilder(); + for (int i = 0; i < request.getEntriesCount(); i++) { + builder.addEntriesBuilder().setIndex(i); + } + responseObserver.onNext(builder.build()); + responseObserver.onCompleted(); + } else { + Exception expectedRpc = expectations.poll(); + responseObserver.onError(expectedRpc); + } + } + + @Override + public void sampleRowKeys( + SampleRowKeysRequest request, StreamObserver responseObserver) { + attemptCounter.incrementAndGet(); + if (expectations.isEmpty()) { + responseObserver.onNext(SampleRowKeysResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } else { + Exception expectedRpc = expectations.poll(); + responseObserver.onError(expectedRpc); + } + } + + @Override + public void checkAndMutateRow( + CheckAndMutateRowRequest request, + StreamObserver responseObserver) { + attemptCounter.incrementAndGet(); + if (expectations.isEmpty()) { + responseObserver.onNext(CheckAndMutateRowResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } else { + Exception expectedRpc = expectations.poll(); + responseObserver.onError(expectedRpc); + } + } + + @Override + public void readModifyWriteRow( + ReadModifyWriteRowRequest request, + StreamObserver responseObserver) { + attemptCounter.incrementAndGet(); + if (expectations.isEmpty()) { + responseObserver.onNext(ReadModifyWriteRowResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } else { + Exception expectedRpc = expectations.poll(); + responseObserver.onError(expectedRpc); + } + } + } +} From eb0148f7f75c1c332464df7528a38b7bce56ad14 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 15 Dec 2023 13:30:20 -0500 Subject: [PATCH 06/13] fix conformance test --- .../bigtable/gaxx/retrying/Callables.java | 37 +++---------------- 1 file changed, 5 insertions(+), 32 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java index 5d2f9ed28d..1ee963dc80 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java @@ -18,16 +18,13 @@ import com.google.api.core.InternalApi; import com.google.api.gax.retrying.ExponentialRetryAlgorithm; import com.google.api.gax.retrying.RetryAlgorithm; -import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.retrying.ScheduledRetryingExecutor; import com.google.api.gax.retrying.StreamingRetryAlgorithm; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.ServerStreamingCallSettings; import com.google.api.gax.rpc.ServerStreamingCallable; -import com.google.api.gax.rpc.StatusCode; import com.google.api.gax.rpc.UnaryCallSettings; import com.google.api.gax.rpc.UnaryCallable; -import java.util.Collection; // TODO: remove this once ApiResultRetryAlgorithm is added to gax. /** @@ -48,23 +45,14 @@ public static UnaryCallable retrying( UnaryCallSettings settings = callSettings; - if (areRetriesDisabled(settings.getRetryableCodes(), settings.getRetrySettings())) { - // When retries are disabled, the total timeout can be treated as the rpc timeout. - settings = - settings - .toBuilder() - .setSimpleTimeoutNoRetries(settings.getRetrySettings().getTotalTimeout()) - .build(); - } - RetryAlgorithm retryAlgorithm = - new UnaryRetryAlgorithm<>( - new ApiResultRetryAlgorithm(), + new RetryAlgorithm<>( + new ApiResultRetryAlgorithm<>(), new ExponentialRetryAlgorithm(settings.getRetrySettings(), clientContext.getClock())); - ScheduledRetryingExecutor retryingExecutor = + ScheduledRetryingExecutor executor = new ScheduledRetryingExecutor<>(retryAlgorithm, clientContext.getExecutor()); - return new RetryingCallable<>( - clientContext.getDefaultCallContext(), innerCallable, retryingExecutor); + + return new RetryingCallable<>(clientContext.getDefaultCallContext(), innerCallable, executor); } public static ServerStreamingCallable retrying( @@ -73,14 +61,6 @@ public static ServerStreamingCallable ClientContext clientContext) { ServerStreamingCallSettings settings = callSettings; - if (areRetriesDisabled(settings.getRetryableCodes(), settings.getRetrySettings())) { - // When retries are disabled, the total timeout can be treated as the rpc timeout. - settings = - settings - .toBuilder() - .setSimpleTimeoutNoRetries(settings.getRetrySettings().getTotalTimeout()) - .build(); - } StreamingRetryAlgorithm retryAlgorithm = new StreamingRetryAlgorithm<>( @@ -93,11 +73,4 @@ public static ServerStreamingCallable return new RetryingServerStreamingCallable<>( innerCallable, retryingExecutor, settings.getResumptionStrategy()); } - - private static boolean areRetriesDisabled( - Collection retryableCodes, RetrySettings retrySettings) { - return retrySettings.getMaxAttempts() == 1 - || retryableCodes.isEmpty() - || (retrySettings.getMaxAttempts() == 0 && retrySettings.getTotalTimeout().isZero()); - } } From 944a93f11dc58e8aa94337fcdf9b0127651f95df Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 15 Dec 2023 13:32:18 -0500 Subject: [PATCH 07/13] remove unused class --- .../gaxx/retrying/UnaryRetryAlgorithm.java | 51 ------------------- 1 file changed, 51 deletions(-) delete mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/UnaryRetryAlgorithm.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/UnaryRetryAlgorithm.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/UnaryRetryAlgorithm.java deleted file mode 100644 index 0966e16881..0000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/UnaryRetryAlgorithm.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright 2023 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.cloud.bigtable.gaxx.retrying; - -import com.google.api.core.InternalApi; -import com.google.api.gax.retrying.ResultRetryAlgorithmWithContext; -import com.google.api.gax.retrying.RetryAlgorithm; -import com.google.api.gax.retrying.RetryingContext; -import com.google.api.gax.retrying.TimedAttemptSettings; -import com.google.api.gax.retrying.TimedRetryAlgorithmWithContext; -import java.util.concurrent.CancellationException; - -/** - * Retry algorithm for unary calls. It'll use the result from result algorithm first and only fall - * back to timedAlgorithm if resultAlgorithm#shouldRetry is false. - */ -@InternalApi -public class UnaryRetryAlgorithm extends RetryAlgorithm { - - public UnaryRetryAlgorithm( - ResultRetryAlgorithmWithContext resultAlgorithm, - TimedRetryAlgorithmWithContext timedAlgorithm) { - super(resultAlgorithm, timedAlgorithm); - } - - @Override - public boolean shouldRetry( - RetryingContext context, - Throwable previousThrowable, - ResponseT previousResponse, - TimedAttemptSettings nextAttemptSettings) - throws CancellationException { - if (getResultAlgorithm().shouldRetry(previousThrowable, previousResponse)) { - return true; - } - return super.shouldRetry(context, previousThrowable, previousResponse, nextAttemptSettings); - } -} From 5f6d5833f2a9e875148d9d635616f5e6fb153d93 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Sun, 17 Dec 2023 14:13:16 -0500 Subject: [PATCH 08/13] address comments --- .../data/v2/stub/EnhancedBigtableStub.java | 4 +- .../mutaterows/MutateRowsAttemptCallable.java | 4 +- .../bigtable/gaxx/retrying/ApiException.java | 32 +++++++ .../retrying/ApiResultRetryAlgorithm.java | 62 ++---------- .../bigtable/gaxx/retrying/Callables.java | 4 +- .../retrying/RetryInfoRetryAlgorithm.java | 95 +++++++++++++++++++ .../bigtable/data/v2/stub/RetryInfoTest.java | 5 +- 7 files changed, 141 insertions(+), 65 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.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 3b441972dc..dc1b10b05c 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 @@ -107,7 +107,7 @@ import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsRetryCompletedCallable; import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsUserCallable; import com.google.cloud.bigtable.data.v2.stub.readrows.RowMergingCallable; -import com.google.cloud.bigtable.gaxx.retrying.ApiResultRetryAlgorithm; +import com.google.cloud.bigtable.gaxx.retrying.RetryInfoRetryAlgorithm; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -763,7 +763,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { RetryAlgorithm retryAlgorithm = new RetryAlgorithm<>( - new ApiResultRetryAlgorithm(), + new RetryInfoRetryAlgorithm<>(), new ExponentialRetryAlgorithm( settings.bulkMutateRowsSettings().getRetrySettings(), clientContext.getClock())); RetryingExecutorWithContext retryingExecutor = diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java index b6184bca44..ddf945b7da 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java @@ -31,7 +31,6 @@ import com.google.bigtable.v2.MutateRowsResponse.Entry; import com.google.cloud.bigtable.data.v2.models.MutateRowsException; import com.google.cloud.bigtable.data.v2.models.MutateRowsException.FailedMutation; -import com.google.cloud.bigtable.gaxx.retrying.ApiResultRetryAlgorithm; import com.google.cloud.bigtable.gaxx.retrying.NonCancellableFuture; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -236,7 +235,8 @@ private void handleAttemptError(Throwable rpcError) { FailedMutation failedMutation = FailedMutation.create(origIndex, entryError); allFailures.add(failedMutation); - if (ApiResultRetryAlgorithm.extractRetryDelay(failedMutation.getError()) == null + if (!com.google.cloud.bigtable.gaxx.retrying.ApiException.isRetryable2( + failedMutation.getError()) && !failedMutation.getError().isRetryable()) { permanentFailures.add(failedMutation); } else { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java new file mode 100644 index 0000000000..d2f676c33b --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java @@ -0,0 +1,32 @@ +/* + * 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.gaxx.retrying; + +import com.google.api.core.InternalApi; + +// TODO: move this to gax later +@InternalApi +public class ApiException { + + // TODO: this should replace the existing ApiException#isRetryable() method, + // but that cant be done in bigtable, so this lives here for now. + public static boolean isRetryable2(Throwable e) { + if (RetryInfoRetryAlgorithm.extractRetryDelay(e) != null) { + return true; + } + return false; + } +} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiResultRetryAlgorithm.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiResultRetryAlgorithm.java index 91d893e3f3..d71a044235 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiResultRetryAlgorithm.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiResultRetryAlgorithm.java @@ -18,46 +18,17 @@ import com.google.api.core.InternalApi; import com.google.api.gax.retrying.BasicResultRetryAlgorithm; import com.google.api.gax.retrying.RetryingContext; -import com.google.api.gax.retrying.TimedAttemptSettings; import com.google.api.gax.rpc.ApiException; -import com.google.protobuf.util.Durations; -import com.google.rpc.RetryInfo; -import io.grpc.Metadata; -import io.grpc.Status; -import io.grpc.protobuf.ProtoUtils; -import org.threeten.bp.Duration; -/** - * For internal use, public for technical reasons. This retry algorithm checks the metadata of an - * exception for additional error details. If the metadata has a RetryInfo field, use the retry - * delay to set the wait time between attempts. - */ +/** For internal use, public for technical reasons. */ @InternalApi public class ApiResultRetryAlgorithm extends BasicResultRetryAlgorithm { - private static final Metadata.Key KEY_RETRY_INFO = - ProtoUtils.keyForProto(RetryInfo.getDefaultInstance()); - - @Override - public TimedAttemptSettings createNextAttempt( - Throwable prevThrowable, ResponseT prevResponse, TimedAttemptSettings prevSettings) { - Duration retryDelay = extractRetryDelay(prevThrowable); - if (retryDelay != null) { - return prevSettings - .toBuilder() - .setRandomizedRetryDelay(retryDelay) - .setAttemptCount(prevSettings.getAttemptCount() + 1) - .build(); - } - return null; - } - /** Returns true if previousThrowable is an {@link ApiException} that is retryable. */ @Override public boolean shouldRetry(Throwable previousThrowable, ResponseT previousResponse) { - return (extractRetryDelay(previousThrowable) != null) - || (previousThrowable instanceof ApiException - && ((ApiException) previousThrowable).isRetryable()); + return (previousThrowable instanceof ApiException) + && ((ApiException) previousThrowable).isRetryable(); } /** @@ -72,30 +43,11 @@ public boolean shouldRetry( if (context.getRetryableCodes() != null) { // Ignore the isRetryable() value of the throwable if the RetryingContext has a specific list // of codes that should be retried. - return extractRetryDelay(previousThrowable) != null - || ((previousThrowable instanceof ApiException) - && context - .getRetryableCodes() - .contains(((ApiException) previousThrowable).getStatusCode().getCode())); + return (previousThrowable instanceof ApiException) + && context + .getRetryableCodes() + .contains(((ApiException) previousThrowable).getStatusCode().getCode()); } return shouldRetry(previousThrowable, previousResponse); } - - public static Duration extractRetryDelay(Throwable throwable) { - if (throwable == null) { - return null; - } - Metadata trailers = Status.trailersFromThrowable(throwable); - if (trailers == null) { - return null; - } - RetryInfo retryInfo = trailers.get(KEY_RETRY_INFO); - if (retryInfo == null) { - return null; - } - if (!retryInfo.hasRetryDelay()) { - return null; - } - return Duration.ofMillis(Durations.toMillis(retryInfo.getRetryDelay())); - } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java index 1ee963dc80..a78e7643b0 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/Callables.java @@ -47,7 +47,7 @@ public static UnaryCallable retrying( RetryAlgorithm retryAlgorithm = new RetryAlgorithm<>( - new ApiResultRetryAlgorithm<>(), + new RetryInfoRetryAlgorithm<>(), new ExponentialRetryAlgorithm(settings.getRetrySettings(), clientContext.getClock())); ScheduledRetryingExecutor executor = new ScheduledRetryingExecutor<>(retryAlgorithm, clientContext.getExecutor()); @@ -64,7 +64,7 @@ public static ServerStreamingCallable StreamingRetryAlgorithm retryAlgorithm = new StreamingRetryAlgorithm<>( - new ApiResultRetryAlgorithm(), + new RetryInfoRetryAlgorithm<>(), new ExponentialRetryAlgorithm(settings.getRetrySettings(), clientContext.getClock())); ScheduledRetryingExecutor retryingExecutor = diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java new file mode 100644 index 0000000000..e18898559c --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java @@ -0,0 +1,95 @@ +package com.google.cloud.bigtable.gaxx.retrying; + +import com.google.api.core.InternalApi; +import com.google.api.gax.retrying.BasicResultRetryAlgorithm; +import com.google.api.gax.retrying.RetryingContext; +import com.google.api.gax.retrying.TimedAttemptSettings; +import com.google.api.gax.rpc.ApiException; +import com.google.common.annotations.VisibleForTesting; +import com.google.protobuf.util.Durations; +import com.google.rpc.RetryInfo; +import io.grpc.Metadata; +import io.grpc.Status; +import io.grpc.protobuf.ProtoUtils; +import org.threeten.bp.Duration; + +// TODO move this algorithm to gax +/** + * This retry algorithm checks the metadata of an exception for additional error details. If the + * metadata has a RetryInfo field, use the retry delay to set the wait time between attempts. + */ +@InternalApi +public class RetryInfoRetryAlgorithm extends BasicResultRetryAlgorithm { + + @VisibleForTesting + public static final Metadata.Key RETRY_INFO_KEY = + ProtoUtils.keyForProto(RetryInfo.getDefaultInstance()); + + @Override + public TimedAttemptSettings createNextAttempt( + Throwable prevThrowable, ResponseT prevResponse, TimedAttemptSettings prevSettings) { + Duration retryDelay = extractRetryDelay(prevThrowable); + if (retryDelay != null) { + return prevSettings + .toBuilder() + .setRandomizedRetryDelay(retryDelay) + .setAttemptCount(prevSettings.getAttemptCount() + 1) + .build(); + } + return null; + } + + /** Returns true if previousThrowable is an {@link ApiException} that is retryable. */ + @Override + public boolean shouldRetry(Throwable previousThrowable, ResponseT previousResponse) { + if (extractRetryDelay(previousThrowable) != null) { + // First check if server wants us to retry + return true; + } + // Server didn't have retry information, use the local status code config. + return (previousThrowable instanceof ApiException + && ((ApiException) previousThrowable).isRetryable()); + } + + /** + * If {@link RetryingContext#getRetryableCodes()} is not null: Returns true if the status code of + * previousThrowable is in the list of retryable code of the {@link RetryingContext}. + * + *

Otherwise it returns the result of {@link #shouldRetry(Throwable, Object)}. + */ + @Override + public boolean shouldRetry( + RetryingContext context, Throwable previousThrowable, ResponseT previousResponse) { + if (extractRetryDelay(previousThrowable) != null) { + // First check if server wants us to retry + return true; + } + if (context.getRetryableCodes() != null) { + // Ignore the isRetryable() value of the throwable if the RetryingContext has a specific list + // of codes that should be retried. + return ((previousThrowable instanceof ApiException) + && context + .getRetryableCodes() + .contains(((ApiException) previousThrowable).getStatusCode().getCode())); + } + return shouldRetry(previousThrowable, previousResponse); + } + + static Duration extractRetryDelay(Throwable throwable) { + if (throwable == null) { + return null; + } + Metadata trailers = Status.trailersFromThrowable(throwable); + if (trailers == null) { + return null; + } + RetryInfo retryInfo = trailers.get(RETRY_INFO_KEY); + if (retryInfo == null) { + return null; + } + if (!retryInfo.hasRetryDelay()) { + return null; + } + return Duration.ofMillis(Durations.toMillis(retryInfo.getRetryDelay())); + } +} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java index b62bd6be20..a853829e63 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java @@ -15,6 +15,7 @@ */ package com.google.cloud.bigtable.data.v2.stub; +import static com.google.cloud.bigtable.gaxx.retrying.RetryInfoRetryAlgorithm.RETRY_INFO_KEY; import static com.google.common.truth.Truth.assertThat; import com.google.api.gax.core.NoCredentialsProvider; @@ -54,7 +55,6 @@ import io.grpc.Metadata; import io.grpc.Status; import io.grpc.StatusRuntimeException; -import io.grpc.protobuf.ProtoUtils; import io.grpc.stub.StreamObserver; import io.grpc.testing.GrpcServerRule; import java.io.IOException; @@ -71,9 +71,6 @@ public class RetryInfoTest { @Rule public GrpcServerRule serverRule = new GrpcServerRule(); - private static final Metadata.Key RETRY_INFO_KEY = - ProtoUtils.keyForProto(RetryInfo.getDefaultInstance()); - private FakeBigtableService service; private BigtableDataClient client; From a3ef1d19dfb9184ef2ebb683094c62872a970f84 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Sun, 17 Dec 2023 15:24:44 -0500 Subject: [PATCH 09/13] add a flag for retry info and tests --- .../data/v2/stub/EnhancedBigtableStub.java | 37 ++- .../v2/stub/EnhancedBigtableStubSettings.java | 31 ++ .../bigtable/gaxx/retrying/ApiException.java | 2 +- .../retrying/RetryInfoRetryAlgorithm.java | 15 + .../EnhancedBigtableStubSettingsTest.java | 70 +++- .../bigtable/data/v2/stub/RetryInfoTest.java | 298 +++++++++++++++++- 6 files changed, 427 insertions(+), 26 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 dc1b10b05c..1bf69b109b 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 @@ -107,6 +107,7 @@ import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsRetryCompletedCallable; import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsUserCallable; import com.google.cloud.bigtable.data.v2.stub.readrows.RowMergingCallable; +import com.google.cloud.bigtable.gaxx.retrying.ApiResultRetryAlgorithm; import com.google.cloud.bigtable.gaxx.retrying.RetryInfoRetryAlgorithm; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; @@ -761,11 +762,18 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { ServerStreamingCallable withBigtableTracer = new BigtableTracerStreamingCallable<>(convertException); - RetryAlgorithm retryAlgorithm = - new RetryAlgorithm<>( - new RetryInfoRetryAlgorithm<>(), - new ExponentialRetryAlgorithm( - settings.bulkMutateRowsSettings().getRetrySettings(), clientContext.getClock())); + RetryAlgorithm retryAlgorithm; + ExponentialRetryAlgorithm exponentialRetryAlgorithm = + new ExponentialRetryAlgorithm( + settings.bulkMutateRowsSettings().getRetrySettings(), clientContext.getClock()); + if (settings.getEnableRetryInfo()) { + retryAlgorithm = + new RetryAlgorithm<>(new RetryInfoRetryAlgorithm<>(), exponentialRetryAlgorithm); + } else { + retryAlgorithm = + new RetryAlgorithm<>(new ApiResultRetryAlgorithm<>(), exponentialRetryAlgorithm); + } + RetryingExecutorWithContext retryingExecutor = new ScheduledRetryingExecutor<>(retryAlgorithm, clientContext.getExecutor()); @@ -1055,8 +1063,13 @@ public Map extract(PingAndWarmRequest request) { private UnaryCallable withRetries( UnaryCallable innerCallable, UnaryCallSettings unaryCallSettings) { - UnaryCallable retrying = - Callables.retrying(innerCallable, unaryCallSettings, clientContext); + UnaryCallable retrying; + if (settings.getEnableRetryInfo()) { + retrying = com.google.cloud.bigtable.gaxx.retrying.Callables.retrying(innerCallable, unaryCallSettings, clientContext); + } else { + retrying = + Callables.retrying(innerCallable, unaryCallSettings, clientContext); + } if (settings.getEnableRoutingCookie()) { return new CookiesUnaryCallable<>(retrying); } @@ -1066,8 +1079,14 @@ private UnaryCallable withRetries( private ServerStreamingCallable withRetries( ServerStreamingCallable innerCallable, ServerStreamingCallSettings serverStreamingCallSettings) { - ServerStreamingCallable retrying = - Callables.retrying(innerCallable, serverStreamingCallSettings, clientContext); + + ServerStreamingCallable retrying; + if (settings.getEnableRetryInfo()) { + retrying = + com.google.cloud.bigtable.gaxx.retrying.Callables.retrying(innerCallable, serverStreamingCallSettings, clientContext); + } else { + retrying = Callables.retrying(innerCallable, serverStreamingCallSettings, clientContext); + } if (settings.getEnableRoutingCookie()) { return new CookiesServerStreamingCallable<>(retrying); } 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 64f44bb52f..8b5a05e4ca 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 @@ -213,6 +213,7 @@ public class EnhancedBigtableStubSettings extends StubSettings primedTableIds; private final Map jwtAudienceMapping; private final boolean enableRoutingCookie; + private final boolean enableRetryInfo; private final ServerStreamingCallSettings readRowsSettings; private final UnaryCallSettings readRowSettings; @@ -255,6 +256,7 @@ private EnhancedBigtableStubSettings(Builder builder) { primedTableIds = builder.primedTableIds; jwtAudienceMapping = builder.jwtAudienceMapping; enableRoutingCookie = builder.enableRoutingCookie; + enableRetryInfo = builder.enableRetryInfo; // Per method settings. readRowsSettings = builder.readRowsSettings.build(); @@ -325,6 +327,14 @@ public boolean getEnableRoutingCookie() { return enableRoutingCookie; } + /** + * Gets if RetryInfo is enabled. If true, client bases retry decision and back off time on server + * returned RetryInfo value. Otherwise, client uses {@link RetrySettings}. + */ + public boolean getEnableRetryInfo() { + return enableRetryInfo; + } + /** Returns a builder for the default ChannelProvider for this service. */ public static InstantiatingGrpcChannelProvider.Builder defaultGrpcTransportProviderBuilder() { return BigtableStubSettings.defaultGrpcTransportProviderBuilder() @@ -608,6 +618,7 @@ public static class Builder extends StubSettings.Builder primedTableIds; private Map jwtAudienceMapping; private boolean enableRoutingCookie; + private boolean enableRetryInfo; private final ServerStreamingCallSettings.Builder readRowsSettings; private final UnaryCallSettings.Builder readRowSettings; @@ -641,6 +652,7 @@ private Builder() { jwtAudienceMapping = DEFAULT_JWT_AUDIENCE_MAPPING; setCredentialsProvider(defaultCredentialsProviderBuilder().build()); this.enableRoutingCookie = true; + this.enableRetryInfo = true; // Defaults provider BigtableStubSettings.Builder baseDefaults = BigtableStubSettings.newBuilder(); @@ -760,6 +772,7 @@ private Builder(EnhancedBigtableStubSettings settings) { primedTableIds = settings.primedTableIds; jwtAudienceMapping = settings.jwtAudienceMapping; enableRoutingCookie = settings.enableRoutingCookie; + enableRetryInfo = settings.enableRetryInfo; // Per method settings. readRowsSettings = settings.readRowsSettings.toBuilder(); @@ -918,6 +931,15 @@ public Builder setEnableRoutingCookie(boolean enableRoutingCookie) { return this; } + /** + * Sets if RetryInfo is enabled. If true, client bases retry decision and back off time on + * server returned RetryInfo value. Otherwise, client uses {@link RetrySettings}. + */ + public Builder setEnableRetryInfo(boolean enableRetryInfo) { + this.enableRetryInfo = enableRetryInfo; + return this; + } + /** * Gets if routing cookie is enabled. If true, client will retry a request with extra metadata * server sent back. @@ -927,6 +949,14 @@ public boolean getEnableRoutingCookie() { return enableRoutingCookie; } + /** + * Gets if RetryInfo is enabled. If true, client bases retry decision and back off time on + * server returned RetryInfo value. Otherwise, client uses {@link RetrySettings}. + */ + public boolean getEnableRetryInfo() { + return enableRetryInfo; + } + /** Returns the builder for the settings used for calls to readRows. */ public ServerStreamingCallSettings.Builder readRowsSettings() { return readRowsSettings; @@ -1054,6 +1084,7 @@ public String toString() { .add("primedTableIds", primedTableIds) .add("jwtAudienceMapping", jwtAudienceMapping) .add("enableRoutingCookie", enableRoutingCookie) + .add("enableRetryInfo", enableRetryInfo) .add("readRowsSettings", readRowsSettings) .add("readRowSettings", readRowSettings) .add("sampleRowKeysSettings", sampleRowKeysSettings) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java index d2f676c33b..b13452934d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java @@ -1,5 +1,5 @@ /* - * Copyright 2018 Google LLC + * Copyright 2023 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java index e18898559c..1fde962903 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java @@ -1,3 +1,18 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.google.cloud.bigtable.gaxx.retrying; import com.google.api.core.InternalApi; 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 2c05ca4ee8..7df5fd1ace 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 @@ -78,6 +78,7 @@ public void settingsAreNotLostTest() { WatchdogProvider watchdogProvider = Mockito.mock(WatchdogProvider.class); Duration watchdogInterval = Duration.ofSeconds(12); boolean enableRoutingCookie = false; + boolean enableRetryInfo = false; EnhancedBigtableStubSettings.Builder builder = EnhancedBigtableStubSettings.newBuilder() @@ -89,7 +90,8 @@ public void settingsAreNotLostTest() { .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) .setStreamWatchdogCheckInterval(watchdogInterval) - .setEnableRoutingCookie(enableRoutingCookie); + .setEnableRoutingCookie(enableRoutingCookie) + .setEnableRetryInfo(enableRetryInfo); verifyBuilder( builder, @@ -101,7 +103,8 @@ public void settingsAreNotLostTest() { credentialsProvider, watchdogProvider, watchdogInterval, - enableRoutingCookie); + enableRoutingCookie, + enableRetryInfo); verifySettings( builder.build(), projectId, @@ -112,7 +115,8 @@ public void settingsAreNotLostTest() { credentialsProvider, watchdogProvider, watchdogInterval, - enableRoutingCookie); + enableRoutingCookie, + enableRetryInfo); verifyBuilder( builder.build().toBuilder(), projectId, @@ -123,7 +127,8 @@ public void settingsAreNotLostTest() { credentialsProvider, watchdogProvider, watchdogInterval, - enableRoutingCookie); + enableRoutingCookie, + enableRetryInfo); } private void verifyBuilder( @@ -136,7 +141,8 @@ private void verifyBuilder( CredentialsProvider credentialsProvider, WatchdogProvider watchdogProvider, Duration watchdogInterval, - boolean enableRoutingCookie) { + boolean enableRoutingCookie, + boolean enableRetryInfo) { assertThat(builder.getProjectId()).isEqualTo(projectId); assertThat(builder.getInstanceId()).isEqualTo(instanceId); assertThat(builder.getAppProfileId()).isEqualTo(appProfileId); @@ -146,6 +152,7 @@ private void verifyBuilder( assertThat(builder.getStreamWatchdogProvider()).isSameInstanceAs(watchdogProvider); assertThat(builder.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); assertThat(builder.getEnableRoutingCookie()).isEqualTo(enableRoutingCookie); + assertThat(builder.getEnableRetryInfo()).isEqualTo(enableRetryInfo); } private void verifySettings( @@ -158,7 +165,8 @@ private void verifySettings( CredentialsProvider credentialsProvider, WatchdogProvider watchdogProvider, Duration watchdogInterval, - boolean enableRoutingCookie) { + boolean enableRoutingCookie, + boolean enableRetryInfo) { assertThat(settings.getProjectId()).isEqualTo(projectId); assertThat(settings.getInstanceId()).isEqualTo(instanceId); assertThat(settings.getAppProfileId()).isEqualTo(appProfileId); @@ -168,6 +176,7 @@ private void verifySettings( assertThat(settings.getStreamWatchdogProvider()).isSameInstanceAs(watchdogProvider); assertThat(settings.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); assertThat(settings.getEnableRoutingCookie()).isEqualTo(enableRoutingCookie); + assertThat(settings.getEnableRetryInfo()).isEqualTo(enableRetryInfo); } @Test @@ -797,17 +806,49 @@ public void routingCookieIsEnabled() throws IOException { CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); Mockito.when(credentialsProvider.getCredentials()).thenReturn(new FakeCredentials()); EnhancedBigtableStubSettings.Builder builder = - EnhancedBigtableStubSettings.newBuilder() - .setProjectId(dummyProjectId) - .setInstanceId(dummyInstanceId) - .setCredentialsProvider(credentialsProvider); + EnhancedBigtableStubSettings.newBuilder() + .setProjectId(dummyProjectId) + .setInstanceId(dummyInstanceId) + .setCredentialsProvider(credentialsProvider); assertThat(builder.getEnableRoutingCookie()).isTrue(); assertThat(builder.build().getEnableRoutingCookie()).isTrue(); assertThat(builder.build().toBuilder().getEnableRoutingCookie()).isTrue(); } + public void enableRetryInfoDefaultValueTest() throws IOException { + String dummyProjectId = "my-project"; + String dummyInstanceId = "my-instance"; + CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); + Mockito.when(credentialsProvider.getCredentials()).thenReturn(new FakeCredentials()); + EnhancedBigtableStubSettings.Builder builder = + EnhancedBigtableStubSettings.newBuilder() + .setProjectId(dummyProjectId) + .setInstanceId(dummyInstanceId) + .setCredentialsProvider(credentialsProvider); + assertThat(builder.getEnableRetryInfo()).isTrue(); + assertThat(builder.build().getEnableRetryInfo()).isTrue(); + assertThat(builder.build().toBuilder().getEnableRetryInfo()).isTrue(); + } + @Test public void routingCookieFalseValueSet() throws IOException { + String dummyProjectId = "my-project"; + String dummyInstanceId = "my-instance"; + CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); + Mockito.when(credentialsProvider.getCredentials()).thenReturn(new FakeCredentials()); + EnhancedBigtableStubSettings.Builder builder = + EnhancedBigtableStubSettings.newBuilder() + .setProjectId(dummyProjectId) + .setInstanceId(dummyInstanceId) + .setEnableRoutingCookie(false) + .setCredentialsProvider(credentialsProvider); + assertThat(builder.getEnableRoutingCookie()).isFalse(); + assertThat(builder.build().getEnableRoutingCookie()).isFalse(); + assertThat(builder.build().toBuilder().getEnableRoutingCookie()).isFalse(); + } + + @Test + public void enableRetryInfoFalseValueTest() throws IOException { String dummyProjectId = "my-project"; String dummyInstanceId = "my-instance"; CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); @@ -816,11 +857,11 @@ public void routingCookieFalseValueSet() throws IOException { EnhancedBigtableStubSettings.newBuilder() .setProjectId(dummyProjectId) .setInstanceId(dummyInstanceId) - .setEnableRoutingCookie(false) + .setEnableRetryInfo(false) .setCredentialsProvider(credentialsProvider); - assertThat(builder.getEnableRoutingCookie()).isFalse(); - assertThat(builder.build().getEnableRoutingCookie()).isFalse(); - assertThat(builder.build().toBuilder().getEnableRoutingCookie()).isFalse(); + assertThat(builder.getEnableRetryInfo()).isFalse(); + assertThat(builder.build().getEnableRetryInfo()).isFalse(); + assertThat(builder.build().toBuilder().getEnableRetryInfo()).isFalse(); } static final String[] SETTINGS_LIST = { @@ -831,6 +872,7 @@ public void routingCookieFalseValueSet() throws IOException { "primedTableIds", "jwtAudienceMapping", "enableRoutingCookie", + "enableRetryInfo", "readRowsSettings", "readRowSettings", "sampleRowKeysSettings", diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java index a853829e63..41037ae067 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java @@ -28,10 +28,14 @@ import com.google.bigtable.v2.BigtableGrpc; import com.google.bigtable.v2.CheckAndMutateRowRequest; import com.google.bigtable.v2.CheckAndMutateRowResponse; +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.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; @@ -43,8 +47,10 @@ import com.google.cloud.bigtable.data.v2.models.BulkMutation; 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.MutateRowsException; 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.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; @@ -73,6 +79,7 @@ public class RetryInfoTest { private FakeBigtableService service; private BigtableDataClient client; + private BigtableDataSettings.Builder settings; private AtomicInteger attemptCounter = new AtomicInteger(); private Duration delay = Duration.newBuilder().setSeconds(1).setNanos(0).build(); @@ -82,7 +89,7 @@ public void setUp() throws IOException { service = new FakeBigtableService(); serverRule.getServiceRegistry().addService(service); - BigtableDataSettings.Builder settings = + settings = BigtableDataSettings.newBuilder() .setProjectId("fake-project") .setInstanceId("fake-instance") @@ -122,6 +129,30 @@ public void testReadRowNonRetryableErrorWithRetryInfo() { assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); } + @Test + public void testReadRowDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + createRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readRow("table", "row"); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); + + attemptCounter.set(0); + ApiException exception = createNonRetryableExceptionWithDelay(delay); + try { + client.readRow("table", "row"); + } catch (ApiException e) { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + } + assertThat(attemptCounter.get()).isEqualTo(1); + } + } + @Test public void testReadRows() { createRetryableExceptionWithDelay(delay); @@ -146,6 +177,30 @@ public void testReadRowsNonRetraybleErrorWithRetryInfo() { assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); } + @Test + public void testReadRowsDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + createRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readRows(Query.create("table")).iterator().hasNext(); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); + + attemptCounter.set(0); + ApiException exception = createNonRetryableExceptionWithDelay(delay); + try { + client.readRows(Query.create("table")).iterator().hasNext(); + } catch (ApiException e) { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + } + assertThat(attemptCounter.get()).isEqualTo(1); + } + } + @Test public void testMutateRows() { createRetryableExceptionWithDelay(delay); @@ -176,6 +231,35 @@ public void testMutateRowsNonRetryableErrorWithRetryInfo() { assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); } + @Test + public void testMutateRowsDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + createRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); + + attemptCounter.set(0); + ApiException exception = createNonRetryableExceptionWithDelay(delay); + try { + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))); + } catch (ApiException e) { + assertThat(((MutateRowsException) e).getFailedMutations().get(0).getError().getStatusCode()) + .isEqualTo(exception.getStatusCode()); + } + assertThat(attemptCounter.get()).isEqualTo(1); + } + } + @Test public void testMutateRow() { createRetryableExceptionWithDelay(delay); @@ -200,6 +284,30 @@ public void testMutateRowNonRetryableErrorWithRetryInfo() { assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(1)); } + @Test + public void testMutateRowDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + createRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); + + attemptCounter.set(0); + ApiException exception = createNonRetryableExceptionWithDelay(delay); + try { + client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); + } catch (ApiException e) { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + } + assertThat(attemptCounter.get()).isEqualTo(1); + } + } + @Test public void testSampleRowKeys() { createRetryableExceptionWithDelay(delay); @@ -226,6 +334,30 @@ public void testSampleRowKeysNonRetryableErrorWithRetryInfo() { assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); } + @Test + public void testSampleRowKeysDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + createRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + client.sampleRowKeys("table"); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); + + attemptCounter.set(0); + ApiException exception = createNonRetryableExceptionWithDelay(delay); + try { + client.sampleRowKeys("table"); + } catch (ApiException e) { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + } + assertThat(attemptCounter.get()).isEqualTo(1); + } + } + @Test public void testCheckAndMutateRow() { createRetryableExceptionWithDelay(delay); @@ -241,6 +373,24 @@ public void testCheckAndMutateRow() { assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); } + @Test + public void testCheckAndMutateDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + ApiException exception = createNonRetryableExceptionWithDelay(delay); + try { + client.checkAndMutateRow( + ConditionalRowMutation.create("table", "key") + .condition(Filters.FILTERS.value().regex("old-value")) + .then(Mutation.create().setCell("cf", "q", "v"))); + } catch (ApiException e) { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + } + assertThat(attemptCounter.get()).isEqualTo(1); + } + } + @Test public void testReadModifyWrite() { createRetryableExceptionWithDelay(delay); @@ -253,6 +403,117 @@ public void testReadModifyWrite() { assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); } + @Test + public void testReadModifyWriteDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + ApiException exception = createNonRetryableExceptionWithDelay(delay); + try { + client.readModifyWriteRow(ReadModifyWriteRow.create("table", "row").append("cf", "q", "v")); + } catch (ApiException e) { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + } + assertThat(attemptCounter.get()).isEqualTo(1); + } + } + + @Test + public void testReadChangeStream() { + createRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testReadChangeStreamNonRetryableErrorWithRetryInfo() { + createNonRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testReadChangeStreamDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + createRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); + + attemptCounter.set(0); + ApiException exception = createNonRetryableExceptionWithDelay(delay); + try { + client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); + } catch (ApiException e) { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + } + assertThat(attemptCounter.get()).isEqualTo(1); + } + } + + @Test + public void testGenerateInitialChangeStreamPartition() { + createRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testGenerateInitialChangeStreamPartitionNonRetryableError() { + createNonRetryableExceptionWithDelay(delay); + + Stopwatch stopwatch = Stopwatch.createStarted(); + client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + } + + @Test + public void testGenerateInitialChangeStreamPartitionDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + + try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { + createRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); + stopwatch.stop(); + + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); + + attemptCounter.set(0); + ApiException exception = createNonRetryableExceptionWithDelay(delay); + try { + client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); + } catch (ApiException e) { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + } + assertThat(attemptCounter.get()).isEqualTo(1); + } + } + private void createRetryableExceptionWithDelay(Duration delay) { Metadata trailers = new Metadata(); RetryInfo retryInfo = RetryInfo.newBuilder().setRetryDelay(delay).build(); @@ -267,7 +528,7 @@ private void createRetryableExceptionWithDelay(Duration delay) { service.expectations.add(exception); } - private void createNonRetryableExceptionWithDelay(Duration delay) { + private ApiException createNonRetryableExceptionWithDelay(Duration delay) { Metadata trailers = new Metadata(); RetryInfo retryInfo = RetryInfo.newBuilder() @@ -282,6 +543,8 @@ private void createNonRetryableExceptionWithDelay(Duration delay) { false); service.expectations.add(exception); + + return exception; } private class FakeBigtableService extends BigtableGrpc.BigtableImplBase { @@ -370,5 +633,36 @@ public void readModifyWriteRow( responseObserver.onError(expectedRpc); } } + + @Override + public void generateInitialChangeStreamPartitions( + GenerateInitialChangeStreamPartitionsRequest request, + StreamObserver responseObserver) { + attemptCounter.incrementAndGet(); + if (expectations.isEmpty()) { + responseObserver.onNext(GenerateInitialChangeStreamPartitionsResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } else { + Exception expectedRpc = expectations.poll(); + responseObserver.onError(expectedRpc); + } + } + + @Override + public void readChangeStream( + ReadChangeStreamRequest request, + StreamObserver responseObserver) { + attemptCounter.incrementAndGet(); + if (expectations.isEmpty()) { + responseObserver.onNext( + ReadChangeStreamResponse.newBuilder() + .setCloseStream(ReadChangeStreamResponse.CloseStream.getDefaultInstance()) + .build()); + responseObserver.onCompleted(); + } else { + Exception expectedRpc = expectations.poll(); + responseObserver.onError(expectedRpc); + } + } } } From 8a92dfd010e57763b778301d09eee8a594d94405 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 18 Dec 2023 18:26:25 -0500 Subject: [PATCH 10/13] format --- .../data/v2/stub/EnhancedBigtableStub.java | 10 ++++--- .../EnhancedBigtableStubSettingsTest.java | 26 +++++++++---------- 2 files changed, 19 insertions(+), 17 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 a1de24c0bc..e72eecb43e 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 @@ -1066,10 +1066,11 @@ private UnaryCallable withRetries( UnaryCallable innerCallable, UnaryCallSettings unaryCallSettings) { UnaryCallable retrying; if (settings.getEnableRetryInfo()) { - retrying = com.google.cloud.bigtable.gaxx.retrying.Callables.retrying(innerCallable, unaryCallSettings, clientContext); - } else { retrying = - Callables.retrying(innerCallable, unaryCallSettings, clientContext); + com.google.cloud.bigtable.gaxx.retrying.Callables.retrying( + innerCallable, unaryCallSettings, clientContext); + } else { + retrying = Callables.retrying(innerCallable, unaryCallSettings, clientContext); } if (settings.getEnableRoutingCookie()) { return new CookiesUnaryCallable<>(retrying); @@ -1084,7 +1085,8 @@ private ServerStreamingCallable withR ServerStreamingCallable retrying; if (settings.getEnableRetryInfo()) { retrying = - com.google.cloud.bigtable.gaxx.retrying.Callables.retrying(innerCallable, serverStreamingCallSettings, clientContext); + com.google.cloud.bigtable.gaxx.retrying.Callables.retrying( + innerCallable, serverStreamingCallSettings, clientContext); } else { retrying = Callables.retrying(innerCallable, serverStreamingCallSettings, clientContext); } 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 5b3ef988ca..22b4aed612 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 @@ -806,10 +806,10 @@ public void routingCookieIsEnabled() throws IOException { CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); Mockito.when(credentialsProvider.getCredentials()).thenReturn(new FakeCredentials()); EnhancedBigtableStubSettings.Builder builder = - EnhancedBigtableStubSettings.newBuilder() - .setProjectId(dummyProjectId) - .setInstanceId(dummyInstanceId) - .setCredentialsProvider(credentialsProvider); + EnhancedBigtableStubSettings.newBuilder() + .setProjectId(dummyProjectId) + .setInstanceId(dummyInstanceId) + .setCredentialsProvider(credentialsProvider); assertThat(builder.getEnableRoutingCookie()).isTrue(); assertThat(builder.build().getEnableRoutingCookie()).isTrue(); @@ -822,10 +822,10 @@ public void enableRetryInfoDefaultValueTest() throws IOException { CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); Mockito.when(credentialsProvider.getCredentials()).thenReturn(new FakeCredentials()); EnhancedBigtableStubSettings.Builder builder = - EnhancedBigtableStubSettings.newBuilder() - .setProjectId(dummyProjectId) - .setInstanceId(dummyInstanceId) - .setCredentialsProvider(credentialsProvider); + EnhancedBigtableStubSettings.newBuilder() + .setProjectId(dummyProjectId) + .setInstanceId(dummyInstanceId) + .setCredentialsProvider(credentialsProvider); assertThat(builder.getEnableRetryInfo()).isTrue(); assertThat(builder.build().getEnableRetryInfo()).isTrue(); assertThat(builder.build().toBuilder().getEnableRetryInfo()).isTrue(); @@ -838,11 +838,11 @@ public void routingCookieFalseValueSet() throws IOException { CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); Mockito.when(credentialsProvider.getCredentials()).thenReturn(new FakeCredentials()); EnhancedBigtableStubSettings.Builder builder = - EnhancedBigtableStubSettings.newBuilder() - .setProjectId(dummyProjectId) - .setInstanceId(dummyInstanceId) - .setEnableRoutingCookie(false) - .setCredentialsProvider(credentialsProvider); + EnhancedBigtableStubSettings.newBuilder() + .setProjectId(dummyProjectId) + .setInstanceId(dummyInstanceId) + .setEnableRoutingCookie(false) + .setCredentialsProvider(credentialsProvider); assertThat(builder.getEnableRoutingCookie()).isFalse(); assertThat(builder.build().getEnableRoutingCookie()).isFalse(); assertThat(builder.build().toBuilder().getEnableRoutingCookie()).isFalse(); From 99c43c57b65927457569a7dac723eb11d3cf4072 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 18 Dec 2023 19:47:13 -0500 Subject: [PATCH 11/13] address comments --- .../data/v2/stub/EnhancedBigtableStub.java | 18 +- .../v2/stub/EnhancedBigtableStubSettings.java | 21 +- .../bigtable/gaxx/retrying/ApiException.java | 2 + .../retrying/RetryInfoRetryAlgorithm.java | 21 +- .../bigtable/data/v2/stub/RetryInfoTest.java | 360 ++++++------------ 5 files changed, 141 insertions(+), 281 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 e72eecb43e..a575aa8607 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 @@ -28,6 +28,7 @@ import com.google.api.gax.grpc.GrpcCallSettings; import com.google.api.gax.grpc.GrpcRawCallableFactory; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; +import com.google.api.gax.retrying.BasicResultRetryAlgorithm; import com.google.api.gax.retrying.ExponentialRetryAlgorithm; import com.google.api.gax.retrying.RetryAlgorithm; import com.google.api.gax.retrying.RetryingExecutorWithContext; @@ -763,18 +764,19 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { ServerStreamingCallable withBigtableTracer = new BigtableTracerStreamingCallable<>(convertException); - RetryAlgorithm retryAlgorithm; - ExponentialRetryAlgorithm exponentialRetryAlgorithm = - new ExponentialRetryAlgorithm( - settings.bulkMutateRowsSettings().getRetrySettings(), clientContext.getClock()); + BasicResultRetryAlgorithm resultRetryAlgorithm; if (settings.getEnableRetryInfo()) { - retryAlgorithm = - new RetryAlgorithm<>(new RetryInfoRetryAlgorithm<>(), exponentialRetryAlgorithm); + resultRetryAlgorithm = new RetryInfoRetryAlgorithm<>(); } else { - retryAlgorithm = - new RetryAlgorithm<>(new ApiResultRetryAlgorithm<>(), exponentialRetryAlgorithm); + resultRetryAlgorithm = new ApiResultRetryAlgorithm<>(); } + RetryAlgorithm retryAlgorithm = + new RetryAlgorithm<>( + resultRetryAlgorithm, + new ExponentialRetryAlgorithm( + settings.bulkMutateRowsSettings().getRetrySettings(), clientContext.getClock())); + RetryingExecutorWithContext retryingExecutor = new ScheduledRetryingExecutor<>(retryAlgorithm, clientContext.getExecutor()); 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 8b5a05e4ca..c9587964c8 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 @@ -331,6 +331,7 @@ public boolean getEnableRoutingCookie() { * Gets if RetryInfo is enabled. If true, client bases retry decision and back off time on server * returned RetryInfo value. Otherwise, client uses {@link RetrySettings}. */ + @BetaApi("RetryInfo is not currently stable and may change in the future") public boolean getEnableRetryInfo() { return enableRetryInfo; } @@ -931,15 +932,6 @@ public Builder setEnableRoutingCookie(boolean enableRoutingCookie) { return this; } - /** - * Sets if RetryInfo is enabled. If true, client bases retry decision and back off time on - * server returned RetryInfo value. Otherwise, client uses {@link RetrySettings}. - */ - public Builder setEnableRetryInfo(boolean enableRetryInfo) { - this.enableRetryInfo = enableRetryInfo; - return this; - } - /** * Gets if routing cookie is enabled. If true, client will retry a request with extra metadata * server sent back. @@ -949,10 +941,21 @@ public boolean getEnableRoutingCookie() { return enableRoutingCookie; } + /** + * Sets if RetryInfo is enabled. If true, client bases retry decision and back off time on + * server returned RetryInfo value. Otherwise, client uses {@link RetrySettings}. + */ + @BetaApi("RetryInfo is not currently stable and may change in the future") + public Builder setEnableRetryInfo(boolean enableRetryInfo) { + this.enableRetryInfo = enableRetryInfo; + return this; + } + /** * Gets if RetryInfo is enabled. If true, client bases retry decision and back off time on * server returned RetryInfo value. Otherwise, client uses {@link RetrySettings}. */ + @BetaApi("RetryInfo is not currently stable and may change in the future") public boolean getEnableRetryInfo() { return enableRetryInfo; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java index b13452934d..7bd85eefa7 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java @@ -21,6 +21,8 @@ @InternalApi public class ApiException { + private ApiException() {} + // TODO: this should replace the existing ApiException#isRetryable() method, // but that cant be done in bigtable, so this lives here for now. public static boolean isRetryable2(Throwable e) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java index 1fde962903..71457f7e9a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java @@ -26,6 +26,7 @@ import io.grpc.Metadata; import io.grpc.Status; import io.grpc.protobuf.ProtoUtils; +import org.checkerframework.checker.nullness.qual.Nullable; import org.threeten.bp.Duration; // TODO move this algorithm to gax @@ -57,13 +58,7 @@ public TimedAttemptSettings createNextAttempt( /** Returns true if previousThrowable is an {@link ApiException} that is retryable. */ @Override public boolean shouldRetry(Throwable previousThrowable, ResponseT previousResponse) { - if (extractRetryDelay(previousThrowable) != null) { - // First check if server wants us to retry - return true; - } - // Server didn't have retry information, use the local status code config. - return (previousThrowable instanceof ApiException - && ((ApiException) previousThrowable).isRetryable()); + return shouldRetry(null, previousThrowable, previousResponse); } /** @@ -74,12 +69,12 @@ public boolean shouldRetry(Throwable previousThrowable, ResponseT previousRespon */ @Override public boolean shouldRetry( - RetryingContext context, Throwable previousThrowable, ResponseT previousResponse) { + @Nullable RetryingContext context, Throwable previousThrowable, ResponseT previousResponse) { if (extractRetryDelay(previousThrowable) != null) { // First check if server wants us to retry return true; } - if (context.getRetryableCodes() != null) { + if (context != null && context.getRetryableCodes() != null) { // Ignore the isRetryable() value of the throwable if the RetryingContext has a specific list // of codes that should be retried. return ((previousThrowable instanceof ApiException) @@ -87,10 +82,14 @@ public boolean shouldRetry( .getRetryableCodes() .contains(((ApiException) previousThrowable).getStatusCode().getCode())); } - return shouldRetry(previousThrowable, previousResponse); + // Server didn't have retry information and there's no retry context, use the local status + // code config. + return previousThrowable instanceof ApiException + && ((ApiException) previousThrowable).isRetryable(); } - static Duration extractRetryDelay(Throwable throwable) { + @Nullable + static Duration extractRetryDelay(@Nullable Throwable throwable) { if (throwable == null) { return null; } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java index 41037ae067..4a9bc3d466 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java @@ -56,7 +56,6 @@ import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; import com.google.common.base.Stopwatch; import com.google.common.collect.Queues; -import com.google.protobuf.Duration; import com.google.rpc.RetryInfo; import io.grpc.Metadata; import io.grpc.Status; @@ -64,6 +63,7 @@ import io.grpc.stub.StreamObserver; import io.grpc.testing.GrpcServerRule; import java.io.IOException; +import java.time.Duration; import java.util.Queue; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Before; @@ -82,7 +82,8 @@ public class RetryInfoTest { private BigtableDataSettings.Builder settings; private AtomicInteger attemptCounter = new AtomicInteger(); - private Duration delay = Duration.newBuilder().setSeconds(1).setNanos(0).build(); + private com.google.protobuf.Duration delay = + com.google.protobuf.Duration.newBuilder().setSeconds(1).setNanos(0).build(); @Before public void setUp() throws IOException { @@ -109,268 +110,127 @@ public void setUp() throws IOException { @Test public void testReadRow() { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readRow("table", "row"); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed(() -> client.readRow("table", "row"), true); } @Test public void testReadRowNonRetryableErrorWithRetryInfo() { - createNonRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readRow("table", "row"); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed(() -> client.readRow("table", "row"), false); } @Test public void testReadRowDisableRetryInfo() throws IOException { settings.stubSettings().setEnableRetryInfo(false); - try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readRow("table", "row"); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); - - attemptCounter.set(0); - ApiException exception = createNonRetryableExceptionWithDelay(delay); - try { - client.readRow("table", "row"); - } catch (ApiException e) { - assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); - } - assertThat(attemptCounter.get()).isEqualTo(1); + try (BigtableDataClient newClient = BigtableDataClient.create(settings.build())) { + verifyRetryInfoCanBeDisabled(() -> newClient.readRow("table", "row")); } } @Test public void testReadRows() { - createRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readRows(Query.create("table")).iterator().hasNext(); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed(() -> client.readRows(Query.create("table")).iterator().hasNext(), true); } @Test public void testReadRowsNonRetraybleErrorWithRetryInfo() { - createNonRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readRows(Query.create("table")).iterator().hasNext(); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed(() -> client.readRows(Query.create("table")).iterator().hasNext(), false); } @Test public void testReadRowsDisableRetryInfo() throws IOException { settings.stubSettings().setEnableRetryInfo(false); - try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readRows(Query.create("table")).iterator().hasNext(); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); - - attemptCounter.set(0); - ApiException exception = createNonRetryableExceptionWithDelay(delay); - try { - client.readRows(Query.create("table")).iterator().hasNext(); - } catch (ApiException e) { - assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); - } - assertThat(attemptCounter.get()).isEqualTo(1); + try (BigtableDataClient newClient = BigtableDataClient.create(settings.build())) { + verifyRetryInfoCanBeDisabled( + () -> newClient.readRows(Query.create("table")).iterator().hasNext()); } } @Test public void testMutateRows() { - createRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - - client.bulkMutateRows( - BulkMutation.create("fake-table") - .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed( + () -> + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))), + true); } @Test public void testMutateRowsNonRetryableErrorWithRetryInfo() { - createNonRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - - client.bulkMutateRows( - BulkMutation.create("fake-table") - .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed( + () -> + client.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))), + false); } @Test public void testMutateRowsDisableRetryInfo() throws IOException { settings.stubSettings().setEnableRetryInfo(false); - try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.bulkMutateRows( - BulkMutation.create("fake-table") - .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); - - attemptCounter.set(0); - ApiException exception = createNonRetryableExceptionWithDelay(delay); - try { - client.bulkMutateRows( - BulkMutation.create("fake-table") - .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v"))); - } catch (ApiException e) { - assertThat(((MutateRowsException) e).getFailedMutations().get(0).getError().getStatusCode()) - .isEqualTo(exception.getStatusCode()); - } - assertThat(attemptCounter.get()).isEqualTo(1); + try (BigtableDataClient newClient = BigtableDataClient.create(settings.build())) { + verifyRetryInfoCanBeDisabled( + () -> + newClient.bulkMutateRows( + BulkMutation.create("fake-table") + .add(RowMutationEntry.create("row-key-1").setCell("cf", "q", "v")))); } } @Test public void testMutateRow() { - createRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(1)); + verifyRetryInfoIsUsed( + () -> client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")), true); } @Test public void testMutateRowNonRetryableErrorWithRetryInfo() { - createNonRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(1)); + verifyRetryInfoIsUsed( + () -> client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")), false); } @Test public void testMutateRowDisableRetryInfo() throws IOException { settings.stubSettings().setEnableRetryInfo(false); - try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); + try (BigtableDataClient newClient = BigtableDataClient.create(settings.build())) { - attemptCounter.set(0); - ApiException exception = createNonRetryableExceptionWithDelay(delay); - try { - client.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v")); - } catch (ApiException e) { - assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); - } - assertThat(attemptCounter.get()).isEqualTo(1); + verifyRetryInfoCanBeDisabled( + () -> newClient.mutateRow(RowMutation.create("table", "key").setCell("cf", "q", "v"))); } } @Test public void testSampleRowKeys() { - createRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - - client.sampleRowKeys("table"); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed(() -> client.sampleRowKeys("table"), true); } @Test public void testSampleRowKeysNonRetryableErrorWithRetryInfo() { - createNonRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - - client.sampleRowKeys("table"); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed(() -> client.sampleRowKeys("table"), false); } @Test public void testSampleRowKeysDisableRetryInfo() throws IOException { settings.stubSettings().setEnableRetryInfo(false); - try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.sampleRowKeys("table"); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); - - attemptCounter.set(0); - ApiException exception = createNonRetryableExceptionWithDelay(delay); - try { - client.sampleRowKeys("table"); - } catch (ApiException e) { - assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); - } - assertThat(attemptCounter.get()).isEqualTo(1); + try (BigtableDataClient newClient = BigtableDataClient.create(settings.build())) { + verifyRetryInfoCanBeDisabled(() -> newClient.sampleRowKeys("table")); } } @Test public void testCheckAndMutateRow() { - createRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - client.checkAndMutateRow( - ConditionalRowMutation.create("table", "key") - .condition(Filters.FILTERS.value().regex("old-value")) - .then(Mutation.create().setCell("cf", "q", "v"))); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed( + () -> + client.checkAndMutateRow( + ConditionalRowMutation.create("table", "key") + .condition(Filters.FILTERS.value().regex("old-value")) + .then(Mutation.create().setCell("cf", "q", "v"))), + true); } @Test @@ -378,7 +238,7 @@ public void testCheckAndMutateDisableRetryInfo() throws IOException { settings.stubSettings().setEnableRetryInfo(false); try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - ApiException exception = createNonRetryableExceptionWithDelay(delay); + ApiException exception = enqueueNonRetryableExceptionWithDelay(delay); try { client.checkAndMutateRow( ConditionalRowMutation.create("table", "key") @@ -393,14 +253,12 @@ public void testCheckAndMutateDisableRetryInfo() throws IOException { @Test public void testReadModifyWrite() { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readModifyWriteRow(ReadModifyWriteRow.create("table", "row").append("cf", "q", "v")); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed( + () -> + client.readModifyWriteRow( + ReadModifyWriteRow.create("table", "row").append("cf", "q", "v")), + true); } @Test @@ -408,7 +266,7 @@ public void testReadModifyWriteDisableRetryInfo() throws IOException { settings.stubSettings().setEnableRetryInfo(false); try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - ApiException exception = createNonRetryableExceptionWithDelay(delay); + ApiException exception = enqueueNonRetryableExceptionWithDelay(delay); try { client.readModifyWriteRow(ReadModifyWriteRow.create("table", "row").append("cf", "q", "v")); } catch (ApiException e) { @@ -420,101 +278,100 @@ public void testReadModifyWriteDisableRetryInfo() throws IOException { @Test public void testReadChangeStream() { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed( + () -> client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(), + true); } @Test public void testReadChangeStreamNonRetryableErrorWithRetryInfo() { - createNonRetryableExceptionWithDelay(delay); - - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed( + () -> client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(), + false); } @Test public void testReadChangeStreamDisableRetryInfo() throws IOException { settings.stubSettings().setEnableRetryInfo(false); - try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); - - attemptCounter.set(0); - ApiException exception = createNonRetryableExceptionWithDelay(delay); - try { - client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(); - } catch (ApiException e) { - assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); - } - assertThat(attemptCounter.get()).isEqualTo(1); + try (BigtableDataClient newClient = BigtableDataClient.create(settings.build())) { + verifyRetryInfoCanBeDisabled( + () -> + newClient + .readChangeStream(ReadChangeStreamQuery.create("table")) + .iterator() + .hasNext()); } } @Test public void testGenerateInitialChangeStreamPartition() { - createRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); - stopwatch.stop(); - - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + verifyRetryInfoIsUsed( + () -> client.generateInitialChangeStreamPartitions("table").iterator().hasNext(), true); } @Test public void testGenerateInitialChangeStreamPartitionNonRetryableError() { - createNonRetryableExceptionWithDelay(delay); + verifyRetryInfoIsUsed( + () -> client.generateInitialChangeStreamPartitions("table").iterator().hasNext(), false); + } + + @Test + public void testGenerateInitialChangeStreamPartitionDisableRetryInfo() throws IOException { + settings.stubSettings().setEnableRetryInfo(false); + try (BigtableDataClient newClient = BigtableDataClient.create(settings.build())) { + verifyRetryInfoCanBeDisabled( + () -> newClient.generateInitialChangeStreamPartitions("table").iterator().hasNext()); + } + } + + private void verifyRetryInfoIsUsed(Runnable runnable, boolean retryableError) { + if (retryableError) { + enqueueRetryableExceptionWithDelay(delay); + } else { + enqueueNonRetryableExceptionWithDelay(delay); + } Stopwatch stopwatch = Stopwatch.createStarted(); - client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); + runnable.run(); stopwatch.stop(); assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isAtLeast(java.time.Duration.ofSeconds(delay.getSeconds())); + assertThat(stopwatch.elapsed()).isAtLeast(Duration.ofSeconds(delay.getSeconds())); } - @Test - public void testGenerateInitialChangeStreamPartitionDisableRetryInfo() throws IOException { + private void verifyRetryInfoCanBeDisabled(Runnable runnable) throws IOException { settings.stubSettings().setEnableRetryInfo(false); try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - createRetryableExceptionWithDelay(delay); + enqueueRetryableExceptionWithDelay(delay); Stopwatch stopwatch = Stopwatch.createStarted(); - client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); + runnable.run(); stopwatch.stop(); assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isLessThan(java.time.Duration.ofSeconds(delay.getSeconds())); + assertThat(stopwatch.elapsed()).isLessThan(Duration.ofSeconds(delay.getSeconds())); attemptCounter.set(0); - ApiException exception = createNonRetryableExceptionWithDelay(delay); + ApiException exception = enqueueNonRetryableExceptionWithDelay(delay); try { - client.generateInitialChangeStreamPartitions("table").iterator().hasNext(); + runnable.run(); } catch (ApiException e) { - assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + if (e instanceof MutateRowsException) { + assertThat( + ((MutateRowsException) e).getFailedMutations().get(0).getError().getStatusCode()) + .isEqualTo(exception.getStatusCode()); + } else { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); + } } assertThat(attemptCounter.get()).isEqualTo(1); } } - private void createRetryableExceptionWithDelay(Duration delay) { + private void enqueueRetryableExceptionWithDelay(com.google.protobuf.Duration delay) { Metadata trailers = new Metadata(); RetryInfo retryInfo = RetryInfo.newBuilder().setRetryDelay(delay).build(); trailers.put(RETRY_INFO_KEY, retryInfo); @@ -528,12 +385,9 @@ private void createRetryableExceptionWithDelay(Duration delay) { service.expectations.add(exception); } - private ApiException createNonRetryableExceptionWithDelay(Duration delay) { + private ApiException enqueueNonRetryableExceptionWithDelay(com.google.protobuf.Duration delay) { Metadata trailers = new Metadata(); - RetryInfo retryInfo = - RetryInfo.newBuilder() - .setRetryDelay(Duration.newBuilder().setSeconds(1).setNanos(0).build()) - .build(); + RetryInfo retryInfo = RetryInfo.newBuilder().setRetryDelay(delay).build(); trailers.put(RETRY_INFO_KEY, retryInfo); ApiException exception = From cf0066135e47c24f65c489bd3cdeb37173625190 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 18 Dec 2023 19:53:21 -0500 Subject: [PATCH 12/13] fix format --- .../bigtable/data/v2/stub/RetryInfoTest.java | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java index 4a9bc3d466..b38e53480c 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/RetryInfoTest.java @@ -253,7 +253,6 @@ public void testCheckAndMutateDisableRetryInfo() throws IOException { @Test public void testReadModifyWrite() { - verifyRetryInfoIsUsed( () -> client.readModifyWriteRow( @@ -278,7 +277,6 @@ public void testReadModifyWriteDisableRetryInfo() throws IOException { @Test public void testReadChangeStream() { - verifyRetryInfoIsUsed( () -> client.readChangeStream(ReadChangeStreamQuery.create("table")).iterator().hasNext(), true); @@ -307,7 +305,6 @@ public void testReadChangeStreamDisableRetryInfo() throws IOException { @Test public void testGenerateInitialChangeStreamPartition() { - verifyRetryInfoIsUsed( () -> client.generateInitialChangeStreamPartitions("table").iterator().hasNext(), true); } @@ -342,33 +339,28 @@ private void verifyRetryInfoIsUsed(Runnable runnable, boolean retryableError) { assertThat(stopwatch.elapsed()).isAtLeast(Duration.ofSeconds(delay.getSeconds())); } - private void verifyRetryInfoCanBeDisabled(Runnable runnable) throws IOException { - settings.stubSettings().setEnableRetryInfo(false); - - try (BigtableDataClient client = BigtableDataClient.create(settings.build())) { - enqueueRetryableExceptionWithDelay(delay); - Stopwatch stopwatch = Stopwatch.createStarted(); - runnable.run(); - stopwatch.stop(); + private void verifyRetryInfoCanBeDisabled(Runnable runnable) { + enqueueRetryableExceptionWithDelay(delay); + Stopwatch stopwatch = Stopwatch.createStarted(); + runnable.run(); + stopwatch.stop(); - assertThat(attemptCounter.get()).isEqualTo(2); - assertThat(stopwatch.elapsed()).isLessThan(Duration.ofSeconds(delay.getSeconds())); + assertThat(attemptCounter.get()).isEqualTo(2); + assertThat(stopwatch.elapsed()).isLessThan(Duration.ofSeconds(delay.getSeconds())); - attemptCounter.set(0); - ApiException exception = enqueueNonRetryableExceptionWithDelay(delay); - try { - runnable.run(); - } catch (ApiException e) { - if (e instanceof MutateRowsException) { - assertThat( - ((MutateRowsException) e).getFailedMutations().get(0).getError().getStatusCode()) - .isEqualTo(exception.getStatusCode()); - } else { - assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); - } + attemptCounter.set(0); + ApiException exception = enqueueNonRetryableExceptionWithDelay(delay); + try { + runnable.run(); + } catch (ApiException e) { + if (e instanceof MutateRowsException) { + assertThat(((MutateRowsException) e).getFailedMutations().get(0).getError().getStatusCode()) + .isEqualTo(exception.getStatusCode()); + } else { + assertThat(e.getStatusCode()).isEqualTo(exception.getStatusCode()); } - assertThat(attemptCounter.get()).isEqualTo(1); } + assertThat(attemptCounter.get()).isEqualTo(1); } private void enqueueRetryableExceptionWithDelay(com.google.protobuf.Duration delay) { From 06b3a189b731c0f717e866154fe7e638b31719b3 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 19 Dec 2023 10:23:25 -0500 Subject: [PATCH 13/13] rename ApiException --- google-cloud-bigtable/pom.xml | 5 ++++- .../data/v2/stub/mutaterows/MutateRowsAttemptCallable.java | 4 ++-- .../gaxx/retrying/{ApiException.java => ApiExceptions.java} | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) rename google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/{ApiException.java => ApiExceptions.java} (94%) diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index e669a2006c..dda6694b65 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -161,7 +161,10 @@ grpc-alts runtime - + + org.checkerframework + checker-qual + com.google.http-client google-http-client diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java index ddf945b7da..269ce79031 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/mutaterows/MutateRowsAttemptCallable.java @@ -31,6 +31,7 @@ import com.google.bigtable.v2.MutateRowsResponse.Entry; import com.google.cloud.bigtable.data.v2.models.MutateRowsException; import com.google.cloud.bigtable.data.v2.models.MutateRowsException.FailedMutation; +import com.google.cloud.bigtable.gaxx.retrying.ApiExceptions; import com.google.cloud.bigtable.gaxx.retrying.NonCancellableFuture; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -235,8 +236,7 @@ private void handleAttemptError(Throwable rpcError) { FailedMutation failedMutation = FailedMutation.create(origIndex, entryError); allFailures.add(failedMutation); - if (!com.google.cloud.bigtable.gaxx.retrying.ApiException.isRetryable2( - failedMutation.getError()) + if (!ApiExceptions.isRetryable2(failedMutation.getError()) && !failedMutation.getError().isRetryable()) { permanentFailures.add(failedMutation); } else { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiExceptions.java similarity index 94% rename from google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java rename to google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiExceptions.java index 7bd85eefa7..4e794fa41a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiException.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/ApiExceptions.java @@ -19,9 +19,9 @@ // TODO: move this to gax later @InternalApi -public class ApiException { +public class ApiExceptions { - private ApiException() {} + private ApiExceptions() {} // TODO: this should replace the existing ApiException#isRetryable() method, // but that cant be done in bigtable, so this lives here for now.