tsreaper commented on code in PR #5776:
URL: https://github.com/apache/paimon/pull/5776#discussion_r2156777394
##########
paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java:
##########
@@ -1110,54 +1088,57 @@ CommitResult tryCommitOnce(
e);
}
- if (commitSnapshotImpl(newSnapshot, deltaStatistics)) {
- LOG.info(
- "Successfully commit snapshot {} to table {} by user {} "
- + "with identifier {} and kind {}.",
+ boolean success;
+ try {
+ success = commitSnapshotImpl(newSnapshot, deltaStatistics);
+ } catch (Exception e) {
+ // commit exception, not sure about the situation and should not
clean up the files
+ LOG.warn("Retry commit for exception.", e);
+ return new RetryResult(latestSnapshot, baseDataFiles, e);
Review Comment:
When committing `APPEND` snapshot, `conflictCheck` will be a
`noConflictCheck`. Let's say `commitSnapshotImpl` throws an exception, but the
commit actually succeeds. If we retry from `tryCommit`, new files will not be
checked for conflicts, and the same new file might be committed twice.
##########
paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java:
##########
@@ -801,11 +801,15 @@ private int tryCommit(
if (System.currentTimeMillis() - startMillis > commitTimeout
|| retryCount >= commitMaxRetries) {
- retryResult.cleanAll();
- throw new RuntimeException(
+ String message =
String.format(
"Commit failed after %s millis with %s
retries, there maybe exist commit conflicts between multiple jobs.",
- commitTimeout, retryCount));
+ commitTimeout, retryCount);
+ if (retryResult.exception != null) {
+ throw new RuntimeException(message, retryResult.exception);
+ } else {
+ throw new RuntimeException(message);
Review Comment:
```suggestion
throw new RuntimeException(message, retryResult.exception);
```
--
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-unsubscr...@paimon.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
