Skip to content

Commit 7b95952

Browse files
shangxinliwgtmac
andauthored
fix: prevent double-commit in Transaction (apache#427)
Prevents Transaction::Commit() from being called multiple times by: - Adding committed_ flag to track transaction state - Checking flag at start of Commit() and returning error if already committed - Setting flag after successful commit (both empty and non-empty updates) - Updating table_ reference after successful catalog update This ensures transactions can only be committed once, preventing unintended side effects and maintaining transaction semantics. --------- Co-authored-by: Gang Wu <ustcwg@gmail.com>
1 parent dba8f92 commit 7b95952

2 files changed

Lines changed: 14 additions & 1 deletion

File tree

src/iceberg/transaction.cc

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,17 @@ Status Transaction::Apply(std::vector<std::unique_ptr<TableUpdate>> updates) {
7575
}
7676

7777
Result<std::shared_ptr<Table>> Transaction::Commit() {
78+
if (committed_) {
79+
return Invalid("Transaction already committed");
80+
}
7881
if (!last_update_committed_) {
7982
return InvalidArgument(
8083
"Cannot commit transaction when previous update is not committed");
8184
}
8285

8386
const auto& updates = metadata_builder_->changes();
8487
if (updates.empty()) {
88+
committed_ = true;
8589
return table_;
8690
}
8791

@@ -98,7 +102,14 @@ Result<std::shared_ptr<Table>> Transaction::Commit() {
98102
}
99103

100104
// XXX: we should handle commit failure and retry here.
101-
return table_->catalog()->UpdateTable(table_->name(), requirements, updates);
105+
ICEBERG_ASSIGN_OR_RAISE(auto updated_table, table_->catalog()->UpdateTable(
106+
table_->name(), requirements, updates));
107+
108+
// Mark as committed and update table reference
109+
committed_ = true;
110+
table_ = std::move(updated_table);
111+
112+
return table_;
102113
}
103114

104115
Result<std::shared_ptr<UpdateProperties>> Transaction::NewUpdateProperties() {

src/iceberg/transaction.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ class ICEBERG_EXPORT Transaction : public std::enable_shared_from_this<Transacti
8080
const bool auto_commit_;
8181
// To make the state simple, we require updates are added and committed in order.
8282
bool last_update_committed_ = true;
83+
// Tracks if transaction has been committed to prevent double-commit
84+
bool committed_ = false;
8385
// Keep track of all created pending updates. Use weak_ptr to avoid circular references.
8486
// This is useful to retry failed updates.
8587
std::vector<std::weak_ptr<PendingUpdate>> pending_updates_;

0 commit comments

Comments
 (0)