feat(trigger): run Event-Plane trigger bodies as one system transaction - #252
Open
EnRaiha wants to merge 1 commit into
Open
feat(trigger): run Event-Plane trigger bodies as one system transaction#252EnRaiha wants to merge 1 commit into
EnRaiha wants to merge 1 commit into
Conversation
Closes epic NodeDB-Lab#165 (deferred actions). A trigger body that failed mid-block left earlier statements applied, and its retry repeated them. The async Event-Plane fire path now stages every fired body into a SystemTxnScope: all statements commit together or roll back together. Hardened design per review (Option B', gaps 1-3): - push_task_into_scope: statement-serial staging extracted from run_tasks_atomically; run_tasks_atomically refactored onto it (system_txn/run.rs) - Gap 1: SystemTxnScope::retain_lease_scope keeps each statement's Arc<QueryLeaseScope> alive until COMMIT's version fence - Gap 2: system_scope threaded through FireTriggersParams + all FireAfter*Params; executor.with_system_scope; cascade fires inherit the caller's scope (all-or-nothing across nested triggers) - Gap 3: is_system_ddl gate rejects ConvertCollection before staging (SystemTxnError::Ddl), fail-fast - commit_scope / rollback_scope helpers; dispatcher/single.rs begins one scope per fired event (async only; sync/deferred untouched) and commits or rolls back by FireReport::has_failure - Sync/deferred/instead/before paths explicitly pass system_scope: None Verified solve-first: cargo check EXIT 0 (no warnings); system_txn 1/1, control::trigger 68/68, event::trigger 12/12.
21 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes epic #165 (deferred actions — last unchecked box). A trigger body that fails mid-block leaves earlier statements applied, and its async retry repeats them. The Event-Plane fire path now stages every fired body into a
SystemTxnScope: all statements commit together or roll back together.Option B′ (scope-bound executor), per the Phase-1 recon: bodies are procedural control flow (
If/While/For), so pre-planning the whole body (Option A) is impossible; instead each statement's tasks route into the scope as they are planned.Gap hardening (review items 1–3)
SystemTxnScope::retain_lease_scopeholds each statement'sArc<QueryLeaseScope>until COMMIT's version fence.system_scopethreaded throughFireTriggersParams+ allFireAfter*Params;StatementExecutor::with_system_scope; nested (cascade) triggers inherit the caller's scope — one failure rolls back every body the cascade touched.is_system_ddlrejects the one DDL-shaped Data-Plane op (MetaOp::ConvertCollection) withSystemTxnError::Ddlbefore staging.Mechanics
push_task_into_scope(statement-serial staging, extracted fromrun_tasks_atomically; the batch function is refactored onto it — behavior identical)commit_scope/rollback_scopeexported helpersdispatcher/single.rs: one scope per fired event (async only —EventSource::User); row + statement fires share it; commit onFireReport::has_failure() == false, rollback otherwisesystem_scope: None— never double-wrapped (they already run inside the client transaction)FireReport::has_failure()Verification (solve-first)
cargo check -p nodedb --lib --tests— EXIT 0, zero warningscontrol::system_txn— 1/1 (DDL gate)control::trigger— 68/68 (no regression from the threading)event::trigger— 12/12 (dispatcher wiring)Known follow-ups (tracked, not blockers): retry-path atomicity wiring in
retry_action.rs(currentlyNone— retries run the failed body non-atomically until it lands), cluster-tests stage-2 coverage (kill mid-trigger → retry/DLQ), deferredbatch.rspath.