aromanenko-dev commented on a change in pull request #11396:
URL: https://github.com/apache/beam/pull/11396#discussion_r435399974
##########
File path:
sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/JdbcIO.java
##########
@@ -1217,6 +1297,16 @@ void set(
return toBuilder().setRetryStrategy(retryStrategy).build();
}
+ /**
+ * When a SQL exception occurs, {@link Write} uses this {@link
RetryConfiguration} to
+ * exponentially back off and retry the statements based on the {@link
RetryConfiguration}
+ * mentioned.
Review comment:
It would helpful to provide a simple example of using - here or in
`JdbcIO` class Javadoc.
##########
File path:
sdks/java/io/jdbc/src/main/java/org/apache/beam/sdk/io/jdbc/JdbcIO.java
##########
@@ -903,6 +907,72 @@ public void teardown() throws Exception {
}
}
+ /**
+ * Builder used to help with retry configuration for {@link JdbcIO}. The
retry configuration
+ * accepts maxAttempts and maxDuration for {@link FluentBackoff}.
+ */
+ @AutoValue
+ public abstract static class RetryConfiguration implements Serializable {
+
+ abstract int getMaxAttempts();
+
+ @Nullable
+ abstract Duration getMaxDuration();
+
+ @Nullable
+ abstract Duration getInitialDuration();
+
+ abstract RetryConfiguration.Builder builder();
+
+ @AutoValue.Builder
+ abstract static class Builder {
+ abstract Builder setMaxAttempts(int maxAttempts);
+
+ abstract Builder setMaxDuration(Duration maxDuration);
+
+ abstract Builder setInitialDuration(Duration initialDuration);
+
+ abstract RetryConfiguration build();
+ }
+
+ public static RetryConfiguration create(int maxAttempts) {
+ checkArgument(maxAttempts > 0, "maxAttempts must be greater than 0");
+ return create(maxAttempts, DEFAULT_MAX_CUMULATIVE_BACKOFF,
DEFAULT_INITIAL_BACKOFF);
+ }
+
+ public static RetryConfiguration create(int maxAttempts, Duration
maxDuration) {
+ checkArgument(maxAttempts > 0, "maxAttempts must be greater than 0");
+ checkArgument(
+ maxDuration != null && maxDuration.isLongerThan(Duration.ZERO),
+ "maxDuration must be greater than 0");
+ return create(maxAttempts, maxDuration, DEFAULT_INITIAL_BACKOFF);
+ }
+
+ public static RetryConfiguration create(Duration initialDuration, int
maxAttempts) {
Review comment:
It's a bit confusing for user to have the overloaded methods where the
same argument (eg, `maxAttempts`) changes its positions. I'd prefer to leave
only one method `create()` with all three arguments to provide - `create(int
maxAttempts, Duration maxDuration, Duration initialDuration)`.
Also, make `DEFAULT_INITIAL_BACKOFF ` and `DEFAULT_MAX_CUMULATIVE_BACKOFF`
as the constants of `RetryConfiguration` and make them public.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
