You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "turcsanyip (via GitHub)" <gi...@apache.org> on 2023/03/20 10:38:14 UTC

[GitHub] [nifi] turcsanyip commented on a diff in pull request #6976: NIFI-11204: Add configurable retry logic for table commits in PutIceberg processor

turcsanyip commented on code in PR #6976:
URL: https://github.com/apache/nifi/pull/6976#discussion_r1141864988


##########
nifi-nar-bundles/nifi-iceberg-bundle/nifi-iceberg-processors/src/main/java/org/apache/nifi/processors/iceberg/PutIceberg.java:
##########
@@ -113,6 +116,42 @@ public class PutIceberg extends AbstractIcebergProcessor {
             .addValidator(StandardValidators.LONG_VALIDATOR)
             .build();
 
+    static final PropertyDescriptor NUMBER_OF_COMMIT_RETRIES = new PropertyDescriptor.Builder()
+            .name("number-of-commit-retries")
+            .displayName("Number of Commit Retries")
+            .description("Number of times to retry a commit before failing.")
+            .required(true)
+            .defaultValue("3")
+            .addValidator(StandardValidators.INTEGER_VALIDATOR)
+            .build();
+
+    static final PropertyDescriptor MINIMUM_COMMIT_WAIT_TIME = new PropertyDescriptor.Builder()
+            .name("minimum-commit-wait-time")
+            .displayName("Minimum Commit Wait Time")
+            .description("Minimum time to wait before retrying a commit.")
+            .required(true)
+            .defaultValue("100 ms")
+            .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+            .build();
+
+    static final PropertyDescriptor MAXIMUM_COMMIT_WAIT_TIME = new PropertyDescriptor.Builder()
+            .name("maximum-commit-wait-time")
+            .displayName("Maximum Commit Wait Time")
+            .description("Maximum time to wait before retrying a commit.")
+            .required(true)
+            .defaultValue("1 min")
+            .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+            .build();
+
+    static final PropertyDescriptor MAXIMUM_COMMIT_DURATION = new PropertyDescriptor.Builder()
+            .name("maximum-commit-duration")
+            .displayName("Maximum Commit Duration")
+            .description("Total retry timeout period for a commit.")
+            .required(true)
+            .defaultValue("30 min")
+            .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
+            .build();

Review Comment:
   I would suggest more consistent and reasonable default values:
   - Number of Commit Retries: 10
   - Maximum Commit Wait Time: 2 sec
   - Maximum Commit Duration: 30 sec (with the increased retry count the overall duration would be max 20 sec + jitter; so 30 min is misleading; furthermore, the thread is blocked while the retry is in progress so it should not take too long)



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org