You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@paimon.apache.org by "dev-donghwan (via GitHub)" <gi...@apache.org> on 2026/09/30 07:51:52 UTC

[PR] [core] Treat a snapshot rename that throws but succeeded as committed [paimon]

dev-donghwan opened a new pull request, #10324:
URL: https://github.com/apache/paimon/pull/10324

### Purpose

When a snapshot rename actually succeeds but the file system reports an error, `RenamingSnapshotCommit` fails the commit without writing the `LATEST` hint. The retry in `FileStoreCommitImpl` then finds the snapshot already committed and returns success, still without writing the hint. If this repeats, `LATEST` stays behind across many successful commits.

`RenamingSnapshotCommit` already handles the same situation when the rename returns `false`. It checks whether the target snapshot file exists with the expected content, treats the commit as done if so, and writes the hint:

```java
boolean committed = fileIO.tryToWriteAtomic(newSnapshotPath, snapshot.toJson());
if (!committed) {
if (!fileIO.exists(newSnapshotPath)) {
throw new IOException("Commit snapshot ... failed and ... not found");
}
committed = snapshot.equals(Snapshot.fromJson(fileIO.readFileUtf8(newSnapshotPath)));
}
if (committed) {
snapshotManager.commitLatestHint(snapshot.id());
}
```

A rename that throws gets none of this. The exception goes straight to `FileStoreCommitImpl`, which logs `Retry commit for exception` and retries. Since #5771, the retry finds its own snapshot and returns success. That avoids a duplicate commit, but nothing on that path writes the hint.

We hit this with S3A on an S3-compatible object store. The copy behind the rename succeeded on the server, but the client could not parse the response. Every commit threw and was then reported as successful on retry, and `LATEST` stayed at the same id for 21 consecutive commits until the hinted snapshot expired. #10323 covers how a stale hint then leaves the table unable to commit, and fixes `findLatest` so the table recovers. This PR fixes one common way the hint falls behind in the first place.

#### Change

If `tryToWriteAtomic` throws but the target snapshot file exists, fall through to the existing "rename returned false" handling: compare the file with the snapshot being committed, and treat the commit as done and write the hint if they match. If the file does not exist, rethrow the original exception as before.

The behaviour stays the same in the other cases:

- The file exists with different content, for example another writer committed the same id: the commit returns `false` and is retried, as before.
- The file cannot be read: the read error is thrown and the commit is retried, as before.

This does not help when writing the hint itself keeps failing. #10323 covers that case.

### Tests

- [x] `RenamingSnapshotCommitTest#testCommitSucceedsAndRenameThrows`: the rename moves the file and then throws. The commit now succeeds and writes `LATEST`. Before this change, the commit threw.
- [x] `RenamingSnapshotCommitTest#testCommitRenameThrowsAndTargetMissing`: the rename throws without moving the file. The original exception is rethrown.
- [x] A table-level check, not included in this PR, with a `FileIO` whose snapshot rename succeeds and then throws. Before this change, `LATEST` stayed at 1 and the fifth commit failed permanently once snapshot 1 expired. After it, `LATEST` follows the latest snapshot.
- [x] All tests under `org.apache.paimon.catalog`, plus the commit tests in `org.apache.paimon.operation`: 203 tests pass.
- [x] Spotless and Checkstyle for `paimon-core`

--
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@paimon.apache.org

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

Re: [PR] [core] Treat a snapshot rename that throws but succeeded as committed [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.

JingsongLi merged PR #10324:
URL: https://github.com/apache/paimon/pull/10324


-- 
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@paimon.apache.org

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

Re: [PR] [core] Treat a snapshot rename that throws but succeeded as committed [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.

JingsongLi commented on PR #10324:
URL: https://github.com/apache/paimon/pull/10324#issuecomment-5943980682

   +1


-- 
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@paimon.apache.org

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

Re: [PR] [core] Treat a snapshot rename that throws but succeeded as committed [paimon]

Posted by "dev-donghwan (via GitHub)" <gi...@apache.org>.

dev-donghwan commented on PR #10324:
URL: https://github.com/apache/paimon/pull/10324#issuecomment-5936566187

   Thanks for the careful review, @Akash3121!
   
   If you have time, I'd also appreciate your thoughts on #10323. It comes from the same incident and covers the read side: once the `LATEST` hint falls behind and the hinted snapshot expires, `findLatest` keeps returning the expired id and the table can't recover on its own. This PR fixes one way the hint falls behind, and #10323 makes the table recover whatever the cause.
   


-- 
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@paimon.apache.org

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