From b3ea0c0f7a759f2f5574afd4525642b70e8c803e Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 13 Mar 2018 23:05:54 -0400 Subject: [PATCH 1/4] Rebase on latest watchdog api changes --- .../data/v2/stub/EnhancedBigtableStub.java | 7 ++- .../v2/stub/EnhancedBigtableStubSettings.java | 3 +- .../EnhancedBigtableStubSettingsTest.java | 52 ++++++++++++++----- 3 files changed, 44 insertions(+), 18 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 d26658cad0b0..379e751d71f9 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 @@ -84,7 +84,9 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) BigtableStubSettings.newBuilder() .setTransportChannelProvider(settings.getTransportChannelProvider()) .setEndpoint(settings.getEndpoint()) - .setCredentialsProvider(settings.getCredentialsProvider()); + .setCredentialsProvider(settings.getCredentialsProvider()) + .setStreamWatchdogProvider(settings.getStreamWatchdogProvider()) + .setStreamWatchdogCheckInterval(settings.getStreamWatchdogCheckInterval()); // ReadRow retries are handled in the overlay: disable retries in the base layer (but make // sure to preserve the exception callable settings). @@ -92,7 +94,6 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) .readRowsSettings() .setSimpleTimeoutNoRetries(Duration.ofHours(2)) .setRetryableCodes(settings.readRowsSettings().getRetryableCodes()) - .setTimeoutCheckInterval(Duration.ZERO) .setIdleTimeout(Duration.ZERO); // SampleRowKeys retries are handled in the overlay: disable retries in the base layer (but make @@ -115,7 +116,6 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) .mutateRowsSettings() .setSimpleTimeoutNoRetries(Duration.ofHours(2)) .setRetryableCodes(settings.mutateRowsSettings().getRetryableCodes()) - .setTimeoutCheckInterval(Duration.ZERO) .setIdleTimeout(Duration.ZERO); // CheckAndMutateRow is a simple passthrough @@ -182,7 +182,6 @@ public ServerStreamingCallable createReadRowsCallable( .setResumptionStrategy(new ReadRowsResumptionStrategy<>(rowAdapter)) .setRetryableCodes(settings.readRowsSettings().getRetryableCodes()) .setRetrySettings(settings.readRowsSettings().getRetrySettings()) - .setTimeoutCheckInterval(settings.readRowsSettings().getTimeoutCheckInterval()) .setIdleTimeout(settings.readRowsSettings().getIdleTimeout()) .build(); 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 271d207a8082..29d3e0a0df1f 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 @@ -201,6 +201,8 @@ private Builder() { .setChannelsPerCpu(2) .setMaxInboundMessageSize(MAX_MESSAGE_SIZE) .build()); + setStreamWatchdogCheckInterval(baseDefaults.getStreamWatchdogCheckInterval()); + setStreamWatchdogProvider(baseDefaults.getStreamWatchdogProvider()); // Per-method settings using baseSettings for defaults. readRowsSettings = ServerStreamingCallSettings.newBuilder(); @@ -208,7 +210,6 @@ private Builder() { readRowsSettings .setRetryableCodes(DEFAULT_RETRY_CODES) .setRetrySettings(DEFAULT_RETRY_SETTINGS) - .setTimeoutCheckInterval(Duration.ofSeconds(10)) .setIdleTimeout(Duration.ofMinutes(5)); sampleRowKeysSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); 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 4e9e49aacb78..3f1c50aed841 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 @@ -25,6 +25,7 @@ import com.google.api.gax.rpc.ServerStreamingCallSettings; import com.google.api.gax.rpc.StatusCode.Code; import com.google.api.gax.rpc.UnaryCallSettings; +import com.google.api.gax.rpc.WatchdogProvider; import com.google.bigtable.admin.v2.InstanceName; import com.google.cloud.bigtable.data.v2.models.ConditionalRowMutation; import com.google.cloud.bigtable.data.v2.models.KeyOffset; @@ -62,18 +63,42 @@ public void settingsAreNotLostTest() { String appProfileId = "my-app-profile-id"; String endpoint = "some.other.host:123"; CredentialsProvider credentialsProvider = Mockito.mock(CredentialsProvider.class); + WatchdogProvider watchdogProvider = Mockito.mock(WatchdogProvider.class); + Duration watchdogInterval = Duration.ofSeconds(12); EnhancedBigtableStubSettings.Builder builder = EnhancedBigtableStubSettings.newBuilder() .setInstanceName(instanceName) .setAppProfileId(appProfileId) .setEndpoint(endpoint) - .setCredentialsProvider(credentialsProvider); + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) + .setStreamWatchdogCheckInterval(watchdogInterval); - verifyBuilder(builder, instanceName, appProfileId, endpoint, credentialsProvider); - verifySettings(builder.build(), instanceName, appProfileId, endpoint, credentialsProvider); verifyBuilder( - builder.build().toBuilder(), instanceName, appProfileId, endpoint, credentialsProvider); + builder, + instanceName, + appProfileId, + endpoint, + credentialsProvider, + watchdogProvider, + watchdogInterval); + verifySettings( + builder.build(), + instanceName, + appProfileId, + endpoint, + credentialsProvider, + watchdogProvider, + watchdogInterval); + verifyBuilder( + builder.build().toBuilder(), + instanceName, + appProfileId, + endpoint, + credentialsProvider, + watchdogProvider, + watchdogInterval); } private void verifyBuilder( @@ -81,11 +106,15 @@ private void verifyBuilder( InstanceName instanceName, String appProfileId, String endpoint, - CredentialsProvider credentialsProvider) { + CredentialsProvider credentialsProvider, + WatchdogProvider watchdogProvider, + Duration watchdogInterval) { assertThat(builder.getInstanceName()).isEqualTo(instanceName); assertThat(builder.getAppProfileId()).isEqualTo(appProfileId); assertThat(builder.getEndpoint()).isEqualTo(endpoint); assertThat(builder.getCredentialsProvider()).isEqualTo(credentialsProvider); + assertThat(builder.getStreamWatchdogProvider()).isSameAs(watchdogProvider); + assertThat(builder.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); } private void verifySettings( @@ -93,11 +122,15 @@ private void verifySettings( InstanceName instanceName, String appProfileId, String endpoint, - CredentialsProvider credentialsProvider) { + CredentialsProvider credentialsProvider, + WatchdogProvider watchdogProvider, + Duration watchdogInterval) { assertThat(settings.getInstanceName()).isEqualTo(instanceName); assertThat(settings.getAppProfileId()).isEqualTo(appProfileId); assertThat(settings.getEndpoint()).isEqualTo(endpoint); assertThat(settings.getCredentialsProvider()).isEqualTo(credentialsProvider); + assertThat(settings.getStreamWatchdogProvider()).isSameAs(watchdogProvider); + assertThat(settings.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); } @Test @@ -132,29 +165,22 @@ public void readRowsIsNotLostTest() { builder .readRowsSettings() - .setTimeoutCheckInterval(Duration.ofSeconds(10)) .setIdleTimeout(Duration.ofMinutes(5)) .setRetryableCodes(Code.ABORTED, Code.DEADLINE_EXCEEDED) .setRetrySettings(retrySettings) .build(); - assertThat(builder.readRowsSettings().getTimeoutCheckInterval()) - .isEqualTo(Duration.ofSeconds(10)); assertThat(builder.readRowsSettings().getIdleTimeout()).isEqualTo(Duration.ofMinutes(5)); assertThat(builder.readRowsSettings().getRetryableCodes()) .containsAllOf(Code.ABORTED, Code.DEADLINE_EXCEEDED); assertThat(builder.readRowsSettings().getRetrySettings()).isEqualTo(retrySettings); - assertThat(builder.build().readRowsSettings().getTimeoutCheckInterval()) - .isEqualTo(Duration.ofSeconds(10)); assertThat(builder.build().readRowsSettings().getIdleTimeout()) .isEqualTo(Duration.ofMinutes(5)); assertThat(builder.build().readRowsSettings().getRetryableCodes()) .containsAllOf(Code.ABORTED, Code.DEADLINE_EXCEEDED); assertThat(builder.build().readRowsSettings().getRetrySettings()).isEqualTo(retrySettings); - assertThat(builder.build().toBuilder().readRowsSettings().getTimeoutCheckInterval()) - .isEqualTo(Duration.ofSeconds(10)); assertThat(builder.build().toBuilder().readRowsSettings().getIdleTimeout()) .isEqualTo(Duration.ofMinutes(5)); assertThat(builder.build().toBuilder().readRowsSettings().getRetryableCodes()) From 1b82a654975c12ec291eb9c4720b30b537fd0cf1 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 13 Mar 2018 23:06:19 -0400 Subject: [PATCH 2/4] Update to latest resumption strategy changes --- .../data/v2/stub/readrows/ReadRowsResumptionStrategy.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsResumptionStrategy.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsResumptionStrategy.java index dfc1ea1ef5d1..5ec34504cb87 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsResumptionStrategy.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/readrows/ReadRowsResumptionStrategy.java @@ -57,7 +57,7 @@ public StreamResumptionStrategy createNew() { } @Override - public void onProgress(RowT response) { + public RowT processResponse(RowT response) { // Last key can come from both the last processed row key and a synthetic row marker. The // synthetic row marker is emitted when the server has read a lot of data that was filtered out. // The row marker can be used to trim the start of the scan, but does not contribute to the row @@ -67,6 +67,7 @@ public void onProgress(RowT response) { // Only real rows count towards the rows limit. numProcessed++; } + return response; } /** From 045205938f77efb17658bc8ebc346683a7778df0 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Tue, 13 Mar 2018 23:06:38 -0400 Subject: [PATCH 3/4] update settings --- .../v2/stub/EnhancedBigtableStubSettings.java | 25 ++++++++----------- .../EnhancedBigtableStubSettingsTest.java | 3 +-- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 29d3e0a0df1f..6bd932c2485c 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 @@ -80,16 +80,16 @@ public class EnhancedBigtableStubSettings extends StubSettings DEFAULT_RETRY_CODES = ImmutableSet.of(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE, Code.ABORTED); + // Copy of default retrying settings in the yaml private static final RetrySettings DEFAULT_RETRY_SETTINGS = RetrySettings.newBuilder() - .setMaxAttempts(10) - .setTotalTimeout(Duration.ofHours(1)) - .setInitialRetryDelay(Duration.ofMillis(100)) + .setInitialRetryDelay(Duration.ofMillis(100L)) .setRetryDelayMultiplier(1.3) - .setMaxRetryDelay(Duration.ofMinutes(1)) - .setInitialRpcTimeout(Duration.ofSeconds(20)) - .setRpcTimeoutMultiplier(1) - .setMaxRpcTimeout(Duration.ofSeconds(20)) + .setMaxRetryDelay(Duration.ofMillis(60000L)) + .setInitialRpcTimeout(Duration.ofMillis(20000L)) + .setRpcTimeoutMultiplier(1.0) + .setMaxRpcTimeout(Duration.ofMillis(20000L)) + .setTotalTimeout(Duration.ofMillis(600000L)) .build(); private final InstanceName instanceName; @@ -209,7 +209,8 @@ private Builder() { /* TODO: copy timeouts, retryCodes & retrySettings from baseSettings.readRows once it exists in GAPIC */ readRowsSettings .setRetryableCodes(DEFAULT_RETRY_CODES) - .setRetrySettings(DEFAULT_RETRY_SETTINGS) + .setRetrySettings( + DEFAULT_RETRY_SETTINGS.toBuilder().setTotalTimeout(Duration.ofHours(1)).build()) .setIdleTimeout(Duration.ofMinutes(5)); sampleRowKeysSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); @@ -218,14 +219,8 @@ private Builder() { .setRetryableCodes(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE, Code.ABORTED) .setRetrySettings(DEFAULT_RETRY_SETTINGS); - // NOTE: This client enforces client side timestamps, which makes all mutations retryable. - // However, since the base GAPIC client allows for server side timestamps, it is not - // configured to enable retries. So the retry settings have to be defined here instead of - // being copied from the BigtableStubSettings. mutateRowSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); - mutateRowSettings - .setRetryableCodes(DEFAULT_RETRY_CODES) - .setRetrySettings(DEFAULT_RETRY_SETTINGS); + copyRetrySettings(baseDefaults.mutateRowSettings(), mutateRowSettings); /* TODO: copy retryCodes & retrySettings from baseSettings.mutateRows once it exists in GAPIC */ mutateRowsSettings = 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 3f1c50aed841..916bcc00d446 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 @@ -393,9 +393,8 @@ public void checkAndMutateRowSettingsAreSane() { } private void verifyRetrySettingAreSane(Set retryCodes, RetrySettings retrySettings) { - assertThat(retryCodes).containsAllOf(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE, Code.ABORTED); + assertThat(retryCodes).containsAllOf(Code.DEADLINE_EXCEEDED, Code.UNAVAILABLE); - assertThat(retrySettings.getMaxAttempts()).isGreaterThan(1); assertThat(retrySettings.getTotalTimeout()).isGreaterThan(Duration.ZERO); assertThat(retrySettings.getInitialRetryDelay()).isGreaterThan(Duration.ZERO); From b755a0da4f31d7c46bd1a1a7e334f2e2015384e5 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Fri, 16 Mar 2018 00:01:43 -0400 Subject: [PATCH 4/4] update gax version --- google-cloud-bom/pom.xml | 8 ++++---- pom.xml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/google-cloud-bom/pom.xml b/google-cloud-bom/pom.xml index 34ee7af61f3d..2dc363447156 100644 --- a/google-cloud-bom/pom.xml +++ b/google-cloud-bom/pom.xml @@ -168,10 +168,10 @@ 0.39.1-alpha-SNAPSHOT 0.39.1-alpha-SNAPSHOT - 1.4.0 - 1.19.0 - 1.19.0 - 0.36.0 + 1.5.0 + 1.20.0 + 1.20.0 + 0.37.0 0.4.0 1.3.0 diff --git a/pom.xml b/pom.xml index 30d6cbbed39b..18c32244e697 100644 --- a/pom.xml +++ b/pom.xml @@ -138,7 +138,7 @@ google-cloud 0.39.1-alpha-SNAPSHOT 1.23.0 - 1.19.0 + 1.20.0 0.9.0 1.9.0 2.0.7.Final