Copilot commented on code in PR #16975:
URL: https://github.com/apache/iceberg/pull/16975#discussion_r3489391036
##########
core/src/main/java/org/apache/iceberg/BaseTransaction.java:
##########
@@ -320,10 +322,16 @@ private void commitReplaceTransaction(boolean orCreate) {
}
}
- // because this is a replace table, it will always completely
replace the table
- // metadata. even if it was just updated.
+ // If the table changed while this transaction was open
(usually a concurrent
+ // commit), rebuild the replacement on the latest metadata so
that commit's changes
+ // are not silently dropped. If the latest metadata is null
the table no longer
+ // exists, so there is nothing to rebuild on and the staged
replacement is committed
+ // as a new table.
if (base != underlyingOps.current()) {
this.base = underlyingOps.current(); // just refreshed
+ if (base != null) {
+ rebaseReplaceOnto(base);
+ }
}
Review Comment:
With the new rebase path, `commitReplaceTransaction` can replay
`PendingUpdate.commit()` on refreshed metadata and then still hit an outer
`CommitFailedException` on `underlyingOps.commit(base, current)` (e.g., another
concurrent commit lands between refresh and CAS). In that case, the outer retry
loop will rebase/replay updates again and may reuse manifests/manifest lists
across attempts, but the `finally` block currently deletes everything
accumulated via `SnapshotUpdate.deleteWith(...)` unconditionally. This can
delete manifest files that ended up being committed in a later attempt (the
simple-transaction path avoids this by filtering `deletedFiles` against
committed manifests).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
