From a7c1e065520c3ebdb803dc1851d0a55ef8d54ef2 Mon Sep 17 00:00:00 2001 From: jiangzhe Date: Fri, 7 Aug 2026 22:18:55 +0800 Subject: [PATCH 1/2] implement private transaction and snapshot --- docs/garbage-collect.md | 21 +- docs/lock-system.md | 19 +- docs/tasks/000262-private-transactions.md | 674 ++++++++++++++++++ docs/transaction-system.md | 125 ++-- docs/unsafe-usage-baseline.md | 4 +- doradb-storage/src/catalog/checkpoint.rs | 39 +- doradb-storage/src/catalog/index.rs | 199 ++---- doradb-storage/src/catalog/storage/columns.rs | 10 +- doradb-storage/src/catalog/storage/ddl.rs | 305 ++++++++ doradb-storage/src/catalog/storage/indexes.rs | 16 +- doradb-storage/src/catalog/storage/mod.rs | 12 +- doradb-storage/src/catalog/storage/tables.rs | 4 +- doradb-storage/src/catalog/table.rs | 328 +++------ doradb-storage/src/recovery/mod.rs | 25 +- doradb-storage/src/session.rs | 309 ++++---- doradb-storage/src/table/access.rs | 8 +- doradb-storage/src/table/gc.rs | 263 ++++--- doradb-storage/src/table/mod.rs | 44 +- doradb-storage/src/table/persistence.rs | 70 +- doradb-storage/src/table/storage.rs | 12 +- doradb-storage/src/trx/mod.rs | 659 ++++++++++------- doradb-storage/src/trx/purge.rs | 16 +- doradb-storage/src/trx/readonly.rs | 79 ++ doradb-storage/src/trx/retention.rs | 69 +- doradb-storage/src/trx/stmt.rs | 119 ++-- doradb-storage/src/trx/sys.rs | 84 +-- 26 files changed, 2241 insertions(+), 1272 deletions(-) create mode 100644 docs/tasks/000262-private-transactions.md create mode 100644 doradb-storage/src/catalog/storage/ddl.rs create mode 100644 doradb-storage/src/trx/readonly.rs diff --git a/docs/garbage-collect.md b/docs/garbage-collect.md index d481ebf5..88dd9b6a 100644 --- a/docs/garbage-collect.md +++ b/docs/garbage-collect.md @@ -119,8 +119,10 @@ checkpointed cold entries into `MemIndex`. The cleanup pass captures: -- a `TrxReadProof<'ctx>` from the cleanup transaction context -- one proof-gated `TableRootSnapshot<'ctx>` containing: +- a mandatory-only `PrivateSnapshot` whose STS is registered in the active GC + watermark +- one `TableRootSnapshot<'snapshot>` directly lifetime-bound to that private + snapshot and containing: - table checkpoint timestamp - `pivot_row_id` - `ColumnBlockIndex` root @@ -158,11 +160,16 @@ the root fence failed, delete-overlay cleanup still runs and and active horizon alongside the completed `MemIndexCleanupStats`. A caller-disabled live pass has no delay. -The complete table pass uses one maintenance transaction and one -`TableRootSnapshot` for all secondary indexes. If publication races with root -capture and the root is not visible to the cleanup transaction, cleanup rolls -back and retries immediately; this transient capture race does not wait for or -report a horizon event. +The complete table pass uses one private snapshot and one `TableRootSnapshot` +for all secondary indexes. The private snapshot has no transaction id, status, +session child state, scan API, locks, undo, commit, or rollback capability. Its +only job is to keep its STS registered until every root-bound read has +finished. + +If publication races with root capture and the root is not visible to the +private snapshot STS, cleanup drops the captured root, deregisters the STS, +yields once, and retries with a fresh registration. This transient capture +race does not wait for or report a horizon event. Delete overlays require overlay-obsolescence proof, not `DiskTree` absence. A unique delete-shadow or non-unique delete-marked exact entry below the captured diff --git a/docs/lock-system.md b/docs/lock-system.md index 1adf8a24..a6c7c902 100644 --- a/docs/lock-system.md +++ b/docs/lock-system.md @@ -399,9 +399,12 @@ DDL call first reserves a typed DDL operation while idle and retains its `&mut Session` borrow while the same entry hosts a private catalog transaction. The private transaction inherits the operation key and allocates only a `TrxID`; it temporarily takes the outer carrier's family box while the -operation `curr_scope` remains owned and immutable. Terminal completion parks -the returned box in the stable entry, and the still-active outer operation -reclaims that exact allocation before acquiring again or closing. +operation `curr_scope` remains owned and immutable. It owns one checked-out +core and strong runtime attachment for its complete lifetime, so catalog +statement boundaries do not move the family authority or core through the +entry. Terminal completion returns the family box through the stable entry, +and the still-active outer operation reclaims that exact allocation before +acquiring again or closing. ### Wait and cancellation behavior @@ -630,11 +633,11 @@ Open + Idle: effectful Session admission resumes ``` An already-admitted typed DDL operation may create a private catalog -transaction in its stable entry while the outer `&mut Session` call remains borrowed. The normal -path is sequential, but cancellation of the whole DDL future can queue -transaction cleanup while DDL scope guards unwind. The DDL carrier and private -transaction transfer the same boxed family authority, serializing cleanup with -outer-scope unwind. +transaction in its stable entry while the outer `&mut Session` call remains +borrowed. Mandatory execution owns that transaction through normal terminal +completion; there is no caller-abandonment cleanup boundary between its +statements. On a supervised panic, the private checkout is synchronously +parked before the operation and its family authority are retained as failed. The public `Session` handle remains movable between threads but is not shareable: its local closed flag uses `Cell`, making the type `Send` and diff --git a/docs/tasks/000262-private-transactions.md b/docs/tasks/000262-private-transactions.md new file mode 100644 index 00000000..0352f22b --- /dev/null +++ b/docs/tasks/000262-private-transactions.md @@ -0,0 +1,674 @@ +--- +id: 000262 +title: Introduce Private Transactions and Maintenance Snapshots +status: proposal # proposal | implemented | superseded +created: 2026-08-07 +github_issue: 958 +--- + +# Task: Introduce Private Transactions and Maintenance Snapshots + +## Summary + +Introduce a crate-private `PrivateTransaction` for mandatory catalog DDL +instead of representing those transactions with the public `Transaction` +facade. + +`PrivateTransaction` owns one `SessionOperationCheckout` for its complete +lifetime. Catalog DDL can therefore execute several statement-effect +boundaries without repeatedly upgrading weak session reachability, checking +engine and entry state, resolving the operation key, and moving `TrxInner` +through the stable entry between statements. Secondary `MemIndex` maintenance +uses a separate lightweight `PrivateSnapshot` that registers only an STS in +the active GC horizon and directly brands captured roots with its lifetime. + +Move logical catalog DDL staging behind `CatalogStorage` methods. Each method +uses one private statement per catalog table that it actually mutates, derives +persisted row objects from validated metadata, and installs exactly one DDL +redo record directly in transaction effects after all catalog statements +succeed. Remove catalog-specific execution, terminal, and DDL-redo APIs from +the public transaction type while preserving the existing commit, rollback, +lock, recovery, and persisted-redo behavior. + +## Context + +Issue Labels: + +- type:task +- priority:medium +- codex + +The public `Transaction` is intentionally a weak foreground facade. Every +`Transaction::exec` checks lifecycle admission, upgrades the exact weak +session, verifies engine health, resolves the stable operation entry, validates +the independent transaction id, and checks `TrxInner` out for one statement. +These checks are necessary for caller-controlled public transactions because +their handle, future, session, or engine may be dropped independently. + +Private transactions have a different contract. They start only after a DDL +operation has transferred to engine-owned mandatory execution. +The accepted operation and its stable `SessionOperationEntry` outlive the +nested transaction, execution is supervised, and every normal path must +consume the transaction through its domain-specific commit or rollback. There +is no supported caller-controlled abandonment boundary between its internal +steps. + +The current implementation nevertheless returns the public `Transaction` from +`MandatoryOperationGuard::begin_private_trx`. Catalog code then calls +`Transaction::stage_catalog_statement`, checks the core back into the entry, +and repeats the complete public checkout path for later work. Secondary +`MemIndex` cleanup similarly starts a public-shaped transaction, checks it out +only to borrow `TrxReadProof`, returns it, and finally invokes a private +rollback method on the same public facade. + +Catalog mutation ownership is also split at the wrong boundary: + +- `catalog/table.rs` owns + `execute_create_table_catalog_staging` and + `execute_drop_table_catalog_cascade`; +- `catalog/index.rs` owns + `execute_create_index_catalog_update` and + `execute_drop_index_catalog_update`; +- those free functions receive both `CatalogStorage` and public + `Transaction`; +- each function groups mutations of several logical catalog tables into one + `Statement`; and +- each function installs DDL redo through `StmtEffects::set_ddl_redo`. + +Task 000261 removed statement-scope logical locks. A `Statement` is now an +effect and rollback boundary only; all catalog-table logical locks acquired by +its operations belong directly to the transaction. Splitting catalog work by +logical table therefore creates no additional lock identity, lock handoff, or +early-release behavior. Repeated access reuses transaction-owned exact claims +until terminal cleanup. + +This work passes the task complexity gate. It is one internal ownership and API +refactor with focused catalog and maintenance consumers. It does not change a +public API contract, persisted catalog schema, redo encoding, recovery +protocol, lock compatibility rule, or DDL publication sequence, and it does +not require a phased rollout. + +Related design history: + +- `docs/transaction-system.md` describes weak public transactions, stable + operation entries, nested private transaction states, and mandatory panic + retention. +- `docs/lock-system.md` defines the + `SessionExplicit -> Operation -> PrivateTransaction` lock-owner topology. +- `docs/tasks/000247-statement-public-transaction-cancellation-ownership.md` + introduced the current distinct public and private statement drop policies. +- `docs/tasks/000249-runtime-owned-table-ddl.md` and + `docs/tasks/000250-runtime-owned-index-ddl.md` moved catalog DDL into + supervised mandatory execution. +- `docs/tasks/000251-runtime-owned-mandatory-maintenance.md` made the active + cleanup transaction part of supervised maintenance resources. +- `docs/tasks/000261-remove-statement-scope-logical-locks.md` removed the final + statement-owned logical claims and lock-scope state. + +The selected design uses a semantic `PrivateTransaction` facade over the +existing mechanical `SessionOperationCheckout`. It intentionally does not +introduce a second carrier such as `TransactionLease`. A thin wrapper around +public `Transaction` was rejected because it would preserve weak reachability, +abandonment policy, and repeated checkout validation. Maintenance instead uses +`PrivateSnapshot`, because MemIndex cleanup needs only a registered STS and +root lifetime: giving it transaction identity, core state, locks, undo, +terminal claims, or rollback would misrepresent its capabilities. + +## Goals + +1. Add one crate-private `PrivateTransaction` type for mandatory nested DDL + transactions. +2. Hold the same checked-out `TrxInner`, stable entry, and strong + `TrxAttachment` for the complete private transaction lifetime. +3. Keep public transaction cancellation, abandonment, weak reachability, and + statement-error semantics confined to public `Transaction`. +4. Reuse existing `TrxInner`, `StmtEffects`, `Statement`, + `SessionOperationCheckout`, `SessionOperationCompletionClaim`, transaction + lock state, commit, and rollback machinery. +5. Make `CatalogStorage` the owner of logical create/drop table and + create/drop index catalog mutations. +6. Use a separate statement-effect boundary for each logical catalog table + that a DDL operation mutates, while retaining batches of rows for the same + catalog table in one statement. +7. Move catalog DDL redo installation from `StmtEffects` to `TrxEffects` and + enforce exactly one transaction-level marker per catalog DDL transaction. +8. Derive catalog row objects inside `CatalogStorage` from already validated + table metadata instead of carrying duplicate row bundles through DDL plans. +9. Let secondary `MemIndex` cleanup retain a lightweight registered + `PrivateSnapshot` whose lifetime directly protects its captured table root. +10. Preserve normal terminal ordering and safely retain a checked-out DDL + private core before mandatory panic publication. +11. Preserve existing catalog contents, DDL redo bytes, recovery + classification, table-file/root publication, runtime installation, and + logical-lock lifetime. + +## Non-Goals + +1. Do not change the public `Session::begin_trx`, `Transaction::exec`, + `Transaction::commit`, `Transaction::rollback`, streaming statement, or + explicit-lock APIs. +2. Do not add public access to `PrivateTransaction`, transaction effects, + catalog row accessors, or DDL redo installation. +3. Do not add private-transaction cancellation, asynchronous Drop rollback, + caller abandonment, savepoints, statement retry, or transaction reuse after + terminal completion. +4. Do not change public statement ordinary-error rollback or future + cancellation behavior. +5. Do not change MVCC visibility, STS/CTS allocation, GC bucket registration, + transaction status, undo ordering, row/index operations, or table + admission. +6. Do not reintroduce statement lock ownership or change transaction-owned + logical-lock compatibility, FIFO behavior, acquisition, or terminal + release. +7. Do not change catalog table definitions, row encodings, primary keys, + checkpoint folding, or catalog recovery validation. +8. Do not change `DDLRedo` variants, numeric codes, serialization, table-root + proof rules, or recovery replay policy. +9. Do not move file creation, root publication, runtime construction, + lifecycle gates, compensation, or runtime/history installation into + `CatalogStorage`. +10. Do not redesign session-operation states beyond the transitions required + for one continuously checked-out private core. +11. Do not alter sessionless `SysTrx` DDL records such as row-page creation, + checkpoint publication, or silent-watermark maintenance. +12. Do not rewrite implemented RFC or task documents; update only live + transaction and lock documentation where current behavior changes. + +## Plan + +### 1. Add the semantic private transaction owner + +Define the crate-private type in `doradb-storage/src/trx/mod.rs`: + +```rust +pub(crate) struct PrivateTransaction { + checkout: Option, +} +``` + +Do not duplicate `trx_id`, `sts`, operation key, weak session reachability, or +engine fields. The checked-out `TrxInner` is the authority for transaction +identity and STS, while `SessionOperationCheckout` already owns: + +- the registry-visible `Arc`; +- the exclusive `Box` containing context, transaction effects, + positive table bindings, transaction lock state, activity state, and + terminal cache policy; and +- the strong `TrxAttachment` containing exact session runtime reachability, + operation and transaction identity, engine access, pool guards, and session + cache access. + +Expose only the crate-private operations required by catalog DDL: + +- `trx_id()` for invariant diagnostics when needed; +- `sts()` from `TrxInner::ctx`; +- direct engine-health validation without weak-session or entry lookup; +- a private statement executor used by catalog storage; +- exact-once transaction-level DDL redo installation; +- consuming catalog commit and rollback; +- synchronous parking of a still-active checkout for mandatory panic + retention. + +Keep `SessionOperationCheckout` as the mechanical carrier shared with public +statements. Do not rename it and do not add `TransactionLease`. + +### 2. Begin directly in the checked-out state + +Change `TransactionSystem`, `MandatoryOperationGuard`, and +`AcceptedDdlScope` private-begin paths to return `PrivateTransaction`. + +Initialize the existing fresh private `TrxInner` through the current +transaction-system STS, transaction-id, GC-bucket, status, and lock-authority +logic. Require the enclosing entry to be `Mandatory(None)` and install the +identity directly as `Mandatory(Some(Running))`, with the core owned by the +new checkout rather than temporarily stored in +`SessionOperationEntry::trx_inner`. Private transaction construction is not +available from caller-owned voluntary operation state. + +Construct one strong `TrxAttachment` from the already-owned +`SessionRuntime`, operation key, and new transaction id. Construct the +`SessionOperationCheckout` directly from the stable entry, initialized core, +and attachment. Do not install an available core and immediately call the +public weak-handle checkout path. + +While the private transaction is active, the entry remains in `Running` and +its `trx_inner` slot remains empty across statements, DDL file/runtime awaits, +and index build work. This preserves registry visibility through the entry's +operation state and transaction id without repeatedly moving the core. + +Retain the existing checked-in `Available` representation for panic parking +and defensive Drop handling. An unintentionally dropped, non-terminal private +transaction returns its checkout to the stable entry; accepted execution then +cannot pass `assert_mandatory_finish_ready` and must fail closed rather than +silently publish terminal success. + +### 3. Reuse statement effects without private checkout cycling + +Implement the private statement executor by borrowing +`SessionOperationCheckout::inner_and_attachment_mut`, creating fresh +`StmtEffects`, and lending the existing `Statement` facade to the callback. +Reuse current row/index operations, transaction runtime views, effect merge, +cancelled-effect folding, and undo data structures. + +At the start of each `CatalogStorage::stage_*` group, validate engine health +once through the retained attachment and convert it to the existing catalog +runtime context. This preserves the current check immediately before catalog +mutation even when a private transaction was started before lengthy build or +drain work. Storage operations still report their own runtime failures +normally; the executor does not repeat lifecycle admission, weak upgrade, +registry lookup, health validation, transaction-id validation, or core +take/return between catalog-table boundaries. + +Preserve the current private catalog callback contract: + +- on callback success, merge statement effects into `TrxEffects`; +- on an ordinary `RuntimeResult` error, also merge all complete and partial + undo/effects into `TrxEffects`, return the original error, and require the + owning DDL path to roll back the complete private transaction; +- on callback panic, discard incomplete statement redo, fold residual + row/index undo into `TrxEffects`, settle the statement facade, and resume the + unwind for mandatory supervision. + +An ordinary private error must not perform statement-local asynchronous +rollback or make the transaction reusable by a caller. This differs +intentionally from public `Transaction::exec` and preserves the current +catalog staging behavior. + +Remove `StmtState::private` once no caller needs a statement state that owns a +whole checkout. Retain `StmtState::public` and its +`CancelPublicTransaction` policy for public statements. If shared effect +settlement helpers are extracted, keep public and private policy decisions +explicit rather than parameterizing them with ambiguous booleans. + +Remove `Transaction::stage_catalog_statement`. The public transaction type +must no longer contain a catalog-specific execution surface. + +### 4. Convert a held checkout directly into terminal ownership + +Add an entry transition that validates the exact private transaction id and +moves `Mandatory(Some(Running))` directly to +`Mandatory(Some(Completing))` while the core remains held by the checkout. + +Add a consuming `SessionOperationCheckout` conversion that disarms checkout +Drop and constructs `SessionOperationCompletionClaim` from the already-owned +entry, core, and attachment. Represent any moved fields with `Option` where +needed; do not use unsafe field extraction. + +Use the resulting claim with the existing +`commit_catalog_transaction`, +and `rollback_catalog_transaction` machinery. Preserve prepared commit, group +redo, undo rollback, lock release, GC deregistration, returned family +authority, cache policy, and outer +`Mandatory(Some(Completing)) -> Mandatory(None)` publication. + +Move `commit_catalog_ddl` and `rollback_catalog_ddl` from public `Transaction` +to `PrivateTransaction`. Remove the public facade's crate-private `engine()` +probe from production callers; the private checkout already retains exact +engine reachability. + +### 5. Preserve mandatory panic retention before dropping resources + +A supervised DDL panic may occur while `PrivateTransaction` still owns the +core outside the stable entry. Before `AcceptedDdlScope::handle_panic` +publishes `FailedRetained`, park the active checkout back into the matching +entry: + +1. settle any currently executing private statement effects before resuming + the original unwind; +2. take the optional private transaction from its DDL progress; +3. synchronously return its core through the existing checked-out-to-available + entry transition; and +4. only then retain the outer operation scope and publish + `FailedRetained`. + +The panic path must not start asynchronous rollback, queue abandoned +transaction cleanup, expose an idle session, or allow checkout Drop to return a +core after the entry is already failed. + +Update all four accepted catalog DDL panic handlers to park the optional +transaction in their progress state before invoking the scope panic policy. +The parking steps must remain synchronous and panic-minimal, and the complete +handler must preserve the non-unwinding contract of +`AcceptedExecution::handle_panic`. + +For maintenance, replace the separate specification/resource/scope owners with +one stateful `MaintenanceExecution` object owned by +`AcceptedMaintenanceScope`. The scope implements `AcceptedExecution` +directly, drops `E` before both normal terminal publication and +`FailedRetained`, and centralizes the mandatory-finish readiness check. +Remove `MaintenanceExecutionSpec::{Resources,PanicLabel}`, `settle_panic`, +the `*Resources` structs, and `AcceptedMaintenanceExecution`. + +### 6. Make `CatalogStorage` own logical catalog DDL mutations + +Add `doradb-storage/src/catalog/storage/ddl.rs` and include it from +`catalog/storage/mod.rs`. Define these crate-private methods on +`CatalogStorage`: + +```rust +async fn stage_create_table( + &self, + trx: &mut PrivateTransaction, + table_id: TableID, + metadata: &TableMetadata, +) -> RuntimeResult<()>; + +async fn stage_drop_table( + &self, + trx: &mut PrivateTransaction, + table_id: TableID, + metadata: &TableMetadata, +) -> RuntimeResult<()>; + +async fn stage_create_index( + &self, + trx: &mut PrivateTransaction, + table_id: TableID, + index_no: IndexNo, + new_metadata: &TableMetadata, +) -> RuntimeResult<()>; + +async fn stage_drop_index( + &self, + trx: &mut PrivateTransaction, + table_id: TableID, + index_no: IndexNo, + old_metadata: &TableMetadata, +) -> RuntimeResult<()>; +``` + +The metadata arguments are already validated and protected by the enclosing +DDL gates. CREATE INDEX receives the post-create metadata, from which it +derives both `next_index_no` and the active `IndexSpec` at `index_no`. DROP +INDEX receives the pre-drop metadata, from which it derives the expected +index-column count. Assert inactive or mismatched metadata as a violated +prepared-plan invariant with table and index identifiers. + +Construct `TableObject`, `ColumnObject`, `IndexObject`, and +`IndexColumnObject` inside this module. `TableMetadata` contains the ordered +column names, value kinds, attributes, active stable index numbers, index +attributes, keys, and next index number needed for all persisted rows. + +Remove `CreateTableCatalogObjects` and the duplicate catalog-object fields +from `CreateTablePlan`. `ValidatedCreateTable` and `CreateTablePlan` retain the +validated `Arc` and allocated table id needed by catalog +staging and runtime construction. Keep the row-object structs and low-level +`tables()`, `columns()`, `indexes()`, `index_columns()`, and +`table_replay_silent_watermarks()` accessors as storage implementation +details accepting `&mut Statement`. + +Move the drop-count assertions into the catalog DDL module so persisted-row +expectations remain beside the mutation that produces their counts. + +Remove the four free staging functions from `catalog/table.rs` and +`catalog/index.rs`. Those modules continue to own validation, DDL gates, +prepared plans, provisional files and roots, runtime construction, +commit/rollback compensation, poisoning policy, and runtime/history +publication. + +### 7. Split statements by mutated catalog table + +Use these private statement boundaries, preserving the listed order: + +| DDL | Statement boundaries | +| --- | --- | +| CREATE TABLE | insert `catalog.tables`; insert all `catalog.columns`; insert all `catalog.indexes`; insert all `catalog.index_columns` | +| DROP TABLE | delete `catalog.index_columns`; delete `catalog.indexes`; delete `catalog.columns`; delete `catalog.tables`; delete optional `catalog.table_replay_silent_watermarks` | +| CREATE INDEX | delete and reinsert `catalog.tables`; insert `catalog.indexes`; insert all `catalog.index_columns` | +| DROP INDEX | delete `catalog.index_columns`; delete `catalog.indexes` | + +All row mutations belonging to the same logical catalog table remain in one +statement. In particular, CREATE INDEX's table-row delete/reinsert is one +`catalog.tables` statement, and all columns or index-column mappings are +batched within their respective table statement. Skip CREATE statements for +an empty optional index or index-column collection; do not manufacture an +empty effect boundary. + +Keep DROP TABLE's silent-watermark delete as its own statement even when no row +exists because absence is a valid result of that attempted logical-table +mutation. Preserve current delete-count and required-row assertions after the +corresponding statement result is available. + +Because task 000261 made all logical claims transaction-owned, this split must +not add lock owners, statement lock cleanup, claim handoff, or release between +boundaries. Successful later access to the same catalog table reuses the +transaction claim. + +### 8. Install DDL redo only at transaction level + +Add an exact-once DDL installation method to `TrxEffects` and delegate to it +through `PrivateTransaction`. The method accepts `DDLRedo`, stores it in the +transaction's `RedoLogs::ddl` slot, and release-asserts that the slot was +previously empty. It returns no replaceable old value. + +Remove `StmtEffects::set_ddl_redo` and its production import of `DDLRedo`. +Statement effects may produce only DML row redo. Keep `RedoLogs` as the shared +merge representation unless a smaller refactor is needed to make the +statement-level absence invariant explicit; do not redesign redo containers or +serialization in this task. + +Each `CatalogStorage::stage_*` method installs its matching marker only after +all catalog-table statements and invariant checks succeed: + +- `DDLRedo::CreateTable(table_id)`; +- `DDLRedo::DropTable(table_id)`; +- `DDLRedo::CreateIndex { table_id, index_no }`; or +- `DDLRedo::DropIndex { table_id, index_no }`. + +If any statement returns an ordinary error, the private transaction contains +the undo needed for all earlier and partial catalog mutations but no DDL +marker. The DDL owner immediately rolls back the complete private transaction. +No commit path may observe catalog DML without its transaction-level marker, +and existing terminal redo invariant checks remain in force. + +Update tests that deliberately construct catalog DML through public +transactions. After `Transaction::exec` merges their DML, install the required +marker through one narrow `#[cfg(test)]` transaction-level helper. Do not +retain a statement-level setter or widen production public APIs for corruption +and recovery tests. Change the cancelled-statement effects test to use DML redo +when verifying that incomplete statement redo is discarded, and add direct +transaction-effects tests for empty and duplicate DDL installation. + +### 9. Migrate catalog DDL progress owners + +Change the transaction field in create/drop table and create/drop index +progress types from `Option` to +`Option`. Call the matching `CatalogStorage::stage_*` +method with validated metadata, then retain the existing phase transitions, +file/root/runtime work, and terminal ordering. + +Consume the private transaction through catalog commit on success and catalog +rollback on every pre-commit failure. Since the private transaction owns a +strong attachment, remove weak-engine-availability branches before rollback; +scope-owned engine and pool access remain authoritative for domain cleanup. + +Preserve the existing policy for failures after catalog commit: perform the +same runtime cleanup or poisoning decisions without attempting to roll back an +already terminal transaction. + +### 10. Add private maintenance snapshots + +Add a lightweight crate-private `PrivateSnapshot` containing an owned +transaction-system guard, registered STS, and GC bucket number. It allocates no +transaction id, mutable core, status object, session child state, locks, undo, +or terminal cleanup task. It exposes only `sts()` and deregisters its STS +synchronously on Drop. + +Extract active-STS registration and deregistration helpers shared with normal +transaction initialization and rollback. Keep `TrxReadProof` exclusively +branded by a borrowed `TrxContext`; a private snapshot cannot mint one. +Generalize only `TableRootSnapshot`'s lifetime marker so a captured root may be +branded either by `TrxReadProof<'ctx>` or directly by +`&'snapshot PrivateSnapshot`. Both constructors must take the real borrowed +capability and no zero-input lifetime constructor may exist. + +Make `MemIndexCleanupExecution` stateful and let it retain an optional +`PrivateSnapshot`. For each cleanup attempt: + +1. register one private snapshot before observing the GC horizon; +2. read its STS and calculate the active GC horizon; +3. preserve the post-start hook and revalidate engine health; +4. borrow the private snapshot directly while capturing and using the table + root snapshot; +5. drop the snapshot-bound root before deregistering the private STS; and +6. yield once and retry with a fresh STS when root publication raced capture. + +Remove explicit checkout, private-transaction state transitions, asynchronous +maintenance rollback, rollback-error combination, and panic parking from this +path. Preserve the unbounded non-busy retry contract, timestamp-fence +reasoning, root visibility hooks, and cleanup outcomes. + +### 11. Update lifecycle documentation and tests + +Update `docs/transaction-system.md` to distinguish: + +- weak, caller-controlled public transaction handles that check out per + operation; +- strongly attached private transactions that own one checkout; +- lightweight private snapshots that own only a registered active STS; +- direct private begin into `Running`; +- no `Available` transition between catalog statements; +- direct held-checkout terminal conversion; and +- required DDL panic parking before `FailedRetained`. + +Update `docs/lock-system.md` only where it describes private transaction +checkout/check-in or family-authority movement. Preserve the three-owner +topology and transaction-lifetime claims introduced by task 000261. + +Audit comments and tests in `session.rs`, `engine.rs`, and transaction modules +for descriptions that still call the private owner a public handle or imply +per-statement private check-in. + +## Implementation Notes + +## Impacts + +| Area | Expected change | +| --- | --- | +| Public transaction API | Public behavior and signatures stay unchanged; crate-private catalog methods and maintenance rollback leave `Transaction`. | +| Private transaction ownership | New semantic facade owns one existing checkout and strong attachment from begin through terminal conversion. | +| Session operation state | Private begin enters `Running` directly; `Available` is used only for panic/defensive parking, not between internal steps. | +| Statement lifecycle | Public cancellation remains in `StmtState`; private catalog execution borrows the long-lived checkout and reuses statement effects. | +| Logical locks | No policy change; all catalog claims remain transaction-owned until commit, rollback, or fatal retention. | +| Catalog API | Four `CatalogStorage::stage_*` methods replace free functions in table/index DDL modules. | +| Catalog plans | CREATE TABLE stops carrying duplicate persisted row objects; storage derives them from validated metadata. | +| Catalog statement granularity | One statement per mutated logical catalog table, with same-table row batches retained. | +| DDL redo | Marker moves from statement effects to the transaction effects exact-once slot; bytes and recovery meaning do not change. | +| DDL runtime flow | Validation, files, roots, gates, compensation, commit order, poisoning, and runtime/history publication stay with table/index modules. | +| Maintenance | Secondary `MemIndex` cleanup uses a lightweight GC-registered `PrivateSnapshot` with no nested session transaction. | +| Maintenance carrier | Stateful execution is owned and settled directly by `AcceptedMaintenanceScope`; there is no separate resources abstraction. | +| Panic supervision | Active DDL private checkout is parked before the stable entry becomes `FailedRetained`; maintenance execution state drops before outer failure publication. | +| Tests | Catalog storage, DDL, transaction effects, recovery corruption helpers, session lifecycle, and maintenance retry tests are updated. | +| Documentation | Live transaction and lock descriptions reflect continuous private checkout ownership. | +| Persistence | No catalog schema, table-file, redo-code, serialization, checkpoint, or recovery-format change. | + +Primary files: + +- `doradb-storage/src/trx/mod.rs` +- `doradb-storage/src/trx/stmt.rs` +- `doradb-storage/src/trx/sys.rs` +- `doradb-storage/src/session.rs` +- `doradb-storage/src/catalog/storage/mod.rs` +- `doradb-storage/src/catalog/storage/ddl.rs` (new) +- `doradb-storage/src/catalog/storage/{tables,columns,indexes}.rs` tests +- `doradb-storage/src/catalog/table.rs` +- `doradb-storage/src/catalog/index.rs` +- `doradb-storage/src/table/gc.rs` +- `doradb-storage/src/recovery/mod.rs` tests +- `docs/transaction-system.md` +- `docs/lock-system.md` + +## Test Cases + +1. Beginning a mandatory private transaction installs the exact operation and + transaction identity directly in `Mandatory(Some(Running))`, leaves the + entry core slot empty, and gives `PrivateTransaction` the initialized core + and strong attachment. +2. Private transaction construction rejects any entry that is not the exact + accepted `Mandatory(None)` DDL operation and cannot start from caller-owned + voluntary state. +3. Two sequential private statement executions use the same `TrxInner` + allocation and never expose `Available` between callbacks. +4. `PrivateTransaction::sts` is sourced from its held `TrxContext`. +5. A successful private statement merges row undo, index undo, and DML redo + into transaction effects without returning the checkout. +6. An ordinary private statement error retains partial undo/effects in the + private transaction, returns the original runtime error, and is fully + reverted by whole-transaction rollback. +7. A private callback panic discards incomplete DML redo, preserves partial + undo in transaction effects, and resumes the original unwind with the + checkout still owned and settled. +8. Public statement success, ordinary-error rollback, fatal rollback, future + cancellation, stream destruction ordering, and abandoned cleanup remain + unchanged after removing `StmtState::private`. +9. Direct held-checkout catalog commit and rollback transition + `Running -> Completing -> Mandatory(None)`, return the same family + authority, deregister the active STS, and preserve transaction status. +10. `PrivateSnapshot` registration contributes its STS to the global GC + watermark and Drop deregisters it exactly once. +11. CREATE TABLE persists one table row, all columns, all active indexes, and + all index-column mappings derived from `TableMetadata`, including a table + with no secondary indexes. +12. CREATE TABLE does not create empty index or index-column statement + boundaries when both collections are empty. +13. DROP TABLE deletes index columns, indexes, columns, the required table row, + and any optional silent watermark in separate ordered statements, with + count assertions matching metadata. +14. CREATE INDEX replaces the table row in one `catalog.tables` statement, + inserts the allocated index row, and inserts all key mappings using the + post-create metadata. +15. DROP INDEX deletes the expected mappings before the index row using the + pre-drop metadata and asserts missing or mismatched prepared metadata. +16. Relation-level catalog staging failures after each successful prior + boundary roll back every catalog row and leave no transaction-level DDL + marker or externally published runtime/root state. +17. All four successful DDL operations install exactly one matching + transaction-level DDL marker after their final catalog statement. +18. Duplicate transaction-level DDL installation release-asserts with the + existing and attempted DDL context, while an empty transaction accepts its + first marker. +19. Statement APIs cannot install DDL redo; cancelled statement tests use DML + redo and continue proving that incomplete redo is discarded. +20. Catalog and recovery tests that intentionally commit direct catalog DML + use only a narrow test-only transaction marker helper after statement + merge and preserve their existing replay outcomes. +21. Existing CREATE/DROP TABLE failure hooks before staging, after staging, + after file/root work, during commit, and after commit retain their current + rollback, cleanup, and poison behavior. +22. Existing CREATE/DROP INDEX build, root publication, commit, cleanup, + recovery proof, and poison tests retain their current outcomes. +23. A supervised DDL panic during a catalog statement and between later DDL + phases parks the private core before `FailedRetained`; dropping the + accepted owner does not panic, lose undo, queue abandoned cleanup, or + expose an idle session. +24. A supervised `MemIndex` cleanup panic drops and deregisters its active + private snapshot before retaining the outer scope, which carries no + nested transaction id. +25. Secondary `MemIndex` cleanup captures a root directly branded by its + private snapshot without an explicit checkout and starts a freshly + registered STS after a root-fence race. +26. Normal, retrying, failed, and panicking maintenance leave no private STS + registration, checked-out core, or unreclaimed family authority after + execution state is settled. +27. Session close and engine shutdown continue waiting for registry-visible + mandatory private transactions and retained failures. +28. Logical-lock tests confirm catalog statements reuse transaction claims and + release them only at private transaction terminal cleanup. +29. Restart, catalog checkpoint, DDL recovery, and index root-proof tests + confirm unchanged persisted redo and catalog state. +30. Run `cargo fmt --check`. +31. Run `cargo clippy --workspace --all-targets -- -D warnings`. +32. Run `cargo nextest run --workspace`. +33. Run alternate-backend lint and tests with + `cargo clippy -p doradb-storage --no-default-features --features libaio --all-targets -- -D warnings` + and + `cargo nextest run -p doradb-storage --no-default-features --features libaio`. +34. Run `tools/style_audit.rs` on the completed branch-diff Rust files. + +## Open Questions + +None. The private transaction owner, private snapshot owner, checkout +lifetime, panic settlement, +maintenance execution ownership, catalog API inputs, catalog statement +boundaries, and transaction-level DDL redo placement are resolved by this +task. diff --git a/docs/transaction-system.md b/docs/transaction-system.md index ad5e0ca7..3d2a1413 100644 --- a/docs/transaction-system.md +++ b/docs/transaction-system.md @@ -148,6 +148,10 @@ For the detailed index design, see [`secondary-index.md`](./secondary-index.md). - **STS (Start Timestamp)**: Acquired at transaction start from a global atomic sequence. - **CTS (Commit Timestamp)**: Acquired at transaction commit. +Mandatory maintenance may register a `PrivateSnapshot` from the same STS +sequence. It participates in the active GC watermark but is not a transaction +and receives no transaction id or status. + ### Transaction Lifecycle #### Execution Phase @@ -161,6 +165,11 @@ then copy a single secondary `DiskTree` root id or build an owned load, and file-internal root reads remain explicit unchecked exceptions outside this runtime transaction contract. +MemIndex cleanup is the separate registered-reader case. Its +`PrivateSnapshot` directly brands the captured `TableRootSnapshot` lifetime; +it cannot mint `TrxReadProof`, and the captured root cannot outlive the active +STS registration. + Each user statement runs through `Transaction::exec(async |stmt| { ... })`. The public `Transaction` is a weak, non-cloneable capability containing weak reachability to its exact `SessionState`, `SessionOperationKey`, and its @@ -225,11 +234,12 @@ wake. Fatal cleanup publishes engine poison before releasing waiters, and waiters check that poison before retrying retained state. This notification protocol does not authorize logical-lock release before redo durability. -During an active transaction, the owning box is checked out for one +During an active public transaction, the owning box is checked out for one non-terminal operation through `SessionOperationCheckout`; ordinary checkout -drop returns the same box through the entry mutex. This keeps repeated ownership -transfers pointer-sized without allocating during statement execution. The -checkout owns a `TrxAttachment` containing the exact `SessionRuntime` and +drop returns the same box through the entry mutex. A private transaction +instead owns one checkout continuously from direct construction through +terminal conversion or synchronous panic parking. The checkout owns a +`TrxAttachment` containing the exact `SessionRuntime` and exposes a copyable `TrxRuntime` value that pairs immutable `TrxContext` with borrowed access to `EngineCore`, its canonical pool guards, and the session-local user-table cache. `TrxContext` never @@ -256,14 +266,17 @@ versioned page tokens through the catalog table's shared insert free list, so catalog insert capacity remains available across sessions without requiring a user-table runtime cache entry. -`StmtState` owns the per-operation checkout and statement effects while +`StmtState` owns the per-operation checkout and statement effects while public `Transaction::exec` is active. It lends one `Statement` facade with direct disjoint borrows of the checked-out `TrxInner`, operation attachment, and effects; DML methods therefore do not resolve the entry or unwrap the carrier. -Normal statement finish returns the core to the available payload position -inside outer `Voluntary` ownership. This check-in -ends only the operation-local lease, not the semantic transaction lifetime; -the weak public `Transaction` remains reusable for its next call. +Normal public statement finish returns the core to its checked-in payload +position inside outer `Voluntary` ownership. This ends only the +operation-local checkout, not the semantic transaction lifetime; the weak +public `Transaction` remains reusable for its next call. Private catalog +statements borrow the core and attachment directly from `PrivateTransaction`, +settle their statement effects into the held `TrxInner`, and never check the +core through the entry between logical catalog-table boundaries. Dropping an unpolled `Transaction::exec` future performs no checkout. Once checkout succeeds, dropping the future is terminal for that public @@ -283,36 +296,45 @@ abandonment, and claim the same entry and core through rolls back inline; it records cleanup intent on the exact entry and queues transaction-system cleanup when the engine is still reachable. -DDL and effectful maintenance start private transactions through their -already-reserved operation authority. A private transaction allocates a new -`TrxID` and boxed core but inherits the outer operation key, installs that box -in the same entry mutex, and does not replace the active slot. During caller -preparation the entry remains `Voluntary(None)`; accepted DDL and maintenance -transfer it to `Mandatory(None)` before starting a child. While mandatory -execution owns that child, `Mandatory(Some(InternalTrxState))` records its -available, checked-out, cleanup, or completion position. Public transactions -use the outer operation states directly and therefore use `Voluntary(None)` -only while checked out. +DDL starts private transactions through its already-reserved operation +authority. `PrivateTransaction` allocates a new `TrxID` and boxed core, +inherits the outer operation key, and constructs a strong `TrxAttachment` from +the accepted operation's `SessionRuntime`. During caller preparation the entry +remains `Voluntary(None)`; accepted DDL transfers it to `Mandatory(None)` +before starting a child. Private begin validates that exact DDL state and publishes +`Mandatory(Some(Running))` directly while the core remains owned by the +private checkout and the entry payload slot remains empty. Public transactions +continue to use weak session reachability and per-operation checkout. Accepted table and index DDL transfer the same entry to `Mandatory(None)` before the runtime task is detached. Their nested catalog transaction follows -`Mandatory(None) -> Mandatory(Some(Available)) -> -Mandatory(Some(Running)) -> Mandatory(Some(Available))`; commit or rollback -claims `Mandatory(Some(Completing))` and clears the child back to -`Mandatory(None)`. That child terminal edge never publishes the outer -operation terminal. Successful accepted execution first proves the exact empty -mandatory state, releases its complete prepared lock scope, and consumes that -proof to publish `Terminal`. A supervised unwind moves any still-owned nested -state to `FailedRetained`; this remains registry-visible and blocks shutdown -instead of exposing an idle session or scheduling competing abandoned cleanup. - -Accepted maintenance uses the same mandatory child transitions as accepted -DDL. Secondary `MemIndex` cleanup installs its private transaction into the -stable entry before any hook, scan, or await. A root-capture race settles that -child completely back to `Mandatory(None)` before a retry installs a fresh -`TrxID`. Normal completion releases the prepared maintenance resources before -publishing the outer terminal state; supervised unwind retains unsafe child -state in `FailedRetained`. +`Mandatory(None) -> Mandatory(Some(Running))`; consuming commit or rollback +converts the held checkout directly to `Mandatory(Some(Completing))` and then +clears the child back to `Mandatory(None)`. The core remains continuously held +across catalog statements, file/root awaits, runtime construction, and index +build work. That child terminal edge never publishes the outer operation +terminal. Successful accepted execution first proves the exact empty mandatory +state, releases its complete prepared lock scope, and consumes that proof to +publish `Terminal`. Before a supervised unwind publishes `FailedRetained`, the +DDL progress owner synchronously parks any active private checkout as +`Mandatory(Some(Available))`; the retained core and entry remain +registry-visible and block shutdown without exposing an idle session or +scheduling abandoned cleanup. + +Accepted maintenance has no nested transaction state. One stateful +`MaintenanceExecution` owns its operation-specific resources inside +`AcceptedMaintenanceScope`, which implements the mandatory +`AcceptedExecution` contract directly. Normal completion drops the execution +state before publishing the outer terminal state; a supervised unwind drops it +before the outer scope publishes `FailedRetained`. + +Secondary `MemIndex` cleanup registers a `PrivateSnapshot` before observing +the GC horizon. The snapshot owns only an active STS registration and directly +brands the captured table-root lifetime. A root-capture race drops both the +root and registration before yielding and retrying with a fresh STS. Normal, +error, and panic paths synchronously deregister the snapshot; the stable +maintenance entry remains `Mandatory(None)` throughout and carries no nested +transaction id. After explicit rollback claims terminal ownership and publishes `Completing`, the claimed transaction core, undo buffers, locks, and session cleanup @@ -332,7 +354,9 @@ optionally contain the nested private-transaction positions `Available`, `Running`, `CleanupReady`, and `Completing`; public transaction checkout is represented by payload ownership within `Voluntary(None)`. Handle-drop intent is orthogonal while a transaction core is checked out, so checkout return -publishes outer `CleanupReady` exactly once. +publishes outer `CleanupReady` exactly once. Normal mandatory private execution +uses `Running` continuously; its `Available` position is reserved for +defensive Drop and mandatory panic parking. Cleanup messages carry `(SessionOperationKey, TrxID)` and stale, replaced, or duplicate hints are neutral. Registry resolution uses only the operation key; the cleanup claim atomically validates the message's `TrxID`, claimable state, @@ -340,7 +364,11 @@ and physical payload ownership under the entry mutex. `Statement` is a borrowed facade over operation-local runtime access and carrier-owned statement-local `StmtEffects`; callers cannot construct or -finish it directly. +finish it directly. Public statements settle through `StmtState`; private +catalog statements use a fresh effect accumulator borrowed alongside the +continuously held checkout. Private ordinary errors merge complete and partial +undo for whole-transaction rollback, while panic settlement discards +incomplete statement redo and folds residual undo before resuming the unwind. Foreground table APIs receive `TrxRuntime` by value when they need pool guards, insert-page cache access, or runtime lock assertions, while pure row MVCC helpers continue to receive `&TrxContext`. When the callback succeeds, @@ -355,10 +383,18 @@ later commit or rollback attempts return an error. Logical lock ownership is tracked outside `TrxContext`. One boxed `FamilyLockAuthority` is allocated per session and moves linearly into `TransactionLockState`, which pairs that root with the transaction -`curr_scope`. `StmtState` retains statement effects, checkout, cancellation, -and Drop policy without logical-lock state. `StreamStmtState` owns only its -transaction checkout and remains last in the stream state so cursor/root state -is destroyed before transaction check-in. +`curr_scope`. Public `StmtState` retains statement effects, checkout, +cancellation, and Drop policy without logical-lock state. `StreamStmtState` +owns only its transaction checkout and remains last in the stream state so +cursor/root state is destroyed before transaction check-in. + +Catalog DDL mutations are owned by `CatalogStorage` and use one private +statement per logical catalog table, retaining same-table row batches in one +effect boundary. `StmtEffects` carries only DML redo. After every catalog-table +statement and invariant check succeeds, `PrivateTransaction` installs exactly +one `DDLRedo` marker directly in `TrxEffects`; an ordinary staging error leaves +all accumulated undo available for whole-transaction rollback and leaves the +transaction-level DDL slot empty. Transaction locks close on commit, rollback, no-op discard, or fatal transaction discard. DDL and maintenance @@ -410,8 +446,9 @@ Finite effectful session maintenance reserves one outer `Maintenance` operation, acquires owned `TableMetadata(S)` followed by `TableData(IS)`, and resolves the exact live runtime before mandatory admission. Freeze, checkpoint, and secondary `MemIndex` cleanup transfer that complete scope into accepted -execution and retain it through their last table/layout/index use. Hot-row-page -counting remains a caller-owned, cancellable scoped observation. These calls +execution. The stateful execution and lock scope are one accepted owner and +remain retained through their last table/layout/index use. Hot-row-page counting +remains a caller-owned, cancellable scoped observation. These calls preserve ordinary `IX` DML and explicit `S` table-reader concurrency while excluding same-table DROP and serializing page freeze/transition against full-table mutation `X`. Grants admitted by a covering explicit session lock diff --git a/docs/unsafe-usage-baseline.md b/docs/unsafe-usage-baseline.md index f9562e7b..5be02ea8 100644 --- a/docs/unsafe-usage-baseline.md +++ b/docs/unsafe-usage-baseline.md @@ -12,12 +12,12 @@ | row | 3 | 6 | 0 | 0 | 0 | 6 | | index | 21 | 13 | 0 | 0 | 3 | 7 | | io | 6 | 21 | 0 | 0 | 1 | 18 | -| trx | 14 | 4 | 0 | 0 | 0 | 4 | +| trx | 15 | 4 | 0 | 0 | 0 | 4 | | lwc | 2 | 4 | 0 | 0 | 0 | 3 | | file | 8 | 11 | 0 | 0 | 2 | 11 | | log | 6 | 0 | 0 | 0 | 0 | 0 | | recovery | 5 | 0 | 0 | 0 | 0 | 0 | -| **total** | **83** | **150** | **0** | **0** | **6** | **130** | +| **total** | **84** | **150** | **0** | **0** | **6** | **130** | ## File Hotspots (top 40) diff --git a/doradb-storage/src/catalog/checkpoint.rs b/doradb-storage/src/catalog/checkpoint.rs index 9ff96580..ba7a0a56 100644 --- a/doradb-storage/src/catalog/checkpoint.rs +++ b/doradb-storage/src/catalog/checkpoint.rs @@ -15,8 +15,7 @@ use crate::quiescent::QuiescentGuard; use crate::recovery::stream::{CatalogSafeRedoSegment, RedoReplayPlanner}; use crate::runtime::mandatory::PreparedExecution; use crate::session::{ - AcceptedMaintenanceScope, MaintenanceExecutionSpec, PreparedMaintenanceExecution, - PreparedMaintenanceScope, + MaintenanceExecution, PreparedMaintenanceExecution, PreparedMaintenanceScope, SessionRuntime, }; use crate::trx::RedoRetentionScope; use crate::trx::sys::{CatalogRedoRetentionProgress, TransactionSystem}; @@ -362,11 +361,6 @@ impl Drop for CatalogCheckpointScope { } } -struct CatalogCheckpointResources { - _catalog_scope: CatalogCheckpointScope, - _redo_scope: RedoRetentionScope, -} - #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum CatalogCheckpointTxnAction { Include, @@ -374,28 +368,28 @@ enum CatalogCheckpointTxnAction { Stop(CatalogCheckpointScanStopReason), } -struct CatalogCheckpointExecution; +struct CatalogCheckpointExecution { + _catalog_scope: CatalogCheckpointScope, + _redo_scope: RedoRetentionScope, +} -impl MaintenanceExecutionSpec for CatalogCheckpointExecution { +impl MaintenanceExecution for CatalogCheckpointExecution { type Output = CatalogCheckpointOutcome; - type Resources = CatalogCheckpointResources; - type PanicLabel = &'static str; const LABEL: &'static str = "checkpoint_catalog"; - async fn execute( - scope: &mut AcceptedMaintenanceScope, - _resources: &mut Self::Resources, - _panic_label: &mut Self::PanicLabel, - ) -> CompletionResult { - let engine = scope.engine(); - let result = engine + async fn execute(&mut self, runtime: &SessionRuntime) -> CompletionResult { + let engine = runtime.core(); + engine .catalog() .checkpoint_prepared(&engine.trx_sys) .await - .map_err(CompletionErrorBridge::capture_runtime_or_fatal); - scope.mark_terminal_ready(); - result + .map_err(CompletionErrorBridge::capture_runtime_or_fatal) + } + + #[inline] + fn panic_diagnostic(&self) -> String { + "accepted catalog checkpoint panicked".to_owned() } } @@ -407,11 +401,10 @@ pub(crate) fn prepare_catalog_checkpoint_operation( ) -> impl PreparedExecution { PreparedMaintenanceExecution::::global( scope, - CatalogCheckpointResources { + CatalogCheckpointExecution { _catalog_scope: catalog_scope, _redo_scope: redo_scope, }, - "accepted catalog checkpoint panicked", ) } diff --git a/doradb-storage/src/catalog/index.rs b/doradb-storage/src/catalog/index.rs index d6e84bfa..44243648 100644 --- a/doradb-storage/src/catalog/index.rs +++ b/doradb-storage/src/catalog/index.rs @@ -1,9 +1,5 @@ use crate::buffer::{EvictableBufferPool, PoolGuard, PoolGuards}; -use crate::catalog::storage::CatalogStorage; -use crate::catalog::{ - Catalog, IndexColumnObject, IndexNo, IndexObject, IndexSpec, TableMetadata, TableObject, - catalog_table_id_from_slot, -}; +use crate::catalog::{Catalog, IndexNo, IndexSpec, TableMetadata, catalog_table_id_from_slot}; use crate::engine::EngineCore; use crate::error::{ CompletionErrorBridge, CompletionResult, DataIntegrityError, DataIntegrityResult, @@ -18,7 +14,6 @@ use crate::index::{ BTreeKey, BTreeKeyEncoder, ColumnBlockIndex, IndexInsert, NonUniqueMemIndex, SecondaryDiskTreeRuntime, SecondaryIndex, UniqueMemIndex, }; -use crate::log::redo::DDLRedo; use crate::obs; use crate::poison::EnginePoisoner; use crate::quiescent::QuiescentGuard; @@ -27,7 +22,7 @@ use crate::runtime::mandatory::{AcceptedExecution, MandatoryTaskMetadata, Prepar use crate::runtime::{POLL_BUDGET, yield_now}; use crate::session::{AcceptedDdlScope, PreparedDdlScope}; use crate::table::{DeleteMarker, Table, TableRuntimeLayout, secondary_disk_tree_encoder}; -use crate::trx::{Transaction, trx_is_committed}; +use crate::trx::{PrivateTransaction, trx_is_committed}; use crate::value::Val; use error_stack::{Report, ResultExt}; use std::any::Any; @@ -158,7 +153,6 @@ pub(crate) struct DropIndexPlan { table: Arc, old_layout: Arc, index_no: IndexNo, - old_index_spec: IndexSpec, new_metadata: Arc, secondary_index_roots: Vec, } @@ -169,7 +163,7 @@ impl DropIndexPlan { let old_layout = table.layout_snapshot(); let old_metadata = old_layout.metadata(); let index_no_usize = usize::from(index_no); - let old_index_spec = old_metadata + old_metadata .idx .index_spec(index_no_usize) .ok_or_else(|| { @@ -177,8 +171,7 @@ impl DropIndexPlan { "drop index target not found: table_id={table_id}, index_no={index_no}, reason=inactive_metadata_slot" )) }) - .disclose()? - .clone(); + .disclose()?; old_layout .secondary_index(index_no_usize) .expect("active index metadata must have a matching runtime index"); @@ -193,7 +186,6 @@ impl DropIndexPlan { table, old_layout, index_no, - old_index_spec, new_metadata, secondary_index_roots, }) @@ -589,14 +581,14 @@ struct CreateIndexProgress { index_no: IndexNo, build_ts: TrxID, phase: CreateIndexBuildPhase, - trx: Option, + trx: Option, staged_index: Option>>, new_layout: Option, } impl CreateIndexProgress { #[inline] - fn new(table_id: TableID, index_no: IndexNo, trx: Transaction) -> Self { + fn new(table_id: TableID, index_no: IndexNo, trx: PrivateTransaction) -> Self { let build_ts = trx.sts(); Self { table_id, @@ -609,6 +601,13 @@ impl CreateIndexProgress { } } + #[inline] + fn park_active_transaction(&mut self) { + if let Some(trx) = self.trx.take() { + trx.park(); + } + } + #[inline] fn build_ts(&self) -> TrxID { self.build_ts @@ -652,8 +651,7 @@ impl CreateIndexProgress { async fn execute_catalog_update( &mut self, engine: &EngineCore, - metadata: &TableMetadata, - index_spec: &IndexSpec, + new_metadata: &TableMetadata, ) -> RuntimeOrFatalResult<()> { debug_assert_eq!(self.phase, CreateIndexBuildPhase::LayoutStaged); let trx = self.trx.as_mut().unwrap_or_else(|| { @@ -662,15 +660,11 @@ impl CreateIndexProgress { self.table_id, self.index_no ) }); - let res = execute_create_index_catalog_update( - &engine.catalog().storage, - trx, - self.table_id, - self.index_no, - metadata, - index_spec, - ) - .await; + let res = engine + .catalog() + .storage + .stage_create_index(trx, self.table_id, self.index_no, new_metadata) + .await; match res { Ok(()) => Ok(()), Err(err) => { @@ -783,13 +777,13 @@ struct DropIndexProgress { table_id: TableID, index_no: IndexNo, phase: DropIndexBuildPhase, - trx: Option, + trx: Option, new_layout: Option, } impl DropIndexProgress { #[inline] - fn new(table_id: TableID, index_no: IndexNo, trx: Transaction) -> Self { + fn new(table_id: TableID, index_no: IndexNo, trx: PrivateTransaction) -> Self { Self { table_id, index_no, @@ -799,6 +793,13 @@ impl DropIndexProgress { } } + #[inline] + fn park_active_transaction(&mut self) { + if let Some(trx) = self.trx.take() { + trx.park(); + } + } + #[inline] fn stage_layout(&mut self, layout: TableRuntimeLayout) { debug_assert_eq!(self.phase, DropIndexBuildPhase::LayoutStaged); @@ -809,7 +810,7 @@ impl DropIndexProgress { async fn execute_catalog_update( &mut self, catalog: &Catalog, - old_index_spec: &IndexSpec, + old_metadata: &TableMetadata, ) -> RuntimeOrFatalResult<()> { debug_assert_eq!(self.phase, DropIndexBuildPhase::LayoutStaged); let trx = self.trx.as_mut().unwrap_or_else(|| { @@ -818,14 +819,10 @@ impl DropIndexProgress { self.table_id, self.index_no ) }); - let res = execute_drop_index_catalog_update( - &catalog.storage, - trx, - self.table_id, - self.index_no, - old_index_spec, - ) - .await; + let res = catalog + .storage + .stage_drop_index(trx, self.table_id, self.index_no, old_metadata) + .await; match res { Ok(()) => Ok(()), Err(err) => { @@ -996,6 +993,9 @@ impl AcceptedExecution for AcceptedCreateIndex { #[inline] async fn handle_panic(&mut self, _panic: Box) -> CompletionErrorBridge { + if let Some(progress) = self.progress.as_mut() { + progress.park_active_transaction(); + } self.scope.handle_panic(); let phase = self .progress @@ -1195,7 +1195,7 @@ impl AcceptedCreateIndex { progress.stage_layout(new_layout); if let Err(err) = progress - .execute_catalog_update(engine, plan.new_metadata.as_ref(), &plan.new_index_spec) + .execute_catalog_update(engine, plan.new_metadata.as_ref()) .await { return Err(CompletionErrorBridge::capture_runtime_or_fatal(err)); @@ -1364,6 +1364,9 @@ impl AcceptedExecution for AcceptedDropIndex { #[inline] async fn handle_panic(&mut self, _panic: Box) -> CompletionErrorBridge { + if let Some(progress) = self.progress.as_mut() { + progress.park_active_transaction(); + } self.scope.handle_panic(); let phase = self .progress @@ -1435,7 +1438,7 @@ impl AcceptedDropIndex { .await; if let Err(err) = progress - .execute_catalog_update(engine.catalog(), &plan.old_index_spec) + .execute_catalog_update(engine.catalog(), plan.old_layout.metadata()) .await { return Err(CompletionErrorBridge::capture_runtime_or_fatal(err)); @@ -1633,13 +1636,11 @@ pub(crate) fn classify_index_ddl_root( } } -async fn rollback_active_ddl_trx(trx: &mut Option) -> RuntimeOrFatalResult<()> { +async fn rollback_active_ddl_trx(trx: &mut Option) -> RuntimeOrFatalResult<()> { let Some(trx) = trx.take() else { return Ok(()); }; - if trx.engine().is_some() { - trx.rollback_catalog_ddl().await?; - } + trx.rollback_catalog_ddl().await?; Ok(()) } @@ -1937,120 +1938,6 @@ async fn destroy_uninstalled_staged_index( .attach("operation=destroy_uninstalled_create_index_runtime") } -#[inline] -async fn execute_drop_index_catalog_update( - storage: &CatalogStorage, - trx: &mut Transaction, - table_id: TableID, - index_no: IndexNo, - old_index_spec: &IndexSpec, -) -> RuntimeResult<()> { - trx.stage_catalog_statement(async |stmt| { - let deleted_columns = storage - .index_columns() - .delete_by_index(stmt, table_id, index_no) - .await?; - assert_eq!( - deleted_columns, - old_index_spec.cols.len(), - "drop-index catalog invariant violated: index-column delete count mismatch, table_id={table_id}, index_no={index_no}" - ); - - let index_deleted = storage - .indexes() - .delete_by_id(stmt, table_id, index_no) - .await?; - assert!( - index_deleted, - "drop-index catalog invariant violated: validated index row is missing, table_id={table_id}, index_no={index_no}" - ); - - assert!( - stmt.effects_mut() - .set_ddl_redo(DDLRedo::DropIndex { table_id, index_no }) - .is_none(), - "drop-index catalog invariant violated: statement already has DDL redo, table_id={table_id}, index_no={index_no}" - ); - Ok(()) - }) - .await -} - -/// Stage catalog metadata for a newly allocated table-local index number. -/// -/// The metadata-change gate serializes index DDL. The table row is deleted and -/// reinserted by this transaction, `index_no` is allocated from `next_index_no`, -/// and index-column numbers are enumerated from the validated index spec. Every -/// inserted catalog primary key is therefore unique by construction. -#[inline] -async fn execute_create_index_catalog_update( - storage: &CatalogStorage, - trx: &mut Transaction, - table_id: TableID, - index_no: IndexNo, - metadata: &TableMetadata, - index_spec: &IndexSpec, -) -> RuntimeResult<()> { - trx.stage_catalog_statement(async |stmt| { - let table_deleted = storage - .tables() - .delete_by_id(stmt, table_id) - .await?; - assert!( - table_deleted, - "create-index catalog invariant violated: validated table row is missing, table_id={table_id}" - ); - - storage - .tables() - .insert( - stmt, - &TableObject { - table_id, - next_index_no: metadata.idx.next_index_no(), - }, - ) - .await?; - - storage - .indexes() - .insert( - stmt, - &IndexObject { - table_id, - index_no, - index_attributes: index_spec.attributes, - }, - ) - .await?; - - for (index_column_no, index_key) in index_spec.cols.iter().enumerate() { - storage - .index_columns() - .insert( - stmt, - &IndexColumnObject { - table_id, - index_no, - index_column_no: index_column_no as u16, - column_no: index_key.col_no, - index_order: index_key.order, - }, - ) - .await?; - } - - assert!( - stmt.effects_mut() - .set_ddl_redo(DDLRedo::CreateIndex { table_id, index_no }) - .is_none(), - "create-index catalog invariant violated: statement already has DDL redo, table_id={table_id}, index_no={index_no}" - ); - Ok(()) - }) - .await -} - #[inline] fn poison_index_after_catalog_commit_with_source( poisoner: &EnginePoisoner, diff --git a/doradb-storage/src/catalog/storage/columns.rs b/doradb-storage/src/catalog/storage/columns.rs index aed6169f..39a9befb 100644 --- a/doradb-storage/src/catalog/storage/columns.rs +++ b/doradb-storage/src/catalog/storage/columns.rs @@ -285,11 +285,11 @@ mod tests { .insert(stmt, &col_43_0) .await .disclose()?; - mark_catalog_ddl(stmt, DDLRedo::CreateTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::CreateTable(TableID::new(42))); trx.commit().await.unwrap(); let mut trx = session.begin_trx().unwrap(); @@ -316,11 +316,11 @@ mod tests { .await .disclose()? ); - mark_catalog_ddl(stmt, DDLRedo::DropTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::DropTable(TableID::new(42))); trx.commit().await.unwrap(); let cols_42 = engine @@ -382,11 +382,11 @@ mod tests { .await .disclose()? ); - mark_catalog_ddl(stmt, DDLRedo::DropTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::DropTable(TableID::new(42))); trx.commit().await.unwrap(); assert!( @@ -464,11 +464,11 @@ mod tests { .await .disclose()?; } - mark_catalog_ddl(stmt, DDLRedo::CreateTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::CreateTable(TableID::new(42))); trx.commit().await.unwrap(); let mut trx = session.begin_trx().unwrap(); @@ -497,11 +497,11 @@ mod tests { .unwrap(), 0 ); - mark_catalog_ddl(stmt, DDLRedo::DropTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::DropTable(TableID::new(42))); trx.commit().await.unwrap(); assert!( diff --git a/doradb-storage/src/catalog/storage/ddl.rs b/doradb-storage/src/catalog/storage/ddl.rs new file mode 100644 index 00000000..a65404c7 --- /dev/null +++ b/doradb-storage/src/catalog/storage/ddl.rs @@ -0,0 +1,305 @@ +use super::{CatalogStorage, ColumnObject, IndexColumnObject, IndexObject, TableObject}; +use crate::catalog::{IndexNo, TableMetadata}; +use crate::error::{RuntimeError, RuntimeResult}; +use crate::id::TableID; +use crate::log::redo::DDLRedo; +use crate::trx::PrivateTransaction; +use error_stack::ResultExt; + +impl CatalogStorage { + /// Stage all persisted catalog rows for a newly allocated table. + pub(crate) async fn stage_create_table( + &self, + trx: &mut PrivateTransaction, + table_id: TableID, + metadata: &TableMetadata, + ) -> RuntimeResult<()> { + validate_catalog_engine_health(trx, "stage_create_table")?; + + let table = TableObject { + table_id, + next_index_no: metadata.idx.next_index_no(), + }; + let columns = metadata + .col + .col_names() + .iter() + .zip(metadata.col.col_types()) + .zip(metadata.col.col_attrs()) + .enumerate() + .map( + |(column_no, ((column_name, column_type), column_attributes))| ColumnObject { + table_id, + column_no: column_no as u16, + column_name: column_name.clone(), + column_type: column_type.kind, + column_attributes: *column_attributes, + }, + ) + .collect::>(); + let indexes = metadata + .idx + .active_indexes() + .map(|(index_no, index_spec)| IndexObject { + table_id, + index_no: index_no as IndexNo, + index_attributes: index_spec.attributes, + }) + .collect::>(); + let index_columns = metadata + .idx + .active_indexes() + .flat_map(|(index_no, index_spec)| { + index_spec + .cols + .iter() + .enumerate() + .map(move |(index_column_no, index_key)| IndexColumnObject { + table_id, + index_no: index_no as IndexNo, + index_column_no: index_column_no as u16, + column_no: index_key.col_no, + index_order: index_key.order, + }) + }) + .collect::>(); + + trx.stage_statement(async |stmt| self.tables().insert(stmt, &table).await) + .await?; + trx.stage_statement(async |stmt| { + for column in &columns { + self.columns().insert(stmt, column).await?; + } + Ok(()) + }) + .await?; + if !indexes.is_empty() { + trx.stage_statement(async |stmt| { + for index in &indexes { + self.indexes().insert(stmt, index).await?; + } + Ok(()) + }) + .await?; + } + if !index_columns.is_empty() { + trx.stage_statement(async |stmt| { + for index_column in &index_columns { + self.index_columns().insert(stmt, index_column).await?; + } + Ok(()) + }) + .await?; + } + trx.install_ddl_redo(DDLRedo::CreateTable(table_id)); + Ok(()) + } + + /// Stage the ordered catalog cascade for a validated table drop. + pub(crate) async fn stage_drop_table( + &self, + trx: &mut PrivateTransaction, + table_id: TableID, + metadata: &TableMetadata, + ) -> RuntimeResult<()> { + validate_catalog_engine_health(trx, "stage_drop_table")?; + + let index_columns_deleted = trx + .stage_statement(async |stmt| { + self.index_columns() + .delete_by_table_id(stmt, table_id) + .await + }) + .await?; + let expected_index_columns = metadata + .idx + .active_indexes() + .map(|(_, spec)| spec.cols.len()) + .sum::(); + assert_eq!( + index_columns_deleted, expected_index_columns, + "drop-table catalog invariant violated: index-column delete count mismatch, table_id={table_id}" + ); + + let indexes_deleted = trx + .stage_statement(async |stmt| self.indexes().delete_by_table_id(stmt, table_id).await) + .await?; + assert_eq!( + indexes_deleted, + metadata.idx.active_index_count(), + "drop-table catalog invariant violated: index delete count mismatch, table_id={table_id}" + ); + + let columns_deleted = trx + .stage_statement(async |stmt| self.columns().delete_by_table_id(stmt, table_id).await) + .await?; + assert_eq!( + columns_deleted, + metadata.col.col_count(), + "drop-table catalog invariant violated: column delete count mismatch, table_id={table_id}" + ); + + let table_deleted = trx + .stage_statement(async |stmt| self.tables().delete_by_id(stmt, table_id).await) + .await?; + assert!( + table_deleted, + "drop-table catalog invariant violated: validated table row is missing, table_id={table_id}" + ); + + trx.stage_statement(async |stmt| { + self.table_replay_silent_watermarks() + .delete_by_table_id(stmt, table_id) + .await + }) + .await?; + + trx.install_ddl_redo(DDLRedo::DropTable(table_id)); + Ok(()) + } + + /// Stage persisted metadata for one newly allocated secondary index. + pub(crate) async fn stage_create_index( + &self, + trx: &mut PrivateTransaction, + table_id: TableID, + index_no: IndexNo, + new_metadata: &TableMetadata, + ) -> RuntimeResult<()> { + validate_catalog_engine_health(trx, "stage_create_index")?; + + let expected_next_index_no = index_no.checked_add(1).unwrap_or_else(|| { + panic!( + "create-index prepared metadata overflow: table_id={table_id}, index_no={index_no}" + ) + }); + assert_eq!( + new_metadata.idx.next_index_no(), + expected_next_index_no, + "create-index prepared metadata mismatch: table_id={table_id}, index_no={index_no}" + ); + let index_spec = new_metadata + .idx + .index_spec(usize::from(index_no)) + .unwrap_or_else(|| { + panic!( + "create-index prepared metadata has inactive index: table_id={table_id}, index_no={index_no}" + ) + }); + + trx.stage_statement(async |stmt| { + let table_deleted = self.tables().delete_by_id(stmt, table_id).await?; + assert!( + table_deleted, + "create-index catalog invariant violated: validated table row is missing, table_id={table_id}" + ); + self.tables() + .insert( + stmt, + &TableObject { + table_id, + next_index_no: new_metadata.idx.next_index_no(), + }, + ) + .await + }) + .await?; + trx.stage_statement(async |stmt| { + self.indexes() + .insert( + stmt, + &IndexObject { + table_id, + index_no, + index_attributes: index_spec.attributes, + }, + ) + .await + }) + .await?; + if !index_spec.cols.is_empty() { + trx.stage_statement(async |stmt| { + for (index_column_no, index_key) in index_spec.cols.iter().enumerate() { + self.index_columns() + .insert( + stmt, + &IndexColumnObject { + table_id, + index_no, + index_column_no: index_column_no as u16, + column_no: index_key.col_no, + index_order: index_key.order, + }, + ) + .await?; + } + Ok(()) + }) + .await?; + } + + trx.install_ddl_redo(DDLRedo::CreateIndex { table_id, index_no }); + Ok(()) + } + + /// Stage the ordered persisted-row deletion for one active secondary index. + pub(crate) async fn stage_drop_index( + &self, + trx: &mut PrivateTransaction, + table_id: TableID, + index_no: IndexNo, + old_metadata: &TableMetadata, + ) -> RuntimeResult<()> { + validate_catalog_engine_health(trx, "stage_drop_index")?; + + assert!( + old_metadata.idx.next_index_no() > index_no, + "drop-index prepared metadata mismatch: table_id={table_id}, index_no={index_no}, next_index_no={}", + old_metadata.idx.next_index_no() + ); + let index_spec = old_metadata + .idx + .index_spec(usize::from(index_no)) + .unwrap_or_else(|| { + panic!( + "drop-index prepared metadata has inactive index: table_id={table_id}, index_no={index_no}" + ) + }); + + let deleted_columns = trx + .stage_statement(async |stmt| { + self.index_columns() + .delete_by_index(stmt, table_id, index_no) + .await + }) + .await?; + assert_eq!( + deleted_columns, + index_spec.cols.len(), + "drop-index catalog invariant violated: index-column delete count mismatch, table_id={table_id}, index_no={index_no}" + ); + + let index_deleted = trx + .stage_statement(async |stmt| { + self.indexes().delete_by_id(stmt, table_id, index_no).await + }) + .await?; + assert!( + index_deleted, + "drop-index catalog invariant violated: validated index row is missing, table_id={table_id}, index_no={index_no}" + ); + + trx.install_ddl_redo(DDLRedo::DropIndex { table_id, index_no }); + Ok(()) + } +} + +#[inline] +fn validate_catalog_engine_health( + trx: &PrivateTransaction, + operation: &'static str, +) -> RuntimeResult<()> { + trx.ensure_engine_healthy() + .change_context(RuntimeError::CatalogAccess) + .attach_with(|| format!("operation={operation}, phase=check_engine_health")) +} diff --git a/doradb-storage/src/catalog/storage/indexes.rs b/doradb-storage/src/catalog/storage/indexes.rs index c692f524..10b6ea3a 100644 --- a/doradb-storage/src/catalog/storage/indexes.rs +++ b/doradb-storage/src/catalog/storage/indexes.rs @@ -489,11 +489,11 @@ mod tests { .insert(stmt, &idx_43_0) .await .disclose()?; - mark_catalog_ddl(stmt, DDLRedo::CreateTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::CreateTable(TableID::new(42))); trx.commit().await.unwrap(); let mut trx = session.begin_trx().unwrap(); @@ -520,11 +520,11 @@ mod tests { .await .disclose()? ); - mark_catalog_ddl(stmt, DDLRedo::DropTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::DropTable(TableID::new(42))); trx.commit().await.unwrap(); let idx_42 = engine @@ -586,11 +586,11 @@ mod tests { .await .disclose()? ); - mark_catalog_ddl(stmt, DDLRedo::DropTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::DropTable(TableID::new(42))); trx.commit().await.unwrap(); assert!( @@ -662,11 +662,11 @@ mod tests { .await .disclose()?; } - mark_catalog_ddl(stmt, DDLRedo::CreateTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::CreateTable(TableID::new(42))); trx.commit().await.unwrap(); let mut trx = session.begin_trx().unwrap(); @@ -695,11 +695,11 @@ mod tests { .unwrap(), 0 ); - mark_catalog_ddl(stmt, DDLRedo::DropTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::DropTable(TableID::new(42))); trx.commit().await.unwrap(); assert!( @@ -783,11 +783,11 @@ mod tests { .await .disclose()?; } - mark_catalog_ddl(stmt, DDLRedo::CreateTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::CreateTable(TableID::new(42))); trx.commit().await.unwrap(); let mut trx = session.begin_trx().unwrap(); @@ -816,11 +816,11 @@ mod tests { .unwrap(), 0 ); - mark_catalog_ddl(stmt, DDLRedo::DropTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::DropTable(TableID::new(42))); trx.commit().await.unwrap(); let remaining_42 = engine @@ -861,11 +861,11 @@ mod tests { .unwrap(), 0 ); - mark_catalog_ddl(stmt, DDLRedo::DropTable(TableID::new(42))); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::DropTable(TableID::new(42))); trx.commit().await.unwrap(); assert!( diff --git a/doradb-storage/src/catalog/storage/mod.rs b/doradb-storage/src/catalog/storage/mod.rs index 02b11d3f..0336b0b1 100644 --- a/doradb-storage/src/catalog/storage/mod.rs +++ b/doradb-storage/src/catalog/storage/mod.rs @@ -1,4 +1,5 @@ mod columns; +mod ddl; mod indexes; mod merge; mod object; @@ -1283,14 +1284,15 @@ pub(crate) mod tests { use crate::index::{ColumnBlockIndex, ColumnDeleteDeltaPatch}; use crate::log::redo::{DDLRedo, RowRedoKind}; use crate::row::ops::{SelectKey, UpdateCol}; - use crate::trx::stmt::Statement; + use crate::trx::Transaction; + use crate::trx::tests::install_transaction_ddl_redo; use crate::value::{Val, ValKind}; use tempfile::TempDir; - /// Attach one catalog DDL marker to the current test statement. - pub(crate) fn mark_catalog_ddl(stmt: &mut Statement<'_>, ddl: DDLRedo) { - let old = stmt.effects_mut().set_ddl_redo(ddl); - debug_assert!(old.is_none()); + /// Attach one catalog DDL marker after test catalog DML has merged. + pub(crate) fn mark_catalog_ddl(trx: &mut Transaction, ddl: DDLRedo) { + install_transaction_ddl_redo(trx, ddl) + .expect("test catalog transaction must remain available"); } fn expect_runtime_report(error: RuntimeOrFatalError) -> Report { diff --git a/doradb-storage/src/catalog/storage/tables.rs b/doradb-storage/src/catalog/storage/tables.rs index ad3e89cc..c88dd192 100644 --- a/doradb-storage/src/catalog/storage/tables.rs +++ b/doradb-storage/src/catalog/storage/tables.rs @@ -194,11 +194,11 @@ mod tests { .insert(stmt, &table101) .await .disclose()?; - mark_catalog_ddl(stmt, DDLRedo::CreateTable(table100.table_id)); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::CreateTable(table100.table_id)); trx.commit().await.unwrap(); let mut trx = session.begin_trx().unwrap(); @@ -225,11 +225,11 @@ mod tests { .await .disclose()? ); - mark_catalog_ddl(stmt, DDLRedo::DropTable(table100.table_id)); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut trx, DDLRedo::DropTable(table100.table_id)); trx.commit().await.unwrap(); assert!( diff --git a/doradb-storage/src/catalog/table.rs b/doradb-storage/src/catalog/table.rs index b25c3a5b..b027c644 100644 --- a/doradb-storage/src/catalog/table.rs +++ b/doradb-storage/src/catalog/table.rs @@ -1,10 +1,6 @@ use crate::buffer::PoolGuards; use crate::catalog::spec::{ActiveIndexSpec, ColumnAttributes, ColumnSpec, IndexNo, IndexSpec}; -use crate::catalog::storage::CatalogStorage; -use crate::catalog::{ - Catalog, ColumnObject, IndexColumnObject, IndexObject, TableObject, catalog_table_id_from_slot, - is_user_table, -}; +use crate::catalog::{Catalog, catalog_table_id_from_slot, is_user_table}; use crate::component::EnginePools; use crate::engine::EngineCore; use crate::error::{ @@ -16,7 +12,6 @@ use crate::file::fs::FileSystem; use crate::file::table_file::{MutableTableFile, TableFile}; use crate::id::{TableID, TrxID}; use crate::index::BlockIndex; -use crate::log::redo::DDLRedo; use crate::map::FastHashSet; use crate::obs; use crate::poison::EnginePoisoner; @@ -26,7 +21,7 @@ use crate::runtime::mandatory::{AcceptedExecution, MandatoryTaskMetadata, Prepar use crate::serde::{Deser, DeserResult, MinBytesHint, Ser, Serde, min_bytes_hint}; use crate::session::{AcceptedDdlScope, PreparedDdlScope}; use crate::table::{Table, TableRedoReplayFloor}; -use crate::trx::Transaction; +use crate::trx::PrivateTransaction; use crate::trx::sys::TransactionSystem; use crate::value::{Val, ValKind, ValType}; use error_stack::{Report, ResultExt}; @@ -55,7 +50,6 @@ const DROP_TABLE_CATALOG_WRITE_TARGETS: [TableID; 5] = [ /// Purely validated public CREATE TABLE input. pub(crate) struct ValidatedCreateTable { - table_spec: super::TableSpec, metadata: Arc, } @@ -71,57 +65,15 @@ impl ValidatedCreateTable { table_spec.columns.clone(), index_specs, )?); - Ok(Self { - table_spec, - metadata, - }) + Ok(Self { metadata }) } /// Bind validated metadata to one gap-tolerant allocated table id. #[inline] pub(crate) fn into_plan(self, table_id: TableID) -> CreateTablePlan { - let table_object = TableObject { - table_id, - next_index_no: self.metadata.idx.next_index_no(), - }; - let column_objects = self - .table_spec - .columns - .into_iter() - .enumerate() - .map(|(col_no, col_spec)| ColumnObject { - table_id, - column_no: col_no as u16, - column_name: col_spec.column_name, - column_type: col_spec.column_type, - column_attributes: col_spec.column_attributes, - }) - .collect(); - let mut index_objects = Vec::new(); - let mut index_column_objects = Vec::new(); - for (index_no, index_spec) in self.metadata.idx.active_indexes() { - index_objects.push(IndexObject { - table_id, - index_no: index_no as u16, - index_attributes: index_spec.attributes, - }); - for (index_column_no, key) in index_spec.cols.iter().enumerate() { - index_column_objects.push(IndexColumnObject { - table_id, - index_no: index_no as u16, - index_column_no: index_column_no as u16, - column_no: key.col_no, - index_order: key.order, - }); - } - } CreateTablePlan { table_id, metadata: self.metadata, - table_object: Some(table_object), - column_objects, - index_objects, - index_column_objects, } } } @@ -130,10 +82,6 @@ impl ValidatedCreateTable { pub(crate) struct CreateTablePlan { table_id: TableID, metadata: Arc, - table_object: Option, - column_objects: Vec, - index_objects: Vec, - index_column_objects: Vec, } /// Owned DROP TABLE target selected under complete target exclusion. @@ -176,13 +124,6 @@ enum CreateTablePhase { Aborted, } -struct CreateTableCatalogObjects { - table: TableObject, - columns: Vec, - indexes: Vec, - index_columns: Vec, -} - enum CreateTableFile { Mutable(MutableTableFile), Published(Arc), @@ -193,7 +134,7 @@ struct CreateTableProgress { table_id: TableID, phase: CreateTablePhase, file: Option, - trx: Option, + trx: Option, staged_table: Option>, } @@ -224,7 +165,7 @@ impl CreateTableProgress { } #[inline] - fn set_catalog_transaction(&mut self, trx: Transaction) { + fn set_catalog_transaction(&mut self, trx: PrivateTransaction) { assert_eq!(self.phase, CreateTablePhase::FileCreated); assert!(self.trx.is_none()); self.trx = Some(trx); @@ -232,24 +173,16 @@ impl CreateTableProgress { } #[inline] - fn mark_catalog_staged(&mut self) { - assert_eq!(self.phase, CreateTablePhase::PrivateTransactionActive); - self.phase = CreateTablePhase::CatalogStaged; + fn park_active_transaction(&mut self) { + if let Some(trx) = self.trx.take() { + trx.park(); + } } #[inline] - fn take_catalog_objects(&mut self) -> CreateTableCatalogObjects { - CreateTableCatalogObjects { - table: self.plan.table_object.take().unwrap_or_else(|| { - panic!( - "create-table plan object moves exactly once: table_id={}", - self.table_id - ) - }), - columns: mem::take(&mut self.plan.column_objects), - indexes: mem::take(&mut self.plan.index_objects), - index_columns: mem::take(&mut self.plan.index_column_objects), - } + fn mark_catalog_staged(&mut self) { + assert_eq!(self.phase, CreateTablePhase::PrivateTransactionActive); + self.phase = CreateTablePhase::CatalogStaged; } #[inline] @@ -406,7 +339,6 @@ impl CreateTableProgress { )); } if let Some(trx) = self.trx.take() - && trx.engine().is_some() && let Err(err) = trx.rollback_catalog_ddl().await && cleanup_error.is_none() { @@ -1329,6 +1261,9 @@ impl AcceptedExecution for AcceptedCreateTable { #[inline] async fn handle_panic(&mut self, _panic: Box) -> CompletionErrorBridge { + if let Some(progress) = self.progress.as_mut() { + progress.park_active_transaction(); + } self.scope.handle_panic(); let phase = match self.progress.as_ref() { Some(progress) => progress.phase, @@ -1392,17 +1327,19 @@ impl AcceptedCreateTable { .reach_phase(TableDdlTestPhase::CreatePrivateTransactionBegun) .await; - let catalog_objects = progress.take_catalog_objects(); - let exec_res = execute_create_table_catalog_staging( - &engine.catalog().storage, - progress - .trx - .as_mut() - .unwrap_or_else(|| panic!("CREATE staging requires private transaction")), - table_id, - catalog_objects, - ) - .await; + let metadata = Arc::clone(progress.metadata()); + let exec_res = engine + .catalog() + .storage + .stage_create_table( + progress + .trx + .as_mut() + .unwrap_or_else(|| panic!("CREATE staging requires private transaction")), + table_id, + &metadata, + ) + .await; if let Err(err) = exec_res { return Err(CompletionErrorBridge::capture_runtime_or_fatal( progress @@ -1533,7 +1470,7 @@ enum DropTablePhase { struct DropTableProgress { plan: DropTablePlan, phase: DropTablePhase, - trx: Option, + trx: Option, } impl DropTableProgress { @@ -1545,6 +1482,13 @@ impl DropTableProgress { trx: None, } } + + #[inline] + fn park_active_transaction(&mut self) { + if let Some(trx) = self.trx.take() { + trx.park(); + } + } } /// Caller-prepared DROP TABLE awaiting mandatory runtime capacity. @@ -1623,6 +1567,9 @@ impl AcceptedExecution for AcceptedDropTable { #[inline] async fn handle_panic(&mut self, _panic: Box) -> CompletionErrorBridge { + if let Some(progress) = self.progress.as_mut() { + progress.park_active_transaction(); + } self.scope.handle_panic(); let phase = match self.progress.as_ref() { Some(progress) => progress.phase, @@ -1703,16 +1650,18 @@ impl AcceptedDropTable { .await; let metadata = table.metadata().clone(); - let exec_res = execute_drop_table_catalog_cascade( - &engine.catalog().storage, - progress - .trx - .as_mut() - .unwrap_or_else(|| panic!("DROP cascade requires private transaction")), - table_id, - &metadata, - ) - .await; + let exec_res = engine + .catalog() + .storage + .stage_drop_table( + progress + .trx + .as_mut() + .unwrap_or_else(|| panic!("DROP cascade requires private transaction")), + table_id, + &metadata, + ) + .await; if let Err(err) = exec_res { if let Some(trx) = progress.trx.take() && let Err(rollback_err) = trx.rollback_catalog_ddl().await @@ -1880,135 +1829,6 @@ fn reject_user_table_primary_key_indexes( Ok(()) } -/// Stage the catalog rows for a newly allocated table. -/// -/// `table_id` is allocated atomically before this call. Every child key is -/// derived by enumerating validated metadata inside that fresh table-id -/// namespace, so catalog insert Operation failures are invariant violations. -#[inline] -async fn execute_create_table_catalog_staging( - storage: &CatalogStorage, - trx: &mut Transaction, - table_id: TableID, - catalog_objects: CreateTableCatalogObjects, -) -> RuntimeResult<()> { - let CreateTableCatalogObjects { - table, - columns, - indexes, - index_columns, - } = catalog_objects; - trx.stage_catalog_statement(async |stmt| { - storage.tables().insert(stmt, &table).await?; - - for column_object in columns { - storage.columns().insert(stmt, &column_object).await?; - } - for index_object in indexes { - storage.indexes().insert(stmt, &index_object).await?; - } - for index_column_object in index_columns { - storage - .index_columns() - .insert(stmt, &index_column_object) - .await?; - } - - let existing = stmt - .effects_mut() - .set_ddl_redo(DDLRedo::CreateTable(table_id)); - // A catalog DDL statement stages exactly one logical DDL effect; the - // create-table path owns the empty redo slot until this point. - assert!( - existing.is_none(), - "create-table catalog staging found existing DDL redo: table_id={table_id}, existing_ddl={existing:?}" - ); - Ok(()) - }) - .await -} - -#[inline] -async fn execute_drop_table_catalog_cascade( - storage: &CatalogStorage, - trx: &mut Transaction, - table_id: TableID, - metadata: &TableMetadata, -) -> RuntimeResult<()> { - trx.stage_catalog_statement(async |stmt| { - let index_columns_deleted = storage - .index_columns() - .delete_by_table_id(stmt, table_id) - .await?; - let indexes_deleted = storage - .indexes() - .delete_by_table_id(stmt, table_id) - .await?; - let columns_deleted = storage - .columns() - .delete_by_table_id(stmt, table_id) - .await?; - let table_deleted = storage - .tables() - .delete_by_id(stmt, table_id) - .await?; - assert!( - table_deleted, - "drop-table catalog invariant violated: validated table row is missing, table_id={table_id}" - ); - storage - .table_replay_silent_watermarks() - .delete_by_table_id(stmt, table_id) - .await?; - - assert_drop_catalog_delete_counts( - table_id, - metadata, - columns_deleted, - indexes_deleted, - index_columns_deleted, - ); - - assert!( - stmt.effects_mut() - .set_ddl_redo(DDLRedo::DropTable(table_id)) - .is_none(), - "drop-table catalog invariant violated: statement already has DDL redo, table_id={table_id}" - ); - Ok(()) - }) - .await -} - -#[inline] -fn assert_drop_catalog_delete_counts( - table_id: TableID, - metadata: &TableMetadata, - columns_deleted: usize, - indexes_deleted: usize, - index_columns_deleted: usize, -) { - let expected_index_columns = metadata - .idx - .active_indexes() - .map(|(_, spec)| spec.cols.len()) - .sum::(); - assert_eq!( - columns_deleted, - metadata.col.col_count(), - "drop-table catalog invariant violated: column delete count mismatch, table_id={table_id}" - ); - assert_eq!( - indexes_deleted, - metadata.idx.active_index_count(), - "drop-table catalog invariant violated: index delete count mismatch, table_id={table_id}" - ); - assert_eq!( - index_columns_deleted, expected_index_columns, - "drop-table catalog invariant violated: index-column delete count mismatch, table_id={table_id}" - ); -} - #[inline] fn finish_drop_table_runtime_retention( engine: &EngineCore, @@ -2116,6 +1936,7 @@ fn validate_primary_key_contract( pub(crate) mod tests { use super::*; use crate::catalog::storage::tables::TABLE_ID_TABLES; + use crate::catalog::storage::tests::mark_catalog_ddl; use crate::catalog::tests::{ assert_dropped_table_floor, assert_dropped_table_runtime, assert_no_dropped_table_operational_state, wait_for_dropped_table_floor, @@ -2135,7 +1956,9 @@ pub(crate) mod tests { use crate::lock::tests::{LockDebugEntryState, TestLockOwner, debug_snapshot}; use crate::lock::{LockMode, LockOwner, LockResource, TableLockMode}; use crate::log::redo::DDLRedo; - use crate::session::tests::{SessionTestExt, active_operation_count, remove_session_for_test}; + use crate::session::tests::{ + SessionTestExt, active_operation_count, active_operation_snapshot, remove_session_for_test, + }; use crate::table::TableTerminal; use crate::table::tests::*; use crate::trx::MAX_SNAPSHOT_TS; @@ -3875,14 +3698,11 @@ pub(crate) mod tests { .await .disclose()?; assert!(deleted); - let old = stmt - .effects_mut() - .set_ddl_redo(DDLRedo::DropTable(table_id)); - debug_assert!(old.is_none()); Ok(()) }) .await .unwrap(); + mark_catalog_ddl(&mut corrupt_trx, DDLRedo::DropTable(table_id)); corrupt_trx.commit().await.unwrap(); let mut drop_session = engine.new_session().unwrap(); @@ -3980,6 +3800,42 @@ pub(crate) mod tests { }); } + #[test] + fn test_create_table_execution_panic_parks_active_private_transaction() { + smol::block_on(async { + let temp_dir = TempDir::new().unwrap(); + let engine = lightweight_test_engine(&temp_dir, "create_table_private_panic").await; + let mut session = engine.new_session().unwrap(); + let session_id = session.id(); + let (table_spec, index_specs) = drop_table_test_spec(); + engine + .inner() + .table_ddl_test + .set_panic_phase(Some(TableDdlTestPhase::CreatePrivateTransactionBegun)); + + let err = session + .create_table(table_spec, index_specs) + .await + .unwrap_err(); + + assert_eq!( + err.report().downcast_ref::().copied(), + Some(FatalError::MandatoryTaskPanic) + ); + assert_eq!(active_operation_count(&engine.inner().session_registry), 1); + let snapshot = active_operation_snapshot(&engine.inner().session_registry, session_id); + assert_eq!( + snapshot.state, + crate::trx::SessionOperationState::FailedRetained + ); + assert!(snapshot.trx_id.is_some()); + + remove_session_for_test(&engine.inner().session_registry, session_id); + drop(session); + engine.shutdown(); + }); + } + #[test] fn test_drop_table_rejects_same_session_explicit_table_lock() { smol::block_on(async { diff --git a/doradb-storage/src/recovery/mod.rs b/doradb-storage/src/recovery/mod.rs index 1bf4d754..d94582a4 100644 --- a/doradb-storage/src/recovery/mod.rs +++ b/doradb-storage/src/recovery/mod.rs @@ -1238,6 +1238,7 @@ mod tests { }; use crate::buffer::PoolRole; use crate::catalog::storage::publish_first_redo_log_seq_for_test; + use crate::catalog::storage::tests::mark_catalog_ddl; use crate::catalog::{ ActiveIndexSpec, ColumnAttributes, ColumnSpec, IndexAttributes, IndexColumnObject, IndexKey, IndexObject, IndexOrder, IndexSpec, TableMetadata, TableObject, TableSpec, @@ -1861,15 +1862,17 @@ mod tests { ) .await .disclose()?; - let old = stmt.effects_mut().set_ddl_redo(DDLRedo::CreateIndex { - table_id, - index_no: 1, - }); - debug_assert!(old.is_none()); Ok(()) }) .await .unwrap(); + mark_catalog_ddl( + &mut trx, + DDLRedo::CreateIndex { + table_id, + index_no: 1, + }, + ); let cts = trx.commit().await.unwrap(); drop(session); cts @@ -1902,15 +1905,17 @@ mod tests { .await .disclose()? ); - let old = stmt.effects_mut().set_ddl_redo(DDLRedo::DropIndex { - table_id, - index_no: 1, - }); - debug_assert!(old.is_none()); Ok(()) }) .await .unwrap(); + mark_catalog_ddl( + &mut trx, + DDLRedo::DropIndex { + table_id, + index_no: 1, + }, + ); let cts = trx.commit().await.unwrap(); drop(session); cts diff --git a/doradb-storage/src/session.rs b/doradb-storage/src/session.rs index 9320aa6b..13935d57 100644 --- a/doradb-storage/src/session.rs +++ b/doradb-storage/src/session.rs @@ -34,9 +34,9 @@ use crate::table::{ prepare_freeze_table_operation, prepare_mem_index_cleanup_operation, }; use crate::trx::{ - RedoRetentionScope, ReleasedTransactionLocks, SessionOperationEntry, SessionOperationKind, - SessionOperationState, Transaction, TrxInner, prepare_catalog_redo_maintenance_operation, - prepare_redo_truncation_operation, + PrivateTransaction, RedoRetentionScope, ReleasedTransactionLocks, SessionOperationEntry, + SessionOperationKind, SessionOperationState, Transaction, TrxInner, + prepare_catalog_redo_maintenance_operation, prepare_redo_truncation_operation, }; use error_stack::{Report, ResultExt}; use event_listener::EventListener; @@ -44,9 +44,7 @@ use futures::future::select_all; use parking_lot::Mutex; use std::any::Any; use std::cell::Cell; -use std::fmt::Display; use std::future::Future; -use std::marker::PhantomData; use std::mem::replace; use std::ops::Deref; use std::sync::atomic::{AtomicU64, Ordering}; @@ -219,7 +217,7 @@ impl AcceptedDdlScope { /// Start one mandatory-owned nested private transaction. #[inline] - pub(crate) fn begin_private_trx(&mut self) -> LifecycleResult { + pub(crate) fn begin_private_trx(&mut self) -> LifecycleResult { self.operation.begin_private_trx() } @@ -306,10 +304,14 @@ impl PreparedMaintenanceScope { Ok(table) } - /// Synchronously consume caller preparation into accepted authority. + /// Synchronously consume caller preparation and execution state into accepted authority. #[inline] - pub(crate) fn accept(self) -> AcceptedMaintenanceScope { + fn accept(self, execution: E) -> AcceptedMaintenanceScope + where + E: MaintenanceExecution, + { AcceptedMaintenanceScope { + execution: Some(execution), operation: self.operation.into_mandatory(), finish_state: MaintenanceFinishState::Executing, } @@ -322,35 +324,38 @@ enum MaintenanceFinishState { FailedRetained, } -/// Runtime-owned maintenance operation and its transferred logical locks. -pub(crate) struct AcceptedMaintenanceScope { +/// Runtime-owned maintenance execution and its transferred logical locks. +pub(crate) struct AcceptedMaintenanceScope +where + E: MaintenanceExecution, +{ + execution: Option, operation: MandatoryOperationGuard, finish_state: MaintenanceFinishState, } -impl AcceptedMaintenanceScope { - /// Return the retained engine runtime. - #[inline] - pub(crate) fn engine(&self) -> &SessionRuntime { - &self.operation.runtime - } - - /// Start one mandatory-owned nested private transaction. - #[inline] - pub(crate) fn begin_private_trx(&mut self) -> LifecycleResult { - self.operation.begin_private_trx() - } +impl AcceptedExecution for AcceptedMaintenanceScope +where + E: MaintenanceExecution, +{ + type Output = E::Output; - /// Verify nested state before returning from accepted execution. #[inline] - pub(crate) fn mark_terminal_ready(&mut self) { + async fn execute(&mut self) -> CompletionResult { + let result = self + .execution + .as_mut() + .unwrap_or_else(|| panic!("accepted maintenance execution is missing")) + .execute(&self.operation.runtime) + .await; self.operation.assert_finish_ready(); self.finish_state = MaintenanceFinishState::TerminalReady; + result } - /// Publish normal completion or retain an invalid finish state. #[inline] - pub(crate) fn finish(&mut self) { + fn finish(&mut self) { + drop(self.execution.take()); let state = replace( &mut self.finish_state, MaintenanceFinishState::FailedRetained, @@ -369,104 +374,85 @@ impl AcceptedMaintenanceScope { } } - /// Retain unsafe nested ownership before the supervisor publishes poison. #[inline] - pub(crate) fn handle_panic(&mut self) { + async fn handle_panic(&mut self, _panic: Box) -> CompletionErrorBridge { + let diagnostic = self + .execution + .as_ref() + .unwrap_or_else(|| panic!("accepted maintenance execution is missing")) + .panic_diagnostic(); + drop(self.execution.take()); self.operation.fail_retained(); self.finish_state = MaintenanceFinishState::FailedRetained; + CompletionErrorBridge::capture( + Report::new(FatalError::MandatoryTaskPanic).attach(diagnostic), + ) } } -impl SessionRuntimeAccess for AcceptedMaintenanceScope { - #[inline] - fn runtime(&self) -> &SessionRuntime { - &self.operation.runtime - } -} - -/// Operation-specific execution specification used by the shared maintenance carrier. -pub(crate) trait MaintenanceExecutionSpec: Send + 'static { +/// Operation-specific state and behavior owned by one accepted maintenance scope. +pub(crate) trait MaintenanceExecution: Send + 'static { /// Terminal output delivered to the maintenance observer. type Output: Send + 'static; - /// Domain resources retained outside the panic-caught execution future. - type Resources: Send + 'static; - /// Mutable diagnostic data attached after an unexpected execution panic. - type PanicLabel: Display + Send + 'static; /// Stable mandatory-runtime diagnostic label. const LABEL: &'static str; - /// Execute one accepted operation while borrowing its retained resources. + /// Execute one accepted operation using its retained session runtime. fn execute( - scope: &mut AcceptedMaintenanceScope, - resources: &mut Self::Resources, - panic_label: &mut Self::PanicLabel, + &mut self, + runtime: &SessionRuntime, ) -> impl Future> + Send; + + /// Build the diagnostic attached after an unexpected execution panic. + fn panic_diagnostic(&self) -> String; } /// Shared caller-prepared carrier for one maintenance execution body. -/// -/// Resource declaration before the maintenance scope preserves domain-resource -/// release before logical locks and the voluntary operation terminal edge. -pub(crate) struct PreparedMaintenanceExecution +pub(crate) struct PreparedMaintenanceExecution where - S: MaintenanceExecutionSpec, + E: MaintenanceExecution, { - resources: S::Resources, + execution: E, scope: PreparedMaintenanceScope, - panic_label: S::PanicLabel, metadata: MandatoryTaskMetadata, - spec: PhantomData, } -impl PreparedMaintenanceExecution +impl PreparedMaintenanceExecution where - S: MaintenanceExecutionSpec, + E: MaintenanceExecution, { /// Build one global catalog/redo maintenance operation. #[inline] - pub(crate) fn global( - scope: PreparedMaintenanceScope, - resources: S::Resources, - panic_label: S::PanicLabel, - ) -> Self { - let metadata = MandatoryTaskMetadata::operation(S::LABEL, Some(scope.key())); + pub(crate) fn global(scope: PreparedMaintenanceScope, execution: E) -> Self { + let metadata = MandatoryTaskMetadata::operation(E::LABEL, Some(scope.key())); Self { - resources, + execution, scope, - panic_label, metadata, - spec: PhantomData, } } /// Build one table-scoped maintenance operation. #[inline] - pub(crate) fn table( - scope: PreparedMaintenanceScope, - resources: S::Resources, - panic_label: S::PanicLabel, - table_id: TableID, - ) -> Self { - let metadata = MandatoryTaskMetadata::table_operation(S::LABEL, scope.key(), table_id); + pub(crate) fn table(scope: PreparedMaintenanceScope, execution: E, table_id: TableID) -> Self { + let metadata = MandatoryTaskMetadata::table_operation(E::LABEL, scope.key(), table_id); Self { - resources, + execution, scope, - panic_label, metadata, - spec: PhantomData, } } } -impl PreparedExecution for PreparedMaintenanceExecution +impl PreparedExecution for PreparedMaintenanceExecution where - S: MaintenanceExecutionSpec, + E: MaintenanceExecution, { - type Output = S::Output; - type Accepted = AcceptedMaintenanceExecution; + type Output = E::Output; + type Accepted = AcceptedMaintenanceScope; - const LABEL: &'static str = S::LABEL; + const LABEL: &'static str = E::LABEL; #[inline] fn metadata(&self) -> MandatoryTaskMetadata { @@ -476,61 +462,11 @@ where #[inline] fn accept(self) -> Self::Accepted { let Self { - resources, + execution, scope, - panic_label, metadata: _, - spec: _, } = self; - AcceptedMaintenanceExecution { - resources: Some(resources), - scope: scope.accept(), - panic_label, - spec: PhantomData, - } - } -} - -/// Shared mandatory-runtime owner for one accepted maintenance execution. -pub(crate) struct AcceptedMaintenanceExecution -where - S: MaintenanceExecutionSpec, -{ - resources: Option, - scope: AcceptedMaintenanceScope, - panic_label: S::PanicLabel, - spec: PhantomData, -} - -impl AcceptedExecution for AcceptedMaintenanceExecution -where - S: MaintenanceExecutionSpec, -{ - type Output = S::Output; - - #[inline] - fn execute(&mut self) -> impl Future> + Send { - S::execute( - &mut self.scope, - self.resources - .as_mut() - .unwrap_or_else(|| panic!("accepted maintenance resources are missing")), - &mut self.panic_label, - ) - } - - #[inline] - fn finish(&mut self) { - drop(self.resources.take()); - self.scope.finish(); - } - - #[inline] - async fn handle_panic(&mut self, _panic: Box) -> CompletionErrorBridge { - self.scope.handle_panic(); - CompletionErrorBridge::capture( - Report::new(FatalError::MandatoryTaskPanic).attach(self.panic_label.to_string()), - ) + scope.accept(execution) } } @@ -690,6 +626,20 @@ impl AdmittedSessionRuntime<'_> { } } +/// Shared runtime view implemented by observer and foreground authorities. +pub(crate) trait SessionRuntimeAccess { + /// Returns the retained exact session runtime. + fn runtime(&self) -> &SessionRuntime; + /// Returns immutable shared engine capabilities. + fn engine(&self) -> &EngineCore { + self.runtime().core() + } + /// Borrows the canonical pool-guard bundle. + fn pool_guards(&self) -> &PoolGuards { + self.runtime().pool_guards() + } +} + /// Strong operation-local reachability to one exact session state. /// /// This typed `Arc` wrapper pins the state reached by a public weak handle. @@ -768,6 +718,13 @@ impl Deref for SessionRuntime { } } +impl SessionRuntimeAccess for SessionRuntime { + #[inline] + fn runtime(&self) -> &SessionRuntime { + self + } +} + /// Weak, non-cloneable public session capability bound to one engine instance. /// /// The engine owns the strong session state in its internal session registry. @@ -1578,20 +1535,6 @@ impl Drop for Session { } } -/// Shared runtime view implemented by observer and foreground authorities. -pub(crate) trait SessionRuntimeAccess { - /// Returns the retained exact session runtime. - fn runtime(&self) -> &SessionRuntime; - /// Returns immutable shared engine capabilities. - fn engine(&self) -> &EngineCore { - self.runtime().core() - } - /// Borrows the canonical pool-guard bundle. - fn pool_guards(&self) -> &PoolGuards { - self.runtime().pool_guards() - } -} - /// One strong runtime/session pin for observer or inspection work. /// /// The creating `Session` method establishes whether normal healthy-runtime or @@ -1983,8 +1926,9 @@ impl MandatoryOperationGuard { /// Mandatory ownership keeps the active slot bound to `entry`, so private /// installation needs only the entry mutex rather than the lifecycle lock. #[inline] - pub(crate) fn begin_private_trx(&mut self) -> LifecycleResult { + pub(crate) fn begin_private_trx(&mut self) -> LifecycleResult { self.reclaim_transaction_authority(); + self.entry.validate_private_transaction_begin()?; let authority = self.authority.take().unwrap_or_else(|| { panic!( "private transaction begin requires family authority: key={}", @@ -3229,27 +3173,24 @@ impl TrxAttachment { } } -/// Starts one private transaction under an existing DDL or maintenance owner. +/// Starts one private transaction under an existing DDL owner. #[inline] fn begin_private_transaction( runtime: &SessionRuntime, entry: &Arc, authority: Box, -) -> LifecycleResult { +) -> LifecycleResult { let kind = entry.kind(); assert!( - matches!( - kind, - SessionOperationKind::Ddl | SessionOperationKind::Maintenance - ), - "private transaction requires DDL or maintenance authority: key={}, kind={}", + kind == SessionOperationKind::Ddl, + "private transaction requires DDL authority: key={}, kind={}", entry.key(), kind.label() ); let inner = Box::new(TrxInner::private()); - Ok(runtime + runtime .trx_sys - .begin_private_trx(runtime.downgrade(), entry, inner, authority)) + .begin_private_trx(runtime.clone(), entry, inner, authority) } async fn wait_for_maintenance_boundary( @@ -3324,8 +3265,8 @@ pub(crate) mod tests { use crate::trx::retention::{ RedoTruncationBlocker, tests::install_redo_cleanup_before_unlink_hook, }; - use crate::trx::tests::trx_inner; - use crate::trx::{MIN_ACTIVE_TRX_ID, MIN_SNAPSHOT_TS, TrxInner}; + use crate::trx::tests::{private_transaction_inner_ptr, trx_inner}; + use crate::trx::{MIN_ACTIVE_TRX_ID, MIN_SNAPSHOT_TS, SessionOperationSnapshot, TrxInner}; use crate::value::{Val, ValKind}; use futures::task::noop_waker; use std::cell::RefCell; @@ -3763,6 +3704,15 @@ pub(crate) mod tests { .count() } + /// Return the coherent snapshot of one test session's active operation. + #[inline] + pub(crate) fn active_operation_snapshot( + registry: &SessionRegistry, + session_id: SessionID, + ) -> SessionOperationSnapshot { + active_operation_entry_for_test(registry, session_id).inspect() + } + /// Returns whether a registered session currently owns its public transaction cache. #[inline] pub(crate) fn session_has_public_trx_cache( @@ -5006,9 +4956,7 @@ pub(crate) mod tests { .await .unwrap(); let session = engine.new_session().unwrap(); - let operation = session - .pin_operation(SessionOperationKind::Maintenance) - .unwrap(); + let operation = session.pin_operation(SessionOperationKind::Ddl).unwrap(); let key = operation.key(); let entry = Arc::clone(&operation.entry); let state = Arc::clone(operation.runtime.state()); @@ -5025,14 +4973,38 @@ pub(crate) mod tests { entry.inspect().state, SessionOperationState::Mandatory(None) ); - let trx = operation.begin_private_trx().unwrap(); - let first_inner = entry - .inner_ptr_for_test() - .expect("private transaction entry must retain its checked-in core"); + let mut trx = operation.begin_private_trx().unwrap(); + let first_inner = private_transaction_inner_ptr(&trx); assert_ne!( first_inner, public_cache_ptr, "private transaction must use a core distinct from the parked public cache" ); + assert_eq!( + entry.inner_ptr_for_test(), + None, + "running private transaction must hold its core outside the entry" + ); + assert_eq!( + entry.inspect().state, + SessionOperationState::Mandatory(Some(crate::trx::InternalTrxState::Running)) + ); + let nested_begin_err = match operation.begin_private_trx() { + Ok(_) => panic!("mandatory operation cannot start a second private transaction"), + Err(err) => err, + }; + assert_eq!( + *nested_begin_err.current_context(), + LifecycleError::ExistingTransaction + ); + trx.stage_statement(async |_stmt| Ok(())).await.unwrap(); + assert_eq!(private_transaction_inner_ptr(&trx), first_inner); + assert_eq!(entry.inner_ptr_for_test(), None); + trx.stage_statement(async |_stmt| Ok(())).await.unwrap(); + assert_eq!(private_transaction_inner_ptr(&trx), first_inner); + assert_eq!( + entry.inspect().state, + SessionOperationState::Mandatory(Some(crate::trx::InternalTrxState::Running)) + ); assert_eq!( state .lifecycle @@ -5052,7 +5024,7 @@ pub(crate) mod tests { assert!(Arc::ptr_eq(&resolved, &entry)); assert_eq!(state.lifecycle.lock().next_operation_id, 2); - trx.rollback().await.unwrap(); + trx.rollback_catalog_ddl().await.unwrap(); let snapshot = entry.inspect(); assert_eq!(snapshot.state, SessionOperationState::Mandatory(None)); assert_eq!(snapshot.trx_id, None); @@ -5075,13 +5047,12 @@ pub(crate) mod tests { assert!(state.lifecycle.lock().change_ev.is_none()); let replacement = operation.begin_private_trx().unwrap(); - let second_inner = entry - .inner_ptr_for_test() - .expect("replacement private transaction must retain its checked-in core"); + let second_inner = private_transaction_inner_ptr(&replacement); assert_ne!( second_inner, public_cache_ptr, "each private transaction must remain separate from the public cache" ); + assert_eq!(entry.inner_ptr_for_test(), None); assert_eq!( state .lifecycle @@ -5092,7 +5063,7 @@ pub(crate) mod tests { Some(public_cache_ptr), "sequential private transactions must leave the public cache parked" ); - replacement.rollback().await.unwrap(); + replacement.rollback_catalog_ddl().await.unwrap(); operation.assert_finish_ready(); operation.finish(); diff --git a/doradb-storage/src/table/access.rs b/doradb-storage/src/table/access.rs index 9eddb050..c43aa3b6 100644 --- a/doradb-storage/src/table/access.rs +++ b/doradb-storage/src/table/access.rs @@ -7872,9 +7872,7 @@ mod tests { ); } - let rows = &stmt - .effects_mut() - .redo_for_test() + let rows = &stmt_tests::statement_redo(stmt) .dml .get(&table_id) .unwrap() @@ -7937,9 +7935,7 @@ mod tests { }) }) .await?; - let rows = &stmt - .effects_mut() - .redo_for_test() + let rows = &stmt_tests::statement_redo(stmt) .dml .get(&table_id) .unwrap() diff --git a/doradb-storage/src/table/gc.rs b/doradb-storage/src/table/gc.rs index 88997463..48c9d6bd 100644 --- a/doradb-storage/src/table/gc.rs +++ b/doradb-storage/src/table/gc.rs @@ -12,14 +12,13 @@ use crate::index::{ UniqueMemIndex, }; use crate::runtime::mandatory::PreparedExecution; +use crate::runtime::yield_now; use crate::session::{ - AcceptedMaintenanceScope, MaintenanceExecutionSpec, PreparedMaintenanceExecution, - PreparedMaintenanceScope, + MaintenanceExecution, PreparedMaintenanceExecution, PreparedMaintenanceScope, SessionRuntime, }; -use crate::trx::{Transaction, TrxReadProof}; +use crate::trx::PrivateSnapshot; use crate::value::Val; use error_stack::{Report, ResultExt}; -use std::fmt; use std::sync::Arc; /// Aggregate result for a full-scan user-table secondary MemIndex cleanup pass. @@ -186,57 +185,44 @@ enum DeleteOverlayProof { #[derive(Clone, Copy, Debug)] enum MemIndexCleanupPhase { Starting, - TransactionActive, + PrivateSnapshotActive, Scanning, - RollingBack, + ReleasingPrivateSnapshot, Finished, } -struct MemIndexCleanupPanicLabel(MemIndexCleanupPhase); - -impl fmt::Display for MemIndexCleanupPanicLabel { - #[inline] - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "accepted secondary MemIndex cleanup panicked: phase={:?}", - self.0 - ) - } -} - enum CleanupIteration { Retry, Finished(RuntimeResult), } -struct MemIndexCleanupResources { - active_trx: Option, +struct MemIndexCleanupExecution { + active_snapshot: Option, table: Arc
, clean_live_entries: bool, + phase: MemIndexCleanupPhase, } -struct MemIndexCleanupExecution; - -impl MaintenanceExecutionSpec for MemIndexCleanupExecution { +impl MaintenanceExecution for MemIndexCleanupExecution { type Output = MemIndexCleanupOutcome; - type Resources = MemIndexCleanupResources; - type PanicLabel = MemIndexCleanupPanicLabel; const LABEL: &'static str = "cleanup_secondary_mem_indexes"; - async fn execute( - scope: &mut AcceptedMaintenanceScope, - resources: &mut Self::Resources, - panic_label: &mut Self::PanicLabel, - ) -> CompletionResult { - let result = execute_mem_index_cleanup_inner(scope, resources, &mut panic_label.0) + async fn execute(&mut self, runtime: &SessionRuntime) -> CompletionResult { + let result = execute_mem_index_cleanup_inner(runtime, self) .await .map_err(CompletionErrorBridge::capture_runtime_or_fatal); - scope.mark_terminal_ready(); - panic_label.0 = MemIndexCleanupPhase::Finished; + self.phase = MemIndexCleanupPhase::Finished; result } + + #[inline] + fn panic_diagnostic(&self) -> String { + format!( + "accepted secondary MemIndex cleanup panicked: phase={:?}", + self.phase + ) + } } /// Prepare secondary MemIndex cleanup for an exact live table runtime. @@ -248,12 +234,12 @@ pub(crate) fn prepare_mem_index_cleanup_operation( let table_id = table.table_id(); PreparedMaintenanceExecution::::table( scope, - MemIndexCleanupResources { - active_trx: None, + MemIndexCleanupExecution { + active_snapshot: None, table, clean_live_entries, + phase: MemIndexCleanupPhase::Starting, }, - MemIndexCleanupPanicLabel(MemIndexCleanupPhase::Starting), table_id, ) } @@ -271,60 +257,46 @@ pub(crate) fn prepare_mem_index_cleanup_operation( /// retained by policy and no live delay is reported. Obsolete delete overlays /// are cleaned independently in either case. async fn execute_mem_index_cleanup_inner( - scope: &mut AcceptedMaintenanceScope, - resources: &mut MemIndexCleanupResources, - phase: &mut MemIndexCleanupPhase, + runtime: &SessionRuntime, + execution: &mut MemIndexCleanupExecution, ) -> RuntimeOrFatalResult { - let table = Arc::clone(&resources.table); - let clean_live_entries = resources.clean_live_entries; - let runtime = scope.engine().clone(); - let trx_sys = &runtime.trx_sys; + let table = Arc::clone(&execution.table); + let clean_live_entries = execution.clean_live_entries; + let engine = runtime.core(); + let trx_sys = &engine.trx_sys; let pool_guards = runtime.pool_guards(); loop { - let trx = scope - .begin_private_trx() - .change_context(RuntimeError::TableAccess) - .attach_with(|| { - format!( - "operation=cleanup_secondary_mem_indexes, table_id={}, phase=begin_transaction", - table.table_id() - ) - })?; - resources.active_trx = Some(trx); - *phase = MemIndexCleanupPhase::TransactionActive; - let cleanup_sts = resources - .active_trx + execution.active_snapshot = Some(trx_sys.register_private_snapshot()); + execution.phase = MemIndexCleanupPhase::PrivateSnapshotActive; + let cleanup_sts = execution + .active_snapshot .as_ref() - .unwrap_or_else(|| panic!("cleanup transaction disappeared after installation")) + .unwrap_or_else(|| panic!("cleanup private snapshot disappeared after registration")) .sts(); let min_active_sts = trx_sys.calc_min_active_sts_for_gc(); #[cfg(test)] - scope - .engine() + engine .maintenance_test - .run_cleanup_after_trx_start_hook() + .run_cleanup_after_private_snapshot_hook() .await; - *phase = MemIndexCleanupPhase::Scanning; + execution.phase = MemIndexCleanupPhase::Scanning; let iteration = { - let trx = resources - .active_trx - .as_mut() - .unwrap_or_else(|| panic!("cleanup transaction disappeared before checkout")); - let checkout = trx - .checkout() - .change_context(RuntimeError::TableAccess) - .attach_with(|| { - format!( - "operation=cleanup_secondary_mem_indexes, table_id={}, phase=checkout_transaction", + let private_snapshot = execution.active_snapshot.as_ref().unwrap_or_else(|| { + panic!("cleanup private snapshot disappeared before root capture") + }); + engine + .poisoner + .ensure_healthy() + .map_err(|err| { + RuntimeOrFatalError::from(err.attach(format!( + "operation=cleanup_secondary_mem_indexes, table_id={}, phase=check_engine_health", table.table_id() - ) - }) - .map_err(RuntimeOrFatalError::from)?; - let proof = checkout.inner().ctx().read_proof(); - let snapshot = table.capture_mem_index_cleanup_snapshot(min_active_sts, &proof); + ))) + })?; + let snapshot = + table.capture_mem_index_cleanup_snapshot(min_active_sts, private_snapshot); if !snapshot.is_visible_to(cleanup_sts) { drop(snapshot); - drop(checkout); CleanupIteration::Retry } else { let cleanup_res = table @@ -335,29 +307,24 @@ async fn execute_mem_index_cleanup_inner( ) .await; drop(snapshot); - drop(checkout); CleanupIteration::Finished(cleanup_res) } }; - *phase = MemIndexCleanupPhase::RollingBack; - let trx = resources - .active_trx - .take() - .unwrap_or_else(|| panic!("cleanup transaction missing before rollback")); - let rollback_res = trx.rollback_table_maintenance().await; + execution.phase = MemIndexCleanupPhase::ReleasingPrivateSnapshot; + drop(execution.active_snapshot.take()); match iteration { CleanupIteration::Retry => { - rollback_res?; // This retry is intentionally unbounded. The captured root was - // published after the transaction started, so retry with a - // fresh STS. Transaction starts and root fences share one + // published after the private snapshot registered, so retry + // with a fresh STS. Snapshot registration and root fences share one // monotonic timestamp source, making the next STS newer than - // this fence unless another root publication races again. The - // awaited rollback above keeps retries from becoming a tight - // busy loop. + // this fence unless another root publication races again. + // Explicitly yield after deregistration so repeated publishers + // cannot turn the unbounded retry into a tight loop. + yield_now().await; } CleanupIteration::Finished(cleanup_res) => { - return finish_secondary_mem_index_cleanup(cleanup_res, rollback_res); + return cleanup_res.map_err(RuntimeOrFatalError::from); } } } @@ -365,15 +332,15 @@ async fn execute_mem_index_cleanup_inner( impl Table { #[inline] - fn capture_mem_index_cleanup_snapshot<'ctx>( + fn capture_mem_index_cleanup_snapshot<'snapshot>( &self, min_active_sts: TrxID, - proof: &TrxReadProof<'ctx>, - ) -> MemIndexCleanupSnapshot<'ctx> { + private_snapshot: &'snapshot PrivateSnapshot, + ) -> MemIndexCleanupSnapshot<'snapshot> { let layout = self.layout_snapshot(); - let (root, root_metadata) = self.with_active_root(proof, |root| { + let (root, root_metadata) = self.with_private_snapshot_root(private_snapshot, |root| { ( - TableRootSnapshot::from_active_root(root, proof), + TableRootSnapshot::from_private_snapshot(root, private_snapshot), Arc::clone(&root.metadata), ) }); @@ -755,18 +722,6 @@ impl Table { } } -#[inline] -fn finish_secondary_mem_index_cleanup( - cleanup_res: RuntimeResult, - rollback_res: RuntimeOrFatalResult<()>, -) -> RuntimeOrFatalResult { - match (cleanup_res, rollback_res) { - (Ok(outcome), Ok(())) => Ok(outcome), - (Err(err), Ok(())) => Err(RuntimeOrFatalError::from(err)), - (_, Err(err)) => Err(err), - } -} - #[inline] async fn compare_delete_unique_cleanup_entry( index: &UniqueMemIndex

, @@ -814,28 +769,28 @@ async fn compare_delete_non_unique_cleanup_entry( #[cfg(test)] mod tests { - use super::finish_secondary_mem_index_cleanup; use crate::catalog::IndexNo; use crate::catalog::tests::wait_for_dropped_table_floor; use crate::engine::Engine; - use crate::error::{DataIntegrityError, LifecycleError, RuntimeError, RuntimeOrFatalError}; + use crate::error::{DataIntegrityError, FatalError, LifecycleError}; use crate::id::{RowID, TrxID}; use crate::index::IndexMask; use crate::session::tests::{ - SessionTestExt, assert_checkpoint_published, wait_for_checkpoint_purge, - wait_for_session_idle, + SessionTestExt, active_operation_snapshot, assert_checkpoint_published, + remove_session_for_test, wait_for_checkpoint_purge, wait_for_session_idle, }; use crate::table::CheckpointOutcome; use crate::table::persistence::test_hooks::set_test_checkpoint_after_trx_start_hook; use crate::table::tests::*; - use crate::trx::MAX_SNAPSHOT_TS; + use crate::trx::{MAX_SNAPSHOT_TS, tests::active_sts_count}; use crate::value::Val; - use error_stack::Report; + use smol::Timer; use std::future::Future; use std::sync::Arc; + use std::time::{Duration, Instant}; use tempfile::TempDir; - fn set_test_cleanup_after_trx_start_hook(engine: &Engine, hook: F) + fn set_test_cleanup_after_private_snapshot_hook(engine: &Engine, hook: F) where F: FnOnce() -> Fut + Send + 'static, Fut: Future + Send + 'static, @@ -843,28 +798,22 @@ mod tests { engine .inner() .maintenance_test - .install_cleanup_after_trx_start_hook(hook); + .install_cleanup_after_private_snapshot_hook(hook); } - #[test] - fn test_secondary_mem_index_cleanup_rollback_error_overrides_cleanup_error() { - let cleanup_err = Report::new(DataIntegrityError::InvalidPayload) - .change_context(RuntimeError::TableAccess); - let rollback_err = - Report::new(LifecycleError::Shutdown).change_context(RuntimeError::TableAccess); - let err = finish_secondary_mem_index_cleanup( - Err(cleanup_err), - Err(RuntimeOrFatalError::Runtime(rollback_err)), - ) - .unwrap_err(); - let RuntimeOrFatalError::Runtime(err) = err else { - panic!("Runtime rollback failure must remain Runtime"); - }; - assert_eq!( - err.downcast_ref::().copied(), - Some(LifecycleError::Shutdown) - ); - assert!(err.downcast_ref::().is_none()); + async fn wait_for_no_active_sts(engine: &Engine) { + let deadline = Instant::now() + Duration::from_secs(5); + loop { + let active = active_sts_count(&engine.inner().trx_sys); + if active == 0 { + return; + } + assert!( + Instant::now() < deadline, + "active STS registrations did not drain before MemIndex cleanup: active={active}" + ); + Timer::after(Duration::from_millis(10)).await; + } } #[test] @@ -884,7 +833,7 @@ mod tests { ); let mut checkpoint_session = engine.new_session().unwrap(); - set_test_cleanup_after_trx_start_hook(&engine, move || async move { + set_test_cleanup_after_private_snapshot_hook(&engine, move || async move { assert_checkpoint_published(&mut checkpoint_session, table_id).await; }); @@ -900,6 +849,44 @@ mod tests { }); } + #[test] + fn test_secondary_mem_index_cleanup_panic_releases_private_snapshot() { + smol::block_on(async { + let temp_dir = TempDir::new().unwrap(); + let engine = + evictable_test_engine(&temp_dir, 64u64 * 1024 * 1024, "cleanup_private_panic") + .await; + let table_id = create_table2_for_test(&engine).await; + wait_for_no_active_sts(&engine).await; + let mut session = engine.new_session().unwrap(); + let session_id = session.id(); + set_test_cleanup_after_private_snapshot_hook(&engine, || async { + panic!("injected MemIndex cleanup panic after private snapshot registration"); + }); + + let err = session + .cleanup_secondary_mem_indexes(table_id, true) + .await + .unwrap_err(); + + assert_eq!( + err.report().downcast_ref::().copied(), + Some(FatalError::MandatoryTaskPanic) + ); + let snapshot = active_operation_snapshot(&engine.inner().session_registry, session_id); + assert_eq!( + snapshot.state, + crate::trx::SessionOperationState::FailedRetained + ); + assert_eq!(snapshot.trx_id, None); + assert_eq!(active_sts_count(&engine.inner().trx_sys), 0); + + remove_session_for_test(&engine.inner().session_registry, session_id); + drop(session); + engine.shutdown(); + }); + } + #[test] fn test_dropped_mem_index_cleanup_observer_still_blocks_drop_until_terminal() { smol::block_on(async { @@ -910,7 +897,7 @@ mod tests { let mut cleanup_session = engine.new_session().unwrap(); let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); - set_test_cleanup_after_trx_start_hook(&engine, move || async move { + set_test_cleanup_after_private_snapshot_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); diff --git a/doradb-storage/src/table/mod.rs b/doradb-storage/src/table/mod.rs index 7c21a382..be2c73f7 100644 --- a/doradb-storage/src/table/mod.rs +++ b/doradb-storage/src/table/mod.rs @@ -53,7 +53,7 @@ use crate::quiescent::QuiescentGuard; use crate::row::ops::{RowUpdateInput, RowUpdateView, SelectKey, UpdateCol}; use crate::row::{RowPage, RowRead, var_len_for_insert}; use crate::runtime::yield_now; -use crate::trx::{TrxContext, TrxReadProof}; +use crate::trx::{PrivateSnapshot, TrxReadProof}; use crate::value::{PAGE_VAR_LEN_INLINE, Val}; use error_stack::{Report, ResultExt}; use parking_lot::Mutex; @@ -315,6 +315,15 @@ impl Table { self.storage.with_active_root(proof, f) } + /// Bind one active root observation under a private maintenance snapshot. + #[inline] + pub(crate) fn with_private_snapshot_root(&self, snapshot: &PrivateSnapshot, f: F) -> R + where + F: for<'root> FnOnce(&'root ActiveRoot) -> R, + { + self.storage.with_private_snapshot_root(snapshot, f) + } + /// Capture an owned table-root snapshot for this table. #[inline] #[cfg_attr( @@ -787,19 +796,32 @@ impl Table { /// The snapshot contains only runtime read contract fields copied from a /// single active-root observation. Publication and allocation internals remain /// behind the table-file boundary. -pub(crate) struct TableRootSnapshot<'ctx> { +pub(crate) struct TableRootSnapshot<'read> { root_ts: TrxID, effective_ts: TrxID, pivot_row_id: RowID, column_block_index_root: BlockID, secondary_index_roots: Vec, deletion_cutoff_ts: TrxID, - _proof: PhantomData<&'ctx TrxContext>, + _read: PhantomData<&'read ()>, } -impl<'ctx> TableRootSnapshot<'ctx> { +impl<'read> TableRootSnapshot<'read> { + #[inline] + fn from_active_root(root: &ActiveRoot, _proof: &TrxReadProof<'read>) -> Self { + Self { + root_ts: root.root_ts, + effective_ts: root.effective_ts(), + pivot_row_id: root.pivot_row_id, + column_block_index_root: root.column_block_index_root, + secondary_index_roots: root.secondary_index_roots.clone(), + deletion_cutoff_ts: root.deletion_cutoff_ts, + _read: PhantomData, + } + } + #[inline] - fn from_active_root(root: &ActiveRoot, _proof: &TrxReadProof<'ctx>) -> Self { + fn from_private_snapshot(root: &ActiveRoot, _snapshot: &'read PrivateSnapshot) -> Self { Self { root_ts: root.root_ts, effective_ts: root.effective_ts(), @@ -807,7 +829,7 @@ impl<'ctx> TableRootSnapshot<'ctx> { column_block_index_root: root.column_block_index_root, secondary_index_roots: root.secondary_index_roots.clone(), deletion_cutoff_ts: root.deletion_cutoff_ts, - _proof: PhantomData, + _read: PhantomData, } } @@ -1207,7 +1229,7 @@ pub(crate) mod tests { checkpoint_retry_after_listener_registration_hook: parking_lot::Mutex>, silent_watermark_mutation_hook: parking_lot::Mutex>, - cleanup_after_trx_start_hook: parking_lot::Mutex>, + cleanup_after_private_snapshot_hook: parking_lot::Mutex>, redo_cleanup_before_unlink_hook: parking_lot::Mutex>, force_lwc_build_error: AtomicBool, freeze_page_state_locked_hook: parking_lot::Mutex>, @@ -1349,14 +1371,14 @@ pub(crate) mod tests { ); } - pub(crate) fn install_cleanup_after_trx_start_hook(&self, hook: F) + pub(crate) fn install_cleanup_after_private_snapshot_hook(&self, hook: F) where F: FnOnce() -> Fut + Send + 'static, Fut: Future + Send + 'static, { let old = self .state - .cleanup_after_trx_start_hook + .cleanup_after_private_snapshot_hook .lock() .replace(Box::new(move || Box::pin(hook()))); assert!( @@ -1409,8 +1431,8 @@ pub(crate) mod tests { } } - pub(crate) async fn run_cleanup_after_trx_start_hook(&self) { - let hook = self.state.cleanup_after_trx_start_hook.lock().take(); + pub(crate) async fn run_cleanup_after_private_snapshot_hook(&self) { + let hook = self.state.cleanup_after_private_snapshot_hook.lock().take(); if let Some(hook) = hook { hook().await; } diff --git a/doradb-storage/src/table/persistence.rs b/doradb-storage/src/table/persistence.rs index 7850eb8c..c892d448 100644 --- a/doradb-storage/src/table/persistence.rs +++ b/doradb-storage/src/table/persistence.rs @@ -26,8 +26,8 @@ use crate::obs; use crate::row::RowPage; use crate::runtime::mandatory::PreparedExecution; use crate::session::{ - AcceptedMaintenanceScope, MaintenanceExecutionSpec, PreparedMaintenanceExecution, - PreparedMaintenanceScope, SessionRuntimeAccess, + MaintenanceExecution, PreparedMaintenanceExecution, PreparedMaintenanceScope, SessionRuntime, + SessionRuntimeAccess, }; #[cfg(test)] use crate::table::tests::MaintenanceTestController; @@ -126,72 +126,60 @@ impl DetachedCheckpointRetryWait { } } -struct FreezeTableResources { +struct FreezeTableExecution { attempt: Option, _root_mutation: TableCheckpointRootMutationScope, table: Arc

, max_rows: usize, } -struct FreezeTableExecution; - -impl MaintenanceExecutionSpec for FreezeTableExecution { +impl MaintenanceExecution for FreezeTableExecution { type Output = FreezeOutcome; - type Resources = FreezeTableResources; - type PanicLabel = &'static str; const LABEL: &'static str = "freeze_table"; - async fn execute( - scope: &mut AcceptedMaintenanceScope, - resources: &mut Self::Resources, - _panic_label: &mut Self::PanicLabel, - ) -> CompletionResult { - let attempt = resources + async fn execute(&mut self, runtime: &SessionRuntime) -> CompletionResult { + let attempt = self .attempt .take() .unwrap_or_else(|| panic!("accepted freeze attempt is missing")); - let result = resources - .table - .freeze_prepared(scope, resources.max_rows, attempt) + self.table + .freeze_prepared(runtime, self.max_rows, attempt) .await - .map_err(CompletionErrorBridge::capture); - scope.mark_terminal_ready(); - result + .map_err(CompletionErrorBridge::capture) + } + + #[inline] + fn panic_diagnostic(&self) -> String { + "accepted table freeze panicked".to_owned() } } -struct CheckpointTableResources { +struct CheckpointTableExecution { attempt: Option, _root_mutation: TableCheckpointRootMutationScope, table: Arc
, } -struct CheckpointTableExecution; - -impl MaintenanceExecutionSpec for CheckpointTableExecution { +impl MaintenanceExecution for CheckpointTableExecution { type Output = CheckpointOutcome; - type Resources = CheckpointTableResources; - type PanicLabel = &'static str; const LABEL: &'static str = "checkpoint_table"; - async fn execute( - scope: &mut AcceptedMaintenanceScope, - resources: &mut Self::Resources, - _panic_label: &mut Self::PanicLabel, - ) -> CompletionResult { - let attempt = resources + async fn execute(&mut self, runtime: &SessionRuntime) -> CompletionResult { + let attempt = self .attempt .take() .unwrap_or_else(|| panic!("accepted checkpoint attempt is missing")); - let result = resources - .table - .checkpoint_prepared(scope, attempt) + self.table + .checkpoint_prepared(runtime, attempt) .await - .map_err(CompletionErrorBridge::capture_runtime_or_fatal); - scope.mark_terminal_ready(); - result + .map_err(CompletionErrorBridge::capture_runtime_or_fatal) + } + + #[inline] + fn panic_diagnostic(&self) -> String { + "accepted table checkpoint panicked".to_owned() } } @@ -919,13 +907,12 @@ pub(crate) fn prepare_freeze_table_operation( let table_id = table.table_id(); Ok(PreparedMaintenanceExecution::::table( scope, - FreezeTableResources { + FreezeTableExecution { attempt: Some(attempt), _root_mutation: root_mutation, table, max_rows, }, - "accepted table freeze panicked", table_id, )) } @@ -947,12 +934,11 @@ pub(crate) fn prepare_checkpoint_table_operation( Ok( PreparedMaintenanceExecution::::table( scope, - CheckpointTableResources { + CheckpointTableExecution { attempt: Some(attempt), _root_mutation: root_mutation, table, }, - "accepted table checkpoint panicked", table_id, ), ) diff --git a/doradb-storage/src/table/storage.rs b/doradb-storage/src/table/storage.rs index 6625d8a8..cc9188d6 100644 --- a/doradb-storage/src/table/storage.rs +++ b/doradb-storage/src/table/storage.rs @@ -6,7 +6,7 @@ use crate::id::BlockID; use crate::index::SecondaryDiskTreeRuntime; use crate::lwc::PersistedLwcBlock; use crate::quiescent::QuiescentGuard; -use crate::trx::TrxReadProof; +use crate::trx::{PrivateSnapshot, TrxReadProof}; use error_stack::Report; use std::sync::Arc; @@ -80,6 +80,16 @@ impl ColumnStorage { f(root) } + /// Bind one active root observation under a private maintenance snapshot. + #[inline] + pub(crate) fn with_private_snapshot_root(&self, _snapshot: &PrivateSnapshot, f: F) -> R + where + F: for<'root> FnOnce(&'root ActiveRoot) -> R, + { + let root = self.file().active_root_unchecked(); + f(root) + } + /// Returns the read-only buffer pool used for persisted blocks. #[inline] pub(crate) fn disk_pool(&self) -> &QuiescentGuard { diff --git a/doradb-storage/src/trx/mod.rs b/doradb-storage/src/trx/mod.rs index b08d6025..b9afa74a 100644 --- a/doradb-storage/src/trx/mod.rs +++ b/doradb-storage/src/trx/mod.rs @@ -18,6 +18,7 @@ mod admission; pub(crate) mod group; pub(crate) mod purge; +mod readonly; pub(crate) mod retention; pub(crate) mod row; pub(crate) mod stmt; @@ -27,6 +28,7 @@ mod sys_trx; pub(crate) mod undo; pub(crate) mod ver_map; +pub(crate) use readonly::PrivateSnapshot; pub(crate) use retention::{ prepare_catalog_redo_maintenance_operation, prepare_redo_truncation_operation, }; @@ -39,9 +41,9 @@ use crate::catalog::{TableCache, is_catalog_table}; use crate::completion::Completion; use crate::engine::EngineCore; use crate::error::{ - CompletionErrorBridge, DiscloseError, DiscloseResultExt, Error, FatalError, LifecycleError, - LifecycleResult, OperationResult, ResourceError, Result, RuntimeError, RuntimeOrFatalError, - RuntimeOrFatalResult, RuntimeResult, SharedFatalError, + CompletionErrorBridge, DiscloseError, DiscloseResultExt, Error, FatalError, FatalResult, + LifecycleError, LifecycleResult, OperationResult, ResourceError, Result, RuntimeError, + RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, SharedFatalError, }; use crate::id::{SessionID, SessionOperationKey, TableID, TrxID}; use crate::lock::{ @@ -69,7 +71,7 @@ use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; pub(crate) use admission::TableAdmissionRequest; pub use stmt::Statement; -use stmt::StmtState; +use stmt::{StmtEffects, StmtState}; pub use stream_stmt::{IndexScanMvccStream, StreamStmt}; /// Minimum snapshot timestamp assigned by the transaction system. pub(crate) const MIN_SNAPSHOT_TS: TrxID = TrxID::new(1); @@ -209,12 +211,6 @@ impl Transaction { SessionOperationCompletionClaim::terminal(entry, attachment) } - /// Best-effort check that the transaction can still reach its engine. - #[inline] - pub(crate) fn engine(&self) -> Option { - self.session.upgrade_for_terminal() - } - /// Returns this transaction's current status timestamp. #[inline] pub fn trx_id(&self) -> TrxID { @@ -298,24 +294,117 @@ impl Transaction { } } - /// Stages one private catalog DDL statement with ordinary exact claims. + /// Commit the transaction. + #[inline] + pub async fn commit(self) -> Result { + let claim = self + .claim_terminal() + .attach("operation=commit_active_transaction") + .disclose()?; + let trx_sys = claim.engine().trx_sys.clone(); + trx_sys.commit_transaction(claim).await + } + + /// Rollback the transaction. + #[inline] + pub async fn rollback(self) -> Result<()> { + let claim = self + .claim_terminal() + .attach("operation=rollback_active_transaction") + .disclose()?; + let trx_sys = claim.engine().trx_sys.clone(); + trx_sys.rollback_transaction(claim).await.disclose() + } +} + +impl Drop for Transaction { + #[inline] + fn drop(&mut self) { + if self.terminal_started { + return; + } + if let Some(runtime) = self.session.upgrade_for_terminal() { + let abandoned = runtime + .state() + .abandon_trx_handle(self.operation_key, self.trx_id); + if abandoned { + let trx_sys = runtime.trx_sys.clone(); + trx_sys.request_abandoned_trx_cleanup(runtime, self.operation_key, self.trx_id); + } + } + } +} + +/// Strongly attached transaction used by mandatory DDL work. +/// +/// Unlike the weak public facade, this owner retains one checked-out core and +/// its exact session runtime attachment from construction through terminal +/// completion or synchronous panic parking. +pub(crate) struct PrivateTransaction { + checkout: Option, +} + +impl PrivateTransaction { + /// Create a private facade over one directly initialized checkout. + #[inline] + fn new(checkout: SessionOperationCheckout) -> Self { + Self { + checkout: Some(checkout), + } + } + + #[inline] + fn checkout(&self) -> &SessionOperationCheckout { + self.checkout + .as_ref() + .expect("active private transaction retains its checkout") + } + + #[inline] + fn checkout_mut(&mut self) -> &mut SessionOperationCheckout { + self.checkout + .as_mut() + .expect("active private transaction retains its checkout") + } + + /// Returns the exact active transaction identity. + #[inline] + pub(crate) fn trx_id(&self) -> TrxID { + self.checkout().inner().trx_id() + } + + /// Returns this transaction's snapshot timestamp. + #[inline] + pub(crate) fn sts(&self) -> TrxID { + self.checkout().inner().sts() + } + + /// Validate the retained engine immediately before private transaction work. + #[inline] + pub(crate) fn ensure_engine_healthy(&self) -> FatalResult<()> { + self.checkout() + .attachment() + .engine() + .poisoner + .ensure_healthy() + } + + /// Execute one private statement without returning the core to its entry. /// - /// A callback panic is settled while the statement carrier still owns its - /// partial effects. Incomplete redo is discarded, residual undo returns to - /// the nested transaction core, and the original unwind resumes for the - /// mandatory supervisor. + /// Ordinary errors retain all complete and partial undo in transaction + /// effects for whole-transaction rollback. A callback panic discards + /// incomplete redo, folds residual undo into the transaction, and resumes + /// the original unwind while this facade still owns the checkout. #[inline] - pub(crate) async fn stage_catalog_statement(&mut self, f: F) -> RuntimeResult + pub(crate) async fn stage_statement(&mut self, f: F) -> RuntimeResult where F: for<'borrow> AsyncFnOnce(&'borrow mut Statement<'_>) -> RuntimeResult, { - let checkout = self - .checkout() - .change_context(RuntimeError::CatalogAccess) - .attach("operation=stage_catalog_statement")?; - let mut stmt_state = StmtState::private(checkout); + let checkout = self.checkout_mut(); + let mut effects = StmtEffects::empty(); let outcome = AssertUnwindSafe(async { - let mut stmt = stmt_state.statement(); + let (inner, attachment) = checkout.inner_and_attachment_mut(); + let mut stmt = Statement::new(inner, attachment, &mut effects); let result = f(&mut stmt).await; stmt.merge_effects(); result @@ -323,44 +412,36 @@ impl Transaction { .catch_unwind() .await; match outcome { - Ok(result) => { - stmt_state.return_ordinary(); - result - } + Ok(result) => result, Err(panic) => { - stmt_state.return_after_mandatory_panic(); + effects.fold_cancelled_into_trx_effects(checkout.inner_mut().effects_mut()); resume_unwind(panic); } } } - /// Commit the transaction. + /// Install the exact catalog DDL marker after every catalog statement succeeds. #[inline] - pub async fn commit(self) -> Result { - let claim = self - .claim_terminal() - .attach("operation=commit_active_transaction") - .disclose()?; - let trx_sys = claim.engine().trx_sys.clone(); - trx_sys.commit_transaction(claim).await + pub(crate) fn install_ddl_redo(&mut self, ddl: DDLRedo) { + self.checkout_mut() + .inner_mut() + .effects_mut() + .install_ddl_redo(ddl); } - /// Rollback the transaction. #[inline] - pub async fn rollback(self) -> Result<()> { - let claim = self - .claim_terminal() - .attach("operation=rollback_active_transaction") - .disclose()?; - let trx_sys = claim.engine().trx_sys.clone(); - trx_sys.rollback_transaction(claim).await.disclose() + fn claim_terminal(mut self) -> LifecycleResult { + self.checkout + .take() + .expect("active private transaction retains its checkout") + .claim_private_terminal() } /// Commit a catalog DDL transaction without crossing the public error boundary. #[inline] pub(crate) async fn commit_catalog_ddl(self) -> RuntimeOrFatalResult { - let session_id = self.operation_key.session_id(); - let trx_id = self.trx_id; + let session_id = self.checkout().entry.key().session_id(); + let trx_id = self.trx_id(); let claim = self .claim_terminal() .change_context(RuntimeError::CatalogAccess) @@ -378,8 +459,8 @@ impl Transaction { /// Roll back a catalog DDL transaction without crossing the public error boundary. #[inline] pub(crate) async fn rollback_catalog_ddl(self) -> RuntimeOrFatalResult<()> { - let session_id = self.operation_key.session_id(); - let trx_id = self.trx_id; + let session_id = self.checkout().entry.key().session_id(); + let trx_id = self.trx_id(); let claim = self .claim_terminal() .change_context(RuntimeError::CatalogAccess) @@ -394,42 +475,10 @@ impl Transaction { trx_sys.rollback_catalog_transaction(claim).await } - /// Roll back an engine-owned table-maintenance transaction without crossing - /// the public error boundary. - #[inline] - pub(crate) async fn rollback_table_maintenance(self) -> RuntimeOrFatalResult<()> { - let session_id = self.operation_key.session_id(); - let trx_id = self.trx_id; - let claim = self - .claim_terminal() - .change_context(RuntimeError::TableAccess) - .attach_with(|| { - format!( - "operation=rollback_table_maintenance, session_id={}, trx_id={}", - session_id, trx_id - ) - }) - .map_err(RuntimeOrFatalError::from)?; - let trx_sys = claim.engine().trx_sys.clone(); - trx_sys.rollback_table_maintenance_transaction(claim).await - } -} - -impl Drop for Transaction { + /// Synchronously return an active core before mandatory panic publication. #[inline] - fn drop(&mut self) { - if self.terminal_started { - return; - } - if let Some(runtime) = self.session.upgrade_for_terminal() { - let abandoned = runtime - .state() - .abandon_trx_handle(self.operation_key, self.trx_id); - if abandoned { - let trx_sys = runtime.trx_sys.clone(); - trx_sys.request_abandoned_trx_cleanup(runtime, self.operation_key, self.trx_id); - } - } + pub(crate) fn park(mut self) { + drop(self.checkout.take()); } } @@ -834,6 +883,17 @@ impl TrxEffects { &mut self.index_undo } + /// Install exactly one catalog DDL marker for this transaction. + #[inline] + pub(crate) fn install_ddl_redo(&mut self, ddl: DDLRedo) { + assert!( + self.redo.ddl.is_none(), + "transaction DDL redo installed more than once: existing_ddl={:?}, attempted_ddl={ddl:?}", + self.redo.ddl + ); + self.redo.ddl = Some(Box::new(ddl)); + } + /// Merges one successful statement's effects into this transaction. #[inline] pub(crate) fn merge_statement_effects( @@ -1008,9 +1068,9 @@ struct SessionOperationEntryInner { state: SessionOperationState, /// Identity of the currently attached transaction, if any. /// - /// One DDL or maintenance operation may run sequential private - /// transactions, so this changes with payload installation and completion - /// under the same mutex rather than being immutable entry identity. + /// One DDL operation may run sequential private transactions, so this + /// changes with payload installation and completion under the same mutex + /// rather than being immutable entry identity. trx_id: Option, /// Heap-stable transaction core allocated once at installation. /// @@ -1141,46 +1201,65 @@ impl SessionOperationEntry { .map(|inner| inner as *const TrxInner as usize) } - /// Installs one private transaction inside a DDL or maintenance entry. + /// Validate that accepted mandatory execution may start a private transaction. #[inline] - pub(crate) fn install_private_transaction(&self, trx_inner: Box) { + pub(crate) fn validate_private_transaction_begin(&self) -> LifecycleResult<()> { assert!( - matches!( - self.kind, - SessionOperationKind::Ddl | SessionOperationKind::Maintenance - ), - "private transaction requires DDL or maintenance operation: key={}, kind={}", + self.kind == SessionOperationKind::Ddl, + "private transaction requires DDL operation: key={}, kind={}", self.key, self.kind.label() ); - let trx_id = trx_inner.trx_id(); + let inner = self.inner.lock(); + if inner.state != SessionOperationState::Mandatory(None) + || inner.outer_foreground_alive + || inner.trx_id.is_some() + || inner.trx_inner.is_some() + || inner.lock_authority_return.is_some() + { + return Err(session_operation_entry_state_err( + self.key, self.kind, &inner, + )); + } + Ok(()) + } + + /// Publish a directly checked-out private transaction as running. + #[inline] + fn install_running_private_transaction(&self, trx_id: TrxID) { let mut inner = self.inner.lock(); - let next_state = match inner.state { - SessionOperationState::Voluntary(None) if inner.outer_foreground_alive => { - SessionOperationState::Voluntary(Some(InternalTrxState::Available)) - } - SessionOperationState::Mandatory(None) if !inner.outer_foreground_alive => { - SessionOperationState::Mandatory(Some(InternalTrxState::Available)) - } - _ => panic!( - "private transaction installation requires empty voluntary or mandatory authority: key={}, state={}, trx_id={:?}", - self.key, - inner.state.label(), - inner.trx_id - ), - }; assert!( - inner.trx_id.is_none() + self.kind == SessionOperationKind::Ddl + && inner.state == SessionOperationState::Mandatory(None) + && !inner.outer_foreground_alive + && inner.trx_id.is_none() && inner.trx_inner.is_none() && inner.lock_authority_return.is_none(), - "private transaction installation requires an empty payload slot: key={}, state={}, trx_id={:?}", + "private transaction direct installation requires empty mandatory authority: key={}, kind={}, state={}, trx_id={:?}", self.key, + self.kind.label(), inner.state.label(), inner.trx_id ); - inner.state = next_state; + inner.state = SessionOperationState::Mandatory(Some(InternalTrxState::Running)); inner.trx_id = Some(trx_id); - inner.trx_inner = Some(trx_inner); + } + + /// Convert one exact held private checkout into terminal ownership. + #[inline] + fn claim_running_private_terminal(&self, trx_id: TrxID) -> LifecycleResult<()> { + let mut inner = self.inner.lock(); + if inner.trx_id != Some(trx_id) + || self.kind != SessionOperationKind::Ddl + || inner.state != SessionOperationState::Mandatory(Some(InternalTrxState::Running)) + || inner.trx_inner.is_some() + { + return Err(session_operation_entry_state_err( + self.key, self.kind, &inner, + )); + } + inner.state = SessionOperationState::Mandatory(Some(InternalTrxState::Completing)); + Ok(()) } #[inline] @@ -1204,7 +1283,7 @@ impl SessionOperationEntry { { None } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance + SessionOperationKind::Ddl if inner.state == SessionOperationState::Voluntary(Some(InternalTrxState::Available)) => { @@ -1212,7 +1291,7 @@ impl SessionOperationEntry { InternalTrxState::Running, ))) } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance + SessionOperationKind::Ddl if inner.state == SessionOperationState::Mandatory(Some(InternalTrxState::Available)) => { @@ -1262,13 +1341,13 @@ impl SessionOperationEntry { { SessionOperationState::Completing } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance + SessionOperationKind::Ddl if inner.state == SessionOperationState::Voluntary(Some(InternalTrxState::Available)) => { SessionOperationState::Voluntary(Some(InternalTrxState::Completing)) } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance + SessionOperationKind::Ddl if inner.state == SessionOperationState::Mandatory(Some(InternalTrxState::Available)) => { @@ -1313,7 +1392,7 @@ impl SessionOperationEntry { { SessionOperationState::Completing } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance + SessionOperationKind::Ddl if inner.outer_foreground_alive && inner.state == SessionOperationState::Voluntary(Some( @@ -1322,7 +1401,7 @@ impl SessionOperationEntry { { SessionOperationState::Voluntary(Some(InternalTrxState::Completing)) } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance + SessionOperationKind::Ddl if !inner.outer_foreground_alive && inner.state == SessionOperationState::CleanupReady => { @@ -1360,14 +1439,14 @@ impl SessionOperationEntry { SessionOperationKind::PublicTransaction => { inner.state == SessionOperationState::Voluntary(None) } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance => { + SessionOperationKind::Ddl => { matches!( inner.state, SessionOperationState::Voluntary(Some(InternalTrxState::Running)) | SessionOperationState::Mandatory(Some(InternalTrxState::Running)) ) } - SessionOperationKind::SessionExplicitLock => false, + SessionOperationKind::Maintenance | SessionOperationKind::SessionExplicitLock => false, }; assert!( running && inner.trx_inner.is_none(), @@ -1386,18 +1465,15 @@ impl SessionOperationEntry { if inner.cleanup_requested { inner.state = match self.kind { SessionOperationKind::PublicTransaction => SessionOperationState::CleanupReady, - SessionOperationKind::Ddl | SessionOperationKind::Maintenance - if inner.outer_foreground_alive => - { + SessionOperationKind::Ddl if inner.outer_foreground_alive => { SessionOperationState::Voluntary(Some(InternalTrxState::CleanupReady)) } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance => { - SessionOperationState::CleanupReady - } - SessionOperationKind::SessionExplicitLock => { + SessionOperationKind::Ddl => SessionOperationState::CleanupReady, + SessionOperationKind::Maintenance | SessionOperationKind::SessionExplicitLock => { panic!( - "explicit-lock operation cannot return a transaction core: key={}", - self.key + "non-transaction operation cannot return a transaction core: key={}, kind={}", + self.key, + self.kind.label() ) } }; @@ -1405,29 +1481,28 @@ impl SessionOperationEntry { } if self.kind != SessionOperationKind::PublicTransaction { inner.state = match self.kind { - SessionOperationKind::Ddl | SessionOperationKind::Maintenance => { - match inner.state { - SessionOperationState::Voluntary(Some(InternalTrxState::Running)) - if inner.outer_foreground_alive => - { - SessionOperationState::Voluntary(Some(InternalTrxState::Available)) - } - SessionOperationState::Mandatory(Some(InternalTrxState::Running)) - if !inner.outer_foreground_alive => - { - SessionOperationState::Mandatory(Some(InternalTrxState::Available)) - } - _ => panic!( - "private transaction return requires matching outer authority: key={}, state={}", - self.key, - inner.state.label() - ), + SessionOperationKind::Ddl => match inner.state { + SessionOperationState::Voluntary(Some(InternalTrxState::Running)) + if inner.outer_foreground_alive => + { + SessionOperationState::Voluntary(Some(InternalTrxState::Available)) } - } - SessionOperationKind::SessionExplicitLock => { + SessionOperationState::Mandatory(Some(InternalTrxState::Running)) + if !inner.outer_foreground_alive => + { + SessionOperationState::Mandatory(Some(InternalTrxState::Available)) + } + _ => panic!( + "private transaction return requires matching outer authority: key={}, state={}", + self.key, + inner.state.label() + ), + }, + SessionOperationKind::Maintenance | SessionOperationKind::SessionExplicitLock => { panic!( - "explicit-lock operation cannot return a transaction core: key={}", - self.key + "non-transaction operation cannot return a transaction core: key={}, kind={}", + self.key, + self.kind.label() ) } SessionOperationKind::PublicTransaction => unreachable!(), @@ -1481,14 +1556,14 @@ impl SessionOperationEntry { } (SessionOperationKind::PublicTransaction, SessionOperationState::Voluntary(None)) | ( - SessionOperationKind::Ddl | SessionOperationKind::Maintenance, + SessionOperationKind::Ddl, SessionOperationState::Voluntary(Some(InternalTrxState::Running)), ) => { inner.cleanup_requested = true; true } ( - SessionOperationKind::Ddl | SessionOperationKind::Maintenance, + SessionOperationKind::Ddl, SessionOperationState::Voluntary(Some(InternalTrxState::Available)), ) => { inner.cleanup_requested = true; @@ -1497,13 +1572,11 @@ impl SessionOperationEntry { true } ( - SessionOperationKind::Ddl | SessionOperationKind::Maintenance, + SessionOperationKind::Ddl, SessionOperationState::Voluntary(Some(InternalTrxState::CleanupReady)), ) | ( - SessionOperationKind::PublicTransaction - | SessionOperationKind::Ddl - | SessionOperationKind::Maintenance, + SessionOperationKind::PublicTransaction | SessionOperationKind::Ddl, SessionOperationState::CleanupReady, ) => { assert!( @@ -1624,24 +1697,20 @@ impl SessionOperationEntry { SessionOperationKind::PublicTransaction => { inner.state == SessionOperationState::Completing } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance - if inner.outer_foreground_alive => - { + SessionOperationKind::Ddl if inner.outer_foreground_alive => { matches!( inner.state, SessionOperationState::Voluntary(Some(InternalTrxState::Completing)) ) } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance + SessionOperationKind::Ddl if inner.state == SessionOperationState::Mandatory(Some(InternalTrxState::Completing)) => { true } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance => { - inner.state == SessionOperationState::Completing - } - SessionOperationKind::SessionExplicitLock => false, + SessionOperationKind::Ddl => inner.state == SessionOperationState::Completing, + SessionOperationKind::Maintenance | SessionOperationKind::SessionExplicitLock => false, }; if inner.trx_id != Some(trx_id) || !completion_owned { return None; @@ -1792,11 +1861,12 @@ impl SessionOperationEntry { /// runtime attachment, and restores the same box on ordinary drop. Statement /// ownership policy lives in /// [`StmtState`], its callback facade is [`Statement`], and terminal commit and -/// rollback use private completion claims. +/// rollback use private completion claims. Public statements construct a fresh +/// checkout per operation; [`PrivateTransaction`] owns one continuously. pub(crate) struct SessionOperationCheckout { entry: Arc, inner: Option>, - attachment: TrxAttachment, + attachment: Option, } impl SessionOperationCheckout { @@ -1806,10 +1876,25 @@ impl SessionOperationCheckout { Ok(Self { entry, inner: Some(inner), - attachment, + attachment: Some(attachment), }) } + /// Own a newly initialized private core without checking it through the entry. + #[inline] + fn private( + entry: Arc, + inner: Box, + attachment: TrxAttachment, + ) -> Self { + entry.install_running_private_transaction(attachment.trx_id()); + Self { + entry, + inner: Some(inner), + attachment: Some(attachment), + } + } + /// Returns this checkout's immutable transaction core. #[inline] pub(crate) fn inner(&self) -> &TrxInner { @@ -1833,13 +1918,20 @@ impl SessionOperationCheckout { .inner .as_mut() .expect("SessionOperationCheckout always owns an inner until fatal discard"); - (inner, &self.attachment) + ( + inner, + self.attachment + .as_ref() + .expect("active checkout retains its transaction attachment"), + ) } /// Returns this checkout's operation-local attachment. #[inline] pub(crate) fn attachment(&self) -> &TrxAttachment { - &self.attachment + self.attachment + .as_ref() + .expect("active checkout retains its transaction attachment") } /// Acquires an explicit transaction-lifetime table lock. @@ -1855,6 +1947,9 @@ impl SessionOperationCheckout { let inner = inner .as_mut() .expect("SessionOperationCheckout always owns an inner until fatal discard"); + let attachment = attachment + .as_ref() + .expect("active checkout retains its transaction attachment"); inner.lock_table(attachment, table_id, mode).await } @@ -1862,14 +1957,15 @@ impl SessionOperationCheckout { #[inline] pub(crate) fn discard_after_fatal_rollback(&mut self) { if let Some(mut inner) = self.inner.take() { - let retention = inner.retain_and_discard_after_fatal_rollback(&self.attachment); - self.attachment - .engine() - .trx_sys - .retain_fatal_rollback(retention); + let attachment = self + .attachment + .as_ref() + .expect("active checkout retains its transaction attachment"); + let retention = inner.retain_and_discard_after_fatal_rollback(attachment); + attachment.engine().trx_sys.retain_fatal_rollback(retention); } self.entry.fail_retained(); - self.attachment.notify_operation_transition(); + self.attachment().notify_operation_transition(); } /// Returns a cancelled public statement directly to cleanup ownership. @@ -1880,8 +1976,28 @@ impl SessionOperationCheckout { .take() .expect("cancelled statement checkout must retain its transaction core"); self.entry.return_cancelled(inner); - self.attachment.notify_operation_transition(); - self.attachment.request_abandoned_cleanup(); + self.attachment().notify_operation_transition(); + self.attachment().request_abandoned_cleanup(); + } + + /// Convert a continuously held private checkout into a terminal claim. + #[inline] + fn claim_private_terminal(mut self) -> LifecycleResult { + let trx_id = self.attachment().trx_id(); + self.entry.claim_running_private_terminal(trx_id)?; + let inner = self + .inner + .take() + .expect("private terminal claim retains its transaction core"); + let attachment = self + .attachment + .take() + .expect("private terminal claim retains its transaction attachment"); + Ok(SessionOperationCompletionClaim { + entry: Arc::clone(&self.entry), + inner: Some(inner), + attachment: Some(attachment), + }) } } @@ -1892,8 +2008,8 @@ impl Drop for SessionOperationCheckout { return; }; if self.entry.return_inner(inner) { - self.attachment.notify_operation_transition(); - self.attachment.request_abandoned_cleanup(); + self.attachment().notify_operation_transition(); + self.attachment().request_abandoned_cleanup(); } } } @@ -3417,7 +3533,72 @@ pub(crate) mod tests { inner } - async fn test_engine(log_file_stem: &str) -> (TempDir, Engine) { + /// Install one transaction-level DDL marker for catalog and recovery tests. + pub(crate) fn install_transaction_ddl_redo( + trx: &mut Transaction, + ddl: DDLRedo, + ) -> LifecycleResult<()> { + let mut checkout = trx.checkout()?; + checkout.inner_mut().effects_mut().install_ddl_redo(ddl); + Ok(()) + } + + /// Return the core allocation held by one running private transaction. + pub(crate) fn private_transaction_inner_ptr(trx: &PrivateTransaction) -> usize { + trx.checkout().inner() as *const TrxInner as usize + } + + /// Return the exact number of snapshot timestamps registered for GC. + pub(crate) fn active_sts_count(trx_sys: &sys::TransactionSystem) -> usize { + trx_sys + .gc_buckets + .iter() + .map(|bucket| { + let active_sts = bucket.active_sts_list.lock(); + active_sts.active.len() - active_sts.deleted.len() + }) + .sum() + } + + /// Install a checked-in private core for entry state-machine tests. + fn install_private_transaction(entry: &SessionOperationEntry, trx_inner: Box) { + let trx_id = trx_inner.trx_id(); + let mut inner = entry.inner.lock(); + assert!( + entry.kind == SessionOperationKind::Ddl, + "test private transaction installation requires DDL operation: key={}, kind={}", + entry.key, + entry.kind.label() + ); + let next_state = match inner.state { + SessionOperationState::Voluntary(None) if inner.outer_foreground_alive => { + SessionOperationState::Voluntary(Some(InternalTrxState::Available)) + } + SessionOperationState::Mandatory(None) if !inner.outer_foreground_alive => { + SessionOperationState::Mandatory(Some(InternalTrxState::Available)) + } + _ => panic!( + "test private transaction installation requires empty authority: key={}, state={}, trx_id={:?}", + entry.key, + inner.state.label(), + inner.trx_id + ), + }; + assert!( + inner.trx_id.is_none() + && inner.trx_inner.is_none() + && inner.lock_authority_return.is_none(), + "test private transaction installation requires an empty payload slot: key={}, state={}, trx_id={:?}", + entry.key, + inner.state.label(), + inner.trx_id + ); + inner.state = next_state; + inner.trx_id = Some(trx_id); + inner.trx_inner = Some(trx_inner); + } + + pub(super) async fn test_engine(log_file_stem: &str) -> (TempDir, Engine) { test_engine_with_mem_size(log_file_stem, 64usize * 1024 * 1024).await } @@ -3702,19 +3883,17 @@ pub(crate) mod tests { let trx_id = MIN_ACTIVE_TRX_ID + 101; let entry = SessionOperationEntry::new( SessionOperationKey::new(session_id, OperationID::new(1)), - SessionOperationKind::Maintenance, + SessionOperationKind::Ddl, ); assert_eq!( entry.inspect().state, SessionOperationState::Voluntary(None) ); - entry.install_private_transaction(Box::new(trx_inner( - trx_id, - TrxID::new(101), - 0, - session_id, - ))); + install_private_transaction( + &entry, + Box::new(trx_inner(trx_id, TrxID::new(101), 0, session_id)), + ); assert_eq!( entry.inspect().state, SessionOperationState::Voluntary(Some(InternalTrxState::Available)) @@ -3774,12 +3953,10 @@ pub(crate) mod tests { entry.inspect().state, SessionOperationState::Mandatory(None) ); - entry.install_private_transaction(Box::new(trx_inner( - trx_id, - TrxID::new(102), - 0, - session_id, - ))); + install_private_transaction( + &entry, + Box::new(trx_inner(trx_id, TrxID::new(102), 0, session_id)), + ); assert_eq!( entry.inspect().state, SessionOperationState::Mandatory(Some(InternalTrxState::Available)) @@ -3828,15 +4005,13 @@ pub(crate) mod tests { let second_trx_id = MIN_ACTIVE_TRX_ID + 201; let entry = SessionOperationEntry::new( SessionOperationKey::new(session_id, OperationID::new(1)), - SessionOperationKind::Maintenance, + SessionOperationKind::Ddl, ); - entry.install_private_transaction(Box::new(trx_inner( - first_trx_id, - TrxID::new(200), - 0, - session_id, - ))); + install_private_transaction( + &entry, + Box::new(trx_inner(first_trx_id, TrxID::new(200), 0, session_id)), + ); let first_inner = entry .take_for_terminal(first_trx_id) .expect("first private transaction can be completed"); @@ -3850,12 +4025,10 @@ pub(crate) mod tests { drop(entry.take_lock_authority_return()); drop(first_inner); - entry.install_private_transaction(Box::new(trx_inner( - second_trx_id, - TrxID::new(201), - 0, - session_id, - ))); + install_private_transaction( + &entry, + Box::new(trx_inner(second_trx_id, TrxID::new(201), 0, session_id)), + ); assert!( !entry.abandon_transaction(first_trx_id), "a stale handle must not abandon the replacement transaction" @@ -3881,14 +4054,12 @@ pub(crate) mod tests { let available_trx_id = MIN_ACTIVE_TRX_ID + 102; let available_entry = SessionOperationEntry::new( SessionOperationKey::new(session_id, OperationID::new(1)), - SessionOperationKind::Maintenance, + SessionOperationKind::Ddl, + ); + install_private_transaction( + &available_entry, + Box::new(trx_inner(available_trx_id, TrxID::new(102), 0, session_id)), ); - available_entry.install_private_transaction(Box::new(trx_inner( - available_trx_id, - TrxID::new(102), - 0, - session_id, - ))); let release = available_entry.release_foreground(); assert!(!release.terminal); assert_eq!(release.cleanup, Some(available_trx_id)); @@ -3903,12 +4074,10 @@ pub(crate) mod tests { SessionOperationKey::new(session_id, OperationID::new(2)), SessionOperationKind::Ddl, ); - terminal_entry.install_private_transaction(Box::new(trx_inner( - terminal_trx_id, - TrxID::new(103), - 0, - session_id, - ))); + install_private_transaction( + &terminal_entry, + Box::new(trx_inner(terminal_trx_id, TrxID::new(103), 0, session_id)), + ); let _inner = terminal_entry .take_for_terminal(terminal_trx_id) .expect("private transaction terminal ownership can be claimed"); @@ -3938,14 +4107,12 @@ pub(crate) mod tests { let running_trx_id = MIN_ACTIVE_TRX_ID + 104; let running_entry = SessionOperationEntry::new( SessionOperationKey::new(session_id, OperationID::new(3)), - SessionOperationKind::Maintenance, + SessionOperationKind::Ddl, + ); + install_private_transaction( + &running_entry, + Box::new(trx_inner(running_trx_id, TrxID::new(104), 0, session_id)), ); - running_entry.install_private_transaction(Box::new(trx_inner( - running_trx_id, - TrxID::new(104), - 0, - session_id, - ))); let inner = running_entry .take_for_checkout(running_trx_id) .expect("private transaction can be checked out"); @@ -4225,16 +4392,13 @@ pub(crate) mod tests { } #[inline] - fn discard_production_transaction_after_fatal_rollback(trx: &mut Transaction) { + fn discard_production_transaction_after_fatal_rollback(engine: &Engine, trx: &mut Transaction) { let sts = trx.sts(); let gc_no = transaction_gc_no(trx); let session_id = trx.operation_key.session_id(); - let engine = trx.engine().expect("test transaction must have engine"); discard_transaction_after_fatal_rollback(trx); - engine.trx_sys.record_rollback_for_purge(gc_no, sts); - if let Some(registry) = engine.session_registry.upgrade() { - remove_session_for_test(®istry, session_id); - } + engine.inner().trx_sys.record_rollback_for_purge(gc_no, sts); + remove_session_for_test(&engine.inner().session_registry, session_id); } /// Add one redo log entry for tests that need a non-readonly transaction. @@ -4248,7 +4412,7 @@ pub(crate) mod tests { trx.exec(async |stmt| { // Simulate one sysbench record: // uint64 + int32 + int32 + char(60) + char(120) - stmt.effects_mut().insert_row_redo( + stmt_tests::statement_effects_mut(stmt).insert_row_redo( USER_TABLE_ID_START, RowRedo { row_id: RowID::new(0), @@ -4837,7 +5001,7 @@ pub(crate) mod tests { let (_temp_dir, engine) = test_engine("redo_stmt_effect_merge").await; let (_session, mut trx) = begin_production_test_transaction(&engine); trx.exec(async |stmt| { - let effects = stmt.effects_mut(); + let effects = stmt_tests::statement_effects_mut(stmt); effects.push_row_undo(OwnedRowUndo::new( TableID::new(12), None, @@ -4870,7 +5034,7 @@ pub(crate) mod tests { }) .unwrap(); - discard_production_transaction_after_fatal_rollback(&mut trx); + discard_production_transaction_after_fatal_rollback(&engine, &mut trx); }); } @@ -4938,7 +5102,7 @@ pub(crate) mod tests { let resource = LockResource::TableMetadata(TableID::new(91_430)); let mut exec = Box::pin(trx.exec(async |stmt| { stmt_tests::acquire_transaction_lock(stmt, resource, LockMode::Shared).await?; - stmt.effects_mut().insert_row_redo( + stmt_tests::statement_effects_mut(stmt).insert_row_redo( TableID::new(91_430), RowRedo { row_id: RowID::new(1), @@ -5165,6 +5329,25 @@ pub(crate) mod tests { effects.debug_assert_redo_invariants(); } + #[test] + fn test_transaction_effects_install_first_ddl_redo() { + let mut effects = TrxEffects::empty(); + effects.install_ddl_redo(DDLRedo::CreateTable(TableID::new(42))); + assert!(matches!( + effects.redo.ddl.as_deref(), + Some(DDLRedo::CreateTable(table_id)) if *table_id == TableID::new(42) + )); + effects.clear_for_rollback(); + } + + #[test] + #[should_panic(expected = "transaction DDL redo installed more than once")] + fn test_transaction_effects_reject_duplicate_ddl_redo() { + let mut effects = TrxEffects::empty(); + effects.install_ddl_redo(DDLRedo::CreateTable(TableID::new(42))); + effects.install_ddl_redo(DDLRedo::DropTable(TableID::new(42))); + } + #[test] fn test_statement_error_rolls_back_only_statement_effects() { smol::block_on(async { @@ -5172,7 +5355,7 @@ pub(crate) mod tests { let (_session, mut trx) = begin_production_test_transaction(&engine); trx.exec(async |stmt| { - stmt.effects_mut().insert_row_redo( + stmt_tests::statement_effects_mut(stmt).insert_row_redo( TableID::new(12), RowRedo { row_id: RowID::new(23), @@ -5186,7 +5369,7 @@ pub(crate) mod tests { let res: Result<()> = trx .exec(async |stmt| { - stmt.effects_mut().insert_row_redo( + stmt_tests::statement_effects_mut(stmt).insert_row_redo( TableID::new(12), RowRedo { row_id: RowID::new(24), @@ -5209,7 +5392,7 @@ pub(crate) mod tests { }) .unwrap(); - discard_production_transaction_after_fatal_rollback(&mut trx); + discard_production_transaction_after_fatal_rollback(&engine, &mut trx); }); } diff --git a/doradb-storage/src/trx/purge.rs b/doradb-storage/src/trx/purge.rs index 45767db2..dd8a93ed 100644 --- a/doradb-storage/src/trx/purge.rs +++ b/doradb-storage/src/trx/purge.rs @@ -271,6 +271,16 @@ impl TransactionSystem { progress } + /// Deregister one active STS and report its causal horizon transition. + #[inline] + pub(super) fn deregister_active_sts(&self, gc_no: usize, sts: TrxID) -> bool { + let progress = self.gc_buckets[gc_no].record_rollback_for_purge(sts); + if let Some(progress) = progress { + let _ = self.purge_tx.send(Purge::ActiveSts(progress)); + } + progress.is_some() + } + /// Record rollback progress and report the causal active-STS transition. /// /// Commit handoffs are non-lossy through `Purge::Committed` because purge @@ -279,11 +289,7 @@ impl TransactionSystem { /// a coalescible scheduling observation. #[inline] pub(crate) fn record_rollback_for_purge(&self, gc_no: usize, sts: TrxID) -> bool { - let progress = self.gc_buckets[gc_no].record_rollback_for_purge(sts); - if let Some(progress) = progress { - let _ = self.purge_tx.send(Purge::ActiveSts(progress)); - } - progress.is_some() + self.deregister_active_sts(gc_no, sts) } #[inline] diff --git a/doradb-storage/src/trx/readonly.rs b/doradb-storage/src/trx/readonly.rs new file mode 100644 index 00000000..c09ff542 --- /dev/null +++ b/doradb-storage/src/trx/readonly.rs @@ -0,0 +1,79 @@ +use crate::id::TrxID; +use crate::quiescent::QuiescentGuard; +use crate::trx::sys::TransactionSystem; + +/// Maintenance-only snapshot registered in the active GC horizon. +/// +/// This owner carries no transaction capabilities or stable session child +/// state. Its active STS protects root snapshots borrowed from it until drop. +pub(crate) struct PrivateSnapshot { + trx_sys: QuiescentGuard, + sts: TrxID, + gc_no: usize, +} + +impl PrivateSnapshot { + /// Return the registered snapshot timestamp. + #[inline] + pub(crate) fn sts(&self) -> TrxID { + self.sts + } +} + +impl Drop for PrivateSnapshot { + #[inline] + fn drop(&mut self) { + self.trx_sys.deregister_active_sts(self.gc_no, self.sts); + } +} + +impl QuiescentGuard { + /// Register one mandatory-maintenance snapshot in the active GC horizon. + #[inline] + pub(crate) fn register_private_snapshot(&self) -> PrivateSnapshot { + let (gc_no, sts) = self.register_active_sts(); + PrivateSnapshot { + trx_sys: self.clone(), + sts, + gc_no, + } + } +} + +#[cfg(test)] +mod tests { + use crate::trx::{ + MAX_SNAPSHOT_TS, + tests::{active_sts_count, test_engine}, + }; + + #[test] + fn test_private_snapshot_registers_and_releases_active_sts() { + smol::block_on(async { + let (_temp_dir, engine) = test_engine("private_snapshot_sts").await; + let trx_sys = engine.inner().trx_sys.clone(); + assert_eq!(active_sts_count(&trx_sys), 0); + + let first = trx_sys.register_private_snapshot(); + let first_sts = first.sts(); + assert_eq!(active_sts_count(&trx_sys), 1); + assert_eq!(trx_sys.min_active_sts(), first_sts); + + let second = trx_sys.register_private_snapshot(); + let second_sts = second.sts(); + assert!(second_sts > first_sts); + assert_eq!(active_sts_count(&trx_sys), 2); + assert_eq!(trx_sys.min_active_sts(), first_sts); + + drop(first); + assert_eq!(active_sts_count(&trx_sys), 1); + assert_eq!(trx_sys.min_active_sts(), second_sts); + drop(second); + assert_eq!(active_sts_count(&trx_sys), 0); + assert_eq!(trx_sys.min_active_sts(), MAX_SNAPSHOT_TS); + + drop(trx_sys); + engine.shutdown(); + }); + } +} diff --git a/doradb-storage/src/trx/retention.rs b/doradb-storage/src/trx/retention.rs index 76ecf5d1..aa6a30c8 100644 --- a/doradb-storage/src/trx/retention.rs +++ b/doradb-storage/src/trx/retention.rs @@ -13,9 +13,8 @@ use crate::recovery::stream::{ }; use crate::runtime::mandatory::PreparedExecution; use crate::session::{ - AcceptedMaintenanceScope, CatalogRedoMaintenanceOutcome, MaintenanceExecutionSpec, - PreparedMaintenanceExecution, PreparedMaintenanceScope, RedoTruncationBlockerInfo, - RedoTruncationOutcome, + CatalogRedoMaintenanceOutcome, MaintenanceExecution, PreparedMaintenanceExecution, + PreparedMaintenanceScope, RedoTruncationBlockerInfo, RedoTruncationOutcome, SessionRuntime, }; #[cfg(test)] use crate::table::tests::MaintenanceTestController; @@ -117,71 +116,61 @@ impl PendingDroppedTableRedoFloor { } } -struct RedoTruncationResources { +struct RedoTruncationExecution { catalog_scope: CatalogCheckpointScope, _redo_scope: RedoRetentionScope, } -struct RedoTruncationExecution; - -impl MaintenanceExecutionSpec for RedoTruncationExecution { +impl MaintenanceExecution for RedoTruncationExecution { type Output = RedoTruncationOutcome; - type Resources = RedoTruncationResources; - type PanicLabel = &'static str; const LABEL: &'static str = "truncate_redo_log"; - async fn execute( - scope: &mut AcceptedMaintenanceScope, - resources: &mut Self::Resources, - _panic_label: &mut Self::PanicLabel, - ) -> CompletionResult { - let engine = scope.engine(); - let result = engine + async fn execute(&mut self, runtime: &SessionRuntime) -> CompletionResult { + let engine = runtime.core(); + engine .trx_sys .truncate_redo_log_prepared( - || resources.catalog_scope.release(), + || self.catalog_scope.release(), #[cfg(test)] &engine.maintenance_test, ) .await - .map_err(CompletionErrorBridge::capture_runtime_or_fatal); - scope.mark_terminal_ready(); - result + .map_err(CompletionErrorBridge::capture_runtime_or_fatal) + } + + #[inline] + fn panic_diagnostic(&self) -> String { + "accepted redo truncation panicked".to_owned() } } -struct CatalogRedoMaintenanceResources { +struct CatalogRedoMaintenanceExecution { catalog_scope: CatalogCheckpointScope, _redo_scope: RedoRetentionScope, } -struct CatalogRedoMaintenanceExecution; - -impl MaintenanceExecutionSpec for CatalogRedoMaintenanceExecution { +impl MaintenanceExecution for CatalogRedoMaintenanceExecution { type Output = CatalogRedoMaintenanceOutcome; - type Resources = CatalogRedoMaintenanceResources; - type PanicLabel = &'static str; const LABEL: &'static str = "checkpoint_catalog_and_truncate_redo_log"; - async fn execute( - scope: &mut AcceptedMaintenanceScope, - resources: &mut Self::Resources, - _panic_label: &mut Self::PanicLabel, - ) -> CompletionResult { - let engine = scope.engine(); - let result = engine + async fn execute(&mut self, runtime: &SessionRuntime) -> CompletionResult { + let engine = runtime.core(); + engine .trx_sys .checkpoint_catalog_and_truncate_redo_log_prepared( - || resources.catalog_scope.release(), + || self.catalog_scope.release(), #[cfg(test)] &engine.maintenance_test, ) .await - .map_err(CompletionErrorBridge::capture_runtime_or_fatal); - scope.mark_terminal_ready(); - result + .map_err(CompletionErrorBridge::capture_runtime_or_fatal) + } + + #[inline] + fn panic_diagnostic(&self) -> String { + "accepted combined catalog/redo maintenance panicked".to_owned() } } @@ -200,11 +189,10 @@ pub(crate) fn prepare_redo_truncation_operation( ) -> impl PreparedExecution { PreparedMaintenanceExecution::::global( scope, - RedoTruncationResources { + RedoTruncationExecution { catalog_scope, _redo_scope: redo_scope, }, - "accepted redo truncation panicked", ) } @@ -216,11 +204,10 @@ pub(crate) fn prepare_catalog_redo_maintenance_operation( ) -> impl PreparedExecution { PreparedMaintenanceExecution::::global( scope, - CatalogRedoMaintenanceResources { + CatalogRedoMaintenanceExecution { catalog_scope, _redo_scope: redo_scope, }, - "accepted combined catalog/redo maintenance panicked", ) } diff --git a/doradb-storage/src/trx/stmt.rs b/doradb-storage/src/trx/stmt.rs index 657768c3..1809ac96 100644 --- a/doradb-storage/src/trx/stmt.rs +++ b/doradb-storage/src/trx/stmt.rs @@ -8,7 +8,7 @@ use crate::error::{ Result, RuntimeError, RuntimeResult, }; use crate::lock::{LockMode, LockResource}; -use crate::log::redo::{DDLRedo, RedoLogs, RowRedo}; +use crate::log::redo::{RedoLogs, RowRedo}; use crate::obs; use crate::row::ops::{ DeleteMvcc, RowMutation, ScanMvcc, SelectKey, SelectMvcc, TableMutationOutcome, UpdateCol, @@ -77,12 +77,6 @@ impl StmtEffects { } } - /// Returns whether this accumulator has no statement-local effects. - #[inline] - pub(crate) fn is_empty(&self) -> bool { - self.row_undo.is_empty() && self.index_undo.is_empty() && self.redo.is_empty() - } - /// Push one row undo entry into this statement. #[inline] pub(crate) fn push_row_undo(&mut self, undo: OwnedRowUndo) { @@ -169,19 +163,6 @@ impl StmtEffects { self.redo.insert_dml(table_id, entry); } - /// Borrow statement-local redo for producer assertions. - #[cfg(test)] - #[inline] - pub(crate) fn redo_for_test(&self) -> &RedoLogs { - &self.redo - } - - /// Replace the statement's deferred DDL redo payload. - #[inline] - pub(crate) fn set_ddl_redo(&mut self, ddl: DDLRedo) -> Option> { - self.redo.ddl.replace(Box::new(ddl)) - } - #[inline] fn push_index_undo(&mut self, index_undo: IndexUndo) { self.index_undo.push(index_undo); @@ -197,13 +178,14 @@ impl StmtEffects { ); } - /// Folds residual cancelled-statement undo into whole-transaction rollback. + /// Folds residual incomplete-statement undo into whole-transaction rollback. /// /// Redo from a statement that did not complete is never commit-visible. /// Undo remains ordered after prior successful statements so whole- - /// transaction rollback unwinds this statement first. + /// transaction rollback unwinds this statement first. Public cancellation + /// and private mandatory panic settlement share this mechanical operation. #[inline] - fn fold_cancelled_into_trx_effects(&mut self, trx_effects: &mut TrxEffects) { + pub(crate) fn fold_cancelled_into_trx_effects(&mut self, trx_effects: &mut TrxEffects) { self.redo.clear(); trx_effects.row_undo_mut().merge(&mut self.row_undo); trx_effects.index_undo_mut().merge(&mut self.index_undo); @@ -254,7 +236,6 @@ impl StmtEffects { #[derive(Clone, Copy)] enum StmtDropAction { CancelPublicTransaction, - PrivateMustComplete, Settled, } @@ -280,16 +261,6 @@ impl StmtState { } } - /// Preserves the current must-complete invariant for private catalog work. - #[inline] - pub(crate) fn private(checkout: SessionOperationCheckout) -> Self { - Self { - effects: StmtEffects::empty(), - drop_action: StmtDropAction::PrivateMustComplete, - checkout: Some(checkout), - } - } - /// Lends one direct callback-facing statement facade. #[inline] pub(crate) fn statement(&mut self) -> Statement<'_> { @@ -325,21 +296,6 @@ impl StmtState { self.checkout = None; } - /// Preserve partial statement undo in the mandatory nested transaction. - /// - /// This path runs before resuming a prepared catalog callback panic. Redo - /// from the incomplete statement is discarded, while row/index undo is - /// checked back into the stable transaction core for outer fatal retention. - #[inline] - pub(crate) fn return_after_mandatory_panic(mut self) { - self.drop_action = StmtDropAction::Settled; - if let Some(checkout) = self.checkout.as_mut() { - self.effects - .fold_cancelled_into_trx_effects(checkout.inner_mut().effects_mut()); - } - self.checkout = None; - } - #[cold] #[inline(never)] fn settle_armed_drop(&mut self) { @@ -352,12 +308,6 @@ impl StmtState { .fold_cancelled_into_trx_effects(checkout.inner_mut().effects_mut()); checkout.return_cancelled(); } - StmtDropAction::PrivateMustComplete => { - assert!( - self.effects.is_empty(), - "private statement must complete effect settlement before drop" - ); - } StmtDropAction::Settled => {} } } @@ -387,6 +337,21 @@ pub struct Statement<'stmt> { } impl<'stmt> Statement<'stmt> { + /// Create a callback-facing statement over borrowed transaction ownership. + #[inline] + pub(crate) fn new( + inner: &'stmt mut TrxInner, + attachment: &'stmt TrxAttachment, + effects: &'stmt mut StmtEffects, + ) -> Self { + Self { + inner, + attachment, + effects, + disable_dml_validation: false, + } + } + /// Disable default DML shape, type, nullability, sparse-update, key, and /// index-scan validation for this statement. /// @@ -411,12 +376,6 @@ impl<'stmt> Statement<'stmt> { ) } - /// Returns mutable access to this statement's effect accumulator. - #[inline] - pub(crate) fn effects_mut(&mut self) -> &mut StmtEffects { - self.effects - } - #[inline] fn runtime_and_effects_mut(&mut self) -> (TrxRuntime<'_>, &mut StmtEffects) { let runtime = TrxRuntime::new( @@ -1001,6 +960,7 @@ pub(crate) mod tests { use crate::id::TrxID; use crate::lock::LockOwner; use crate::lock::tests::debug_snapshot; + use crate::log::redo::RowRedoKind; use crate::session::{SessionState, tests as session_tests}; use crate::trx::sys::tests as sys_tests; use crate::trx::undo::tests::{pause_next_index_rollback, pause_next_row_rollback}; @@ -1057,6 +1017,18 @@ pub(crate) mod tests { stmt.runtime_and_effects_mut() } + #[inline] + pub(crate) fn statement_effects_mut<'borrow>( + stmt: &'borrow mut Statement<'_>, + ) -> &'borrow mut StmtEffects { + stmt.effects + } + + #[inline] + pub(crate) fn statement_redo<'borrow>(stmt: &'borrow Statement<'_>) -> &'borrow RedoLogs { + &stmt.effects.redo + } + #[inline] fn trx_lock_owner(trx: &mut Transaction) -> Result { let checkout = trx.checkout().disclose()?; @@ -1122,15 +1094,18 @@ pub(crate) mod tests { .count() } - #[test] - fn test_stmt_effects_empty() { - let effects = StmtEffects::empty(); - assert!(effects.is_empty()); + fn assert_stmt_effects_empty(effects: &StmtEffects) { assert!(effects.row_undo.is_empty()); assert!(effects.index_undo.is_empty()); assert!(effects.redo.is_empty()); } + #[test] + fn test_stmt_effects_empty() { + let effects = StmtEffects::empty(); + assert_stmt_effects_empty(&effects); + } + #[test] fn test_cancelled_stmt_effects_fold_undo_and_discard_redo() { let mut trx_effects = TrxEffects::empty(); @@ -1159,12 +1134,18 @@ pub(crate) mod tests { SelectKey::new(0, vec![]), true, ); - effects.set_ddl_redo(DDLRedo::CreateTable(TableID::new(42))); + effects.insert_row_redo( + TableID::new(42), + RowRedo { + row_id: RowID::new(2), + kind: RowRedoKind::Delete(None), + }, + ); let cancelled_row_undo = from_ref(&*effects.row_undo[0]); effects.fold_cancelled_into_trx_effects(&mut trx_effects); - assert!(effects.is_empty()); + assert_stmt_effects_empty(&effects); assert_eq!(trx_effects.row_undo.len(), 2); assert_eq!(trx_effects.row_undo[0].table_id, TableID::new(41)); assert_eq!(trx_effects.row_undo[1].table_id, TableID::new(42)); @@ -1344,13 +1325,13 @@ pub(crate) mod tests { // statement rollback ever runs row rollback before index // rollback, this test fails before the injected index // rollback error can discard the statement safely. - stmt.effects_mut().push_row_undo(OwnedRowUndo::new( + statement_effects_mut(stmt).push_row_undo(OwnedRowUndo::new( TableID::new(99_999_999), None, RowID::new(24), RowUndoKind::Delete, )); - stmt.effects_mut().push_delete_index_undo( + statement_effects_mut(stmt).push_delete_index_undo( TableID::new(12), RowID::new(23), SelectKey::new(0, vec![]), diff --git a/doradb-storage/src/trx/sys.rs b/doradb-storage/src/trx/sys.rs index d1f10337..9037dfe3 100644 --- a/doradb-storage/src/trx/sys.rs +++ b/doradb-storage/src/trx/sys.rs @@ -9,8 +9,8 @@ use crate::component::{ use crate::conf::TrxSysConfig; use crate::error::{ CompletionErrorBridge, DataIntegrityError, DataIntegrityResult, DiscloseError, - DiscloseResultExt, Error, FatalError, FatalResult, MultiDomainResultExt, Result, RuntimeError, - RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, + DiscloseResultExt, Error, FatalError, FatalResult, LifecycleResult, MultiDomainResultExt, + Result, RuntimeError, RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, }; use crate::file::fs::FileSystem; use crate::file::table_file::{MutableTableFile, OldRoot, TableFile}; @@ -36,8 +36,9 @@ use crate::trx::sys_trx::SysTrx; use crate::trx::{ FailedPrecommitCleanupJob, FailedPrecommitReason, FatalRollbackRetention, MAX_COMMIT_TS, MAX_SNAPSHOT_TS, MIN_ACTIVE_TRX_ID, MIN_SNAPSHOT_TS, PrecommitTrx, PreparedTrx, - PreparedTrxPayload, ReleasedTransactionLocks, SessionOperationCleanupJob, - SessionOperationCompletionClaim, SessionOperationEntry, Transaction, TrxInner, + PreparedTrxPayload, PrivateTransaction, ReleasedTransactionLocks, SessionOperationCheckout, + SessionOperationCleanupJob, SessionOperationCompletionClaim, SessionOperationEntry, + Transaction, TrxInner, }; #[cfg(test)] use crate::trx::{PrepareListenerResult, SessionOperationState}; @@ -1056,28 +1057,17 @@ impl TransactionSystem { Ok(table_file) } - /// Initialize one ready transaction core and register its active snapshot. + /// Register one snapshot timestamp in an active GC bucket. #[inline] - fn init_trx( - &self, - session_id: SessionID, - inner: &mut TrxInner, - authority: Box, - ) -> (TrxID, TrxID) { + pub(super) fn register_active_sts(&self) -> (usize, TrxID) { let gc_no = self.next_gc_no(); let gc_bucket = &self.gc_buckets[gc_no]; - // Add to active sts list. let mut g = gc_bucket.active_sts_list.lock(); - // With bucket lock, we can make sure all transactions are ordered by STS. + // Holding the bucket lock preserves STS order within this active list. let sts = TrxID::new(self.ts.fetch_add(1, Ordering::SeqCst)); - let trx_id = TrxID::new(sts.as_u64() | (1 << 63)); debug_assert!(sts < MAX_SNAPSHOT_TS); - debug_assert!(trx_id >= MIN_ACTIVE_TRX_ID); g.insert(sts); if g.len() == 1 { - // Only when the previous list is empty, we should update min_active_sts - // as STS of current transaction. - // In this case, current value of min_active_sts should be MAX. debug_assert!( TrxID::new(gc_bucket.min_active_sts.load(Ordering::Relaxed)) == MAX_SNAPSHOT_TS ); @@ -1085,7 +1075,21 @@ impl TransactionSystem { .min_active_sts .store(sts.as_u64(), Ordering::Relaxed); } - drop(g); // release bucket lock. + drop(g); + (gc_no, sts) + } + + /// Initialize one ready transaction core and register its active snapshot. + #[inline] + fn init_trx( + &self, + session_id: SessionID, + inner: &mut TrxInner, + authority: Box, + ) -> (TrxID, TrxID) { + let (gc_no, sts) = self.register_active_sts(); + let trx_id = TrxID::new(sts.as_u64() | (1 << 63)); + debug_assert!(trx_id >= MIN_ACTIVE_TRX_ID); inner.init(trx_id, sts, gc_no, session_id, authority); (trx_id, sts) } @@ -1105,19 +1109,22 @@ impl TransactionSystem { (handle, entry) } - /// Create a private transaction inside an existing stable operation entry. + /// Create a private transaction inside an existing stable DDL entry. #[inline] pub(crate) fn begin_private_trx( &self, - session: WeakSessionRef, + runtime: SessionRuntime, enclosing_entry: &Arc, mut inner: Box, authority: Box, - ) -> Transaction { + ) -> LifecycleResult { + enclosing_entry.validate_private_transaction_begin()?; let operation_key = enclosing_entry.key(); - let (trx_id, sts) = self.init_trx(operation_key.session_id(), inner.as_mut(), authority); - enclosing_entry.install_private_transaction(inner); - Transaction::new(session, operation_key, trx_id, sts) + let (trx_id, _sts) = self.init_trx(operation_key.session_id(), inner.as_mut(), authority); + let attachment = TrxAttachment::new(runtime, operation_key, trx_id); + let checkout = + SessionOperationCheckout::private(Arc::clone(enclosing_entry), inner, attachment); + Ok(PrivateTransaction::new(checkout)) } /// Allocate a timestamp fence for a runtime state transition. @@ -1269,22 +1276,6 @@ impl TransactionSystem { .await } - /// Roll back an engine-owned table-maintenance transaction with typed domains. - #[inline] - pub(crate) async fn rollback_table_maintenance_transaction( - &self, - claim: SessionOperationCompletionClaim, - ) -> RuntimeOrFatalResult<()> { - let completion = - self.enqueue_terminal_rollback(claim, "rollback table maintenance transaction"); - Self::wait_terminal_rollback_runtime_or_fatal( - completion, - RuntimeError::TableAccess, - "wait for table maintenance terminal rollback cleanup", - ) - .await - } - /// Queue terminal rollback cleanup and return the observer completion. #[inline] fn enqueue_terminal_rollback( @@ -2079,12 +2070,13 @@ pub(crate) mod tests { } fn capture_transaction_cleanup_state( + engine: &Engine, trx: &Transaction, ) -> (Arc, Arc) { - let runtime = trx.engine().expect("test transaction must have runtime"); - let entry = runtime - .state() - .resolve_operation(trx.operation_key) + let (entry, _) = engine + .inner() + .session_registry + .try_resolve_operation(trx.operation_key) .expect("test transaction must resolve"); let status = { let inner_slot = entry.inner.lock(); @@ -2477,7 +2469,7 @@ pub(crate) mod tests { }) .await .unwrap(); - let (entry, status) = capture_transaction_cleanup_state(&trx); + let (entry, status) = capture_transaction_cleanup_state(&engine, &trx); let err = trx.commit().await.unwrap_err(); From 5f0e1b19ba546ef39dbc1f50fe1dd1e5f04d0822 Mon Sep 17 00:00:00 2001 From: jiangzhe Date: Fri, 7 Aug 2026 22:47:07 +0800 Subject: [PATCH 2/2] resolve task --- docs/tasks/000262-private-transactions.md | 881 +++++++--------------- docs/tasks/next-id | 2 +- 2 files changed, 254 insertions(+), 629 deletions(-) diff --git a/docs/tasks/000262-private-transactions.md b/docs/tasks/000262-private-transactions.md index 0352f22b..c46a1d62 100644 --- a/docs/tasks/000262-private-transactions.md +++ b/docs/tasks/000262-private-transactions.md @@ -1,7 +1,7 @@ --- id: 000262 title: Introduce Private Transactions and Maintenance Snapshots -status: proposal # proposal | implemented | superseded +status: implemented # proposal | implemented | superseded created: 2026-08-07 github_issue: 958 --- @@ -10,25 +10,26 @@ github_issue: 958 ## Summary -Introduce a crate-private `PrivateTransaction` for mandatory catalog DDL -instead of representing those transactions with the public `Transaction` -facade. - -`PrivateTransaction` owns one `SessionOperationCheckout` for its complete -lifetime. Catalog DDL can therefore execute several statement-effect -boundaries without repeatedly upgrading weak session reachability, checking -engine and entry state, resolving the operation key, and moving `TrxInner` -through the stable entry between statements. Secondary `MemIndex` maintenance -uses a separate lightweight `PrivateSnapshot` that registers only an STS in -the active GC horizon and directly brands captured roots with its lifetime. - -Move logical catalog DDL staging behind `CatalogStorage` methods. Each method -uses one private statement per catalog table that it actually mutates, derives -persisted row objects from validated metadata, and installs exactly one DDL -redo record directly in transaction effects after all catalog statements -succeed. Remove catalog-specific execution, terminal, and DDL-redo APIs from -the public transaction type while preserving the existing commit, rollback, -lock, recovery, and persisted-redo behavior. +Mandatory catalog DDL now uses a crate-private `PrivateTransaction` instead of +the weak public `Transaction` facade. The private owner retains one +`SessionOperationCheckout`, transaction core, stable operation entry, and +strong runtime attachment from begin through commit, rollback, or synchronous +panic parking. + +Catalog DDL mutations moved behind `CatalogStorage`. Each logical catalog table +is mutated in its own statement-effect boundary, while transaction-owned locks +remain held until terminal cleanup. The single DDL redo marker is installed +directly in transaction effects only after all catalog staging succeeds. + +Secondary `MemIndex` cleanup no longer creates a transaction merely to obtain +a timestamp and root lifetime. It registers a lightweight `PrivateSnapshot` +that participates in the GC watermark and directly brands captured table +roots until the snapshot is dropped. + +Mandatory maintenance now uses a stateful execution object rather than +separate execution-specification, resource, and panic-settlement abstractions. +The accepted scope owns and drops that state before terminal or failed-retained +publication. ## Context @@ -38,637 +39,261 @@ Issue Labels: - priority:medium - codex -The public `Transaction` is intentionally a weak foreground facade. Every -`Transaction::exec` checks lifecycle admission, upgrades the exact weak -session, verifies engine health, resolves the stable operation entry, validates -the independent transaction id, and checks `TrxInner` out for one statement. -These checks are necessary for caller-controlled public transactions because -their handle, future, session, or engine may be dropped independently. - -Private transactions have a different contract. They start only after a DDL -operation has transferred to engine-owned mandatory execution. -The accepted operation and its stable `SessionOperationEntry` outlive the -nested transaction, execution is supervised, and every normal path must -consume the transaction through its domain-specific commit or rollback. There -is no supported caller-controlled abandonment boundary between its internal -steps. - -The current implementation nevertheless returns the public `Transaction` from -`MandatoryOperationGuard::begin_private_trx`. Catalog code then calls -`Transaction::stage_catalog_statement`, checks the core back into the entry, -and repeats the complete public checkout path for later work. Secondary -`MemIndex` cleanup similarly starts a public-shaped transaction, checks it out -only to borrow `TrxReadProof`, returns it, and finally invokes a private -rollback method on the same public facade. - -Catalog mutation ownership is also split at the wrong boundary: - -- `catalog/table.rs` owns - `execute_create_table_catalog_staging` and - `execute_drop_table_catalog_cascade`; -- `catalog/index.rs` owns - `execute_create_index_catalog_update` and - `execute_drop_index_catalog_update`; -- those free functions receive both `CatalogStorage` and public - `Transaction`; -- each function groups mutations of several logical catalog tables into one - `Statement`; and -- each function installs DDL redo through `StmtEffects::set_ddl_redo`. - -Task 000261 removed statement-scope logical locks. A `Statement` is now an -effect and rollback boundary only; all catalog-table logical locks acquired by -its operations belong directly to the transaction. Splitting catalog work by -logical table therefore creates no additional lock identity, lock handoff, or -early-release behavior. Repeated access reuses transaction-owned exact claims -until terminal cleanup. - -This work passes the task complexity gate. It is one internal ownership and API -refactor with focused catalog and maintenance consumers. It does not change a -public API contract, persisted catalog schema, redo encoding, recovery -protocol, lock compatibility rule, or DDL publication sequence, and it does -not require a phased rollout. - -Related design history: - -- `docs/transaction-system.md` describes weak public transactions, stable - operation entries, nested private transaction states, and mandatory panic - retention. -- `docs/lock-system.md` defines the - `SessionExplicit -> Operation -> PrivateTransaction` lock-owner topology. -- `docs/tasks/000247-statement-public-transaction-cancellation-ownership.md` - introduced the current distinct public and private statement drop policies. -- `docs/tasks/000249-runtime-owned-table-ddl.md` and - `docs/tasks/000250-runtime-owned-index-ddl.md` moved catalog DDL into - supervised mandatory execution. -- `docs/tasks/000251-runtime-owned-mandatory-maintenance.md` made the active - cleanup transaction part of supervised maintenance resources. -- `docs/tasks/000261-remove-statement-scope-logical-locks.md` removed the final - statement-owned logical claims and lock-scope state. - -The selected design uses a semantic `PrivateTransaction` facade over the -existing mechanical `SessionOperationCheckout`. It intentionally does not -introduce a second carrier such as `TransactionLease`. A thin wrapper around -public `Transaction` was rejected because it would preserve weak reachability, -abandonment policy, and repeated checkout validation. Maintenance instead uses -`PrivateSnapshot`, because MemIndex cleanup needs only a registered STS and -root lifetime: giving it transaction identity, core state, locks, undo, -terminal claims, or rollback would misrepresent its capabilities. +The public transaction facade is intentionally weak and caller-controlled. +Each public operation must re-establish session reachability, engine health, +operation identity, and exclusive core ownership because its handle, session, +future, or engine can disappear independently. + +Mandatory DDL has a different lifetime. Its accepted operation and stable +session entry are engine-owned, supervised, and guaranteed to outlive the +nested transaction. Reusing the public facade imposed repeated weak upgrades, +registry lookups, identity checks, and core checkout/check-in cycles that did +not express that stronger ownership contract. + +The prior secondary-index cleanup path also used a public-shaped private +transaction even though it needed no transaction identity, mutable core, +status, locks, undo, statements, or terminal protocol. Its actual requirements +were a snapshot timestamp registered in the active GC horizon and a lifetime +that protected a captured table root. + +Task 000261 had already removed statement-scope logical locks. Catalog +statements are therefore effect and rollback boundaries only; splitting +catalog work by logical table does not create additional lock owners or +release claims early. + +Preceding tasks 000247, 000249, 000250, 000251, and 000261 established the +public/private cancellation boundary, mandatory DDL and maintenance ownership, +and transaction-owned logical claims used by this implementation. + +The change is internal. It preserves the public transaction API, persisted +catalog schema, redo encoding, recovery protocol, lock compatibility, and DDL +publication order. ## Goals -1. Add one crate-private `PrivateTransaction` type for mandatory nested DDL +1. Give mandatory catalog DDL a strongly attached private transaction owner. +2. Retain one checked-out core across all private statements and DDL awaits. +3. Keep weak reachability and caller-abandonment behavior exclusive to public transactions. -2. Hold the same checked-out `TrxInner`, stable entry, and strong - `TrxAttachment` for the complete private transaction lifetime. -3. Keep public transaction cancellation, abandonment, weak reachability, and - statement-error semantics confined to public `Transaction`. -4. Reuse existing `TrxInner`, `StmtEffects`, `Statement`, - `SessionOperationCheckout`, `SessionOperationCompletionClaim`, transaction - lock state, commit, and rollback machinery. -5. Make `CatalogStorage` the owner of logical create/drop table and - create/drop index catalog mutations. -6. Use a separate statement-effect boundary for each logical catalog table - that a DDL operation mutates, while retaining batches of rows for the same - catalog table in one statement. -7. Move catalog DDL redo installation from `StmtEffects` to `TrxEffects` and - enforce exactly one transaction-level marker per catalog DDL transaction. -8. Derive catalog row objects inside `CatalogStorage` from already validated - table metadata instead of carrying duplicate row bundles through DDL plans. -9. Let secondary `MemIndex` cleanup retain a lightweight registered - `PrivateSnapshot` whose lifetime directly protects its captured table root. -10. Preserve normal terminal ordering and safely retain a checked-out DDL - private core before mandatory panic publication. -11. Preserve existing catalog contents, DDL redo bytes, recovery - classification, table-file/root publication, runtime installation, and - logical-lock lifetime. +4. Make `CatalogStorage` own logical catalog row staging. +5. Use one statement-effect boundary per mutated catalog table without + changing transaction-level lock ownership. +6. Install exactly one transaction-level DDL redo marker after successful + catalog staging. +7. Use a registered, transaction-free snapshot for secondary `MemIndex` + cleanup. +8. Tie captured table-root lifetimes to the actual transaction read proof or + private snapshot that protects them. +9. Preserve safe mandatory panic retention and all normal terminal ordering. ## Non-Goals -1. Do not change the public `Session::begin_trx`, `Transaction::exec`, - `Transaction::commit`, `Transaction::rollback`, streaming statement, or - explicit-lock APIs. -2. Do not add public access to `PrivateTransaction`, transaction effects, - catalog row accessors, or DDL redo installation. -3. Do not add private-transaction cancellation, asynchronous Drop rollback, - caller abandonment, savepoints, statement retry, or transaction reuse after - terminal completion. -4. Do not change public statement ordinary-error rollback or future - cancellation behavior. -5. Do not change MVCC visibility, STS/CTS allocation, GC bucket registration, - transaction status, undo ordering, row/index operations, or table - admission. -6. Do not reintroduce statement lock ownership or change transaction-owned - logical-lock compatibility, FIFO behavior, acquisition, or terminal - release. -7. Do not change catalog table definitions, row encodings, primary keys, - checkpoint folding, or catalog recovery validation. -8. Do not change `DDLRedo` variants, numeric codes, serialization, table-root - proof rules, or recovery replay policy. -9. Do not move file creation, root publication, runtime construction, - lifecycle gates, compensation, or runtime/history installation into - `CatalogStorage`. -10. Do not redesign session-operation states beyond the transitions required - for one continuously checked-out private core. -11. Do not alter sessionless `SysTrx` DDL records such as row-page creation, - checkpoint publication, or silent-watermark maintenance. -12. Do not rewrite implemented RFC or task documents; update only live - transaction and lock documentation where current behavior changes. +1. No changes to public transaction, statement, stream, or explicit-lock APIs. +2. No public access to private transactions, transaction effects, catalog + staging internals, or DDL redo installation. +3. No private-transaction cancellation, savepoints, retry, caller + abandonment, or asynchronous Drop rollback. +4. No changes to MVCC visibility, STS/CTS allocation, undo ordering, lock + compatibility, or table admission. +5. No catalog schema, row encoding, redo-code, checkpoint-format, or recovery + policy changes. +6. No movement of file creation, root publication, runtime construction, + lifecycle gates, or compensation into `CatalogStorage`. +7. No generalization of `TrxReadProof` beyond a real borrowed `TrxContext`. ## Plan -### 1. Add the semantic private transaction owner - -Define the crate-private type in `doradb-storage/src/trx/mod.rs`: - -```rust -pub(crate) struct PrivateTransaction { - checkout: Option, -} -``` - -Do not duplicate `trx_id`, `sts`, operation key, weak session reachability, or -engine fields. The checked-out `TrxInner` is the authority for transaction -identity and STS, while `SessionOperationCheckout` already owns: - -- the registry-visible `Arc`; -- the exclusive `Box` containing context, transaction effects, - positive table bindings, transaction lock state, activity state, and - terminal cache policy; and -- the strong `TrxAttachment` containing exact session runtime reachability, - operation and transaction identity, engine access, pool guards, and session - cache access. - -Expose only the crate-private operations required by catalog DDL: - -- `trx_id()` for invariant diagnostics when needed; -- `sts()` from `TrxInner::ctx`; -- direct engine-health validation without weak-session or entry lookup; -- a private statement executor used by catalog storage; -- exact-once transaction-level DDL redo installation; -- consuming catalog commit and rollback; -- synchronous parking of a still-active checkout for mandatory panic - retention. - -Keep `SessionOperationCheckout` as the mechanical carrier shared with public -statements. Do not rename it and do not add `TransactionLease`. - -### 2. Begin directly in the checked-out state - -Change `TransactionSystem`, `MandatoryOperationGuard`, and -`AcceptedDdlScope` private-begin paths to return `PrivateTransaction`. - -Initialize the existing fresh private `TrxInner` through the current -transaction-system STS, transaction-id, GC-bucket, status, and lock-authority -logic. Require the enclosing entry to be `Mandatory(None)` and install the -identity directly as `Mandatory(Some(Running))`, with the core owned by the -new checkout rather than temporarily stored in -`SessionOperationEntry::trx_inner`. Private transaction construction is not -available from caller-owned voluntary operation state. - -Construct one strong `TrxAttachment` from the already-owned -`SessionRuntime`, operation key, and new transaction id. Construct the -`SessionOperationCheckout` directly from the stable entry, initialized core, -and attachment. Do not install an available core and immediately call the -public weak-handle checkout path. - -While the private transaction is active, the entry remains in `Running` and -its `trx_inner` slot remains empty across statements, DDL file/runtime awaits, -and index build work. This preserves registry visibility through the entry's -operation state and transaction id without repeatedly moving the core. - -Retain the existing checked-in `Available` representation for panic parking -and defensive Drop handling. An unintentionally dropped, non-terminal private -transaction returns its checkout to the stable entry; accepted execution then -cannot pass `assert_mandatory_finish_ready` and must fail closed rather than -silently publish terminal success. - -### 3. Reuse statement effects without private checkout cycling - -Implement the private statement executor by borrowing -`SessionOperationCheckout::inner_and_attachment_mut`, creating fresh -`StmtEffects`, and lending the existing `Statement` facade to the callback. -Reuse current row/index operations, transaction runtime views, effect merge, -cancelled-effect folding, and undo data structures. - -At the start of each `CatalogStorage::stage_*` group, validate engine health -once through the retained attachment and convert it to the existing catalog -runtime context. This preserves the current check immediately before catalog -mutation even when a private transaction was started before lengthy build or -drain work. Storage operations still report their own runtime failures -normally; the executor does not repeat lifecycle admission, weak upgrade, -registry lookup, health validation, transaction-id validation, or core -take/return between catalog-table boundaries. - -Preserve the current private catalog callback contract: - -- on callback success, merge statement effects into `TrxEffects`; -- on an ordinary `RuntimeResult` error, also merge all complete and partial - undo/effects into `TrxEffects`, return the original error, and require the - owning DDL path to roll back the complete private transaction; -- on callback panic, discard incomplete statement redo, fold residual - row/index undo into `TrxEffects`, settle the statement facade, and resume the - unwind for mandatory supervision. - -An ordinary private error must not perform statement-local asynchronous -rollback or make the transaction reusable by a caller. This differs -intentionally from public `Transaction::exec` and preserves the current -catalog staging behavior. - -Remove `StmtState::private` once no caller needs a statement state that owns a -whole checkout. Retain `StmtState::public` and its -`CancelPublicTransaction` policy for public statements. If shared effect -settlement helpers are extracted, keep public and private policy decisions -explicit rather than parameterizing them with ambiguous booleans. - -Remove `Transaction::stage_catalog_statement`. The public transaction type -must no longer contain a catalog-specific execution surface. - -### 4. Convert a held checkout directly into terminal ownership - -Add an entry transition that validates the exact private transaction id and -moves `Mandatory(Some(Running))` directly to -`Mandatory(Some(Completing))` while the core remains held by the checkout. - -Add a consuming `SessionOperationCheckout` conversion that disarms checkout -Drop and constructs `SessionOperationCompletionClaim` from the already-owned -entry, core, and attachment. Represent any moved fields with `Option` where -needed; do not use unsafe field extraction. - -Use the resulting claim with the existing -`commit_catalog_transaction`, -and `rollback_catalog_transaction` machinery. Preserve prepared commit, group -redo, undo rollback, lock release, GC deregistration, returned family -authority, cache policy, and outer -`Mandatory(Some(Completing)) -> Mandatory(None)` publication. - -Move `commit_catalog_ddl` and `rollback_catalog_ddl` from public `Transaction` -to `PrivateTransaction`. Remove the public facade's crate-private `engine()` -probe from production callers; the private checkout already retains exact -engine reachability. - -### 5. Preserve mandatory panic retention before dropping resources - -A supervised DDL panic may occur while `PrivateTransaction` still owns the -core outside the stable entry. Before `AcceptedDdlScope::handle_panic` -publishes `FailedRetained`, park the active checkout back into the matching -entry: - -1. settle any currently executing private statement effects before resuming - the original unwind; -2. take the optional private transaction from its DDL progress; -3. synchronously return its core through the existing checked-out-to-available - entry transition; and -4. only then retain the outer operation scope and publish - `FailedRetained`. - -The panic path must not start asynchronous rollback, queue abandoned -transaction cleanup, expose an idle session, or allow checkout Drop to return a -core after the entry is already failed. - -Update all four accepted catalog DDL panic handlers to park the optional -transaction in their progress state before invoking the scope panic policy. -The parking steps must remain synchronous and panic-minimal, and the complete -handler must preserve the non-unwinding contract of -`AcceptedExecution::handle_panic`. - -For maintenance, replace the separate specification/resource/scope owners with -one stateful `MaintenanceExecution` object owned by -`AcceptedMaintenanceScope`. The scope implements `AcceptedExecution` -directly, drops `E` before both normal terminal publication and -`FailedRetained`, and centralizes the mandatory-finish readiness check. -Remove `MaintenanceExecutionSpec::{Resources,PanicLabel}`, `settle_panic`, -the `*Resources` structs, and `AcceptedMaintenanceExecution`. - -### 6. Make `CatalogStorage` own logical catalog DDL mutations - -Add `doradb-storage/src/catalog/storage/ddl.rs` and include it from -`catalog/storage/mod.rs`. Define these crate-private methods on -`CatalogStorage`: - -```rust -async fn stage_create_table( - &self, - trx: &mut PrivateTransaction, - table_id: TableID, - metadata: &TableMetadata, -) -> RuntimeResult<()>; - -async fn stage_drop_table( - &self, - trx: &mut PrivateTransaction, - table_id: TableID, - metadata: &TableMetadata, -) -> RuntimeResult<()>; - -async fn stage_create_index( - &self, - trx: &mut PrivateTransaction, - table_id: TableID, - index_no: IndexNo, - new_metadata: &TableMetadata, -) -> RuntimeResult<()>; - -async fn stage_drop_index( - &self, - trx: &mut PrivateTransaction, - table_id: TableID, - index_no: IndexNo, - old_metadata: &TableMetadata, -) -> RuntimeResult<()>; -``` - -The metadata arguments are already validated and protected by the enclosing -DDL gates. CREATE INDEX receives the post-create metadata, from which it -derives both `next_index_no` and the active `IndexSpec` at `index_no`. DROP -INDEX receives the pre-drop metadata, from which it derives the expected -index-column count. Assert inactive or mismatched metadata as a violated -prepared-plan invariant with table and index identifiers. - -Construct `TableObject`, `ColumnObject`, `IndexObject`, and -`IndexColumnObject` inside this module. `TableMetadata` contains the ordered -column names, value kinds, attributes, active stable index numbers, index -attributes, keys, and next index number needed for all persisted rows. - -Remove `CreateTableCatalogObjects` and the duplicate catalog-object fields -from `CreateTablePlan`. `ValidatedCreateTable` and `CreateTablePlan` retain the -validated `Arc` and allocated table id needed by catalog -staging and runtime construction. Keep the row-object structs and low-level -`tables()`, `columns()`, `indexes()`, `index_columns()`, and -`table_replay_silent_watermarks()` accessors as storage implementation -details accepting `&mut Statement`. - -Move the drop-count assertions into the catalog DDL module so persisted-row -expectations remain beside the mutation that produces their counts. - -Remove the four free staging functions from `catalog/table.rs` and -`catalog/index.rs`. Those modules continue to own validation, DDL gates, -prepared plans, provisional files and roots, runtime construction, -commit/rollback compensation, poisoning policy, and runtime/history -publication. +### Private transaction ownership + +`PrivateTransaction` owns an optional `SessionOperationCheckout`. The checkout +contains the stable entry, initialized `TrxInner`, and strong +`TrxAttachment`; no weak session handle or duplicate identity fields are +stored. -### 7. Split statements by mutated catalog table +Private begin is available only to accepted mandatory DDL. It validates +`Mandatory(None)`, allocates the ordinary STS, transaction id, GC bucket, +status, and family authority, then publishes +`Mandatory(Some(Running))` while the core remains in the private checkout. +The entry's checked-in core slot stays empty across statements and DDL awaits. -Use these private statement boundaries, preserving the listed order: +Private statement execution borrows the retained core and attachment, creates +fresh `StmtEffects`, and reuses the ordinary `Statement` facade: -| DDL | Statement boundaries | +- success merges statement effects into transaction effects; +- ordinary error also retains complete and partial undo for whole-transaction + rollback; and +- panic discards incomplete statement redo, folds residual undo into the + transaction, and resumes unwinding under mandatory supervision. + +Commit and rollback convert the held checkout directly into a +`SessionOperationCompletionClaim`, moving +`Running -> Completing -> Mandatory(None)` without checking the core into the +entry between statements. Existing prepare, group commit, rollback, lock +release, GC deregistration, and family-authority return machinery remains +authoritative. + +Defensive Drop parks a still-active private checkout as `Available`. On a +supervised DDL panic, the DDL progress owner parks that checkout synchronously +before the outer operation publishes `FailedRetained`, ensuring undo and lock +ownership remain reachable from the stable entry. + +### Catalog staging and DDL redo + +`CatalogStorage` implements create/drop table and create/drop index staging. +Validated `TableMetadata` is the source for persisted table, column, index, +and index-column rows, eliminating duplicate catalog row bundles from DDL +plans. + +Final statement boundaries are: + +| DDL | Ordered logical-table statements | | --- | --- | -| CREATE TABLE | insert `catalog.tables`; insert all `catalog.columns`; insert all `catalog.indexes`; insert all `catalog.index_columns` | -| DROP TABLE | delete `catalog.index_columns`; delete `catalog.indexes`; delete `catalog.columns`; delete `catalog.tables`; delete optional `catalog.table_replay_silent_watermarks` | -| CREATE INDEX | delete and reinsert `catalog.tables`; insert `catalog.indexes`; insert all `catalog.index_columns` | -| DROP INDEX | delete `catalog.index_columns`; delete `catalog.indexes` | - -All row mutations belonging to the same logical catalog table remain in one -statement. In particular, CREATE INDEX's table-row delete/reinsert is one -`catalog.tables` statement, and all columns or index-column mappings are -batched within their respective table statement. Skip CREATE statements for -an empty optional index or index-column collection; do not manufacture an -empty effect boundary. - -Keep DROP TABLE's silent-watermark delete as its own statement even when no row -exists because absence is a valid result of that attempted logical-table -mutation. Preserve current delete-count and required-row assertions after the -corresponding statement result is available. - -Because task 000261 made all logical claims transaction-owned, this split must -not add lock owners, statement lock cleanup, claim handoff, or release between -boundaries. Successful later access to the same catalog table reuses the -transaction claim. - -### 8. Install DDL redo only at transaction level - -Add an exact-once DDL installation method to `TrxEffects` and delegate to it -through `PrivateTransaction`. The method accepts `DDLRedo`, stores it in the -transaction's `RedoLogs::ddl` slot, and release-asserts that the slot was -previously empty. It returns no replaceable old value. - -Remove `StmtEffects::set_ddl_redo` and its production import of `DDLRedo`. -Statement effects may produce only DML row redo. Keep `RedoLogs` as the shared -merge representation unless a smaller refactor is needed to make the -statement-level absence invariant explicit; do not redesign redo containers or -serialization in this task. - -Each `CatalogStorage::stage_*` method installs its matching marker only after -all catalog-table statements and invariant checks succeed: - -- `DDLRedo::CreateTable(table_id)`; -- `DDLRedo::DropTable(table_id)`; -- `DDLRedo::CreateIndex { table_id, index_no }`; or -- `DDLRedo::DropIndex { table_id, index_no }`. - -If any statement returns an ordinary error, the private transaction contains -the undo needed for all earlier and partial catalog mutations but no DDL -marker. The DDL owner immediately rolls back the complete private transaction. -No commit path may observe catalog DML without its transaction-level marker, -and existing terminal redo invariant checks remain in force. - -Update tests that deliberately construct catalog DML through public -transactions. After `Transaction::exec` merges their DML, install the required -marker through one narrow `#[cfg(test)]` transaction-level helper. Do not -retain a statement-level setter or widen production public APIs for corruption -and recovery tests. Change the cancelled-statement effects test to use DML redo -when verifying that incomplete statement redo is discarded, and add direct -transaction-effects tests for empty and duplicate DDL installation. - -### 9. Migrate catalog DDL progress owners - -Change the transaction field in create/drop table and create/drop index -progress types from `Option` to -`Option`. Call the matching `CatalogStorage::stage_*` -method with validated metadata, then retain the existing phase transitions, -file/root/runtime work, and terminal ordering. - -Consume the private transaction through catalog commit on success and catalog -rollback on every pre-commit failure. Since the private transaction owns a -strong attachment, remove weak-engine-availability branches before rollback; -scope-owned engine and pool access remain authoritative for domain cleanup. - -Preserve the existing policy for failures after catalog commit: perform the -same runtime cleanup or poisoning decisions without attempting to roll back an -already terminal transaction. - -### 10. Add private maintenance snapshots - -Add a lightweight crate-private `PrivateSnapshot` containing an owned -transaction-system guard, registered STS, and GC bucket number. It allocates no -transaction id, mutable core, status object, session child state, locks, undo, -or terminal cleanup task. It exposes only `sts()` and deregisters its STS -synchronously on Drop. - -Extract active-STS registration and deregistration helpers shared with normal -transaction initialization and rollback. Keep `TrxReadProof` exclusively -branded by a borrowed `TrxContext`; a private snapshot cannot mint one. -Generalize only `TableRootSnapshot`'s lifetime marker so a captured root may be -branded either by `TrxReadProof<'ctx>` or directly by -`&'snapshot PrivateSnapshot`. Both constructors must take the real borrowed -capability and no zero-input lifetime constructor may exist. - -Make `MemIndexCleanupExecution` stateful and let it retain an optional -`PrivateSnapshot`. For each cleanup attempt: - -1. register one private snapshot before observing the GC horizon; -2. read its STS and calculate the active GC horizon; -3. preserve the post-start hook and revalidate engine health; -4. borrow the private snapshot directly while capturing and using the table - root snapshot; -5. drop the snapshot-bound root before deregistering the private STS; and -6. yield once and retry with a fresh STS when root publication raced capture. - -Remove explicit checkout, private-transaction state transitions, asynchronous -maintenance rollback, rollback-error combination, and panic parking from this -path. Preserve the unbounded non-busy retry contract, timestamp-fence -reasoning, root visibility hooks, and cleanup outcomes. - -### 11. Update lifecycle documentation and tests - -Update `docs/transaction-system.md` to distinguish: - -- weak, caller-controlled public transaction handles that check out per - operation; -- strongly attached private transactions that own one checkout; -- lightweight private snapshots that own only a registered active STS; -- direct private begin into `Running`; -- no `Available` transition between catalog statements; -- direct held-checkout terminal conversion; and -- required DDL panic parking before `FailedRetained`. - -Update `docs/lock-system.md` only where it describes private transaction -checkout/check-in or family-authority movement. Preserve the three-owner -topology and transaction-lifetime claims introduced by task 000261. - -Audit comments and tests in `session.rs`, `engine.rs`, and transaction modules -for descriptions that still call the private owner a public handle or imply -per-statement private check-in. +| CREATE TABLE | tables, columns, indexes, index columns | +| DROP TABLE | index columns, indexes, columns, tables, optional silent watermark | +| CREATE INDEX | replace tables row, indexes, index columns | +| DROP INDEX | index columns, indexes | + +Rows for the same catalog table remain batched in one statement. Empty +optional CREATE collections do not create empty statements. DROP count and +metadata invariants remain beside the storage mutation that enforces them. + +Statement effects produce only DML redo. After every catalog statement and +invariant succeeds, `PrivateTransaction` installs one matching `DDLRedo` +marker into `TrxEffects`. Duplicate installation is a hard invariant failure. +An ordinary staging error leaves all rollback effects in the private +transaction but installs no DDL marker. + +### Stateful maintenance and private snapshots + +`MaintenanceExecution` is a stateful trait whose implementer owns its domain +resources and panic-diagnostic phase. `AcceptedMaintenanceScope` directly +implements accepted execution, owns `Option`, and drops `E` before normal +terminal publication or failed-retained publication. + +`PrivateSnapshot` lives in `trx/readonly.rs` and owns: + +- a transaction-system guard; +- one registered STS; and +- its GC bucket number. + +It exposes only `sts()` and synchronously deregisters on Drop. It has no +session slot or transaction capabilities. + +`TableRootSnapshot` has explicit constructors for either an active +`TrxReadProof` or a borrowed `PrivateSnapshot`. Both constructors require the +real capability reference, so neither proof can mint an arbitrary lifetime. + +Each secondary `MemIndex` cleanup attempt registers a fresh private snapshot, +calculates the GC horizon, captures and scans a root borrowed from that +snapshot, drops the root, then drops the snapshot. A root-publication race +deregisters the attempt and yields before retrying with a newer STS. + +### Correctness invariants + +- Public transactions retain their weak, caller-controlled behavior. +- Private transactions start only under mandatory DDL authority. +- One private core remains checked out until terminal conversion or panic + parking. +- Catalog logical claims remain transaction-owned across statement boundaries. +- Catalog DML cannot commit without exactly one transaction-level DDL marker. +- `TrxReadProof` remains branded by `TrxContext`. +- Private-root access cannot outlive its registered `PrivateSnapshot`. +- Maintenance execution state drops before the accepted operation becomes + terminal or failed-retained. +- No private STS remains registered after success, retry, error, or panic. ## Implementation Notes +Implemented task 000262 with strongly attached private DDL transactions, +transaction-free maintenance snapshots, stateful maintenance execution, and +storage-owned catalog staging. Public APIs and persisted formats are +unchanged. + +Material implementation outcomes: + +- `PrivateTransaction` owns one checkout across all private statements and + converts it directly into existing commit or rollback ownership. +- CREATE/DROP TABLE and CREATE/DROP INDEX progress owners retain the private + transaction and synchronously park it before mandatory panic retention. +- `CatalogStorage` derives persisted rows from validated metadata and stages + each logical catalog table separately. +- DDL redo moved from `StmtEffects` to an exact-once slot in `TrxEffects`. +- Secondary `MemIndex` cleanup uses `PrivateSnapshot`; private transactions + are now DDL-only. +- `PrivateSnapshot`, registration, Drop cleanup, and focused tests were moved + into `trx/readonly.rs`. +- Root lifetime constructors require either the real transaction read proof or + the real private snapshot. The proposed arbitrary-lifetime + `TrxReadProof::registered` approach was rejected. +- The separate maintenance `Resources` abstraction and `settle_panic` hook + were removed. Domain execution objects now own their resources and describe + their current panic phase. +- Task-local test-only inherent methods were removed or replaced with free + helpers under `trx::tests`; production types expose no new test API. + +Review and verification found one test-only timing issue: catalog setup may +briefly retain an unrelated active STS. The cleanup panic test now waits for +setup registrations to drain before proving that panic releases the private +snapshot. + +Final verification: + +- mandatory task-resolution style gate passed for 21 branch-diff Rust files; +- focused ownership, panic, DDL marker, recovery, and cleanup tests passed; +- `cargo nextest run --workspace` passed 1,698 tests; and +- the alternate `libaio` suite passed 1,588 tests. + ## Impacts -| Area | Expected change | +| Area | Implemented effect | | --- | --- | -| Public transaction API | Public behavior and signatures stay unchanged; crate-private catalog methods and maintenance rollback leave `Transaction`. | -| Private transaction ownership | New semantic facade owns one existing checkout and strong attachment from begin through terminal conversion. | -| Session operation state | Private begin enters `Running` directly; `Available` is used only for panic/defensive parking, not between internal steps. | -| Statement lifecycle | Public cancellation remains in `StmtState`; private catalog execution borrows the long-lived checkout and reuses statement effects. | -| Logical locks | No policy change; all catalog claims remain transaction-owned until commit, rollback, or fatal retention. | -| Catalog API | Four `CatalogStorage::stage_*` methods replace free functions in table/index DDL modules. | -| Catalog plans | CREATE TABLE stops carrying duplicate persisted row objects; storage derives them from validated metadata. | -| Catalog statement granularity | One statement per mutated logical catalog table, with same-table row batches retained. | -| DDL redo | Marker moves from statement effects to the transaction effects exact-once slot; bytes and recovery meaning do not change. | -| DDL runtime flow | Validation, files, roots, gates, compensation, commit order, poisoning, and runtime/history publication stay with table/index modules. | -| Maintenance | Secondary `MemIndex` cleanup uses a lightweight GC-registered `PrivateSnapshot` with no nested session transaction. | -| Maintenance carrier | Stateful execution is owned and settled directly by `AcceptedMaintenanceScope`; there is no separate resources abstraction. | -| Panic supervision | Active DDL private checkout is parked before the stable entry becomes `FailedRetained`; maintenance execution state drops before outer failure publication. | -| Tests | Catalog storage, DDL, transaction effects, recovery corruption helpers, session lifecycle, and maintenance retry tests are updated. | -| Documentation | Live transaction and lock descriptions reflect continuous private checkout ownership. | -| Persistence | No catalog schema, table-file, redo-code, serialization, checkpoint, or recovery-format change. | - -Primary files: - -- `doradb-storage/src/trx/mod.rs` -- `doradb-storage/src/trx/stmt.rs` -- `doradb-storage/src/trx/sys.rs` -- `doradb-storage/src/session.rs` -- `doradb-storage/src/catalog/storage/mod.rs` -- `doradb-storage/src/catalog/storage/ddl.rs` (new) -- `doradb-storage/src/catalog/storage/{tables,columns,indexes}.rs` tests -- `doradb-storage/src/catalog/table.rs` -- `doradb-storage/src/catalog/index.rs` -- `doradb-storage/src/table/gc.rs` -- `doradb-storage/src/recovery/mod.rs` tests -- `docs/transaction-system.md` -- `docs/lock-system.md` +| Public API | No behavior or signature change | +| Private DDL | Strong checkout ownership from begin through terminal | +| Session state | Direct private begin into `Running`; `Available` reserved for parking | +| Statements | Private statements borrow the retained checkout | +| Catalog storage | Owns logical DDL row staging and metadata derivation | +| Logical locks | Remain transaction-owned until terminal cleanup | +| DDL redo | Installed exactly once in transaction effects | +| Maintenance | Stateful execution with no separate resource carrier | +| MemIndex cleanup | Uses a GC-registered `PrivateSnapshot` | +| Root lifetimes | Branded by the actual read proof or private snapshot | +| Panic handling | DDL cores park and maintenance state drops before retention | +| Persistence | No schema, encoding, redo, checkpoint, or recovery-format change | +| Operations | No migration or rollout action required | ## Test Cases -1. Beginning a mandatory private transaction installs the exact operation and - transaction identity directly in `Mandatory(Some(Running))`, leaves the - entry core slot empty, and gives `PrivateTransaction` the initialized core - and strong attachment. -2. Private transaction construction rejects any entry that is not the exact - accepted `Mandatory(None)` DDL operation and cannot start from caller-owned - voluntary state. -3. Two sequential private statement executions use the same `TrxInner` - allocation and never expose `Available` between callbacks. -4. `PrivateTransaction::sts` is sourced from its held `TrxContext`. -5. A successful private statement merges row undo, index undo, and DML redo - into transaction effects without returning the checkout. -6. An ordinary private statement error retains partial undo/effects in the - private transaction, returns the original runtime error, and is fully - reverted by whole-transaction rollback. -7. A private callback panic discards incomplete DML redo, preserves partial - undo in transaction effects, and resumes the original unwind with the - checkout still owned and settled. -8. Public statement success, ordinary-error rollback, fatal rollback, future - cancellation, stream destruction ordering, and abandoned cleanup remain - unchanged after removing `StmtState::private`. -9. Direct held-checkout catalog commit and rollback transition - `Running -> Completing -> Mandatory(None)`, return the same family - authority, deregister the active STS, and preserve transaction status. -10. `PrivateSnapshot` registration contributes its STS to the global GC - watermark and Drop deregisters it exactly once. -11. CREATE TABLE persists one table row, all columns, all active indexes, and - all index-column mappings derived from `TableMetadata`, including a table - with no secondary indexes. -12. CREATE TABLE does not create empty index or index-column statement - boundaries when both collections are empty. -13. DROP TABLE deletes index columns, indexes, columns, the required table row, - and any optional silent watermark in separate ordered statements, with - count assertions matching metadata. -14. CREATE INDEX replaces the table row in one `catalog.tables` statement, - inserts the allocated index row, and inserts all key mappings using the - post-create metadata. -15. DROP INDEX deletes the expected mappings before the index row using the - pre-drop metadata and asserts missing or mismatched prepared metadata. -16. Relation-level catalog staging failures after each successful prior - boundary roll back every catalog row and leave no transaction-level DDL - marker or externally published runtime/root state. -17. All four successful DDL operations install exactly one matching - transaction-level DDL marker after their final catalog statement. -18. Duplicate transaction-level DDL installation release-asserts with the - existing and attempted DDL context, while an empty transaction accepts its - first marker. -19. Statement APIs cannot install DDL redo; cancelled statement tests use DML - redo and continue proving that incomplete redo is discarded. -20. Catalog and recovery tests that intentionally commit direct catalog DML - use only a narrow test-only transaction marker helper after statement - merge and preserve their existing replay outcomes. -21. Existing CREATE/DROP TABLE failure hooks before staging, after staging, - after file/root work, during commit, and after commit retain their current - rollback, cleanup, and poison behavior. -22. Existing CREATE/DROP INDEX build, root publication, commit, cleanup, - recovery proof, and poison tests retain their current outcomes. -23. A supervised DDL panic during a catalog statement and between later DDL - phases parks the private core before `FailedRetained`; dropping the - accepted owner does not panic, lose undo, queue abandoned cleanup, or - expose an idle session. -24. A supervised `MemIndex` cleanup panic drops and deregisters its active - private snapshot before retaining the outer scope, which carries no - nested transaction id. -25. Secondary `MemIndex` cleanup captures a root directly branded by its - private snapshot without an explicit checkout and starts a freshly - registered STS after a root-fence race. -26. Normal, retrying, failed, and panicking maintenance leave no private STS - registration, checked-out core, or unreclaimed family authority after - execution state is settled. -27. Session close and engine shutdown continue waiting for registry-visible - mandatory private transactions and retained failures. -28. Logical-lock tests confirm catalog statements reuse transaction claims and - release them only at private transaction terminal cleanup. -29. Restart, catalog checkpoint, DDL recovery, and index root-proof tests - confirm unchanged persisted redo and catalog state. -30. Run `cargo fmt --check`. -31. Run `cargo clippy --workspace --all-targets -- -D warnings`. -32. Run `cargo nextest run --workspace`. -33. Run alternate-backend lint and tests with - `cargo clippy -p doradb-storage --no-default-features --features libaio --all-targets -- -D warnings` - and - `cargo nextest run -p doradb-storage --no-default-features --features libaio`. -34. Run `tools/style_audit.rs` on the completed branch-diff Rust files. +Completed coverage verifies: + +1. Mandatory private begin publishes the exact running identity while the + checkout owns the core. +2. Sequential private statements retain the same core allocation and do not + expose an available state between callbacks. +3. Private statement success, ordinary error, and panic settle effects with + the intended whole-transaction policy. +4. Direct private commit and rollback preserve status, lock, GC, and family + authority terminal behavior. +5. DDL panic paths park active private cores before `FailedRetained`. +6. Public statement success, error rollback, cancellation, stream teardown, + and abandoned cleanup remain unchanged. +7. All four catalog DDL operations stage the expected rows in ordered + logical-table boundaries. +8. Catalog staging failures roll back earlier and partial mutations without a + DDL marker. +9. Successful DDL installs exactly one marker; duplicate installation fails. +10. Recovery and corruption tests retain CREATE/DROP TABLE and INDEX replay + behavior with the narrow test-only marker helper. +11. `PrivateSnapshot` registration affects the GC horizon and Drop + deregisters it. +12. MemIndex cleanup retries root races with a fresh snapshot and releases its + snapshot on success and supervised panic. +13. Captured roots cannot outlive the transaction read proof or private + snapshot used to create them. +14. Session close and engine shutdown continue respecting registry-visible + mandatory and failed-retained operations. +15. Workspace and alternate-backend suites pass with formatting, clippy, and + repository style checks. ## Open Questions -None. The private transaction owner, private snapshot owner, checkout -lifetime, panic settlement, -maintenance execution ownership, catalog API inputs, catalog statement -boundaries, and transaction-level DDL redo placement are resolved by this -task. +None. No follow-up work was deferred from this task. diff --git a/docs/tasks/next-id b/docs/tasks/next-id index facc274a..96c23a2f 100644 --- a/docs/tasks/next-id +++ b/docs/tasks/next-id @@ -1 +1 @@ -000262 +000263