arthurgaubil commented on code in PR #9749:
URL: https://github.com/apache/paimon/pull/9749#discussion_r4027408633
##########
paimon-iceberg/src/main/java/org/apache/paimon/iceberg/IcebergRestMetadataCommitter.java:
##########
@@ -254,6 +256,37 @@ private void commitMetadataImpl(
((BaseTable) icebergTable)
.operations()
.commit(((BaseTable) icebergTable).operations().current(),
updatedForCommit);
+ } catch (CommitStateUnknownException e) {
+ // The catalog returned an ambiguous response, so we cannot tell
whether this commit
+ // was applied server-side. Either way the next attempt reloads
the table and runs
+ // checkBase() against that live state: if it landed, the base
matches and the next
+ // commit proceeds normally; if it did not, checkBase() sees the
drift and the table
+ // is rebuilt from the current file set. Failing here does not
resolve the ambiguity,
+ // it only takes down every other table the job is syncing.
+ LOG.warn(
+ "Commit to rest catalog returned an ambiguous response for
table {}, snapshot"
+ + " {}; not failing the commit, the next attempt
will reconcile.",
+ icebergTableIdentifier,
+ updatedForCommit.currentSnapshot() == null
+ ? null
+ : updatedForCommit.currentSnapshot().snapshotId(),
+ e);
+ } catch (CommitFailedException e) {
Review Comment:
You’re right. I changed the committer to retry from fresh catalog state and
only return after publication is confirmed. Exhausted retries now propagate, so
cleanup is skipped and the durable retry obligation is preserved. I also added
failure-injection coverage for rejected publication, ambiguous
applied/not-applied outcomes, retention safety, and the final commit.
Addressed in 969f70fcd
Also let me know if you would want `MAX_COMMIT_ATTEMPTS` to be exposed and
customizable though `IcebergOptions`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
