Skip to content

refactor: introduce TransactionContext to decouple Transaction and PendingUpdate#591

Merged
wgtmac merged 1 commit intoapache:mainfrom
wgtmac:refactor_txn
Mar 16, 2026
Merged

refactor: introduce TransactionContext to decouple Transaction and PendingUpdate#591
wgtmac merged 1 commit intoapache:mainfrom
wgtmac:refactor_txn

Conversation

@wgtmac
Copy link
Member

@wgtmac wgtmac commented Mar 13, 2026

Add TransactionContext to own the shared state (table, metadata_builder, kind) between Transaction and PendingUpdate. Both now hold a shared_ptr instead of PendingUpdate holding a weak_ptr.

This fixes two issues:

  • pending_updates_ was weak_ptr, so dropping a PendingUpdate would silently break Finalize/retry; now Transaction holds shared_ptr
  • Table::New*() no longer creates a temporary Transaction; it creates a TransactionContext directly and passes it to the PendingUpdate, removing the circular dependency

Also clean up related redundancy:

  • Hoist Transaction::Kind to a standalone enum class TransactionKind
  • Remove Transaction::kind_ (duplicate of ctx_->kind)
  • Remove auto_commit machinery; PendingUpdate::Commit() now calls txn->Commit() explicitly on the table-created path
  • TransactionContext::Make returns Result to propagate null table errors

…endingUpdate

  Add TransactionContext to own the shared state (table, metadata_builder,
  kind) between Transaction and PendingUpdate. Both now hold a
  shared_ptr<TransactionContext> instead of PendingUpdate holding a
  weak_ptr<Transaction>.

  This fixes two issues:
  - pending_updates_ was weak_ptr, so dropping a PendingUpdate would
    silently break Finalize/retry; now Transaction holds shared_ptr
  - Table::New*() no longer creates a temporary Transaction; it creates
    a TransactionContext directly and passes it to the PendingUpdate,
    removing the circular dependency

  Also clean up related redundancy:
  - Hoist Transaction::Kind to a standalone enum class TransactionKind
  - Remove Transaction::kind_ (duplicate of ctx_->kind)
  - Remove auto_commit machinery; PendingUpdate::Commit() now calls
    txn->Commit() explicitly on the table-created path
  - TransactionContext::Make returns Result to propagate null table errors
Copy link
Collaborator

@zhjwpku zhjwpku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@WZhuo WZhuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wgtmac
Copy link
Member Author

wgtmac commented Mar 16, 2026

Thanks all for the review!

@wgtmac wgtmac merged commit 8fdf346 into apache:main Mar 16, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants