[core] Introduce timeout for commit retry avoid long time loop #4668

xuzifu666 · 2024-12-09T10:54:24Z

Purpose

When execute drop partition or expire partition with large numbers partitions(eg: table with large number of files and stream job with 1 min ckp), would block batch job for overwrite in a long time loop, so add a timeout option for commit retry, user can quick get job result in a specific timeout to handle their batch job(which can adjust ckp time interval or stop stream job to finish their batch job).
FileLength change to 4000 for CoreOption lines was near 3000 which would block code compile.

Linked issue: close #xxx

Tests

API and Format

Documentation

JingsongLi · 2024-12-10T02:08:49Z

paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java

@@ -134,6 +135,7 @@ public class FileStoreCommitImpl implements FileStoreCommit {
    private final List<CommitCallback> commitCallbacks;
    private final StatsFileHandler statsFileHandler;
    private final BucketMode bucketMode;
+    private final Optional<Duration> commitMaxTimeout;


do not use optional for class member, you can use nullable field here.

JingsongLi · 2024-12-10T02:09:56Z

paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java

@@ -1066,6 +1082,16 @@ public void compactManifest() {
                                commitMaxRetries));
            }
            retryCount++;
+
+            if (commitMaxTimeout.isPresent()


Please merge into if (retryCount >= commitMaxRetries).

JingsongLi · 2024-12-10T02:10:03Z

paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java

@@ -754,6 +759,16 @@ private int tryCommit(
                                commitMaxRetries));
            }
            retryCount++;
+
+            if (commitMaxTimeout.isPresent()


Please merge into if (retryCount >= commitMaxRetries).

JingsongLi · 2024-12-10T03:09:52Z

docs/layouts/shortcodes/generated/core_configuration.html

@@ -134,6 +134,12 @@
            <td>Integer</td>
            <td>Maximum number of retries when commit failed.</td>
        </tr>
+        <tr>
+            <td><h5>commit.max-timeout</h5></td>


max timeout seems weird... it is just timeout?
commit.timeout

Make sense，changed to commit.timeout，Thanks~ @JingsongLi

JingsongLi · 2024-12-10T03:58:18Z

paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java

@@ -166,7 +168,8 @@ public FileStoreCommitImpl(
            BucketMode bucketMode,
            @Nullable Integer manifestReadParallelism,
            List<CommitCallback> commitCallbacks,
-            int commitMaxRetries) {
+            int commitMaxRetries,
+            @Nullable Duration commitMaxTimeout) {


Can you modify all MaxTimeout to Timeout?

JingsongLi · 2024-12-10T03:59:16Z

paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java

@@ -134,6 +135,7 @@ public class FileStoreCommitImpl implements FileStoreCommit {
    private final List<CommitCallback> commitCallbacks;
    private final StatsFileHandler statsFileHandler;
    private final BucketMode bucketMode;
+    @Nullable private Duration commitMaxTimeout;


store millis here.

JingsongLi · 2024-12-10T05:00:15Z

paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java

@@ -134,6 +134,7 @@ public class FileStoreCommitImpl implements FileStoreCommit {
    private final List<CommitCallback> commitCallbacks;
    private final StatsFileHandler statsFileHandler;
    private final BucketMode bucketMode;
+    @Nullable private long commitTimeout;


remove @nullable

JingsongLi · 2024-12-10T05:00:18Z

paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java

@@ -166,7 +167,8 @@ public FileStoreCommitImpl(
            BucketMode bucketMode,
            @Nullable Integer manifestReadParallelism,
            List<CommitCallback> commitCallbacks,
-            int commitMaxRetries) {
+            int commitMaxRetries,
+            @Nullable long commitTimeout) {


remove @nullable

Done. Thanks for reminder @JingsongLi

JingsongLi

+1

xuyu added 3 commits December 9, 2024 17:33

[core] Introduce max-timeout for commit retry avoid long time loop

e60a720

fix style

a047998

fix

416a2d0

JingsongLi reviewed Dec 10, 2024

View reviewed changes

xuyu added 3 commits December 10, 2024 11:00

fix

38b13c7

fix

ae475ef

fix

ba648c7

JingsongLi reviewed Dec 10, 2024

View reviewed changes

fix doc

4303d95

JingsongLi reviewed Dec 10, 2024

View reviewed changes

xuyu added 3 commits December 10, 2024 12:09

fix name

d51d7a7

fix mills

3a0b504

1

5aaa389

xuzifu666 changed the title ~~[core] Introduce max-timeout for commit retry avoid long time loop~~ [core] Introduce timeout for commit retry avoid long time loop Dec 10, 2024

fix doc

d16020b

JingsongLi reviewed Dec 10, 2024

View reviewed changes

remove nullabl

ce9cb25

JingsongLi approved these changes Dec 10, 2024

View reviewed changes

JingsongLi merged commit 8b4a77a into apache:master Dec 10, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Introduce timeout for commit retry avoid long time loop #4668

[core] Introduce timeout for commit retry avoid long time loop #4668

xuzifu666 commented Dec 9, 2024 •

edited

Loading

JingsongLi Dec 10, 2024 •

edited

Loading

JingsongLi Dec 10, 2024

JingsongLi Dec 10, 2024

JingsongLi Dec 10, 2024

xuzifu666 Dec 10, 2024

JingsongLi Dec 10, 2024

JingsongLi Dec 10, 2024

JingsongLi Dec 10, 2024

JingsongLi Dec 10, 2024 •

edited

Loading

xuzifu666 Dec 10, 2024

JingsongLi left a comment

[core] Introduce timeout for commit retry avoid long time loop #4668

[core] Introduce timeout for commit retry avoid long time loop #4668

Conversation

xuzifu666 commented Dec 9, 2024 • edited Loading

Purpose

Tests

API and Format

Documentation

JingsongLi Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JingsongLi Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JingsongLi left a comment

Choose a reason for hiding this comment

xuzifu666 commented Dec 9, 2024 •

edited

Loading

JingsongLi Dec 10, 2024 •

edited

Loading

JingsongLi Dec 10, 2024 •

edited

Loading