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. batch table properties atomic commit using CAS (NoLock)
   5. release locks if any
   6. rollback ALL if table has been modified concurrently or exception
   
   removed the guards in 2nd commit. 
   
   I think having guards in place makes sense to prevent ending up in an 
inconsistent state. Calling rollback after commit could potentially remove 
already committed data; however, that would require introducing an additional 
`CommitMode` flag (see the first commit).



##########
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. batch table properties atomic commit using CAS (NoLock)
   5. release locks if any
   6. rollback ALL if table has been modified concurrently or exception
   
   removed the guards in 2nd commit. 
   
   I think having guards in place makes sense to prevent ending up in an 
inconsistent state. Calling rollback after commit could potentially remove 
already committed data; however, that would require introducing an additional 
`CommitMode` flag (see the first commit).



-- 
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]

Reply via email to