From c023cad1a664d23eb875bbd167a8039d62665b55 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 27 Dec 2018 13:53:26 -0500 Subject: [PATCH 1/4] Track overall attempt counts in TimedAttemptSettings This is extracted from https://github.com/googleapis/gax-java/pull/613#discussion_r229900863. Currently for streaming RPCs TimedAttemptSettings only tracks how many attempts occurred trying to receive the next message. Once the message is received, the attempt count is reset. This adds a new field to TimedAttemptSettings that will track overall count so that it can be used in a future integration with opencensus (or be used in a new retry algorithm) --- .../retrying/ExponentialRetryAlgorithm.java | 2 ++ .../gax/retrying/StreamingRetryAlgorithm.java | 1 + .../api/gax/retrying/TimedAttemptSettings.java | 18 +++++++++++++++++- .../ExponentialRetryAlgorithmTest.java | 2 ++ .../ServerStreamingAttemptCallableTest.java | 1 + 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/gax/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java b/gax/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java index 976f0c19c..cad48323c 100644 --- a/gax/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java +++ b/gax/src/main/java/com/google/api/gax/retrying/ExponentialRetryAlgorithm.java @@ -72,6 +72,7 @@ public TimedAttemptSettings createFirstAttempt() { .setRpcTimeout(globalSettings.getInitialRpcTimeout()) .setRandomizedRetryDelay(Duration.ZERO) .setAttemptCount(0) + .setOverallAttemptCount(0) .setFirstAttemptStartTimeNanos(clock.nanoTime()) .build(); } @@ -113,6 +114,7 @@ public TimedAttemptSettings createNextAttempt(TimedAttemptSettings prevSettings) .setRpcTimeout(Duration.ofMillis(newRpcTimeout)) .setRandomizedRetryDelay(Duration.ofMillis(nextRandomLong(newRetryDelay))) .setAttemptCount(prevSettings.getAttemptCount() + 1) + .setOverallAttemptCount(prevSettings.getOverallAttemptCount() + 1) .setFirstAttemptStartTimeNanos(prevSettings.getFirstAttemptStartTimeNanos()) .build(); } diff --git a/gax/src/main/java/com/google/api/gax/retrying/StreamingRetryAlgorithm.java b/gax/src/main/java/com/google/api/gax/retrying/StreamingRetryAlgorithm.java index 764e66386..52d58f9c7 100644 --- a/gax/src/main/java/com/google/api/gax/retrying/StreamingRetryAlgorithm.java +++ b/gax/src/main/java/com/google/api/gax/retrying/StreamingRetryAlgorithm.java @@ -68,6 +68,7 @@ public TimedAttemptSettings createNextAttempt( createFirstAttempt() .toBuilder() .setFirstAttemptStartTimeNanos(prevSettings.getFirstAttemptStartTimeNanos()) + .setOverallAttemptCount(prevSettings.getOverallAttemptCount()) .build(); } } diff --git a/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java b/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java index 3884ba754..9cd61c15b 100644 --- a/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java +++ b/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java @@ -58,9 +58,19 @@ public abstract class TimedAttemptSettings { */ public abstract Duration getRandomizedRetryDelay(); - /** The attempt count. It is a zero-based value (first attempt will have this value set to 0). */ + /** + * The attempt count. It is a zero-based value (first attempt will have this value set to 0). For + * streamed RPCs this will be reset after every successful message. + */ public abstract int getAttemptCount(); + /** + * The overall attempt count. It is a zero-based value (first attempt will have this value set to + * 0). This will be the sum of all attempt counts for a streaming RPC and will be equal to {@link + * #getAttemptCount()} for unary RPCs. + */ + public abstract int getOverallAttemptCount(); + /** * The start time of the first attempt. Note that this value is dependent on the actual {@link * ApiClock} used during the process. @@ -99,6 +109,12 @@ public abstract static class Builder { */ public abstract Builder setAttemptCount(int value); + /** + * Set the overall attempt count. It is a zero-based value (first attempt will have this value + * set to 0). + */ + public abstract Builder setOverallAttemptCount(int value); + /** * Set the start time of the first attempt. Note that this value is dependent on the actual * {@link ApiClock} used during the process. diff --git a/gax/src/test/java/com/google/api/gax/retrying/ExponentialRetryAlgorithmTest.java b/gax/src/test/java/com/google/api/gax/retrying/ExponentialRetryAlgorithmTest.java index 418aad7e9..81d536ef5 100644 --- a/gax/src/test/java/com/google/api/gax/retrying/ExponentialRetryAlgorithmTest.java +++ b/gax/src/test/java/com/google/api/gax/retrying/ExponentialRetryAlgorithmTest.java @@ -63,6 +63,7 @@ public void testCreateFirstAttempt() { // Checking only the most core values, to not make this test too implementation specific. assertEquals(0, attempt.getAttemptCount()); + assertEquals(0, attempt.getOverallAttemptCount()); assertEquals(Duration.ZERO, attempt.getRetryDelay()); assertEquals(Duration.ZERO, attempt.getRandomizedRetryDelay()); assertEquals(Duration.ofMillis(1L), attempt.getRpcTimeout()); @@ -76,6 +77,7 @@ public void testCreateNextAttempt() { // Checking only the most core values, to not make this test too implementation specific. assertEquals(1, secondAttempt.getAttemptCount()); + assertEquals(1, secondAttempt.getOverallAttemptCount()); assertEquals(Duration.ofMillis(1L), secondAttempt.getRetryDelay()); assertEquals(Duration.ofMillis(1L), secondAttempt.getRandomizedRetryDelay()); assertEquals(Duration.ofMillis(2L), secondAttempt.getRpcTimeout()); diff --git a/gax/src/test/java/com/google/api/gax/rpc/ServerStreamingAttemptCallableTest.java b/gax/src/test/java/com/google/api/gax/rpc/ServerStreamingAttemptCallableTest.java index 743b2928c..66582dd70 100644 --- a/gax/src/test/java/com/google/api/gax/rpc/ServerStreamingAttemptCallableTest.java +++ b/gax/src/test/java/com/google/api/gax/rpc/ServerStreamingAttemptCallableTest.java @@ -399,6 +399,7 @@ private static class FakeRetryingFuture extends AbstractApiFuture RetrySettings.newBuilder().setTotalTimeout(Duration.ofHours(1)).build()) .setFirstAttemptStartTimeNanos(0) .setAttemptCount(0) + .setOverallAttemptCount(0) .setRandomizedRetryDelay(Duration.ofMillis(1)) .setRetryDelay(Duration.ofMillis(1)) .setRpcTimeout(Duration.ofMinutes(1)) From 2abda47aafb9e6397f6fd736690a80226685b67d Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 27 Dec 2018 14:17:28 -0500 Subject: [PATCH 2/4] fix tests --- .../test/java/com/google/api/gax/rpc/AttemptCallableTest.java | 1 + .../java/com/google/api/gax/rpc/CheckingAttemptCallableTest.java | 1 + 2 files changed, 2 insertions(+) diff --git a/gax/src/test/java/com/google/api/gax/rpc/AttemptCallableTest.java b/gax/src/test/java/com/google/api/gax/rpc/AttemptCallableTest.java index a05a1e40a..cb5133620 100644 --- a/gax/src/test/java/com/google/api/gax/rpc/AttemptCallableTest.java +++ b/gax/src/test/java/com/google/api/gax/rpc/AttemptCallableTest.java @@ -64,6 +64,7 @@ public void setUp() { TimedAttemptSettings.newBuilder() .setGlobalSettings(RetrySettings.newBuilder().build()) .setAttemptCount(0) + .setOverallAttemptCount(0) .setFirstAttemptStartTimeNanos(0) .setRetryDelay(Duration.ofSeconds(1)) .setRandomizedRetryDelay(Duration.ofSeconds(1)) diff --git a/gax/src/test/java/com/google/api/gax/rpc/CheckingAttemptCallableTest.java b/gax/src/test/java/com/google/api/gax/rpc/CheckingAttemptCallableTest.java index 650f1590f..92554616b 100644 --- a/gax/src/test/java/com/google/api/gax/rpc/CheckingAttemptCallableTest.java +++ b/gax/src/test/java/com/google/api/gax/rpc/CheckingAttemptCallableTest.java @@ -64,6 +64,7 @@ public void setUp() { TimedAttemptSettings.newBuilder() .setGlobalSettings(RetrySettings.newBuilder().build()) .setAttemptCount(0) + .setOverallAttemptCount(0) .setFirstAttemptStartTimeNanos(0) .setRetryDelay(Duration.ofSeconds(1)) .setRandomizedRetryDelay(Duration.ofSeconds(1)) From de4783ef1ad3e2d4549c9b569ebfe4012797db21 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 27 Dec 2018 14:19:15 -0500 Subject: [PATCH 3/4] set new property to 0 to maintain backwards compat --- .../java/com/google/api/gax/retrying/TimedAttemptSettings.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java b/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java index 9cd61c15b..046f9aa2c 100644 --- a/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java +++ b/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java @@ -80,7 +80,8 @@ public abstract class TimedAttemptSettings { public abstract Builder toBuilder(); public static Builder newBuilder() { - return new AutoValue_TimedAttemptSettings.Builder(); + return new AutoValue_TimedAttemptSettings.Builder() + .setOverallAttemptCount(0); } @AutoValue.Builder From 34ace162c8a46f9be02867074c68a8f241e390e0 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Thu, 27 Dec 2018 14:20:32 -0500 Subject: [PATCH 4/4] format --- .../java/com/google/api/gax/retrying/TimedAttemptSettings.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java b/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java index 046f9aa2c..89462e19b 100644 --- a/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java +++ b/gax/src/main/java/com/google/api/gax/retrying/TimedAttemptSettings.java @@ -80,8 +80,7 @@ public abstract class TimedAttemptSettings { public abstract Builder toBuilder(); public static Builder newBuilder() { - return new AutoValue_TimedAttemptSettings.Builder() - .setOverallAttemptCount(0); + return new AutoValue_TimedAttemptSettings.Builder().setOverallAttemptCount(0); } @AutoValue.Builder