deniskuzZ commented on code in PR #15389:
URL: https://github.com/apache/iceberg/pull/15389#discussion_r2840290133
##########
core/src/main/java/org/apache/iceberg/BaseTransaction.java:
##########
@@ -78,6 +78,7 @@ enum TransactionType {
private TableMetadata base;
private TableMetadata current;
private boolean hasLastOpCommitted;
+ private boolean isAborted = false;
Review Comment:
Multi-table commit:
table loop {
1. stage every change: `BaseTransaction#commit` ->
`StagingTableOperations#doCommit`, writes metadata file only, skipping the
locking
2. lock if enabled (HMSLock)
3. verify whether Base metadata location is same as the current table
metadata location
}
4. table properties batch commit using CAS (NoLock)
5. release locks if any
6. **rollback ALL** if table has been modified concurrently or exception -
Currently, we rely on reflection to invoke
`BaseTransaction#cleanUpOnCommitFailure`, and addressing that limitation is the
purpose of this PR.
--------
As discussed, removed the guards in [2nd
commit](https://github.com/apache/iceberg/pull/15389/changes/6719771c5894e482a552679510b095de57e2df12).
I think having guards in place makes sense to avoid ending up in an
inconsistent state. Calling rollback after a commit is destructive, as it would
remove already committed data.
On the other hand, adding such guards would require introducing `CommitMode`
flag (see the [1st
commit](https://github.com/apache/iceberg/pull/15389/changes/e4ba7b3b4afa2647fa0f2a6a6d2d1124c799674f#diff-fb713bd8803fd14e254ff3cb7ceb44183e2ceef244d9b011f8e08df23aa662bfR274-R275)).
--
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]
