diff --git a/docs/backlogs/000171-exact-family-lock-system-redesign.md b/docs/backlogs/000171-exact-family-lock-system-redesign.md index 594926df..b6f290b1 100644 --- a/docs/backlogs/000171-exact-family-lock-system-redesign.md +++ b/docs/backlogs/000171-exact-family-lock-system-redesign.md @@ -10,9 +10,27 @@ Prerequisites docs/backlogs/000169-separate-session-operation-lock-scopes.md and ## Deferred From (Optional) +docs/tasks/000249-runtime-owned-table-ddl.md; +docs/rfcs/0026-engine-owned-mandatory-background-runtime.md Phase 2 ## Deferral Context (Optional) +- Defer Reason: Task 000249 must establish complete caller-prepared table-DDL + authority without widening RFC-0026 Phase 2 into the cross-cutting lock + representation redesign. The broader redesign should begin after the current + mandatory-runtime RFC phases establish their operation ownership patterns. +- Findings: Accepted table DDL owns catalog metadata-S and data-IX claims under + its operation owner, while its nested private transaction performs the + catalog writes. Reacquiring those claims for the transaction is + correctness-safe through current same-family coverage, but duplicates lock + manager grants and owner-cache entries. `PreparedCatalogWriteAuthority` + therefore remains a narrow borrowed proof that reuses the operation claims + and preserves the no-reacquisition acceptance boundary. +- Direction Hint: Unify operation and nested-transaction claims under one + exact-family authority model, then remove the special prepared catalog-write + path without adding duplicate grants, a generic lock-bypass flag, or another + ownership-transfer protocol. Preserve the prepared-statement panic settlement + introduced by task 000249 independently of the lock representation. ## Scope Hint diff --git a/docs/benchmark-tool.md b/docs/benchmark-tool.md index a3e593ac..e06d4c47 100644 --- a/docs/benchmark-tool.md +++ b/docs/benchmark-tool.md @@ -270,7 +270,7 @@ doradb-bench --root target/doradb-bench/index-scan run index-scan --num 10000 -- doradb-bench --root target/doradb-bench/index-scan cleanup ``` -## RFC-0025 Successful-Path Measurements +## RFC-0025 and RFC-0026 Successful-Path Measurements The new workloads complete the pre-RFC successful-path shapes needed by RFC-0025: @@ -278,7 +278,7 @@ RFC-0025: - Phase 1/2 statement and transaction evidence uses `stmt-noop` and `trx-noop`. - Phase 2's no-per-item stream budget uses `index-stream`. -- Phase 4's successful table-DDL path uses `table-ddl`. +- RFC-0026 Phase 2's runtime-owned table-DDL path uses `table-ddl`. - Phase 5's successful index-DDL path uses `index-ddl`. - Existing insert, lookup, table-scan, and index-scan workloads remain the row/index/page-loop evidence. diff --git a/docs/engine-component-lifetime.md b/docs/engine-component-lifetime.md index eb944fe8..e86f01b1 100644 --- a/docs/engine-component-lifetime.md +++ b/docs/engine-component-lifetime.md @@ -183,6 +183,18 @@ shutdown. Conversely, a prepared caller future retained without being polled still owns its voluntary resources and can block shutdown until it resumes or drops. +`CREATE TABLE` and `DROP TABLE` are the first production users of this caller +contract. The session validates input, reserves its DDL operation, and acquires +the complete target/catalog lock set while the future is still cancellable. +After caller capacity is available, synchronous acceptance transfers the +operation entry, locks, immutable execution plan, and exact table runtime (for +DROP) to the mandatory owner. The public future then waits only through the +execution-inert observer. Normal completion drops settled progress, releases +the prepared locks, and only then publishes the outer operation terminal. +Unexpected execution unwind instead retains `FailedRetained`, publishes +mandatory-runtime poison, and releases the caller permit after the accepted +owner is dropped. + The supervisor catches both synchronous future construction and polling unwinds while the accepted operation or cleanup job remains in an outer owner. Its domain policy first releases or moves residual unsafe ownership into fatal diff --git a/docs/lock-system.md b/docs/lock-system.md index 9ee0ba85..fd431d8d 100644 --- a/docs/lock-system.md +++ b/docs/lock-system.md @@ -120,8 +120,8 @@ The current implementation uses the following table-level mapping: | Explicit shared table lock | session `S` | session `S` | explicit session | | Explicit exclusive table lock | session `S` | session `X` | explicit session | | Freeze/checkpoint | scoped `S` | scoped `IS` | maintenance operation | -| CREATE TABLE on a new id | scoped `X` | none | DDL operation | -| DROP TABLE | scoped `X` | scoped `X` | DDL operation | +| CREATE TABLE on a new id | target `X`; catalog slots 0-3 `S` | catalog slots 0-3 `IX` | prepared DDL operation, then mandatory owner | +| DROP TABLE | target `X`; catalog slots 0-4 `S` | target `X`; catalog slots 0-4 `IX` | prepared DDL operation, then mandatory owner | | CREATE/DROP INDEX | scoped `X` | scoped `X` | DDL operation | On first touch, statement metadata protection is handed to the transaction @@ -138,6 +138,15 @@ Successfully bound reads therefore retain metadata protection until transaction commit or rollback. Repeated operations use the transaction binding and lock cache. +Table CREATE/DROP acquire their complete fixed lock sequences while the public +session future is still cancellable. Winning mandatory capacity synchronously +transfers the same `OwnerLockState` and operation owner to accepted execution; +there is no release/reacquire window. Catalog statements receive a typed +prepared-write authority that proves metadata S plus data IX for each catalog +table and bypasses ordinary transaction lock acquisition. Index DDL, +maintenance, and ordinary catalog statements continue through their existing +lock-manager paths. + Recovery, purge, and no-transaction replay do not acquire logical locks. They run at lifecycle boundaries where foreground lock owners do not exist. Logical lock state is volatile and is never reconstructed from redo. diff --git a/docs/public-error-audit.csv b/docs/public-error-audit.csv index b11d7705..0de1575a 100644 --- a/docs/public-error-audit.csv +++ b/docs/public-error-audit.csv @@ -1,8 +1,6 @@ file,function_or_method,disclose_calls doradb-storage/src/catalog/index.rs,create_index_for_session,29 doradb-storage/src/catalog/index.rs,drop_index_for_session,16 -doradb-storage/src/catalog/table.rs,create_table_for_session,11 -doradb-storage/src/catalog/table.rs,drop_table_for_session,12 doradb-storage/src/engine.rs,Engine::new_session,1 doradb-storage/src/engine.rs,Engine::try_shutdown,1 doradb-storage/src/engine.rs,bootstrap_inner,23 @@ -20,9 +18,9 @@ doradb-storage/src/session.rs,Session::checkpoint_table_with_wait,3 doradb-storage/src/session.rs,Session::cleanup_secondary_mem_indexes,3 doradb-storage/src/session.rs,Session::close,3 doradb-storage/src/session.rs,Session::create_index,1 -doradb-storage/src/session.rs,Session::create_table,1 +doradb-storage/src/session.rs,Session::create_table,4 doradb-storage/src/session.rs,Session::drop_index,1 -doradb-storage/src/session.rs,Session::drop_table,1 +doradb-storage/src/session.rs,Session::drop_table,4 doradb-storage/src/session.rs,Session::freeze_table,3 doradb-storage/src/session.rs,Session::list_table_ids,1 doradb-storage/src/session.rs,Session::lock_table,2 diff --git a/docs/rfcs/0026-engine-owned-mandatory-background-runtime.md b/docs/rfcs/0026-engine-owned-mandatory-background-runtime.md index 00988160..03738fa5 100644 --- a/docs/rfcs/0026-engine-owned-mandatory-background-runtime.md +++ b/docs/rfcs/0026-engine-owned-mandatory-background-runtime.md @@ -1177,10 +1177,12 @@ focused validation. only where one poll has materially unbounded work. - Non-goals: Do not migrate index DDL, redesign table lifecycle/catalog semantics, or parallelize one table DDL. - - Task Doc: `docs/tasks/TBD.md` - - Task Issue: `#0` - - Phase Status: `pending` - - Implementation Summary: `pending` + - Task Doc: `docs/tasks/000249-runtime-owned-table-ddl.md` + - Task Issue: `#924` + - Phase Status: done + - Implementation Summary: Implemented caller-prepared, mandatory-runtime-owned CREATE TABLE and DROP TABLE with complete operation lock scopes, nested private transactions, supervised completion and panic retention, deterministic cross-thread tests, and validated benchmark parity. [Task Resolve Sync: docs/tasks/000249-runtime-owned-table-ddl.md @ 2026-08-02] + - Related Backlogs: + - `docs/backlogs/000171-exact-family-lock-system-redesign.md` - **Phase 3: Runtime-Owned Index DDL** - Scope: Prepare create/drop index on the caller, including authoritative diff --git a/docs/table-file.md b/docs/table-file.md index 1e3faaad..009ce115 100644 --- a/docs/table-file.md +++ b/docs/table-file.md @@ -202,6 +202,18 @@ publication or enqueue handoff. Consequently `DROP TABLE` either closes a reversible workflow immediately or asynchronously drains the publisher that already won admission. +Public table DDL owns only preparation. `CREATE TABLE` validates its schema and +prepares all logical locks before mandatory acceptance; the runtime-owned task +then creates the provisional file, starts the private catalog transaction, +publishes the initial root, builds the runtime, and preserves the existing +precommit compensation policy. `DROP TABLE` similarly transfers the exact +current-live runtime and complete lock scope before it closes the lifecycle and +waits for any admitted publisher. Dropping the public future after acceptance +does not abandon either file workflow. Ordinary failures still compensate +inside accepted CREATE execution, while a panic or unsafe post-gate DROP +failure is retained and poisons storage rather than running fallible cleanup +from a destructor. + ### 7.1 Data Checkpoint Publication Data checkpoint publishes: diff --git a/docs/tasks/000249-runtime-owned-table-ddl.md b/docs/tasks/000249-runtime-owned-table-ddl.md new file mode 100644 index 00000000..a89a0aa0 --- /dev/null +++ b/docs/tasks/000249-runtime-owned-table-ddl.md @@ -0,0 +1,1278 @@ +--- +id: 000249 +title: Runtime-Owned Table DDL +status: implemented # proposal | implemented | superseded +created: 2026-08-01 +github_issue: 924 +--- + +# Task: Runtime-Owned Table DDL + +## Summary + +Implement Phase 2 of RFC-0026 by moving accepted `CREATE TABLE` and +`DROP TABLE` execution from the caller executor to the engine-owned mandatory +runtime. Keep caller-owned preparation in `session.rs`: validate public input, +reserve one DDL session operation, acquire the complete target and catalog +logical-lock scope, and wait for mandatory capacity while that preparation +remains cancellable. Keep the prepared and accepted execution implementations, +effectful catalog/file/table-lifecycle work, compensation, and result policy in +the catalog table module. + +Use one operation-owned, lifetime-free lock scope across the acceptance +boundary. Start the private catalog transaction only after acceptance, and +teach catalog statements to consume a typed proof of already-prepared catalog +write authority without reacquiring metadata/data locks. Extend the stable +session-operation entry so a mandatory operation can own and settle its nested +private transaction before releasing operation locks and publishing terminal +state. + +Preserve the existing CREATE rollback/file/runtime compensation boundaries and +DROP irreversible-gate poison policy. Before acceptance, caller cancellation +releases every partial or complete preparation grant. After acceptance, caller +or completion-observer drop is execution-inert. Only +`AcceptedExecution::execute` may unwind; `finish`, `handle_panic`, and resource +release are infallible and non-panicking as required by the Phase 1 mandatory +runtime contract. + +## Context + +`Issue Labels:` +`- type:task` +`- priority:medium` +`- codex` + +`Parent RFC:` +`- docs/rfcs/0026-engine-owned-mandatory-background-runtime.md` + +This task is RFC-0026 Phase 2, **Runtime-Owned Table DDL**. Phase 1 is complete +through task `000248` and issue `#922`; it provides: + +- the fixed engine-owned mandatory executor and caller-capacity admission; +- the consuming `PreparedExecution -> AcceptedExecution` handoff; +- exclusive completion producer/observer ownership; +- panic supervision with the accepted owner outside the caught future; +- compact `Voluntary` and `Mandatory` session-operation states; +- concurrent transaction cleanup and ordered shutdown drain. + +The Phase 2 prerequisite remains an implementation gate: focused Phase 1 +acceptance, panic, cleanup, and shutdown tests must pass with both one and +multiple mandatory runner threads before table DDL integration is considered +complete. + +Phase 2 resolves its local catalog-authority choice with a narrow owned +operation-lock scope plus a typed prepared-catalog-write capability. It does +not begin a private transaction before capacity and does not adopt a +transaction-owned lock state across acceptance. This avoids retaining an +active transaction timestamp while prepared work waits for runtime capacity +and avoids a second lock-owner transfer protocol. + +The approved source boundary is: + +- `session.rs` owns `prepare_create_table` and `prepare_drop_table`, complete + caller-side lock acquisition, mandatory submission, and observer waiting; +- `catalog/table.rs` owns create/drop execution plans, the four + prepared/accepted execution carriers, phase progress, compensation, and + catalog-specific invariants; +- the current monolithic `create_table_for_session` and + `drop_table_for_session` functions are removed; +- `SessionDdlContext` remains for index DDL until RFC-0026 Phase 3. + +The current `Session::create_table` path validates user primary-key policy and +table metadata, atomically allocates a table ID, acquires only target metadata +`X`, creates a provisional file, starts a private transaction, stages four +catalog tables, publishes the file/root, builds the runtime, commits, and +installs the runtime on the caller executor. + +The current `Session::drop_table` path acquires target metadata/data `X`, +validates both the current runtime and catalog row, starts a private +transaction, closes and drains the table lifecycle, stages a five-table +catalog cascade, commits, retains the dropped runtime, and requests purge on +the caller executor. `DropTableProgressGuard::drop` poisons if that future is +abandoned after the lifecycle gate. + +Both catalog staging paths currently call transaction statement helpers that +acquire catalog-table metadata `S` and data `IX` after the target DDL locks. +Those hidden operation-lock awaits cannot remain in mandatory execution. +`FreshLockGuard` and `ScopedTableDdlLocks` borrow `&LockManager`, so neither can +cross the required `'static` accepted-task boundary. + +The existing create-table allocator is an `AtomicU64::fetch_add` with a +monotonic user/catalog namespace boundary. Checkpoint metadata persists the +next ID, recovery advances it from recovered CREATE records, and recovery +removes table files absent from recovered current or retained-drop state. +Allocated gaps are allowed. Therefore preparation does not probe current +runtime or catalog storage for a duplicate CREATE ID. An impossible catalog +primary-key or runtime-map duplicate is an execution invariant, not a +recoverable preparation outcome. + +Public user-table primary-key rejection is a different contract and remains. +Task `000206` deliberately keeps `IndexAttributes::PK` internal to catalog +table definitions and requires public `CREATE TABLE` and `CREATE INDEX` to +reject it. This pure input validation is performed before operation +reservation and creates no runtime effect. + +DROP receives an arbitrary external table ID and must still distinguish +`TableNotFound` while retaining the exact current `Arc`. It therefore +performs one current-live runtime lookup after target exclusion. It does not +perform a second catalog-row lookup, a duplicate foreground-live check, or a +separate post-lock health check. Missing catalog rows after a current-live +runtime has been selected are engine invariants handled by the execution-side +catalog cascade and mandatory panic policy. + +The immediately following RFC phase, Phase 3 runtime-owned index DDL, assumes +this task establishes the production session wrapper, operation-entry +transition, typed completion/error observation, prepared catalog authority, +and deterministic cross-thread gate-testing pattern. This task must leave that +assumption intact without migrating index DDL itself. + +Relevant design and implementation sources: + +- `docs/rfcs/0026-engine-owned-mandatory-background-runtime.md` +- `docs/tasks/000248-mandatory-operation-driver-and-concurrent-cleanup-executor.md` +- `docs/tasks/000206-catalog-primary-key-contract.md` +- `docs/architecture.md` +- `docs/engine-component-lifetime.md` +- `docs/transaction-system.md` +- `docs/lock-system.md` +- `docs/table-file.md` +- `docs/process/coding-guidance.md` +- `docs/process/unit-test.md` +- `doradb-storage/src/runtime/mandatory.rs` +- `doradb-storage/src/session.rs` +- `doradb-storage/src/catalog/table.rs` +- `doradb-storage/src/catalog/mod.rs` +- `doradb-storage/src/catalog/storage/{tables,columns,indexes,table_replay_silent_watermarks}.rs` +- `doradb-storage/src/trx/{mod,stmt}.rs` +- `doradb-storage/src/lock/{mod,state}.rs` +- `doradb-storage/src/error.rs` + +## Goals + +1. Split public CREATE/DROP into caller-owned preparation followed by + runtime-owned accepted execution at the first operation-effect boundary. +2. Keep preparation orchestration in `session.rs` and catalog-specific + prepared/accepted execution implementations in catalog modules. +3. Acquire every target and catalog table-level logical lock required by the + accepted call graph before mandatory capacity admission. +4. Transfer one owned `LockManager` guard and exact `OwnerLockState` across + acceptance; release partial preparation cleanly on error or caller drop. +5. Prove that accepted CREATE/DROP performs no `LockManager` acquisition or + operation-lock reacquisition. +6. Start the private catalog transaction only after acceptance and represent + its available, running, completing, empty, and fatal-retained states inside + the enclosing `Mandatory` session operation. +7. Preserve existing public metadata validation, user-PK rejection, + `TableNotFound`, explicit-session-lock conflict, and typed error semantics + without adding redundant ID or catalog-row preflight reads. +8. Preserve CREATE precommit compensation, post-root/commit poison, and + postcommit runtime-install policy. +9. Preserve DROP pre-gate rollback, irreversible lifecycle drain, post-gate + poison, dropped-runtime retention, and purge requests. +10. Make observer drop after acceptance semantically inert at every execution + phase, including when the public session future is abandoned. +11. Keep progress and operation resources in the accepted owner outside the + unwind-caught execution future so panic supervision can publish + `FailedRetained`, poison, and release safe logical resources. +12. Ensure only accepted execution may unwind. Make normal finish, panic + handling, progress/lock Drop, and terminal resource release contain no + panicking assertions or fallible domain cleanup. +13. Release or safely retain all nested transaction and progress ownership + before releasing operation locks, and release locks before normal session + terminal publication. +14. Provide deterministic, engine-scoped, cross-thread tests for preparation, + capacity, acceptance, every reversible/irreversible execution phase, + panic, final release, and shutdown. +15. Establish the production integration pattern required by RFC-0026 Phase 3 + without adding successful transaction/statement hot-path work. + +## Non-Goals + +1. Do not migrate `CREATE INDEX` or `DROP INDEX`; those remain RFC-0026 + Phase 3. +2. Do not migrate checkpoint, redo truncation, index cleanup, or other + maintenance; those remain later RFC phases. +3. Do not redesign table metadata, user primary-key support, table-ID + allocation, catalog schema, catalog redo, table files, recovery format, or + dropped-table retention semantics. +4. Do not remove public user-table `IndexAttributes::PK` rejection or weaken + static catalog primary-key validation. +5. Do not add a CREATE ID-existence query, catalog primary-key preflight, or + DROP catalog-row preflight merely to recheck allocator/catalog invariants. +6. Do not bypass the catalog MVCC engine's inherent unique-index enforcement + or existing row/key shape validation. +7. Do not redesign `LockManager`, add a generic prepared-lock plan API, add + deadlock detection, add lock leases, or revoke a retained caller + preparation. +8. Do not transfer a pre-acceptance private transaction or transaction lock + owner into the runtime. +9. Do not add mandatory scheduler priorities, adaptive capacity, work + stealing, a task registry, or domain-specific DDL workers. +10. Do not parallelize one CREATE/DROP operation or add speculative + cooperative yields where existing awaits already bound a poll. +11. Do not retry DDL automatically, reopen a table after the DROP lifecycle + gate, or reinterpret existing ordinary/fatal failures. +12. Do not run fallible compensation from `handle_panic` or any Drop + implementation after an arbitrary unwind. +13. Do not modify historical completed task documents. RFC-0025 is already + explicitly superseded; update it only if implementation finds a remaining + normative statement that conflicts with RFC-0026. +14. Do not change `.config/nextest.toml` or introduce a second test runner or + timeout policy. + +## Plan + +### 1. Split the public session call paths at the first operation effect + +Refactor `Session::create_table` to follow this sequence: + +```text +pure TableSpec/IndexSpec validation + -> reserve SessionOperationKind::Ddl + -> allocate gap-tolerant TableID + -> build owned CreateTablePlan + -> acquire complete PreparedTableDdlLocks + -> construct catalog::PreparedCreateTable + -> await mandatory caller capacity + -> synchronous accept and detached runtime spawn + -> drop caller mandatory-runtime guard + -> await CompletionObserver +``` + +Refactor `Session::drop_table` to follow: + +```text +reject non-user ID + -> reserve SessionOperationKind::Ddl + -> reject same-session explicit target lock + -> acquire complete PreparedTableDdlLocks + -> resolve exact current-live Arc
under target exclusion + -> construct catalog::PreparedDropTable + -> await mandatory caller capacity + -> synchronous accept and detached runtime spawn + -> drop caller mandatory-runtime guard + -> await CompletionObserver +``` + +Add private `prepare_create_table` and `prepare_drop_table` helpers in +`session.rs`. They own sequencing and cancellation. Catalog code may expose +pure constructors for catalog-specific plan objects and fixed catalog write +target lists, but it must not reacquire the session operation or drive +caller-side preparation. + +Remove `create_table_for_session` and `drop_table_for_session`. Keep +`create_index_for_session`, `drop_index_for_session`, and `SessionDdlContext` +unchanged except for imports or shared helper movement that is mechanically +required. + +Clone the mandatory-runtime access guard from the pinned engine before moving +the session operation into the prepared carrier. Once `submit` returns the +observer, explicitly drop that caller guard before `observer.wait()`. The +mandatory task and permit retain their own runtime guards; the observer must +not become an engine/runtime lifetime authority. + +### 2. Keep only necessary preparation validation + +CREATE pure validation remains before operation reservation: + +- reject user-supplied primary-key index attributes; +- validate column/index shape, referenced column numbers, empty keys, + nullability, duplicate PK metadata, and other existing + `TableMetadata::try_new` rules; +- construct `Arc` and catalog row objects from owned input. + +Allocate the table ID only after pure input succeeds. Cancellation after ID +allocation may leave a gap. This is intentional and already compatible with +checkpoint/recovery allocator semantics. + +Do not query the current runtime map, metadata history, or `catalog.tables` +under the new target lock. The allocator provides uniqueness. Keep target +metadata `X` because it protects the newly published runtime from admission +until CREATE has settled its nested transaction and completed final lock +release, not because preparation expects an ID collision. + +Do not add an explicit catalog primary-key uniqueness scan. Catalog insert +continues using its ordinary unique-index mutation. Any impossible duplicate +reported by the catalog mutation remains an invariant assertion inside +accepted execution and is caught by mandatory panic supervision. + +DROP validation remains deliberately asymmetric because its table ID is +caller-supplied: + +- reject catalog/out-of-range IDs before reserving an operation; +- reject an explicit target lock held by the same session before waiting; +- after complete target exclusion, call the synchronous current-live catalog + runtime lookup exactly once; +- return `OperationError::TableNotFound` when absent and otherwise retain that + exact `Arc
` in `DropTablePlan`. + +Do not call `ensure_user_table_catalog_row` during DROP preparation. Do not add +a second `check_foreground_live`; the current-live map plus target exclusion +selects the authoritative target, while `start_drop_lifecycle` remains the +execution-side transition. Do not add a separate health check after the lock +wait; `mandatory::submit` owns the health/capacity race and releases the +prepared carrier if admission is closed or poisoned. + +Keep existing catalog row/value and primary-key shape validation inside +catalog statement mutation. Those checks validate trusted write construction +against the static catalog schema; they are not an existence preflight and +are outside this task's redundant-read removal. + +### 3. Add one lifetime-free prepared table-DDL lock scope + +Add the following crate-private session-owned shapes, with final naming allowed +to follow local style: + +```rust +pub(crate) struct PreparedTableDdlLocks { + lock_manager: QuiescentGuard, + locks: OwnerLockState, +} + +pub(crate) struct PreparedTableDdlScope { + operation: Option, + locks: Option, +} + +pub(crate) struct AcceptedTableDdlScope { + operation: MandatoryOperationGuard, + locks: Option, + finish_state: TableDdlFinishState, +} +``` + +`PreparedTableDdlLocks` clones the component-owned lock-manager guard and +creates `OwnerLockState` with `SessionOperationPin::operation_lock_owner()`. +Acquisition records each successfully granted resource immediately. If the +current awaited request is cancelled, the lock manager's waiter guard removes +it; if any later acquisition or preparation step fails, dropping the owned +scope releases all previously recorded grants. + +Its Drop implementation calls only the existing idempotent +`OwnerLockState::release_all` path. It must not assert the release count, call +`assert_cleared`, format an invariant report, poison, or perform fallible +cleanup. Exact release behavior is verified by tests outside Drop. + +`PreparedTableDdlScope` explicitly drops/takes the lock scope before the +foreground `SessionOperationPin` so preparation release order is: + +```text +cancel current waiter + -> release every granted operation lock + -> publish foreground operation release/Terminal +``` + +Catalog table code exposes fixed write-target slices derived from the catalog +tables it actually mutates: + +- CREATE: `tables`, `columns`, `indexes`, `index_columns`; +- DROP: the same four plus `table_replay_silent_watermarks`. + +Acquire requests in canonical `LockResource` order: + +CREATE, 9 grants: + +1. target user-table `TableMetadata(table_id)` in `X`; +2. catalog slots 0 through 3 `TableMetadata` in ascending ID order, each `S`; +3. catalog slots 0 through 3 `TableData` in ascending ID order, each `IX`. + +DROP, 12 grants: + +1. target user-table `TableMetadata(table_id)` in `X`; +2. catalog slots 0 through 4 `TableMetadata` in ascending ID order, each `S`; +3. target user-table `TableData(table_id)` in `X`; +4. catalog slots 0 through 4 `TableData` in ascending ID order, each `IX`. + +User table IDs are below catalog table IDs, so these sequences obey the global +metadata-before-data and ascending-ID rule. Do not allocate or expose a +general-purpose lock-plan abstraction for these two fixed lists. + +### 4. Make capacity admission and acceptance a zero-await ownership edge + +Implement these catalog-owned carriers: + +```rust +pub(crate) struct PreparedCreateTable { + scope: PreparedTableDdlScope, + plan: CreateTablePlan, + metadata: MandatoryTaskMetadata, +} + +pub(crate) struct AcceptedCreateTable { + scope: AcceptedTableDdlScope, + progress: CreateTableProgress, +} + +pub(crate) struct PreparedDropTable { + scope: PreparedTableDdlScope, + plan: DropTablePlan, + metadata: MandatoryTaskMetadata, +} + +pub(crate) struct AcceptedDropTable { + scope: AcceptedTableDdlScope, + progress: DropTableProgress, +} +``` + +Implement `PreparedExecution` for both prepared types and +`AcceptedExecution` for both accepted types. Use stable labels +`create_table` and `drop_table`, the exact session operation key, and table ID +in immutable mandatory metadata. + +All vectors, metadata, task diagnostics, and phase containers required to +construct the accepted value must already be allocated before capacity wins. +`PreparedExecution::accept` only destructures owned fields, calls the Phase 1 +consuming `SessionOperationPin::into_mandatory` transition, and constructs the +accepted value. It contains no await, error return, test panic hook, catalog +lookup, file operation, transaction begin, lock acquisition, or expected +rejection. + +The current Phase 1 handoff uses `unwrap`/`assert` to re-resolve and validate +the active entry even though `SessionOperationPin` already owns that exact +entry and no nested transaction is permitted before acceptance. Production +table DDL makes acceptance non-panicking: use the retained entry and exclusive +pin as the ownership proof, perform the direct +`Voluntary(None) -> Mandatory(None)` transition while holding the required +lifecycle mutex, and remove panic-capable relookup from this accepted adapter. +Preserve the lock-order and lifecycle notification behavior. Prove the state +precondition with type/ownership construction and focused tests rather than an +assertion outside the supervised execution future. + +Capacity saturation retains `PreparedCreateTable` or `PreparedDropTable` and +the complete lock scope in the caller future. It does not consume a mandatory +permit or create a detached task until capacity succeeds. Dropping that caller +future releases the preparation normally. A retained but unpolled future may +retain its locks and keep shutdown busy, as documented by RFC-0026. + +### 5. Add typed prepared catalog-write authority + +Introduce a narrow borrowed capability in the transaction statement layer, +approximately: + +```rust +pub(crate) struct PreparedCatalogWriteAuthority<'a> { + locks: &'a OwnerLockState, +} +``` + +Only an accepted prepared-operation lock scope creates this view. Its +catalog-table write assertion checks the authoritative owner-local cache for: + +- `TableMetadata(catalog_table_id)` covered by `S`; +- `TableData(catalog_table_id)` covered by `IX`. + +Do not re-read the lock manager after acceptance. The owned lock state is the +proof and the sole release record. A missing prepared grant is an internal +execution invariant and may assert only while the accepted `execute` future is +inside the mandatory unwind boundary. + +Add a private transaction entry such as +`Transaction::stage_prepared_catalog_statement(authority, callback)`. +`StmtState`/`Statement` may carry an optional borrowed capability for the +duration of that private statement. Catalog insert and primary-key delete +then: + +1. when prepared authority is present, assert exact table coverage and skip + `acquire_table_write_metadata_lock` and + `acquire_table_write_data_lock`; +2. otherwise use the current lock-aware path unchanged; +3. preserve existing DML shape/key validation, statement effects, undo, redo, + rollback, and catalog error narrowing. + +Use the prepared entry only from accepted table DDL. Existing foreground index +DDL, maintenance, catalog tests, and ordinary transaction statements continue +through `stage_catalog_statement` and acquire their normal transaction locks. +Do not add a public or generic boolean `skip_locks` flag. + +Logical table locks are completely prepared. Row undo/CDB ownership, page/tree +latches, IO completion, redo/group commit, lifecycle drain, and other +execution-internal synchronization remain allowed after acceptance because +they are not hidden `LockManager` operation authority. + +### 6. Extend mandatory operations to own a nested private transaction + +Factor private-transaction construction so both `SessionOperationPin` and +`MandatoryOperationGuard` can start it with the exact operation key, kind, +session state, engine, and stable entry. The table DDL adapter calls +`MandatoryOperationGuard::begin_private_trx` only from accepted execution. + +Extend every relevant `SessionOperationEntry` transition exhaustively: + +```text +Voluntary(None) + -- accept_mandatory --> +Mandatory(None) + -- install private transaction --> +Mandatory(Some(Available)) + -- statement checkout --> +Mandatory(Some(Running)) + -- ordinary statement return --> +Mandatory(Some(Available)) + -- commit/rollback terminal claim --> +Mandatory(Some(Completing)) + -- matching transaction finish --> +Mandatory(None) +``` + +Preserve the equivalent `Voluntary(Some(...))` behavior for unmigrated index +DDL and maintenance. + +Update: + +- `install_private_transaction`; +- `take_for_checkout`; +- `return_inner`; +- `take_for_terminal`; +- `take_for_cleanup` where an already-owned terminal path requires it; +- `finish_transaction`; +- shutdown/inspection labels and exhaustive matches; +- focused transition tests. + +An accepted private transaction is owned by the mandatory execution. Normal +error paths explicitly commit or roll it back before returning. Dropping its +`Transaction` handle while the operation is `Mandatory` must not submit a +competing abandoned-transaction cleanup. An arbitrary execution unwind is +instead preserved through the accepted panic policy and +`FailedRetained`. Shutdown must not claim a nested cleanup out from under the +accepted task. + +Successful nested commit or rollback returns to `Mandatory(None)`, never +directly to outer `Terminal`. Outer terminal publication is reserved until +progress resources and operation locks are released. + +### 7. Represent every execution effect in accepted progress + +Construct catalog-specific owned plans before acceptance: + +```rust +pub(crate) struct CreateTablePlan { + table_id: TableID, + metadata: Arc, + table_object: TableObject, + column_objects: Vec, + index_objects: Vec, + index_column_objects: Vec, +} + +pub(crate) struct DropTablePlan { + table_id: TableID, + table: Arc
, +} +``` + +Evolve `CreateTableProgress` so it exists before the provisional file and owns +options for every resource that may survive an await or unwind: + +- immutable plan; +- phase; +- mutable/provisional table file; +- private `Transaction`; +- published `Arc`; +- staged `Arc
`; +- commit timestamp until installation. + +Use phases equivalent to: + +```text +Prepared +FileCreated +PrivateTransactionActive +CatalogStaged +FilePublished +RuntimeBuilt +CatalogCommitted +Installed | Aborted +``` + +Add an owned `DropTableProgress` containing the plan/table, phase, optional +private transaction, and any retained terminal values needed across awaits: + +```text +Prepared +PrivateTransactionActive +LifecycleClosed +DrainComplete +CatalogStaged +CatalogCommitted +RuntimeRetained +``` + +Progress methods may use invariant assertions only when invoked from +`AcceptedExecution::execute`. Progress Drop must contain no assertion, +debug assertion, poison call, fallible cleanup, or phase-dependent ownership +decision. + +### 8. Execute and compensate CREATE inside the mandatory runtime + +The accepted CREATE sequence is: + +```text +execution test hook before first effect + -> create provisional table file + -> retain file in CreateTableProgress + -> begin mandatory-nested private transaction + -> stage four catalog tables with prepared authority + -> publish the table-file root + -> build the user-table runtime + -> commit catalog DDL + -> install the current-live runtime + -> prove nested Mandatory(None) +``` + +The provisional file remains the first operation effect. No file creation, +catalog mutation, transaction begin, lifecycle transition, or runtime +publication occurs in caller preparation. + +Preserve the current compensation matrix: + +- file creation failure: return the typed runtime/IO failure; no catalog + transaction exists; +- private transaction begin failure after file creation: delete the + provisional file inside `execute`; +- catalog staging, file publication, runtime build, or injected precommit + failure: settle statement effects, roll back the private transaction, + destroy any staged runtime, and delete the provisional file; +- cleanup failure before commit: preserve the cleanup/fatal policy and poison + where the existing workflow does, without replacing a stronger fatal + reason; +- catalog commit failure after table-root publication: destroy the staged + runtime, poison, and retain the file for diagnosis/recovery; +- successful commit followed by a failed current-runtime map insertion: + assert the impossible duplicate inside `execute`; mandatory panic policy + poisons and retains the operation instead of reporting an ordinary ID + conflict; +- success: install the runtime, move owned values to their terminal + destinations, and request normal final publication. + +Keep all fallible deletion, rollback, runtime destruction, and poison-source +selection inside `execute`, even when handling an ordinary error. `finish` and +Drop are not compensators. + +### 9. Execute and retain DROP inside the mandatory runtime + +The accepted DROP sequence is: + +```text +begin mandatory-nested private transaction + -> start_drop_lifecycle + -> await foreground/runtime publication drain + -> stage five-table catalog cascade with prepared authority + -> commit catalog DDL + -> publish dropped-runtime/replay-floor retention + -> request dropped-table and metadata-history purge + -> prove nested Mandatory(None) +``` + +Preserve the current policy: + +- private transaction begin failure: no lifecycle effect; +- `start_drop_lifecycle` failure: roll back the private transaction and return + the operation error without poisoning; +- after lifecycle close, catalog cascade failure: best-effort rollback inside + `execute`, retain the original cascade error as the poison source, and never + reopen the table; +- after lifecycle close, commit failure: poison with the current + Runtime-or-Fatal source; +- dropped-runtime retention failure: poison; +- success: preserve the effective replay floor, retained runtime, and both + purge requests. + +The catalog cascade continues asserting that its current-live target row +exists. That assertion is now explicitly an accepted-execution invariant: +preparation does not scan the row merely to turn corruption into +`TableNotFound`. + +Remove `DropTableProgressGuard`. Caller or observer cancellation can no longer +abandon accepted execution, and panic policy is centralized in +`AcceptedDropTable::handle_panic`. + +### 10. Preserve typed errors across completion + +Preparation errors remain in native Operation, Runtime, Lifecycle, or Fatal +domains and are disclosed only at the public `Session` boundary before +submission. + +Accepted execution returns `CompletionResult` directly. Add only the narrow +crate-private conversions required to turn existing source-bearing +Operation/Runtime/Fatal carrier arms into `CompletionErrorBridge`; do not +capture or reconstruct the public `Error` wrapper and do not collapse a +stronger Fatal reason into Runtime. + +The completion observer remains the sole move-once consumer. Waiting discloses +the canonical typed report to the public caller. Dropping the observer marks +the result unobserved but does not touch the accepted operation. The mandatory +runtime logs unobserved ordinary failures and retains/poisons before publishing +fatal completion according to its Phase 1 policy. + +### 11. Enforce an execution-only panic boundary + +Follow the Phase 1 `AcceptedExecution` contract literally: + +- only construction/polling of `execute` is inside `catch_unwind` and may + panic; +- `finish` and `handle_panic` run outside that catch and must not unwind; +- after either starts resource settlement, there is no second recovery + protocol. + +Move or keep all phase assertions, invariant assertions, deliberate panic test +hooks, and panic-capable invariant ownership conversions inside `execute`. +Specifically: + +- remove the current `CreateTableProgress::drop` `debug_assert`; +- remove the DROP progress guard that poisons from Drop; +- do not put `assert`, `debug_assert`, `unwrap`, `expect`, panic hooks, or + fallible domain cleanup in the new lock/scope/progress Drop paths; +- do not assert released lock counts during `finish` or Drop; +- do not perform file deletion, rollback, runtime destruction, catalog work, + or lifecycle transitions from `handle_panic`. + +Audit every RAII value that can be dropped while the caught execution future +is unwinding, not only fields of the outer accepted carrier. In particular, +the current private `StmtState::PrivateMustComplete` Drop assertion, +`SessionOperationCheckout::return_inner` assertions, terminal +`SessionOperationCompletionClaim`, and assertion-bearing `PreparedTrx` Drop +cannot be left on an accepted table-DDL unwind path that could double-panic or +drop rollback ownership. + +Add a mandatory prepared-catalog statement panic-settlement path. Catch a +callback unwind while the `StmtState` owner is still structurally available, +disarm its must-complete Drop policy, clear partial statement redo, fold +residual row/index undo into the nested transaction core, release statement +locks, and return that core directly to a `FailedRetained` mandatory entry +through a non-panicking retention method; then resume the original unwind so +the outer mandatory supervisor handles it. Normal statement success and typed +error behavior remain unchanged. + +Likewise, audit the private transaction terminal edge before calling the +existing commit/rollback machinery. Any active completion claim or prepared +transaction that can still own rollback-relevant state across an await must +either remain in an accepted/transaction-system owner outside the caught +borrowed future or already have crossed an existing supervised, non-lossy +handoff. Do not allow an active `TrxInner`, undo payload, lock state, or +terminal attachment to be destroyed by an assertion-bearing Drop during +unwind. Reuse the Phase 1 completion-claim and failed-precommit retention +patterns; do not introduce a second generic supervisor. + +Split normal mandatory terminal validation from non-panicking publication. +Use a private finish state equivalent to: + +```rust +enum TableDdlFinishState { + Executing, + TerminalReady, + FailedRetained, +} +``` + +At the common normal execution epilogue, verify that the private transaction is +gone and the stable entry is exactly `Mandatory(None)`. This verification may +assert because it is still inside `execute`. Transition the accepted scope to +`TerminalReady` only after that validation succeeds. + +`AcceptedExecution::finish` then uses only non-panicking actions: + +```text +drop/take already-settled progress owners + -> release PreparedTableDdlLocks + -> require the already-established TerminalReady state + -> publish outer Terminal and registry removal +``` + +Refactor the current assertion-bearing `MandatoryOperationGuard::finish` into +an execution-side validation operation and a state-gated terminal transition +that cannot panic. If `finish` defensively observes `Executing`, publish fatal +retention/poison through a non-panicking fallback rather than asserting or +exposing the session as idle. + +On an `execute` unwind, `handle_panic`: + +1. records immutable operation kind/key/table ID/last phase through the + existing mandatory metadata/diagnostic path; +2. calls the non-panicking mandatory `fail_retained` transition; +3. marks the accepted finish state `FailedRetained`; +4. returns the canonical `MandatoryTaskPanic` completion bridge. + +It does not guess whether arbitrary partially completed work can be reversed. +The generic supervisor then poisons the mandatory runtime and completes the +observer. When the accepted carrier is subsequently dropped, its disarmed +mandatory guard and progress owners are inert and its operation lock scope +uses only idempotent release. The caller permit is released after the accepted +owner drops. A retained nested transaction core stays in the stable +`FailedRetained` entry so shutdown continues to observe the unsafe residual. + +### 12. Replace thread-local DDL hooks with deterministic cross-thread hooks + +The existing CREATE failure hook is thread-local and cannot control execution +that resumes on a mandatory runner. Replace it with one test-only, +engine-scoped controller shared by the session preparation and catalog +execution paths. Keep parallel test engines isolated; do not use one +process-global mutable phase selector without an existing serialization guard. + +Use events, channels, barriers, and explicit phase acknowledgements. Do not +use wall-clock sleeps as evidence. Cover hooks around: + +- before/after each target and catalog lock request/grant; +- partial preparation release; +- complete preparation before capacity; +- capacity waiting and winning; +- immediately before and after synchronous acceptance; +- accepted before first effect; +- provisional file creation; +- private transaction begin; +- catalog staging; +- file/root publication; +- runtime build; +- catalog commit; +- CREATE runtime installation; +- DROP lifecycle close; +- DROP drain wait/completion; +- DROP retained-runtime publication; +- normal final lock release and outer terminal publication. + +Failure hooks return typed failures from `execute`. Panic hooks exist only at +accepted execution phases. Preparation, `accept`, `finish`, `handle_panic`, +and Drop hooks may block or observe where appropriate but must never inject a +panic. + +Instrument lock-manager acquisition in tests so an accepted operation can +assert that every request occurred before acceptance. A transaction-owner +metadata/data request after acceptance must fail the test rather than merely +eventually succeeding. + +Add explicit cooperative yields only if measurement shows a single poll does +materially unbounded synchronous work. Existing file/transaction/catalog/drain +awaits already provide scheduling points; do not add unconditional yield +overhead speculatively. + +### 13. Update documentation, RFC phase state, and measurements + +Update current documentation to describe table DDL as caller-prepared and +mandatory-runtime-owned after acceptance: + +- `docs/engine-component-lifetime.md`: add the concrete production table-DDL + use of the Phase 1 acceptance/observer contract; +- `docs/transaction-system.md`: document + `Mandatory(Some(InternalTrxState))`, nested completion back to + `Mandatory(None)`, and `FailedRetained`; +- `docs/lock-system.md`: replace stale nested foreground-DDL cancellation and + future handoff wording for table DDL while leaving index/maintenance scope + explicit; +- `docs/table-file.md`: audit CREATE provisional-file and DROP lifecycle + ownership wording for caller-versus-runtime accuracy; +- `docs/benchmark-tool.md`: map `table-ddl` to RFC-0026 Phase 2 rather than the + superseded RFC-0025 Phase 4 plan. + +Audit RFC-0025 and legacy tests for foreground-driver/handoff assumptions. +RFC-0025 already states that RFC-0026 controls post-Phase-2 execution design, +so avoid historical churn when no normative conflict remains. + +At `$task-resolve`, synchronize RFC-0026 Phase 2: + +- Task Doc: `docs/tasks/000249-runtime-owned-table-ddl.md`; +- Task Issue: the created issue number when available; +- Phase Status: `done`; +- a concise implementation summary with the resolve-sync marker; +- any related backlog produced by implementation review. + +Do not change Phase 2 scope, prerequisites, phase-local choices, non-goals, or +Phase 3 assumptions unless implementation evidence requires an explicit RFC +correction. Phase 3 should continue to cite the production wrapper, +operation-entry, error-observation, and deterministic-gate pattern established +here. + +Run paired repeated `doradb-bench run table-ddl` samples on equivalent fresh +roots and report median and dispersion for successful create/drop cycles. +One mandatory scheduling hop is expected. Queue delay and execution latency +must remain visible. The task adds no mandatory work to ordinary transaction +begin/commit, statements, lookup, insert, or stream paths; any repeatable +regression there blocks resolution. + +### 14. Control the phase-specific risks + +The principal correctness risks and required mitigations are: + +- **Incomplete prepared authority:** omitting one catalog table or leaving one + transaction lock acquisition after acceptance can deadlock/starve mandatory + capacity. Keep catalog target lists beside the actual staging/cascade code, + assert coverage inside execution, and instrument every lock request in + tests. +- **Nested-state regression:** adding `Mandatory(Some(...))` branches to only + the happy path could make rollback, terminal completion, shutdown, or stale + cleanup identities incorrect. Keep every `SessionOperationEntry` match + exhaustive and add direct transition tests before end-to-end DDL tests. +- **Panic outside supervision or double panic:** `finish`, `handle_panic`, and + nested unwinding Drops run outside or during the sole catch boundary. + Remove assertion-based resource Drop, use state-gated non-panicking + transitions, and test unwind at statement, transaction-terminal, and + catalog/table phase boundaries. +- **Unsafe residual release:** arbitrary unwind may leave catalog undo, a + published root, a closed table lifecycle, or a staged runtime. Prefer + `FailedRetained` plus engine poison over speculative compensation; release + only logical operation locks and ordinary Rust owners proven safe after + retention. +- **Caller-side lock retention:** complete preparation can hold target/catalog + locks while capacity is saturated or a live future stops being polled. This + is an accepted RFC consequence; preserve deterministic shutdown diagnostics + and document it rather than adding revocation. +- **Cross-thread test blindness:** thread-local hooks can falsely pass while + production work runs elsewhere. Use engine-scoped event-driven hooks and run + the focused matrix with one and multiple runners. +- **Scheduling overhead:** successful DDL gains a queue/cross-thread hop. + Measure fresh-root latency and dispersion; do not hide queue time. Keep + transaction/statement hot paths unchanged and treat their repeatable + regression as a blocker. +- **Scope expansion into index/maintenance:** shared helpers may reveal later + needs, but this phase implements only the narrow table/catalog authority + proven by CREATE/DROP. Record broader gates, chunking, or scheduling work for + the owning RFC phase or a backlog item. + +## Implementation Notes + +- `CREATE TABLE` and `DROP TABLE` now prepare their complete fixed logical-lock + sets in the caller, transfer one owned DDL scope through mandatory admission, + and execute all file, catalog, transaction, and lifecycle effects on the + engine-owned mandatory runtime. Prepared catalog statements consume typed + operation-lock authority without transaction or statement lock-manager + acquisition. +- Mandatory session operations now support nested private-transaction states + and state-gated normal finalization. Accepted execution panic retains unsafe + nested state as `FailedRetained`; normal and panic settlement release the + operation locks without fallible destructor cleanup. Test-only DDL phases, + gates, and failure injection are engine-scoped helpers inside + `catalog::table::tests`. +- Implementation review simplified ownership without changing behavior: + CREATE file ownership is one mutually exclusive progress enum; public and + private transaction initialization return only the values their callers + need; finish readiness is represented directly by the accepted-scope state; + and DDL preparation plus mandatory submission/supervision are inherent + methods on their owning guards. The mandatory operation guard intentionally + retains the exact stable entry so nested transaction state does not require + lifecycle relookup. +- `PreparedCatalogWriteAuthority` remains a deliberate phase-local bridge. + Reacquiring the catalog claims for the nested transaction is correctness-safe + under current same-family coverage but would duplicate manager grants and + owner-cache entries and weaken the prepared no-reacquisition boundary. + Backlog `000171` owns unifying operation and transaction claims in the + exact-family lock redesign and removing this special path afterward. +- Release measurements on 2026-08-01 used one thread/session, `log-sync=none`, + and equivalent fresh roots. Seven one-cycle `table-ddl` samples had a + candidate median of 585,711 ns per create/drop cycle (range + 372,919-1,537,467 ns) versus 638,670 ns on `origin/main` (range + 353,835-1,097,798 ns). The distributions overlap substantially and show no + repeatable regression from the mandatory scheduling hop. +- Five 500,000-operation hot-path samples showed candidate medians of + 296.962 ns/op for `trx-noop` and 73.433 ns/op for `stmt-noop`, versus + 308.449 ns/op and 74.357 ns/op respectively on `origin/main`. The benchmark + reports caller-visible aggregate latency, so the mandatory queue and + execution contribution remains included rather than split into synthetic + sub-measurements. +- Final verification passed 1,621 workspace tests, 1,528 alternate-`libaio` + tests, focused preparation/acceptance/panic/cleanup coverage, formatting and + diff checks, and the mandatory style audit over 11 branch-diff Rust files. + +## Impacts + +### Primary implementation + +- `doradb-storage/src/session.rs` + - `Session::{create_table,drop_table}` + - new caller preparation helpers + - `SessionOperationPin` + - `MandatoryOperationGuard` + - prepared/accepted table-DDL scopes + - session-operation registry tests and test hook access +- `doradb-storage/src/catalog/table.rs` + - remove `create_table_for_session` and `drop_table_for_session` + - `CreateTableProgress` + - remove `DropTableProgressGuard` + - add `DropTableProgress` + - create/drop plans + - `PreparedCreateTable` / `AcceptedCreateTable` + - `PreparedDropTable` / `AcceptedDropTable` + - catalog target lists, staging/cascade, compensation, and failure hooks +- `doradb-storage/src/catalog/mod.rs` + - current-live runtime access used by DROP preparation + - CREATE runtime-install invariant tests +- `doradb-storage/src/trx/mod.rs` + - mandatory nested private-transaction entry transitions + - mandatory finish validation and non-panicking publication + - terminal/rollback/cleanup matches and tests +- `doradb-storage/src/trx/stmt.rs` + - `PreparedCatalogWriteAuthority` + - prepared catalog statement staging + - catalog insert/delete lock bypass under typed authority +- `doradb-storage/src/lock/state.rs` + - reuse `OwnerLockState` as exact operation grant record and authority proof; + add only narrow read/access support if required +- `doradb-storage/src/lock/mod.rs` + - acquisition instrumentation/tests; no manager redesign +- `doradb-storage/src/error.rs` + - narrow native carrier-to-completion conversion used by accepted DDL +- `doradb-storage/src/runtime/mandatory.rs` + - production use of Phase 1 prepared/accepted APIs + - remove obsolete production `dead_code` expectations + - no scheduler or supervision topology redesign + +### Documentation and validation + +- `docs/engine-component-lifetime.md` +- `docs/transaction-system.md` +- `docs/lock-system.md` +- `docs/table-file.md` +- `docs/benchmark-tool.md` +- `docs/rfcs/0026-engine-owned-mandatory-background-runtime.md` at resolve +- `doradb-bench` existing `table-ddl` workload and lifecycle tests; production + benchmark code changes only if needed to expose already-required queue versus + execution measurements + +### Public behavior + +- Public method signatures and successful catalog/table semantics do not + change. +- Before acceptance, dropping CREATE/DROP remains cancellation and releases + preparation. +- After acceptance, dropping the public future or observer no longer abandons + table DDL; the engine completes, compensates, poisons, or safely retains it. +- A live but unpolled pre-acceptance future may retain logical locks and keep + shutdown busy by documented design. +- Accepted table DDL consumes mandatory caller capacity from acceptance through + normal finish or panic retention/release. + +### Performance + +- Successful CREATE/DROP adds one mandatory-capacity check and executor hop. +- Preparation may retain complete operation locks while waiting for capacity. +- No CREATE duplicate-ID/catalog-row lookup is added. +- DROP removes the extra catalog-row lookup and uses one synchronous + current-live runtime resolution under exclusion. +- Ordinary public transaction and statement hot paths do not use the mandatory + runtime or prepared catalog authority. + +## Test Cases + +### A. Pure preflight and validation + +1. Construct but do not poll `Session::create_table`; assert no operation ID, + table ID, waiter/grant, permit, task, file, or catalog effect. +2. Construct but do not poll `Session::drop_table`; assert the same. +3. Invalid CREATE columns/index shapes fail before session-operation + reservation and table-ID allocation. +4. User `IndexAttributes::PK` fails before operation reservation, locks, or file + creation and preserves task `000206` behavior. +5. A catalog-range/non-user DROP ID fails before operation reservation and + lock acquisition. +6. CREATE ID allocation remains atomic/monotonic; cancellation after allocation + may consume one gap without probing or reusing a current ID. +7. Instrument CREATE preparation to prove it performs no runtime-map, + metadata-history, or `catalog.tables` duplicate-ID lookup. +8. A missing DROP target returns `TableNotFound` after target exclusion and + performs no catalog-row scan. +9. DROP rejects a same-session explicit target lock without entering a + self-conflicting lock wait. + +### B. Prepared lock acquisition and caller cancellation + +10. Assert CREATE owns exactly 9 grants with the approved resources, modes, + operation owner, and canonical order. +11. Assert DROP owns exactly 12 grants with the approved resources, modes, + operation owner, and canonical order. +12. Block and drop preparation during each target/catalog metadata and data + lock wait; assert the current waiter and every earlier grant are released + exactly once. +13. Inject an error after each partial successful grant and assert the same + cleanup. +14. Retain a fully prepared but unpolled future and assert locks remain held, + its session entry is `Voluntary`, and `try_shutdown` reports it as busy. +15. Drop that retained future and assert locks release before outer operation + terminal publication and shutdown progress wakes. +16. Queue a conflicting public transaction behind prepared CREATE/DROP, cancel + preparation, and assert the transaction proceeds without a protection gap + or leaked waiter. +17. Run distinct-table prepared DDL concurrently and assert no unintended + target-table conflict beyond shared catalog write locks. + +### C. Capacity and atomic acceptance + +18. Configure one caller permit and hold the first accepted task before its + first effect. Fully prepare a second table DDL and assert it owns all locks + but has no permit, accepted task, or `Mandatory` state. +19. Drop the capacity-waiting second future and assert complete preparation + release. +20. Release capacity and prove one non-yielding poll moves the exact + `PreparedTableDdlScope` once, transitions + `Voluntary(None) -> Mandatory(None)` once, spawns, and detaches. +21. Allow a runner to poll immediately at acceptance and assert no race can + expose idle/terminal state before the accepted owner exists. +22. Poison or close mandatory admission while capacity is pending; assert the + prepared owner releases and no effect begins. +23. Assert accepted metadata carries the exact operation key, table ID, and + stable CREATE/DROP label. + +### D. No hidden operation-lock acquisition + +24. Instrument every `LockManager` acquisition by owner/resource/time and prove + all accepted CREATE requests occurred before acceptance. +25. Prove the same for DROP. +26. Fail the test if the nested private transaction attempts catalog metadata + `S` or data `IX` after acceptance. +27. For each actual CREATE catalog table, assert prepared authority covers + metadata `S` and data `IX`. +28. For each actual DROP catalog table, including replay silent watermarks, + assert the same. +29. Remove one synthetic prepared grant before an accepted catalog call and + assert the coverage invariant panics inside `execute`, is caught, poisons, + and does not kill the runner. +30. Exercise the ordinary non-prepared catalog statement path and assert it + still acquires transaction locks. + +### E. Mandatory nested transaction states + +31. Cover + `Mandatory(None) -> Mandatory(Available) -> Mandatory(Running) -> + Mandatory(Available)` for successful statement checkout/return. +32. Cover terminal claim and successful commit back to `Mandatory(None)`. +33. Cover explicit rollback back to `Mandatory(None)`. +34. Cover statement error plus whole-private-transaction rollback. +35. Assert nested transaction completion never publishes outer `Terminal`. +36. Assert outer normal terminal is published only after prepared lock release. +37. Drop a nested transaction during an injected execute panic and assert no + competing abandoned-cleanup task is submitted. +38. Assert a retained nested core remains visible as `FailedRetained` and + blocks shutdown. +39. Preserve unmigrated `Voluntary(Some(...))` index DDL and maintenance state + tests. + +### F. Observer and public-future drop + +40. Drop the public CREATE future/observer immediately after acceptance and + before first effect; assert CREATE still reaches a terminal result. +41. Repeat after file creation, private transaction begin, catalog staging, + file publication, runtime build, commit, and before/after installation. +42. Drop the public DROP future/observer before lifecycle close, while drain is + pending, after drain, during catalog cascade, during commit, and before + retained-runtime publication. +43. Race observer drop with result publication and assert the output is + consumed or logged exactly once without influencing execution. +44. Close or abandon the public `Session` after acceptance and assert the + registry retains the exact mandatory operation until finalization. + +### G. CREATE ordinary failures and compensation + +45. File creation failure creates no transaction/catalog/runtime state. +46. Private transaction begin failure deletes the provisional file. +47. Failure after catalog staging rolls back catalog effects and deletes the + file. +48. Failure during file/root publication performs existing precommit cleanup. +49. Failure during runtime build destroys staged state, rolls back, and deletes + the file. +50. Failure after runtime build and before commit follows the same policy. +51. Commit failure after root publication destroys the staged runtime, poisons, + and retains the file according to current recovery policy. +52. An impossible runtime-map duplicate asserts only inside execute, reaches + mandatory panic handling, poisons, and never returns an ordinary ID + conflict. +53. Successful CREATE installs one current-live runtime with matching catalog + rows/file metadata and releases all operation/catalog locks. + +### H. DROP ordinary failures and irreversible policy + +54. Private transaction begin failure leaves the table live. +55. `start_drop_lifecycle` failure rolls back without poisoning. +56. Hold foreground/runtime publication work and assert DROP waits in the + accepted drain phase while target/catalog locks and permit remain owned. +57. Catalog cascade failure after lifecycle close preserves the original + source, attempts rollback, poisons, and never reopens foreground access. +58. Inject rollback failure after the gate and assert it is diagnostic cleanup, + not a replacement for the original poison source. +59. Commit failure after the gate poisons and retains the unsafe residual. +60. Dropped-runtime/replay-floor retention failure poisons. +61. Successful DROP deletes the five catalog row families, publishes retained + runtime/floor state, requests both purge classes, and releases locks only + afterward. +62. Concurrent DROP of the same table waits for the first target lock and then + observes `TableNotFound` from the current-live map without a stale Arc. + +### I. Panic-only execution and non-panicking settlement + +63. Inject an execution panic before CREATE's first effect and after each + representative CREATE phase. +64. Inject an execution panic before DROP's lifecycle gate and after each + representative irreversible DROP phase. +65. Assert each panic calls `handle_panic`, publishes + `MandatoryTaskPanic`, poisons, moves the entry to `FailedRetained`, releases + the caller permit, and leaves the executor runner alive. +66. Assert panic hooks cannot fire in preparation, `accept`, `finish`, + `handle_panic`, or any Drop implementation. +67. Exercise both production `PreparedExecution::accept` implementations + inside `catch_unwind`; assert the retained-entry handoff does not unwind + and moves the exact operation/lock scope once. +68. Exercise `AcceptedCreateTable::finish` and + `AcceptedDropTable::finish` under normal ready states inside + `catch_unwind`; assert no unwind and correct release-before-terminal order. +69. Exercise both `handle_panic` implementations inside `catch_unwind`; assert + no unwind and no fallible compensation. +70. Drop progress and prepared lock scopes for every synthetic phase inside + `catch_unwind`; assert no debug assertion, poison-from-progress-Drop, or + release panic. +71. Panic from inside a prepared catalog statement after residual row/index + effects exist; assert there is no double panic, partial redo is discarded, + undo/core ownership reaches `FailedRetained`, and the outer supervisor + completes. +72. Panic while a nested catalog transaction owns a terminal claim or prepared + transaction before its supervised terminal handoff; assert no active core, + undo, lock state, or attachment is dropped and no assertion-bearing + destructor double-panics. +73. Exercise the defensive missing-terminal-ready state and assert fatal + retention/poison rather than panic or idle publication. +74. Assert accepted panic releases operation locks only through the + non-panicking outer resource scope after safe fatal retention and generic + poison publication. + +### J. Shutdown, runner count, recovery, and regression + +75. With one runner, hold accepted CREATE/DROP at each execution-internal await + and assert blocking shutdown drains rather than cancels it. +76. Repeat focused ownership, panic, and shutdown tests with multiple runners. +77. Assert `try_shutdown` distinguishes retained `Voluntary` preparation, + accepted `Mandatory` table DDL, nonzero caller permits, and + `FailedRetained`. +78. Race shutdown admission close with prepared capacity waiting and with + accepted work; only the prepared waiter is rejected/cancelled. +79. Assert mandatory runtime workers stop only after accepted table DDL and + internal cleanup drain and the executor is empty. +80. Recover after successful CREATE, successful DROP, injected precommit + rollback, and post-root/commit poison residue; preserve existing catalog, + allocator, file cleanup, replay-floor, and dropped-runtime invariants. +81. Re-run existing table DDL, catalog checkpoint, dropped-table purge, + metadata-history purge, file cleanup, explicit lock, DML admission, and + recovery tests. +82. Run the standard workspace nextest pass: + + ```bash + rtk cargo nextest run --workspace + ``` + +83. Run the alternate storage backend because this task changes table-file + creation/publication and recovery-observable behavior: + + ```bash + rtk cargo nextest run -p doradb-storage --no-default-features --features libaio + ``` + +84. Run normal build/lint/style validation required by repository guidance and + `$task-resolve`, including the mandatory style audit for branch-modified + Rust files. +85. Run repeated release-mode `table-ddl` benchmarks on equivalent fresh roots, + report median/dispersion and queue versus execution observations, and + compare ordinary transaction/statement baselines for unintended hot-path + regression. + +## Open Questions + +No blocking design questions remain. + +- `docs/backlogs/000171-exact-family-lock-system-redesign.md` owns the deferred + unification of operation and nested-transaction lock claims and eventual + removal of the phase-local prepared catalog-write authority. This does not + change RFC-0026 Phase 3 prerequisites. diff --git a/docs/tasks/next-id b/docs/tasks/next-id index a7eb3ca7..12fe2aa2 100644 --- a/docs/tasks/next-id +++ b/docs/tasks/next-id @@ -1 +1 @@ -000249 +000250 diff --git a/docs/transaction-system.md b/docs/transaction-system.md index 2ba5aa81..844a21cf 100644 --- a/docs/transaction-system.md +++ b/docs/transaction-system.md @@ -267,17 +267,30 @@ DDL and 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. While the outer foreground -authority remains attached, `ForegroundRunning(Some(InternalTrxState))` +authority remains attached, `Voluntary(Some(InternalTrxState))` records the private transaction's available, checked-out, cleanup, or completion position. Public transactions use the outer operation states directly and therefore use -`ForegroundRunning(None)` only while checked out. A private transaction's terminal -callback clears the child and returns the entry to `ForegroundRunning(None)`; -only dropping the outer foreground authority can publish the operation terminal -and return an open session to idle. One outer operation may run sequential -private transactions, so the entry's optional `TrxID` changes only at -installation and terminal completion while remaining protected by that same -mutex. +`Voluntary(None)` only while checked out. + +Accepted table DDL transfers the same entry to `Mandatory(None)` before the +runtime task is detached. Its 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. + +Unmigrated index DDL and maintenance retain the voluntary private-transaction +path. Their private terminal callback clears the child and returns the entry to +`Voluntary(None)`; only dropping the outer foreground authority publishes the +operation terminal and returns an open session to idle. One outer operation +may run sequential private transactions, so the entry's optional `TrxID` +changes only at installation and terminal completion under that same mutex. After explicit rollback claims terminal ownership and publishes `RollingBack`, the claimed transaction core, undo buffers, locks, and session cleanup @@ -381,18 +394,23 @@ guards, table/layout owners, and logical locks, and then sleeps. This lets same-table DROP acquire metadata X and publish terminal lifecycle state; the listener carries that change into the next bounded recheck. -`CREATE TABLE` allocates a distinct id and then holds `TableMetadata(X)` for -that id while it creates the deterministic table file, stages catalog rows, -builds the per-id runtime, commits the catalog transaction, and publishes the -current history/runtime entry. The initial table-file root uses the create -transaction STS as `root_ts`. Keeping metadata X through current publication -prevents first touch from observing a partially published table. - -`DROP TABLE` prechecks the id-only runtime and catalog row, acquires -`TableMetadata(X)` followed by `TableData(X)`, and then revalidates the target -under those table-local locks before crossing the terminal lifecycle gate. A -drop that waits for an already-admitted checkpoint publisher therefore does -not delay CREATE or DROP for unrelated table ids. Transaction and statement +`CREATE TABLE` validates metadata before reservation, allocates a distinct +gap-tolerant id, and caller-prepares target metadata X plus metadata-S/data-IX +authority for the four catalog tables it writes. Mandatory acceptance then +owns those locks while it creates the deterministic table file, runs its nested +catalog transaction without further manager acquisition, builds the per-id +runtime, commits, and publishes the current history/runtime entry. The initial +table-file root uses the create transaction STS as `root_ts`. + +`DROP TABLE` rejects non-user ids and same-session explicit target locks before +waiting, then caller-prepares target metadata/data X plus metadata-S/data-IX +authority for all five cascade catalog tables. Under target exclusion it +selects the exact current-live `Arc
` without an extra catalog-row scan. +Mandatory execution begins the nested transaction, closes and drains the +terminal lifecycle, performs the catalog cascade, commits, and publishes +dropped-runtime/replay-floor retention. A drop waiting for an already-admitted +checkpoint publisher therefore does not delay CREATE or DROP for unrelated +table ids when runner capacity is available. Transaction and statement rollback drop their operation-local table caches and transaction bindings before releasing the logical locks that authorize those runtime owners. CREATE INDEX and DROP INDEX also take same-table `TableMetadata(X)`. That grant diff --git a/doradb-storage/src/catalog/table.rs b/doradb-storage/src/catalog/table.rs index d4fccad4..dc3a91f2 100644 --- a/doradb-storage/src/catalog/table.rs +++ b/doradb-storage/src/catalog/table.rs @@ -1,11 +1,14 @@ use crate::buffer::PoolGuards; use crate::catalog::spec::{ActiveIndexSpec, ColumnAttributes, ColumnSpec, IndexNo, IndexSpec}; -use crate::catalog::{ColumnObject, IndexColumnObject, IndexObject, TableObject, is_user_table}; +use crate::catalog::{ + ColumnObject, IndexColumnObject, IndexObject, TableObject, catalog_table_id_from_slot, + is_user_table, +}; use crate::engine::EngineRef; use crate::error::{ - DiscloseError, DiscloseResultExt, FatalError, FatalResult, InternalError, InternalResult, - IoResult, OperationError, OperationOrRuntimeResult, OperationResult, Result, RuntimeError, - RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, + CompletionErrorBridge, CompletionResult, FatalError, FatalResult, InternalError, + InternalResult, IoResult, OperationError, OperationOrRuntimeResult, OperationResult, + RuntimeError, RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, }; use crate::file::table_file::{MutableTableFile, TableFile}; use crate::id::{TableID, TrxID}; @@ -15,107 +18,235 @@ use crate::map::FastHashSet; use crate::obs; use crate::row::ops::SelectKey; use crate::row::{Row, RowRead}; +use crate::runtime::mandatory::{AcceptedExecution, MandatoryTaskMetadata, PreparedExecution}; use crate::serde::{Deser, DeserResult, MinBytesHint, Ser, Serde, min_bytes_hint}; -use crate::session::{SessionDdlContext, SessionOperationPin}; +use crate::session::{AcceptedTableDdlScope, PreparedTableDdlScope}; use crate::table::{Table, TableRedoReplayFloor}; -use crate::trx::Transaction; +use crate::trx::{PreparedCatalogWriteAuthority, Transaction}; use crate::value::{Val, ValKind, ValType}; use error_stack::{Report, ResultExt}; use semistr::SemiStr; +use std::any::Any; use std::mem; use std::ops::Index; use std::result::Result as StdResult; use std::sync::Arc; #[cfg(test)] -use tests::{ - CreateTableTestFailure, maybe_fail_create_table, - maybe_poison_before_create_table_catalog_commit, -}; - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum CreateTablePhase { - Init, - CatalogStaged, - FilePublished, - RuntimeBuilt, - CatalogCommitted, - Installed, - Aborted, +use tests::{CreateTableTestFailure, TableDdlTestPhase}; + +const CREATE_TABLE_CATALOG_WRITE_TARGETS: [TableID; 4] = [ + catalog_table_id_from_slot(0), + catalog_table_id_from_slot(1), + catalog_table_id_from_slot(2), + catalog_table_id_from_slot(3), +]; +const DROP_TABLE_CATALOG_WRITE_TARGETS: [TableID; 5] = [ + catalog_table_id_from_slot(0), + catalog_table_id_from_slot(1), + catalog_table_id_from_slot(2), + catalog_table_id_from_slot(3), + catalog_table_id_from_slot(4), +]; + +/// Purely validated public CREATE TABLE input. +pub(crate) struct ValidatedCreateTable { + table_spec: super::TableSpec, + metadata: Arc, } -impl CreateTablePhase { +impl ValidatedCreateTable { + /// Validate public metadata before reserving a session operation or table id. #[inline] - fn is_terminal(self) -> bool { - matches!(self, Self::Installed | Self::Aborted) + pub(crate) fn try_new( + table_spec: super::TableSpec, + index_specs: Vec, + ) -> OperationResult { + reject_user_table_primary_key_indexes(&index_specs, "create_table")?; + let metadata = Arc::new(TableMetadata::try_new( + table_spec.columns.clone(), + index_specs, + )?); + Ok(Self { + table_spec, + 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, + } } } -struct DropTableProgressGuard { - engine: EngineRef, +/// Owned CREATE TABLE execution plan transferred across mandatory acceptance. +pub(crate) struct CreateTablePlan { table_id: TableID, - armed: bool, + metadata: Arc, + table_object: Option, + column_objects: Vec, + index_objects: Vec, + index_column_objects: Vec, +} + +/// Owned DROP TABLE target selected under complete target exclusion. +pub(crate) struct DropTablePlan { + table_id: TableID, + table: Option>, } -impl DropTableProgressGuard { +impl DropTablePlan { + /// Retain the exact current-live runtime selected during preparation. #[inline] - fn new(engine: EngineRef, table_id: TableID) -> Self { + pub(crate) fn new(table_id: TableID, table: Arc
) -> Self { Self { - engine, table_id, - armed: true, + table: Some(table), } } #[inline] - fn disarm(&mut self) { - self.armed = false; + fn take_table(&mut self) -> Arc
{ + self.table.take().unwrap_or_else(|| { + panic!( + "drop-table plan runtime moves exactly once: table_id={}", + self.table_id + ) + }) } } -impl Drop for DropTableProgressGuard { - #[inline] - fn drop(&mut self) { - if self.armed { - let _ = - poison_drop_table_after_gate(&self.engine, self.table_id, "drop_future_abandoned"); - } - } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum CreateTablePhase { + Prepared, + FileCreated, + PrivateTransactionActive, + CatalogStaged, + FilePublished, + RuntimeBuilt, + CatalogCommitted, + Installed, + Aborted, +} + +struct CreateTableCatalogObjects { + table: TableObject, + columns: Vec, + indexes: Vec, + index_columns: Vec, +} + +enum CreateTableFile { + Mutable(MutableTableFile), + Published(Arc), } struct CreateTableProgress { + plan: CreateTablePlan, table_id: TableID, phase: CreateTablePhase, - mutable_file: Option, + file: Option, trx: Option, - table_file: Option>, staged_table: Option>, } impl CreateTableProgress { #[inline] - fn new(table_id: TableID, mutable_file: MutableTableFile) -> Self { + fn new(plan: CreateTablePlan) -> Self { + let table_id = plan.table_id; Self { + plan, table_id, - phase: CreateTablePhase::Init, - mutable_file: Some(mutable_file), + phase: CreateTablePhase::Prepared, + file: None, trx: None, - table_file: None, staged_table: None, } } + #[inline] + fn metadata(&self) -> &Arc { + &self.plan.metadata + } + + #[inline] + fn set_provisional_file(&mut self, mutable_file: MutableTableFile) { + assert_eq!(self.phase, CreateTablePhase::Prepared); + self.file = Some(CreateTableFile::Mutable(mutable_file)); + self.phase = CreateTablePhase::FileCreated; + } + #[inline] fn set_catalog_transaction(&mut self, trx: Transaction) { - debug_assert!(self.trx.is_none()); + assert_eq!(self.phase, CreateTablePhase::FileCreated); + assert!(self.trx.is_none()); self.trx = Some(trx); + self.phase = CreateTablePhase::PrivateTransactionActive; } #[inline] fn mark_catalog_staged(&mut self) { - debug_assert_eq!(self.phase, CreateTablePhase::Init); + assert_eq!(self.phase, CreateTablePhase::PrivateTransactionActive); self.phase = CreateTablePhase::CatalogStaged; } + #[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), + } + } + #[inline] async fn publish_file(&mut self, engine: &EngineRef) -> RuntimeResult<()> { debug_assert_eq!(self.phase, CreateTablePhase::CatalogStaged); @@ -124,10 +255,13 @@ impl CreateTableProgress { .as_ref() .expect("catalog transaction is staged before file publish") .sts(); - let mutable_file = self - .mutable_file + let file = self + .file .take() .expect("mutable create-table file is present before publish"); + let CreateTableFile::Mutable(mutable_file) = file else { + panic!("create-table file is mutable before publish"); + }; let table_file = engine .trx_sys .publish_table_file_root(mutable_file, root_ts, true) @@ -139,7 +273,7 @@ impl CreateTableProgress { self.table_id ) })?; - self.table_file = Some(table_file); + self.file = Some(CreateTableFile::Published(table_file)); self.phase = CreateTablePhase::FilePublished; Ok(()) } @@ -151,11 +285,10 @@ impl CreateTableProgress { engine: &EngineRef, ) -> RuntimeResult<()> { debug_assert_eq!(self.phase, CreateTablePhase::FilePublished); - let table_file = Arc::clone( - self.table_file - .as_ref() - .expect("published table file is present before runtime build"), - ); + let Some(CreateTableFile::Published(table_file)) = self.file.as_ref() else { + panic!("published table file is present before runtime build"); + }; + let table_file = Arc::clone(table_file); let active_root = table_file.active_root_unchecked(); let blk_idx = BlockIndex::new( engine.meta_pool.clone_inner(), @@ -228,10 +361,13 @@ impl CreateTableProgress { #[inline] fn delete_provisional_file(&mut self, engine: &EngineRef) -> IoResult<()> { - if let Some(mutable_file) = self.mutable_file.take() { - let _ = mutable_file.try_delete(); + match self.file.take() { + Some(CreateTableFile::Mutable(mutable_file)) => { + let _ = mutable_file.try_delete(); + } + Some(CreateTableFile::Published(table_file)) => drop(table_file), + None => {} } - let _ = self.table_file.take(); engine.table_fs.delete_user_table_file(self.table_id) } @@ -333,18 +469,6 @@ impl CreateTableProgress { } } -impl Drop for CreateTableProgress { - #[inline] - fn drop(&mut self) { - debug_assert!( - self.phase.is_terminal(), - "create-table progress dropped in non-terminal phase: table_id={}, phase={:?}", - self.table_id, - self.phase - ); - } -} - /// Sparse secondary-index metadata slots keyed by stable table-local index number. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct IndexSpecs { @@ -1131,259 +1255,563 @@ impl Deser for TableBriefMetadata { } } -/// Create a new user table for a session-level DDL request. -pub(crate) async fn create_table_for_session( - session: SessionOperationPin, - table_spec: super::TableSpec, - index_specs: Vec, -) -> Result { - let ctx = SessionDdlContext::new(&session) - .attach("operation=create_table") - .disclose()?; - let engine = ctx.engine.clone(); - let guards = ctx.pool_guards.clone(); - reject_user_table_primary_key_indexes(&index_specs, "create_table").disclose()?; - let metadata = Arc::new( - TableMetadata::try_new(table_spec.columns.clone(), index_specs.clone()).disclose()?, - ); - let table_id = engine.catalog().next_table_id(); - let _metadata_lock = engine - .lock_manager() - .acquire_create_table_metadata_lock(table_id, ctx.owner) - .await - .attach_with(|| format!("operation=create_table, table_id={table_id}")) - .disclose()?; - let uninit_table_file = engine - .table_fs - .create_table_file(table_id, Arc::clone(&metadata), false) - .disclose()?; - - let table_object = TableObject { - table_id, - next_index_no: metadata.idx.next_index_no(), - }; - let column_objects: Vec<_> = table_spec - .columns - .iter() - .enumerate() - .map(|(col_no, col_spec)| ColumnObject { - table_id, - column_no: col_no as u16, - column_name: col_spec.column_name.clone(), - column_type: col_spec.column_type, - column_attributes: col_spec.column_attributes, - }) - .collect(); +/// Caller-prepared CREATE TABLE awaiting mandatory runtime capacity. +pub(crate) struct PreparedCreateTable { + scope: PreparedTableDdlScope, + plan: CreateTablePlan, + metadata: MandatoryTaskMetadata, +} - let mut index_objects = Vec::new(); - let mut index_column_objects = Vec::new(); - for (index_no, index_spec) in 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, ik) 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: ik.col_no, - index_order: ik.order, - }); +impl PreparedCreateTable { + /// Build one fully prepared CREATE TABLE carrier. + #[inline] + pub(crate) fn new(scope: PreparedTableDdlScope, plan: CreateTablePlan) -> Self { + let metadata = MandatoryTaskMetadata::table_operation( + ::LABEL, + scope.key(), + plan.table_id, + ); + Self { + scope, + plan, + metadata, } } +} - let mut progress = CreateTableProgress::new(table_id, uninit_table_file); - let mut trx = match session.begin_private_trx().attach("operation=create_table") { - Ok(trx) => trx, - Err(err) => { - let err = match progress.delete_provisional_file(&engine) { - Ok(()) => err, - Err(cleanup_err) => err.attach(cleanup_err.attach(format!( - "create table provisional-file cleanup failed: table_id={table_id}" - ))), - }; - progress.phase = CreateTablePhase::Aborted; - return Err(err.disclose()); - } - }; +impl PreparedExecution for PreparedCreateTable { + type Output = TableID; + type Accepted = AcceptedCreateTable; - let exec_res = execute_create_table_catalog_staging( - &engine, - &mut trx, - table_id, - table_object, - column_objects, - index_objects, - index_column_objects, - ) - .await; - progress.set_catalog_transaction(trx); - if let Err(err) = exec_res { - return Err(progress - .abort_before_catalog_commit(&engine, &guards, "catalog_staging", err) - .await - .disclose()); + const LABEL: &'static str = "create_table"; + + #[inline] + fn metadata(&self) -> MandatoryTaskMetadata { + self.metadata.clone() } - progress.mark_catalog_staged(); - #[cfg(test)] - if let Err(err) = maybe_fail_create_table(CreateTableTestFailure::AfterCatalogStaged) { - return Err(progress - .abort_before_catalog_commit(&engine, &guards, "test_after_catalog_staging", err) - .await - .disclose()); + #[inline] + fn accept(self) -> Self::Accepted { + let Self { + scope, + plan, + metadata: _, + } = self; + let table_id = plan.table_id; + AcceptedCreateTable { + scope: scope.accept(), + table_id, + progress: Some(CreateTableProgress::new(plan)), + } } +} - if let Err(err) = progress.publish_file(&engine).await { - return Err(progress - .abort_before_catalog_commit(&engine, &guards, "file_publish", err) - .await - .disclose()); +/// Mandatory-runtime owner of accepted CREATE TABLE execution. +pub(crate) struct AcceptedCreateTable { + scope: AcceptedTableDdlScope, + table_id: TableID, + progress: Option, +} + +impl AcceptedExecution for AcceptedCreateTable { + type Output = TableID; + + #[inline] + async fn execute(&mut self) -> CompletionResult { + let result = self.execute_inner().await; + self.scope.mark_terminal_ready(); + result } - #[cfg(test)] - if let Err(err) = maybe_fail_create_table(CreateTableTestFailure::AfterFilePublished) { - return Err(progress - .abort_before_catalog_commit(&engine, &guards, "test_after_file_publish", err) - .await - .disclose()); + #[inline] + fn finish(&mut self) { + drop(self.progress.take()); + self.scope.finish(); } - if let Err(err) = progress.build_runtime(&guards, &engine).await { - return Err(progress - .abort_before_catalog_commit(&engine, &guards, "runtime_build", err) - .await - .disclose()); + #[inline] + async fn handle_panic(&mut self, _panic: Box) -> CompletionErrorBridge { + self.scope.handle_panic(); + let phase = match self.progress.as_ref() { + Some(progress) => progress.phase, + None => CreateTablePhase::Aborted, + }; + CompletionErrorBridge::capture(Report::new(FatalError::MandatoryTaskPanic).attach(format!( + "accepted CREATE TABLE panicked: table_id={}, phase={:?}", + self.table_id, phase + ))) } +} - #[cfg(test)] - if let Err(err) = maybe_fail_create_table(CreateTableTestFailure::AfterRuntimeBuilt) { - return Err(progress - .abort_before_catalog_commit(&engine, &guards, "test_after_runtime_build", err) - .await - .disclose()); +impl AcceptedCreateTable { + async fn execute_inner(&mut self) -> CompletionResult { + let scope = &mut self.scope; + let progress = self + .progress + .as_mut() + .unwrap_or_else(|| panic!("accepted CREATE progress exists during execution")); + let engine = scope.engine().clone(); + let guards = scope.pool_guards(); + let table_id = progress.table_id; + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::CreateBeforeFirstEffect) + .await; + + let mutable_file = engine + .table_fs + .create_table_file(table_id, Arc::clone(progress.metadata()), false) + .map_err(CompletionErrorBridge::capture)?; + progress.set_provisional_file(mutable_file); + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::CreateFileCreated) + .await; + + let trx = match scope.begin_private_trx() { + Ok(trx) => trx, + Err(err) => { + let source_debug = format!("{err:?}"); + progress.phase = CreateTablePhase::Aborted; + if let Err(cleanup_err) = progress.delete_provisional_file(&engine) { + return Err(CompletionErrorBridge::capture(cleanup_err.attach(format!( + "create table provisional-file cleanup failed after transaction begin: table_id={table_id}, source_error={source_debug}" + )))); + } + return Err(CompletionErrorBridge::capture( + err.attach("operation=create_table, phase=begin_private_transaction"), + )); + } + }; + progress.set_catalog_transaction(trx); + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::CreatePrivateTransactionBegun) + .await; + + let catalog_objects = progress.take_catalog_objects(); + let authority = scope.catalog_write_authority(); + let exec_res = execute_create_table_catalog_staging( + &engine, + progress + .trx + .as_mut() + .unwrap_or_else(|| panic!("CREATE staging requires private transaction")), + authority, + table_id, + catalog_objects, + ) + .await; + if let Err(err) = exec_res { + return Err(capture_runtime_or_fatal( + progress + .abort_before_catalog_commit(&engine, &guards, "catalog_staging", err) + .await, + )); + } + progress.mark_catalog_staged(); + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::CreateCatalogStaged) + .await; + + #[cfg(test)] + if let Err(err) = engine + .table_ddl_test + .maybe_fail_create(CreateTableTestFailure::AfterCatalogStaged) + { + return Err(capture_runtime_or_fatal( + progress + .abort_before_catalog_commit( + &engine, + &guards, + "test_after_catalog_staging", + err, + ) + .await, + )); + } + + if let Err(err) = progress.publish_file(&engine).await { + return Err(capture_runtime_or_fatal( + progress + .abort_before_catalog_commit(&engine, &guards, "file_publish", err) + .await, + )); + } + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::CreateFilePublished) + .await; + + #[cfg(test)] + if let Err(err) = engine + .table_ddl_test + .maybe_fail_create(CreateTableTestFailure::AfterFilePublished) + { + return Err(capture_runtime_or_fatal( + progress + .abort_before_catalog_commit(&engine, &guards, "test_after_file_publish", err) + .await, + )); + } + + if let Err(err) = progress.build_runtime(&guards, &engine).await { + return Err(capture_runtime_or_fatal( + progress + .abort_before_catalog_commit(&engine, &guards, "runtime_build", err) + .await, + )); + } + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::CreateRuntimeBuilt) + .await; + + #[cfg(test)] + if let Err(err) = engine + .table_ddl_test + .maybe_fail_create(CreateTableTestFailure::AfterRuntimeBuilt) + { + return Err(capture_runtime_or_fatal( + progress + .abort_before_catalog_commit(&engine, &guards, "test_after_runtime_build", err) + .await, + )); + } + + #[cfg(test)] + engine + .table_ddl_test + .maybe_poison_before_create_commit(&engine); + + let create_cts = match progress.commit_catalog().await { + Ok(create_cts) => create_cts, + Err(err) => { + return Err(capture_runtime_or_fatal( + progress + .abort_after_root_publish_commit_error( + &engine, + &guards, + "catalog_commit", + err, + ) + .await, + )); + } + }; + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::CreateCatalogCommitted) + .await; + + assert!( + progress.install_runtime(&engine, create_cts), + "allocated CREATE TABLE id duplicated current runtime during accepted execution: table_id={table_id}" + ); + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::CreateRuntimeInstalled) + .await; + + Ok(table_id) } +} - #[cfg(test)] - maybe_poison_before_create_table_catalog_commit(&engine); +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum DropTablePhase { + Prepared, + PrivateTransactionActive, + LifecycleClosed, + DrainComplete, + CatalogStaged, + CatalogCommitted, + RuntimeRetained, +} - let create_cts = match progress.commit_catalog().await { - Ok(create_cts) => create_cts, - Err(err) => { - return Err(progress - .abort_after_root_publish_commit_error(&engine, &guards, "catalog_commit", err) - .await - .disclose()); +struct DropTableProgress { + plan: DropTablePlan, + phase: DropTablePhase, + trx: Option, +} + +impl DropTableProgress { + #[inline] + fn new(plan: DropTablePlan) -> Self { + Self { + plan, + phase: DropTablePhase::Prepared, + trx: None, } - }; + } +} - if !progress.install_runtime(&engine, create_cts) { - return Err( - poison_create_table_after_commit(&engine, table_id, "runtime_install").disclose(), +/// Caller-prepared DROP TABLE awaiting mandatory runtime capacity. +pub(crate) struct PreparedDropTable { + scope: PreparedTableDdlScope, + plan: DropTablePlan, + metadata: MandatoryTaskMetadata, +} + +impl PreparedDropTable { + /// Build one fully prepared DROP TABLE carrier. + #[inline] + pub(crate) fn new(scope: PreparedTableDdlScope, plan: DropTablePlan) -> Self { + let metadata = MandatoryTaskMetadata::table_operation( + ::LABEL, + scope.key(), + plan.table_id, ); + Self { + scope, + plan, + metadata, + } + } +} + +impl PreparedExecution for PreparedDropTable { + type Output = (); + type Accepted = AcceptedDropTable; + + const LABEL: &'static str = "drop_table"; + + #[inline] + fn metadata(&self) -> MandatoryTaskMetadata { + self.metadata.clone() } - Ok(table_id) + #[inline] + fn accept(self) -> Self::Accepted { + let Self { + scope, + plan, + metadata: _, + } = self; + let table_id = plan.table_id; + AcceptedDropTable { + scope: scope.accept(), + table_id, + progress: Some(DropTableProgress::new(plan)), + } + } } -/// Logically drop an existing user table for a session-level DDL request. -pub(crate) async fn drop_table_for_session( - session: SessionOperationPin, +/// Mandatory-runtime owner of accepted DROP TABLE execution. +pub(crate) struct AcceptedDropTable { + scope: AcceptedTableDdlScope, table_id: TableID, -) -> Result<()> { - let ctx = SessionDdlContext::new(&session) - .attach("operation=drop_table") - .disclose()?; - let engine = ctx.engine.clone(); - let lock_manager = engine.lock_manager(); - reject_non_user_table_id(table_id, "drop_table").disclose()?; - lock_manager - .reject_table_ddl_explicit_session_lock(table_id, ctx.owner) - .attach("operation=drop_table") - .disclose()?; - let _table_locks = lock_manager - .acquire_table_ddl_locks(table_id, ctx.owner) - .await - .attach_with(|| format!("operation=drop_table, table_id={table_id}")) - .disclose()?; - let table = validated_drop_table_target(&ctx.pool_guards, &engine, table_id) - .await - .disclose()?; - engine.poisoner.ensure_healthy().disclose()?; - - let mut trx = session - .begin_private_trx() - .attach("operation=drop_table") - .disclose()?; - - let drain = match table.start_drop_lifecycle() { - Ok(drain) => drain, - Err(err) => { - if let Err(rollback_err) = trx.rollback_catalog_ddl().await { - return Err(rollback_err.disclose()); + progress: Option, +} + +impl AcceptedExecution for AcceptedDropTable { + type Output = (); + + #[inline] + async fn execute(&mut self) -> CompletionResult { + let result = self.execute_inner().await; + self.scope.mark_terminal_ready(); + result + } + + #[inline] + fn finish(&mut self) { + drop(self.progress.take()); + self.scope.finish(); + } + + #[inline] + async fn handle_panic(&mut self, _panic: Box) -> CompletionErrorBridge { + self.scope.handle_panic(); + let phase = match self.progress.as_ref() { + Some(progress) => progress.phase, + None => DropTablePhase::Prepared, + }; + CompletionErrorBridge::capture(Report::new(FatalError::MandatoryTaskPanic).attach(format!( + "accepted DROP TABLE panicked: table_id={}, phase={:?}", + self.table_id, phase + ))) + } +} + +impl AcceptedDropTable { + async fn execute_inner(&mut self) -> CompletionResult<()> { + let scope = &mut self.scope; + let progress = self + .progress + .as_mut() + .unwrap_or_else(|| panic!("accepted DROP progress exists during execution")); + let engine = scope.engine().clone(); + let table_id = progress.plan.table_id; + let table = progress.plan.take_table(); + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::DropBeforeFirstEffect) + .await; + + let trx = scope.begin_private_trx().map_err(|err| { + CompletionErrorBridge::capture( + err.attach("operation=drop_table, phase=begin_private_transaction"), + ) + })?; + progress.trx = Some(trx); + progress.phase = DropTablePhase::PrivateTransactionActive; + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::DropPrivateTransactionBegun) + .await; + + let drain = match table.start_drop_lifecycle() { + Ok(drain) => drain, + Err(err) => { + let source_debug = format!("{err:?}"); + let trx = progress.trx.take().unwrap_or_else(|| { + panic!("DROP lifecycle failure requires private transaction") + }); + if let Err(rollback_err) = trx.rollback_catalog_ddl().await { + return Err(capture_runtime_or_fatal(rollback_err.attach_with(|| { + format!( + "drop table rollback failed after lifecycle rejection: table_id={table_id}, source_error={source_debug}" + ) + }))); + } + return Err(CompletionErrorBridge::capture(err)); } - return Err(err.disclose()); - } - }; - let mut drop_progress = DropTableProgressGuard::new(engine.clone(), table_id); - drain.wait().await; - - let metadata = table.metadata().clone(); - let exec_res = execute_drop_table_catalog_cascade(&engine, &mut trx, table_id, &metadata).await; - if let Err(err) = exec_res { - // The lifecycle gate is already irreversible. Preserve the catalog - // cascade failure as the poison source; rollback is best-effort cleanup - // and its error must not replace the failure that crossed the gate. - if let Err(rollback_err) = trx.rollback_catalog_ddl().await { - let rollback_err = rollback_err.attach_with(|| { + }; + progress.phase = DropTablePhase::LifecycleClosed; + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::DropLifecycleClosed) + .await; + + drain.wait().await; + progress.phase = DropTablePhase::DrainComplete; + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::DropDrainComplete) + .await; + + let metadata = table.metadata().clone(); + let authority = scope.catalog_write_authority(); + let exec_res = execute_drop_table_catalog_cascade( + &engine, + progress + .trx + .as_mut() + .unwrap_or_else(|| panic!("DROP cascade requires private transaction")), + authority, + 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 + { + let rollback_err = rollback_err.attach_with(|| { format!( "best-effort DROP TABLE rollback failed after lifecycle gate: table_id={table_id}" ) }); - obs::error!( - "event=ddl_cleanup component=catalog action=rollback_drop_table result=error error={rollback_err:?}" - ); - } - return Err(poison_error_source( - &engine, - RuntimeOrFatalError::from(err), - FatalError::Poisoned, - format!( - "drop table failed after lifecycle gate: table_id={table_id}, operation=catalog_cascade" - ), - ) - .disclose()); - } - - let drop_cts = match trx.commit_catalog_ddl().await { - Ok(drop_cts) => drop_cts, - Err(err) => { - return Err(poison_error_source( + obs::error!( + "event=ddl_cleanup component=catalog action=rollback_drop_table result=error error={rollback_err:?}" + ); + } + return Err(capture_runtime_or_fatal(poison_error_source( &engine, - err, + RuntimeOrFatalError::from(err), FatalError::Poisoned, format!( - "drop table failed after lifecycle gate: table_id={table_id}, operation=commit" + "drop table failed after lifecycle gate: table_id={table_id}, operation=catalog_cascade" ), - ) - .disclose()); + ))); } - }; + progress.phase = DropTablePhase::CatalogStaged; - let replay_floor = engine - .catalog() - .effective_user_table_redo_replay_floor(table_id, table.redo_replay_floor_snapshot()); - finish_drop_table_runtime_retention(&engine, table_id, table, drop_cts, replay_floor) - .disclose()?; - drop_progress.disarm(); - // Foreground DROP TABLE stops at logical removal. The catalog map retains - // the dropped runtime and replay floor until purge and catalog checkpoint - // finish the physical cleanup obligations. - engine.trx_sys.request_dropped_table_purge(); - engine.trx_sys.request_metadata_history_purge(); - Ok(()) + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::DropCatalogStaged) + .await; + + let trx = progress + .trx + .take() + .unwrap_or_else(|| panic!("DROP commit requires private transaction")); + let drop_cts = match trx.commit_catalog_ddl().await { + Ok(drop_cts) => drop_cts, + Err(err) => { + return Err(capture_runtime_or_fatal(poison_error_source( + &engine, + err, + FatalError::Poisoned, + format!( + "drop table failed after lifecycle gate: table_id={table_id}, operation=commit" + ), + ))); + } + }; + progress.phase = DropTablePhase::CatalogCommitted; + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::DropCatalogCommitted) + .await; + + let replay_floor = engine + .catalog() + .effective_user_table_redo_replay_floor(table_id, table.redo_replay_floor_snapshot()); + finish_drop_table_runtime_retention(&engine, table_id, table, drop_cts, replay_floor) + .map_err(CompletionErrorBridge::capture)?; + progress.phase = DropTablePhase::RuntimeRetained; + + #[cfg(test)] + engine + .table_ddl_test + .reach_phase(TableDdlTestPhase::DropRuntimeRetained) + .await; + + engine.trx_sys.request_dropped_table_purge(); + engine.trx_sys.request_metadata_history_purge(); + Ok(()) + } +} + +/// Return the fixed catalog tables written by CREATE TABLE. +#[inline] +pub(crate) const fn create_table_catalog_write_targets() -> &'static [TableID] { + &CREATE_TABLE_CATALOG_WRITE_TARGETS +} + +/// Return the fixed catalog tables written by DROP TABLE. +#[inline] +pub(crate) const fn drop_table_catalog_write_targets() -> &'static [TableID] { + &DROP_TABLE_CATALOG_WRITE_TARGETS } /// Reject table ids outside user-managed catalog space. @@ -1454,6 +1882,14 @@ pub(crate) fn reject_user_table_primary_key_index( ))) } +#[inline] +fn capture_runtime_or_fatal(error: RuntimeOrFatalError) -> CompletionErrorBridge { + match error { + RuntimeOrFatalError::Runtime(report) => CompletionErrorBridge::capture(report), + RuntimeOrFatalError::Fatal(report) => CompletionErrorBridge::capture(report), + } +} + #[inline] fn reject_user_table_primary_key_indexes( index_specs: &[IndexSpec], @@ -1465,21 +1901,6 @@ fn reject_user_table_primary_key_indexes( Ok(()) } -async fn validated_drop_table_target( - guards: &PoolGuards, - engine: &EngineRef, - table_id: TableID, -) -> OperationOrRuntimeResult> { - reject_non_user_table_id(table_id, "drop_table")?; - let Some(table) = engine.catalog().get_table(table_id).await else { - return Err(Report::new(OperationError::TableNotFound) - .attach(format!("drop table runtime lookup: table_id={table_id}")) - .into()); - }; - ensure_user_table_catalog_row(guards, engine, table_id, "drop_table").await?; - Ok(table) -} - /// Stage the catalog rows for a newly allocated table. /// /// `table_id` is allocated atomically before this call. Every child key is @@ -1489,21 +1910,25 @@ async fn validated_drop_table_target( async fn execute_create_table_catalog_staging( engine: &EngineRef, trx: &mut Transaction, + authority: PreparedCatalogWriteAuthority<'_>, table_id: TableID, - table_object: TableObject, - column_objects: Vec, - index_objects: Vec, - index_column_objects: Vec, + catalog_objects: CreateTableCatalogObjects, ) -> RuntimeResult<()> { - trx.stage_catalog_statement(async |stmt| { + let CreateTableCatalogObjects { + table, + columns, + indexes, + index_columns, + } = catalog_objects; + trx.stage_prepared_catalog_statement(authority, async |stmt| { engine .catalog() .storage .tables() - .insert(stmt, &table_object) + .insert(stmt, &table) .await?; - for column_object in column_objects { + for column_object in columns { engine .catalog() .storage @@ -1511,7 +1936,7 @@ async fn execute_create_table_catalog_staging( .insert(stmt, &column_object) .await?; } - for index_object in index_objects { + for index_object in indexes { engine .catalog() .storage @@ -1519,7 +1944,7 @@ async fn execute_create_table_catalog_staging( .insert(stmt, &index_object) .await?; } - for index_column_object in index_column_objects { + for index_column_object in index_columns { engine .catalog() .storage @@ -1546,10 +1971,11 @@ async fn execute_create_table_catalog_staging( async fn execute_drop_table_catalog_cascade( engine: &EngineRef, trx: &mut Transaction, + authority: PreparedCatalogWriteAuthority<'_>, table_id: TableID, metadata: &TableMetadata, ) -> RuntimeResult<()> { - trx.stage_catalog_statement(async |stmt| { + trx.stage_prepared_catalog_statement(authority, async |stmt| { let index_columns_deleted = engine .catalog() .storage @@ -1675,22 +2101,6 @@ fn poison_drop_table_after_gate( engine.poisoner.poison(report).into_report() } -#[inline] -fn poison_create_table_after_commit( - engine: &EngineRef, - table_id: TableID, - operation: &'static str, -) -> Report { - let report = Report::new(FatalError::Poisoned).attach(format!( - "create table failed after catalog commit: table_id={table_id}, operation={operation}" - )); - obs::error!( - "event=engine_poison component=catalog_table action=poison result=error error={:?}", - report - ); - engine.poisoner.poison(report).into_report() -} - /// Fatalizes a typed catalog source while retaining its physical evidence. #[inline] fn poison_error_source( @@ -1753,7 +2163,7 @@ fn validate_primary_key_contract( } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; use crate::catalog::storage::tables::TABLE_ID_TABLES; use crate::catalog::tests::{ @@ -1771,10 +2181,10 @@ mod tests { }; use crate::id::{SessionID, TrxID}; use crate::io::install_storage_backend_test_hook; - use crate::lock::tests::{LockDebugEntryState, try_acquire}; - use crate::lock::{LockMode, LockOwner, LockResource, TableLockMode}; + use crate::lock::tests::{LockDebugEntryState, debug_snapshot, try_acquire}; + use crate::lock::{LockMode, LockOwner, LockResource, LockScope, TableLockMode}; use crate::log::redo::DDLRedo; - use crate::session::tests::{SessionTestExt, active_operation_count}; + use crate::session::tests::{SessionTestExt, active_operation_count, remove_session_for_test}; use crate::table::TableTerminal; use crate::table::tests::*; use crate::trx::MAX_SNAPSHOT_TS; @@ -1795,32 +2205,123 @@ mod tests { PoisonBeforeCatalogCommit, } - thread_local! { - static CREATE_TABLE_FAILURE: Cell> = const { Cell::new(None) }; - } + #[derive(Clone, Copy, Debug, PartialEq, Eq)] + pub(super) enum TableDdlTestPhase { + CreateBeforeFirstEffect, + CreateFileCreated, + CreatePrivateTransactionBegun, + CreateCatalogStaged, + CreateFilePublished, + CreateRuntimeBuilt, + CreateCatalogCommitted, + CreateRuntimeInstalled, + DropBeforeFirstEffect, + DropPrivateTransactionBegun, + DropLifecycleClosed, + DropDrainComplete, + DropCatalogStaged, + DropCatalogCommitted, + DropRuntimeRetained, + } + + struct TableDdlTestGate { + phase: TableDdlTestPhase, + entered: flume::Sender<()>, + release: flume::Receiver<()>, + } + + /// Per-engine CREATE/DROP fault controller shared across runtime threads. + #[derive(Default)] + pub(crate) struct TableDdlTestController { + create_failure: parking_lot::Mutex>, + panic_phase: parking_lot::Mutex>, + gate: parking_lot::Mutex>, + } + + impl TableDdlTestController { + #[inline] + fn set_create_failure(&self, failure: Option) { + *self.create_failure.lock() = failure; + } - fn set_create_table_failure(failure: Option) { - CREATE_TABLE_FAILURE.with(|slot| slot.set(failure)); - } + #[inline] + pub(super) fn maybe_fail_create( + &self, + failure: CreateTableTestFailure, + ) -> RuntimeResult<()> { + if *self.create_failure.lock() == Some(failure) { + return Err(Report::new(RuntimeError::CatalogAccess) + .attach("operation=test_create_table_phase_failure")); + } + Ok(()) + } - pub(super) fn maybe_fail_create_table(failure: CreateTableTestFailure) -> RuntimeResult<()> { - if CREATE_TABLE_FAILURE.with(|slot| slot.get()) == Some(failure) { - return Err(Report::new(RuntimeError::CatalogAccess) - .attach("operation=test_create_table_phase_failure")); + #[inline] + pub(super) fn maybe_poison_before_create_commit(&self, engine: &EngineRef) { + if *self.create_failure.lock() + == Some(CreateTableTestFailure::PoisonBeforeCatalogCommit) + { + let _ = engine + .poisoner + .poison(Report::new(FatalError::Poisoned).attach("forced create-table poison")); + } } - Ok(()) - } - pub(super) fn maybe_poison_before_create_table_catalog_commit(engine: &EngineRef) { - if CREATE_TABLE_FAILURE.with(|slot| slot.get()) - == Some(CreateTableTestFailure::PoisonBeforeCatalogCommit) - { - let _ = engine - .poisoner - .poison(Report::new(FatalError::Poisoned).attach("forced create-table poison")); + fn install_gate( + &self, + phase: TableDdlTestPhase, + ) -> (flume::Receiver<()>, flume::Sender<()>) { + let (entered_tx, entered_rx) = flume::bounded(1); + let (release_tx, release_rx) = flume::bounded(1); + let previous = self.gate.lock().replace(TableDdlTestGate { + phase, + entered: entered_tx, + release: release_rx, + }); + assert!( + previous.is_none(), + "table DDL test gate is already installed" + ); + (entered_rx, release_tx) + } + + fn set_panic_phase(&self, phase: Option) { + *self.panic_phase.lock() = phase; + } + + pub(super) async fn reach_phase(&self, phase: TableDdlTestPhase) { + let should_panic = { + let mut panic_phase = self.panic_phase.lock(); + if *panic_phase == Some(phase) { + *panic_phase = None; + true + } else { + false + } + }; + if should_panic { + panic!("injected accepted table DDL panic: phase={phase:?}"); + } + let gate = { + let mut slot = self.gate.lock(); + if slot.as_ref().is_some_and(|gate| gate.phase == phase) { + slot.take() + } else { + None + } + }; + let Some(gate) = gate else { + return; + }; + let _ = gate.entered.send_async(()).await; + let _ = gate.release.recv_async().await; } } + fn set_create_table_failure(engine: &Engine, failure: Option) { + engine.inner().table_ddl_test.set_create_failure(failure); + } + fn assert_invalid_metadata(err: Error, expected_message: &str) { assert!(err.is_kind(crate::error::ErrorKind::Operation)); assert_eq!( @@ -1847,6 +2348,16 @@ mod tests { } } + async fn wait_for_table_terminal(table: &Table, expected: TableTerminal) { + loop { + let changed = table.lifecycle.listener(); + if table.lifecycle.inspect_terminal() == expected { + return; + } + changed.await; + } + } + fn assert_no_user_table_publication(engine: &Engine, table_id: TableID) { assert!(engine.catalog().get_table_now(table_id).is_none()); assert!( @@ -2665,9 +3176,9 @@ mod tests { let table_file_path = engine.inner().table_fs.user_table_file_path(table_id); let (table_spec, index_specs) = drop_table_test_spec(); - set_create_table_failure(Some(CreateTableTestFailure::AfterCatalogStaged)); + set_create_table_failure(&engine, Some(CreateTableTestFailure::AfterCatalogStaged)); let res = session.create_table(table_spec, index_specs).await; - set_create_table_failure(None); + set_create_table_failure(&engine, None); let err = res.unwrap_err(); assert_eq!( @@ -2750,9 +3261,9 @@ mod tests { let table_file_path = engine.inner().table_fs.user_table_file_path(table_id); let (table_spec, index_specs) = drop_table_test_spec(); - set_create_table_failure(Some(CreateTableTestFailure::AfterFilePublished)); + set_create_table_failure(&engine, Some(CreateTableTestFailure::AfterFilePublished)); let res = session.create_table(table_spec, index_specs).await; - set_create_table_failure(None); + set_create_table_failure(&engine, None); let err = res.unwrap_err(); assert_eq!( @@ -2782,9 +3293,9 @@ mod tests { let table_file_path = engine.inner().table_fs.user_table_file_path(table_id); let (table_spec, index_specs) = drop_table_test_spec(); - set_create_table_failure(Some(CreateTableTestFailure::AfterRuntimeBuilt)); + set_create_table_failure(&engine, Some(CreateTableTestFailure::AfterRuntimeBuilt)); let res = session.create_table(table_spec, index_specs).await; - set_create_table_failure(None); + set_create_table_failure(&engine, None); let err = res.unwrap_err(); assert_eq!( @@ -2814,9 +3325,12 @@ mod tests { let table_file_path = engine.inner().table_fs.user_table_file_path(table_id); let (table_spec, index_specs) = drop_table_test_spec(); - set_create_table_failure(Some(CreateTableTestFailure::PoisonBeforeCatalogCommit)); + set_create_table_failure( + &engine, + Some(CreateTableTestFailure::PoisonBeforeCatalogCommit), + ); let res = session.create_table(table_spec, index_specs).await; - set_create_table_failure(None); + set_create_table_failure(&engine, None); let err = res.unwrap_err(); assert_eq!( @@ -3380,7 +3894,7 @@ mod tests { } #[test] - fn test_drop_table_rejects_runtime_missing_catalog_row_before_gate() { + fn test_drop_table_missing_catalog_row_panics_under_mandatory_supervision() { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; @@ -3410,17 +3924,88 @@ mod tests { let mut drop_session = engine.new_session().unwrap(); let table = table_for_internal_assertion(&engine, table_id); - let before = table_ddl_snapshot(&engine, table_id, &table); let err = drop_session.drop_table(table_id).await.unwrap_err(); assert_eq!( - err.report().downcast_ref::().copied(), - Some(OperationError::TableNotFound) + err.report().downcast_ref::().copied(), + Some(FatalError::MandatoryTaskPanic) ); - assert_table_ddl_snapshot_unchanged(&before, &engine, table_id, &table); - assert!(!drop_session.in_trx().unwrap()); - assert!(engine.inner().poisoner.poison_error().is_none()); + assert_eq!( + engine + .inner() + .poisoner + .poison_error() + .as_ref() + .map(|error| *error.current_context()), + Some(FatalError::MandatoryTaskPanic) + ); + assert_eq!(table.lifecycle.inspect_terminal(), TableTerminal::Dropping); + assert_checkpoint_workflow_closed(&table); assert!(engine.catalog().get_table(table_id).await.is_some()); + assert_eq!(active_operation_count(&engine.inner().session_registry), 1); + let shutdown_err = engine.try_shutdown().unwrap_err(); + assert_eq!( + shutdown_err + .report() + .downcast_ref::() + .copied(), + Some(LifecycleError::ShutdownBusy) + ); + + // FailedRetained deliberately keeps rollback-owned row state alive + // and blocks destructive component teardown. This test process is + // the final owner of the poisoned synthetic engine. + mem::forget((engine, temp_dir, drop_session, table)); + }); + } + + #[test] + fn test_create_table_execution_panic_before_first_effect_is_supervised() { + smol::block_on(async { + let temp_dir = TempDir::new().unwrap(); + let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; + let table_id = engine.catalog().curr_next_table_id(); + 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::CreateBeforeFirstEffect)); + + let err = session + .create_table(table_spec, index_specs) + .await + .unwrap_err(); + + assert_eq!( + err.report().downcast_ref::().copied(), + Some(FatalError::MandatoryTaskPanic) + ); + assert_eq!( + engine + .inner() + .poisoner + .poison_error() + .as_ref() + .map(|error| *error.current_context()), + Some(FatalError::MandatoryTaskPanic) + ); + assert_no_user_table_publication(&engine, table_id); + assert!(!Path::new(&engine.inner().table_fs.user_table_file_path(table_id)).exists()); + assert_eq!(active_operation_count(&engine.inner().session_registry), 1); + let shutdown_err = engine.try_shutdown().unwrap_err(); + assert_eq!( + shutdown_err + .report() + .downcast_ref::() + .copied(), + Some(LifecycleError::ShutdownBusy) + ); + + remove_session_for_test(&engine.inner().session_registry, session_id); + drop(session); + engine.shutdown(); }); } @@ -3491,6 +4076,8 @@ mod tests { let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; let table_id = create_table2_for_test(&engine).await; let other_table_id = create_table2_for_test(&engine).await; + let mut purge_blocker_session = engine.new_session().unwrap(); + let purge_blocker = purge_blocker_session.begin_trx().unwrap(); let table = table_for_internal_assertion(&engine, table_id); let (root_lease, publish_lease) = begin_checkpoint_publish_for_test(&table); @@ -3500,6 +4087,7 @@ mod tests { futures::poll!(waiting_drop.as_mut()), std::task::Poll::Pending )); + wait_for_table_terminal(&table, TableTerminal::Dropping).await; assert_eq!(table.lifecycle.inspect_terminal(), TableTerminal::Dropping); let mut other_session = engine.new_session().unwrap(); @@ -3511,6 +4099,7 @@ mod tests { drop(publish_lease); drop(table); waiting_drop.await.unwrap(); + purge_blocker.rollback().await.unwrap(); }); } @@ -3520,6 +4109,8 @@ mod tests { let temp_dir = TempDir::new().unwrap(); let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; let table_id = create_table2_for_test(&engine).await; + let mut purge_blocker_session = engine.new_session().unwrap(); + let purge_blocker = purge_blocker_session.begin_trx().unwrap(); let table = table_for_internal_assertion(&engine, table_id); let (root_lease, publish_lease) = begin_checkpoint_publish_for_test(&table); @@ -3529,6 +4120,7 @@ mod tests { futures::poll!(waiting_drop.as_mut()), std::task::Poll::Pending )); + wait_for_table_terminal(&table, TableTerminal::Dropping).await; assert_eq!(table.lifecycle.inspect_terminal(), TableTerminal::Dropping); let mut create_session = engine.new_session().unwrap(); @@ -3554,6 +4146,7 @@ mod tests { drop(publish_lease); drop(table); waiting_drop.await.unwrap(); + purge_blocker.rollback().await.unwrap(); }); } @@ -3563,6 +4156,8 @@ mod tests { let temp_dir = TempDir::new().unwrap(); let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; let table_id = create_table2_for_test(&engine).await; + let mut purge_blocker_session = engine.new_session().unwrap(); + let purge_blocker = purge_blocker_session.begin_trx().unwrap(); let table = table_for_internal_assertion(&engine, table_id); let (root_lease, publish_lease) = begin_checkpoint_publish_for_test(&table); let mut drop_session = engine.new_session().unwrap(); @@ -3620,41 +4215,270 @@ mod tests { lock_owner, LockResource::TableData(table_id), )); + purge_blocker.rollback().await.unwrap(); + }); + } + + #[test] + fn test_abandoned_create_future_before_first_effect_is_inert() { + smol::block_on(async { + let temp_dir = TempDir::new().unwrap(); + let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; + let table_id = engine.catalog().curr_next_table_id(); + let (entered, release) = engine + .inner() + .table_ddl_test + .install_gate(TableDdlTestPhase::CreateBeforeFirstEffect); + let mut create_session = engine.new_session().unwrap(); + let (table_spec, index_specs) = drop_table_test_spec(); + let mut create_fut = Box::pin(create_session.create_table(table_spec, index_specs)); + + assert!(matches!( + futures::poll!(create_fut.as_mut()), + std::task::Poll::Pending + )); + entered.recv_async().await.unwrap(); + drop(create_fut); + release.send_async(()).await.unwrap(); + + let mut verify_session = engine.new_session().unwrap(); + verify_session + .lock_table(table_id, TableLockMode::Shared) + .await + .unwrap(); + assert!(engine.catalog().get_table_now(table_id).is_some()); + verify_session.unlock_table(table_id).unwrap(); + verify_session.drop_table(table_id).await.unwrap(); + assert!(engine.inner().poisoner.poison_error().is_none()); + }); + } + + #[test] + fn test_abandoned_drop_future_before_first_effect_is_inert() { + smol::block_on(async { + let temp_dir = TempDir::new().unwrap(); + let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; + let table_id = create_table2_for_test(&engine).await; + let (entered, release) = engine + .inner() + .table_ddl_test + .install_gate(TableDdlTestPhase::DropBeforeFirstEffect); + let mut drop_session = engine.new_session().unwrap(); + let mut drop_fut = Box::pin(drop_session.drop_table(table_id)); + + assert!(matches!( + futures::poll!(drop_fut.as_mut()), + std::task::Poll::Pending + )); + entered.recv_async().await.unwrap(); + drop(drop_fut); + release.send_async(()).await.unwrap(); + + let mut verify_session = engine.new_session().unwrap(); + let err = verify_session + .lock_table(table_id, TableLockMode::Shared) + .await + .unwrap_err(); + assert_eq!( + err.report().downcast_ref::().copied(), + Some(OperationError::TableNotFound) + ); + assert!(engine.inner().poisoner.poison_error().is_none()); + }); + } + + #[test] + fn test_accepted_table_ddl_owns_exact_prepared_lock_sets() { + smol::block_on(async { + let temp_dir = TempDir::new().unwrap(); + let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; + + let create_table_id = engine.catalog().curr_next_table_id(); + let (create_entered, create_release) = engine + .inner() + .table_ddl_test + .install_gate(TableDdlTestPhase::CreateBeforeFirstEffect); + let mut create_session = engine.new_session().unwrap(); + let create_session_id = create_session.id(); + let (table_spec, index_specs) = drop_table_test_spec(); + let mut create_fut = Box::pin(create_session.create_table(table_spec, index_specs)); + assert!(matches!( + futures::poll!(create_fut.as_mut()), + std::task::Poll::Pending + )); + create_entered.recv_async().await.unwrap(); + + let create_owner = ddl_lock_owner( + &engine, + create_session_id, + LockResource::TableMetadata(create_table_id), + ) + .expect("accepted CREATE should retain its operation owner"); + assert_eq!(lock_entry_count(&engine, create_owner), 9); + assert!(has_lock_entry( + &engine, + create_owner, + LockResource::TableMetadata(create_table_id), + LockMode::Exclusive, + LockDebugEntryState::Granted, + )); + for &catalog_table_id in create_table_catalog_write_targets() { + assert!(has_lock_entry( + &engine, + create_owner, + LockResource::TableMetadata(catalog_table_id), + LockMode::Shared, + LockDebugEntryState::Granted, + )); + assert!(has_lock_entry( + &engine, + create_owner, + LockResource::TableData(catalog_table_id), + LockMode::IntentExclusive, + LockDebugEntryState::Granted, + )); + } + let (create_staged, create_staged_release) = engine + .inner() + .table_ddl_test + .install_gate(TableDdlTestPhase::CreateCatalogStaged); + create_release.send_async(()).await.unwrap(); + create_staged.recv_async().await.unwrap(); + assert_eq!(lock_entry_count(&engine, create_owner), 9); + assert!( + debug_snapshot(engine.inner().lock_manager()) + .entries + .into_iter() + .filter(|entry| entry.owner.family().session_id() == create_session_id) + .all(|entry| matches!(entry.owner.scope(), LockScope::Operation(_))), + "accepted CREATE must not acquire transaction/statement catalog locks" + ); + create_staged_release.send_async(()).await.unwrap(); + assert_eq!(create_fut.await.unwrap(), create_table_id); + + let (drop_entered, drop_release) = engine + .inner() + .table_ddl_test + .install_gate(TableDdlTestPhase::DropBeforeFirstEffect); + let mut drop_session = engine.new_session().unwrap(); + let drop_session_id = drop_session.id(); + let mut drop_fut = Box::pin(drop_session.drop_table(create_table_id)); + assert!(matches!( + futures::poll!(drop_fut.as_mut()), + std::task::Poll::Pending + )); + drop_entered.recv_async().await.unwrap(); + + let drop_owner = ddl_lock_owner( + &engine, + drop_session_id, + LockResource::TableMetadata(create_table_id), + ) + .expect("accepted DROP should retain its operation owner"); + assert_eq!(lock_entry_count(&engine, drop_owner), 12); + assert!(has_lock_entry( + &engine, + drop_owner, + LockResource::TableMetadata(create_table_id), + LockMode::Exclusive, + LockDebugEntryState::Granted, + )); + assert!(has_lock_entry( + &engine, + drop_owner, + LockResource::TableData(create_table_id), + LockMode::Exclusive, + LockDebugEntryState::Granted, + )); + for &catalog_table_id in drop_table_catalog_write_targets() { + assert!(has_lock_entry( + &engine, + drop_owner, + LockResource::TableMetadata(catalog_table_id), + LockMode::Shared, + LockDebugEntryState::Granted, + )); + assert!(has_lock_entry( + &engine, + drop_owner, + LockResource::TableData(catalog_table_id), + LockMode::IntentExclusive, + LockDebugEntryState::Granted, + )); + } + let (drop_staged, drop_staged_release) = engine + .inner() + .table_ddl_test + .install_gate(TableDdlTestPhase::DropCatalogStaged); + drop_release.send_async(()).await.unwrap(); + drop_staged.recv_async().await.unwrap(); + assert_eq!(lock_entry_count(&engine, drop_owner), 12); + assert!( + debug_snapshot(engine.inner().lock_manager()) + .entries + .into_iter() + .filter(|entry| entry.owner.family().session_id() == drop_session_id) + .all(|entry| matches!(entry.owner.scope(), LockScope::Operation(_))), + "accepted DROP must not acquire transaction/statement catalog locks" + ); + drop_staged_release.send_async(()).await.unwrap(); + drop_fut.await.unwrap(); }); } #[test] - fn test_abandoned_drop_future_after_terminal_gate_poisons_storage() { + fn test_abandoned_drop_future_after_acceptance_is_inert() { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; let table_id = create_table2_for_test(&engine).await; + let mut purge_blocker_session = engine.new_session().unwrap(); + let purge_blocker = purge_blocker_session.begin_trx().unwrap(); let table = table_for_internal_assertion(&engine, table_id); let (root_lease, publish_lease) = begin_checkpoint_publish_for_test(&table); + let (lifecycle_closed, lifecycle_release) = engine + .inner() + .table_ddl_test + .install_gate(TableDdlTestPhase::DropLifecycleClosed); let mut drop_session = engine.new_session().unwrap(); - let before = table_ddl_snapshot(&engine, table_id, &table); let mut drop_fut = Box::pin(drop_session.drop_table(table_id)); assert!(matches!( futures::poll!(drop_fut.as_mut()), std::task::Poll::Pending )); + lifecycle_closed.recv_async().await.unwrap(); assert_eq!(table.lifecycle.inspect_terminal(), TableTerminal::Dropping); assert_checkpoint_workflow_closed(&table); drop(drop_fut); - let poison = engine - .inner() - .poisoner - .poison_error() - .expect("abandoned terminal drop should poison storage"); - assert_eq!(*poison.current_context(), FatalError::Poisoned); - assert_table_logical_snapshot_unchanged(&before, &engine, table_id); + assert!( + engine.inner().poisoner.poison_error().is_none(), + "dropping the observer must not poison accepted DDL" + ); assert_eq!(table.lifecycle.inspect_terminal(), TableTerminal::Dropping); + lifecycle_release.send_async(()).await.unwrap(); + + let mut lock_session = engine.new_session().unwrap(); + let mut lock_fut = Box::pin(lock_session.lock_table(table_id, TableLockMode::Shared)); + assert!(matches!( + futures::poll!(lock_fut.as_mut()), + std::task::Poll::Pending + )); drop(publish_lease); drop(root_lease); drop(table); + let err = lock_fut.await.unwrap_err(); + assert_eq!( + err.report().downcast_ref::().copied(), + Some(OperationError::TableNotFound) + ); + assert!( + engine.inner().poisoner.poison_error().is_none(), + "accepted DROP should complete after its observer is dropped" + ); + purge_blocker.rollback().await.unwrap(); drop(drop_session); engine.shutdown(); }); diff --git a/doradb-storage/src/completion.rs b/doradb-storage/src/completion.rs index 5af476c3..553db3d8 100644 --- a/doradb-storage/src/completion.rs +++ b/doradb-storage/src/completion.rs @@ -12,13 +12,6 @@ use event_listener::{Event, listener}; use parking_lot::Mutex; use std::mem::replace; -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "exclusive take is reserved for prepared mandatory adapters" - ) -)] enum CompletionState { Running, Completed(CompletionResult), @@ -26,13 +19,6 @@ enum CompletionState { } /// Result of an exclusive completion take attempt. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "exclusive take is reserved for prepared mandatory adapters" - ) -)] pub(crate) enum CompletionTake { /// The producer has not completed yet. Pending, @@ -109,13 +95,6 @@ impl Completion { } /// Exclusively moves the terminal result out of this completion. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "exclusive take is reserved for prepared mandatory adapters" - ) - )] #[inline] pub(crate) fn try_take_result(&self) -> CompletionTake { let mut state = self.state.lock(); @@ -130,13 +109,6 @@ impl Completion { } /// Waits for and exclusively moves the terminal result. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "exclusive take is reserved for prepared mandatory adapters" - ) - )] #[inline] pub(crate) async fn wait_take_result(&self) -> CompletionResult { loop { diff --git a/doradb-storage/src/engine.rs b/doradb-storage/src/engine.rs index 0b174e8d..9674eea0 100644 --- a/doradb-storage/src/engine.rs +++ b/doradb-storage/src/engine.rs @@ -6,6 +6,8 @@ //! model that this module enforces with the component registry. use crate::buffer::PoolRole; use crate::buffer::SharedPoolEvictorWorkers; +#[cfg(test)] +use crate::catalog::table::tests::TableDdlTestController; use crate::catalog::{Catalog, CatalogConfig}; use crate::component::{ ComponentRegistry, DiskPoolConfig, EnginePools, IndexPoolConfig, MetaPoolConfig, @@ -695,6 +697,9 @@ pub(crate) struct EngineInner { pub(crate) session_registry: SessionRegistry, /// Monotonically increasing engine-local session identity source. next_session_id: AtomicU64, + /// Per-engine table-DDL fault and phase controller. + #[cfg(test)] + pub(crate) table_ddl_test: TableDdlTestController, lifecycle: EngineLifecycle, } @@ -921,6 +926,8 @@ async fn bootstrap_inner(config: EngineConfig) -> Result { lock_manager, session_registry: SessionRegistry::new(), next_session_id: AtomicU64::new(FIRST_SESSION_ID.as_u64()), + #[cfg(test)] + table_ddl_test: TableDdlTestController::default(), lifecycle: EngineLifecycle::new(), }; Ok(Engine { diff --git a/doradb-storage/src/lock/mod.rs b/doradb-storage/src/lock/mod.rs index f1616c97..59061c71 100644 --- a/doradb-storage/src/lock/mod.rs +++ b/doradb-storage/src/lock/mod.rs @@ -398,27 +398,6 @@ impl LockManager { Ok((metadata_guard, data_guard)) } - /// Acquires metadata-X for one freshly allocated CREATE TABLE id. - #[inline] - pub(crate) async fn acquire_create_table_metadata_lock<'a>( - &'a self, - table_id: TableID, - owner: LockOwner, - ) -> OperationResult> { - let resource = LockResource::TableMetadata(table_id); - let grant = self - .acquire_with_grant(resource, LockMode::Exclusive, owner) - .await?; - FreshLockGuard::new(self, resource, owner, grant).map_or_else( - || { - panic!( - "create-table metadata lock invariant violated: fresh table id reused an existing owner grant, table_id={table_id}, owner={owner:?}" - ) - }, - Ok, - ) - } - /// Acquires scoped exclusive table DDL locks. #[inline] pub(crate) async fn acquire_table_ddl_locks<'a>( @@ -1397,36 +1376,6 @@ pub(crate) mod tests { let _ = LockOwner::session_explicit(SessionID::new(7)).statement(1); } - #[test] - fn create_table_metadata_guard_holds_only_fresh_metadata_x() { - smol::block_on(async { - let manager = LockManager::new(); - let table_id = TableID::new(42); - let owner = LockOwner::operation(SessionOperationKey::new( - SessionID::new(7), - OperationID::new(1), - )); - let guard = manager - .acquire_create_table_metadata_lock(table_id, owner) - .await - .unwrap(); - - assert_eq!( - debug_snapshot(&manager).entries, - vec![LockDebugEntry { - resource: table_metadata(table_id), - mode: LockMode::Exclusive, - owner, - state: LockDebugEntryState::Granted, - queue_order: None, - }] - ); - - drop(guard); - assert!(debug_snapshot(&manager).entries.is_empty()); - }); - } - fn count_entries( snapshot: &LockDebugSnapshot, resource: LockResource, diff --git a/doradb-storage/src/runtime/mandatory.rs b/doradb-storage/src/runtime/mandatory.rs index 3a4993e7..1303c436 100644 --- a/doradb-storage/src/runtime/mandatory.rs +++ b/doradb-storage/src/runtime/mandatory.rs @@ -5,7 +5,7 @@ use crate::error::{ CompletionErrorBridge, CompletionResult, ConfigError, ConfigResult, DiscloseError, FatalError, LifecycleError, LifecycleResult, Result, RuntimeError, RuntimeResult, SharedFatalError, }; -use crate::id::SessionOperationKey; +use crate::id::{SessionOperationKey, TableID}; use crate::obs; use crate::poison::EnginePoisoner; use crate::quiescent::{QuiescentBox, QuiescentGuard}; @@ -28,13 +28,6 @@ use std::thread::JoinHandle; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum MandatoryTaskClass { /// Caller-submitted DDL or maintenance operation. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) - )] Operation, /// Engine-internal transaction cleanup obligation. TransactionCleanup, @@ -56,6 +49,7 @@ pub(crate) struct MandatoryTaskMetadata { class: MandatoryTaskClass, label: &'static str, session_operation: Option, + table_id: Option, } impl MandatoryTaskMetadata { @@ -76,6 +70,22 @@ impl MandatoryTaskMetadata { class: MandatoryTaskClass::Operation, label, session_operation, + table_id: None, + } + } + + /// Build caller table-DDL metadata. + #[inline] + pub(crate) const fn table_operation( + label: &'static str, + session_operation: SessionOperationKey, + table_id: TableID, + ) -> Self { + Self { + class: MandatoryTaskClass::Operation, + label, + session_operation: Some(session_operation), + table_id: Some(table_id), } } @@ -89,17 +99,20 @@ impl MandatoryTaskMetadata { class: MandatoryTaskClass::TransactionCleanup, label, session_operation, + table_id: None, } } #[inline] fn diagnostic(&self) -> String { format!( - "task_class={}, task_label={}, session_operation={}", + "task_class={}, task_label={}, session_operation={}, table_id={}", self.class.label(), self.label, self.session_operation - .map_or_else(|| "none".to_owned(), |key| key.to_string()) + .map_or_else(|| "none".to_owned(), |key| key.to_string()), + self.table_id + .map_or_else(|| "none".to_owned(), |table_id| table_id.to_string()) ) } } @@ -110,13 +123,6 @@ struct MandatoryAdmissionState { } struct MandatoryAdmission { - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) - )] limit: usize, state: Mutex, changed: Event, @@ -135,13 +141,6 @@ impl MandatoryAdmission { } } - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) - )] async fn acquire( &self, runtime: QuiescentGuard, @@ -165,13 +164,6 @@ impl MandatoryAdmission { } } - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) - )] #[inline] fn release(&self) { let mut state = self.state.lock(); @@ -210,13 +202,6 @@ impl MandatoryAdmission { } } -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) -)] struct MandatoryPermit { runtime: Option>, } @@ -318,13 +303,6 @@ impl Drop for MandatoryInternalPermit { } /// Completely caller-prepared execution awaiting mandatory admission. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 defines the Phase 2 production adapter boundary" - ) -)] pub(crate) trait PreparedExecution: Send + 'static { /// Terminal output delivered to the sole observer. type Output: Send + 'static; @@ -342,13 +320,6 @@ pub(crate) trait PreparedExecution: Send + 'static { } /// Accepted execution whose resources remain outside its panic-caught future. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 defines the Phase 2 production adapter boundary" - ) -)] pub(crate) trait AcceptedExecution: Send + 'static { /// Terminal output delivered to the sole observer. type Output: Send + 'static; @@ -396,26 +367,12 @@ pub(crate) trait MandatoryInternalTask: Send + 'static { fn publish_panic(&mut self, error: CompletionErrorBridge); } -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) -)] enum ObservationState { Attached, Detached, Consumed, } -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) -)] struct MandatoryCompletion { completion: Completion, observation: Mutex, @@ -424,13 +381,6 @@ struct MandatoryCompletion { impl MandatoryCompletion { #[inline] - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) - )] fn endpoints( metadata: MandatoryTaskMetadata, ) -> (CompletionProducer, CompletionObserver) { @@ -447,13 +397,6 @@ impl MandatoryCompletion { ) } - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) - )] #[inline] fn handle_unobserved(&self, result: CompletionResult) { match result { @@ -468,25 +411,16 @@ impl MandatoryCompletion { } } -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) -)] struct CompletionProducer { inner: Arc>, } impl CompletionProducer { - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) - )] + #[inline] + fn metadata(&self) -> &MandatoryTaskMetadata { + &self.inner.metadata + } + #[inline] fn complete(self, result: CompletionResult) { let observation = self.inner.observation.lock(); @@ -513,13 +447,6 @@ impl CompletionProducer { } /// Sole, execution-inert observer for one mandatory caller operation. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) -)] pub(crate) struct CompletionObserver { inner: Arc>, armed: bool, @@ -527,13 +454,6 @@ pub(crate) struct CompletionObserver { impl CompletionObserver { /// Wait for the mandatory task and disclose its terminal result. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) - )] #[inline] pub(crate) async fn wait(mut self) -> Result { let result = self.inner.completion.wait_take_result().await; @@ -849,141 +769,124 @@ impl MandatoryRuntimeWorkersOwned { } } -/// Submit a fully prepared caller operation. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) -)] -pub(crate) async fn submit( - runtime: &QuiescentGuard, - prepared: E, -) -> LifecycleResult> -where - E: PreparedExecution, -{ - let poison_listener = runtime.poisoner.listener(); - if let Err(error) = runtime.poisoner.ensure_healthy() { - runtime.admission.close(); - return Err(error - .change_context(LifecycleError::RuntimeUnavailable) - .attach("phase=mandatory_admission_health_check")); - } - let acquire = runtime.admission.acquire(runtime.clone()); - futures::pin_mut!(acquire); - futures::pin_mut!(poison_listener); - let permit = match select(acquire, poison_listener).await { - Either::Left((result, _)) => result?, - Either::Right((_, _)) => { - runtime.admission.close(); - return Err(runtime - .poisoner - .ensure_healthy() - .expect_err("poison event requires a published fatal reason") +impl QuiescentGuard { + /// Submit a fully prepared caller operation. + pub(crate) async fn submit( + &self, + prepared: E, + ) -> LifecycleResult> + where + E: PreparedExecution, + { + let poison_listener = self.poisoner.listener(); + if let Err(error) = self.poisoner.ensure_healthy() { + self.admission.close(); + return Err(error .change_context(LifecycleError::RuntimeUnavailable) - .attach("phase=mandatory_admission_poison_wake")); + .attach("phase=mandatory_admission_health_check")); } - }; - // Winning admission is the poison-race linearization point. A later - // poison does not cancel work that is already admitted and accounted. - let metadata = prepared.metadata(); - let (producer, observer) = MandatoryCompletion::endpoints(metadata.clone()); - - // No await or expected rejection exists below this ownership edge. - let accepted = prepared.accept(); - let task_runtime = runtime.clone(); - runtime - .executor - .spawn(supervise_accepted( - task_runtime, - accepted, - producer, - metadata, - permit, - )) - .detach(); - Ok(observer) -} + let acquire = self.admission.acquire(self.clone()); + futures::pin_mut!(acquire); + futures::pin_mut!(poison_listener); + let permit = match select(acquire, poison_listener).await { + Either::Left((result, _)) => result?, + Either::Right((_, _)) => { + self.admission.close(); + return Err(self + .poisoner + .ensure_healthy() + .expect_err("poison event requires a published fatal reason") + .change_context(LifecycleError::RuntimeUnavailable) + .attach("phase=mandatory_admission_poison_wake")); + } + }; + // Winning admission is the poison-race linearization point. A later + // poison does not cancel work that is already admitted and accounted. + let metadata = prepared.metadata(); + let (producer, observer) = MandatoryCompletion::endpoints(metadata); + + // No await or expected rejection exists below this ownership edge. + let accepted = prepared.accept(); + let task_runtime = self.clone(); + self.executor + .spawn(task_runtime.supervise_accepted(accepted, producer, permit)) + .detach(); + Ok(observer) + } -/// Synchronously submit an existing internal correctness obligation. -#[inline] -pub(crate) fn submit_internal( - runtime: &QuiescentGuard, - job: J, -) -> StdResult<(), J> -where - J: MandatoryInternalTask, -{ - let Some(permit) = runtime.internal_admission.try_acquire(runtime.clone()) else { - return Err(job); - }; - let metadata = job.metadata(); - let task_runtime = runtime.clone(); - runtime - .executor - .spawn(supervise_internal(task_runtime, job, metadata, permit)) - .detach(); - Ok(()) -} + /// Synchronously submit an existing internal correctness obligation. + #[inline] + pub(crate) fn submit_internal(&self, job: J) -> StdResult<(), J> + where + J: MandatoryInternalTask, + { + let Some(permit) = self.internal_admission.try_acquire(self.clone()) else { + return Err(job); + }; + let metadata = job.metadata(); + let task_runtime = self.clone(); + self.executor + .spawn(task_runtime.supervise_internal(job, metadata, permit)) + .detach(); + Ok(()) + } -/// Supervise one accepted caller operation through terminal publication. -/// -/// The accepted owner remains outside the unwind-caught borrowed execution -/// future. Normal finish and panic policy methods are non-unwinding by -/// contract. The owner drops before its permit releases caller capacity. -async fn supervise_accepted( - runtime: QuiescentGuard, - mut accepted: A, - producer: CompletionProducer, - metadata: MandatoryTaskMetadata, - permit: MandatoryPermit, -) where - A: AcceptedExecution, -{ - let outcome = AssertUnwindSafe(async { accepted.execute().await }) - .catch_unwind() - .await; - match outcome { - Ok(result) => { - accepted.finish(); - producer.complete(result); - } - Err(panic) => { - let error = accepted.handle_panic(panic).await; - runtime.poison_mandatory_panic(&metadata); - producer.complete(Err::(error)); + /// Supervise one accepted caller operation through terminal publication. + /// + /// The accepted owner remains outside the unwind-caught borrowed execution + /// future. Normal finish and panic policy methods are non-unwinding by + /// contract. The owner drops before its permit releases caller capacity. + async fn supervise_accepted( + self, + mut accepted: A, + producer: CompletionProducer, + permit: MandatoryPermit, + ) where + A: AcceptedExecution, + { + let outcome = AssertUnwindSafe(async { accepted.execute().await }) + .catch_unwind() + .await; + match outcome { + Ok(result) => { + accepted.finish(); + producer.complete(result); + } + Err(panic) => { + let error = accepted.handle_panic(panic).await; + self.poison_mandatory_panic(producer.metadata()); + producer.complete(Err::(error)); + } } + drop(accepted); + drop(permit); } - drop(accepted); - drop(permit); -} -/// Supervise one engine-internal obligation through terminal handling. -/// -/// Only the borrowed run future is unwind-caught. Preservation and panic -/// publication are non-unwinding by contract. The job drops before its -/// permit publishes internal-admission drain progress. -async fn supervise_internal( - runtime: QuiescentGuard, - mut job: J, - metadata: MandatoryTaskMetadata, - permit: MandatoryInternalPermit, -) where - J: MandatoryInternalTask, -{ - if AssertUnwindSafe(async { job.run().await }) - .catch_unwind() - .await - .is_err() + /// Supervise one engine-internal obligation through terminal handling. + /// + /// Only the borrowed run future is unwind-caught. Preservation and panic + /// publication are non-unwinding by contract. The job drops before its + /// permit publishes internal-admission drain progress. + async fn supervise_internal( + self, + mut job: J, + metadata: MandatoryTaskMetadata, + permit: MandatoryInternalPermit, + ) where + J: MandatoryInternalTask, { - job.preserve_after_panic(); - let fatal = runtime.poison_mandatory_panic(&metadata); - job.publish_panic(fatal.into_completion_bridge()); + if AssertUnwindSafe(async { job.run().await }) + .catch_unwind() + .await + .is_err() + { + job.preserve_after_panic(); + let fatal = self.poison_mandatory_panic(&metadata); + job.publish_panic(fatal.into_completion_bridge()); + } + drop(job); + drop(permit); } - drop(job); - drop(permit); } #[cfg(test)] @@ -1243,15 +1146,13 @@ mod tests { let moves = Arc::new(AtomicUsize::new(0)); let finishes = Arc::new(AtomicUsize::new(0)); - let observer = submit( - &mandatory, - SyntheticPrepared { + let observer = mandatory + .submit(SyntheticPrepared { moves: Arc::clone(&moves), finishes: Arc::clone(&finishes), - }, - ) - .await - .unwrap(); + }) + .await + .unwrap(); assert_eq!(observer.wait().await.unwrap().0, 1); assert_eq!(moves.load(Ordering::Relaxed), 1); assert_eq!(finishes.load(Ordering::Relaxed), 1); @@ -1285,16 +1186,14 @@ mod tests { let completion = Arc::new(Completion::new()); assert!( - submit_internal( - &mandatory, - SyntheticPanicInternal { + mandatory + .submit_internal(SyntheticPanicInternal { preserved: Arc::clone(&preserved), published: Arc::clone(&published), dropped: Arc::clone(&dropped), completion: Arc::clone(&completion), - }, - ) - .is_ok(), + }) + .is_ok(), "open internal admission accepts the task" ); let error = completion.wait_result().await.unwrap_err(); @@ -1333,20 +1232,22 @@ mod tests { let registry = builder.finish(); let mandatory = registry.dependency::(); - let observer = submit( - &mandatory, - ExecutePanicPrepared { + let observer = mandatory + .submit(ExecutePanicPrepared { dropped: Arc::clone(&dropped), finishes: Arc::clone(&finishes), handled: Arc::clone(&handled), - }, - ) - .await - .unwrap(); + }) + .await + .unwrap(); let error = observer.wait().await.unwrap_err(); assert_eq!(error.kind(), ErrorKind::Fatal); assert_eq!(finishes.load(Ordering::Relaxed), 0); assert_eq!(handled.load(Ordering::Relaxed), 1); + let poison = mandatory.poisoner.poison_error().unwrap(); + let poison = format!("{poison:?}"); + assert!(poison.contains("task_class=operation"), "{poison}"); + assert!(poison.contains("task_label=execute_panic"), "{poison}"); (registry, mandatory) }); diff --git a/doradb-storage/src/session.rs b/doradb-storage/src/session.rs index 92f47d7b..e823055d 100644 --- a/doradb-storage/src/session.rs +++ b/doradb-storage/src/session.rs @@ -1,16 +1,20 @@ use crate::buffer::page::VersionedPageID; use crate::buffer::{BufferPool, PoolGuards}; use crate::catalog::{ - CatalogCheckpointOutcome, IndexNo, IndexSpec, TableSpec, create_index_for_session, - create_table_for_session, drop_index_for_session, drop_table_for_session, + CatalogCheckpointOutcome, DropTablePlan, IndexNo, IndexSpec, PreparedCreateTable, + PreparedDropTable, TableSpec, ValidatedCreateTable, create_index_for_session, + create_table_catalog_write_targets, drop_index_for_session, drop_table_catalog_write_targets, + reject_non_user_table_id, }; use crate::engine::{EngineInner, EngineRef, WeakEngineRef}; use crate::error::{ DiscloseError, DiscloseResultExt, FatalError, LifecycleError, LifecycleResult, - MultiDomainResultExt, OperationResult, Result, + MultiDomainResultExt, OperationError, OperationResult, Result, }; use crate::id::{OperationID, SessionID, SessionOperationKey, TableID, TrxID}; -use crate::lock::{FreshLockGuard, LockManager, LockMode, LockOwner, LockResource, TableLockMode}; +use crate::lock::{ + FreshLockGuard, LockManager, LockMode, LockOwner, LockResource, OwnerLockState, TableLockMode, +}; use crate::map::{FastDashMap, FastHashMap}; use crate::notify::EventNotifyOnDrop; use crate::quiescent::QuiescentGuard; @@ -23,14 +27,15 @@ use crate::table::{ MemIndexCleanupOutcome, Table, }; use crate::trx::{ - ReleasedTransactionLocks, SessionOperationEntry, SessionOperationKind, SessionOperationState, - Transaction, TrxInner, + PreparedCatalogWriteAuthority, ReleasedTransactionLocks, SessionOperationEntry, + SessionOperationKind, SessionOperationState, Transaction, TrxInner, }; use error_stack::{Report, ResultExt}; use event_listener::EventListener; use futures::future::select_all; use parking_lot::Mutex; use std::cell::Cell; +use std::mem::replace; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Weak}; @@ -126,6 +131,247 @@ impl SessionDdlContext { } } +/// Lifetime-free logical-lock scope prepared for one table DDL operation. +pub(crate) struct PreparedTableDdlLocks { + lock_manager: QuiescentGuard, + locks: OwnerLockState, +} + +impl PreparedTableDdlLocks { + #[inline] + fn new(operation: &SessionOperationPin) -> Self { + Self { + lock_manager: operation.engine.lock_manager().clone(), + locks: OwnerLockState::new(operation.operation_lock_owner()), + } + } + + #[inline] + async fn acquire_create( + &mut self, + table_id: TableID, + catalog_targets: &[TableID], + ) -> OperationResult<()> { + self.locks + .acquire( + &self.lock_manager, + LockResource::TableMetadata(table_id), + LockMode::Exclusive, + ) + .await?; + for &catalog_table_id in catalog_targets { + self.locks + .acquire( + &self.lock_manager, + LockResource::TableMetadata(catalog_table_id), + LockMode::Shared, + ) + .await?; + } + for &catalog_table_id in catalog_targets { + self.locks + .acquire( + &self.lock_manager, + LockResource::TableData(catalog_table_id), + LockMode::IntentExclusive, + ) + .await?; + } + Ok(()) + } + + #[inline] + async fn acquire_drop( + &mut self, + table_id: TableID, + catalog_targets: &[TableID], + ) -> OperationResult<()> { + self.locks + .acquire( + &self.lock_manager, + LockResource::TableMetadata(table_id), + LockMode::Exclusive, + ) + .await?; + for &catalog_table_id in catalog_targets { + self.locks + .acquire( + &self.lock_manager, + LockResource::TableMetadata(catalog_table_id), + LockMode::Shared, + ) + .await?; + } + self.locks + .acquire( + &self.lock_manager, + LockResource::TableData(table_id), + LockMode::Exclusive, + ) + .await?; + for &catalog_table_id in catalog_targets { + self.locks + .acquire( + &self.lock_manager, + LockResource::TableData(catalog_table_id), + LockMode::IntentExclusive, + ) + .await?; + } + Ok(()) + } + + #[inline] + fn catalog_write_authority(&self) -> PreparedCatalogWriteAuthority<'_> { + PreparedCatalogWriteAuthority::new(&self.locks) + } +} + +impl Drop for PreparedTableDdlLocks { + #[inline] + fn drop(&mut self) { + self.locks.release_all(&self.lock_manager); + } +} + +/// Caller-owned DDL preparation transferred atomically at mandatory acceptance. +/// +/// Lock fields precede the foreground pin so ordinary cancellation releases +/// grants before publishing the outer foreground terminal edge. +pub(crate) struct PreparedTableDdlScope { + locks: PreparedTableDdlLocks, + operation: SessionOperationPin, +} + +impl PreparedTableDdlScope { + /// Prepare the fixed CREATE TABLE lock set in canonical resource order. + #[inline] + pub(crate) async fn create( + operation: SessionOperationPin, + table_id: TableID, + catalog_targets: &[TableID], + ) -> OperationResult { + let mut locks = PreparedTableDdlLocks::new(&operation); + locks.acquire_create(table_id, catalog_targets).await?; + Ok(Self { locks, operation }) + } + + /// Prepare the fixed DROP TABLE lock set in canonical resource order. + #[inline] + pub(crate) async fn drop_table( + operation: SessionOperationPin, + table_id: TableID, + catalog_targets: &[TableID], + ) -> OperationResult { + let mut locks = PreparedTableDdlLocks::new(&operation); + locks.acquire_drop(table_id, catalog_targets).await?; + Ok(Self { locks, operation }) + } + + /// Return the exact operation key carried into mandatory diagnostics. + #[inline] + pub(crate) fn key(&self) -> SessionOperationKey { + self.operation.key() + } + + /// Return the retained engine while caller preparation still owns the scope. + #[inline] + pub(crate) fn engine(&self) -> &EngineRef { + &self.operation.engine + } + + /// Synchronously consume caller preparation into accepted authority. + #[inline] + pub(crate) fn accept(self) -> AcceptedTableDdlScope { + let Self { locks, operation } = self; + AcceptedTableDdlScope { + operation: operation.into_mandatory(), + locks: Some(locks), + finish_state: TableDdlFinishState::Executing, + } + } +} + +enum TableDdlFinishState { + Executing, + TerminalReady, + FailedRetained, +} + +/// Runtime-owned table-DDL operation and its transferred logical locks. +pub(crate) struct AcceptedTableDdlScope { + operation: MandatoryOperationGuard, + locks: Option, + finish_state: TableDdlFinishState, +} + +impl AcceptedTableDdlScope { + /// Return the retained engine runtime. + #[inline] + pub(crate) fn engine(&self) -> &EngineRef { + &self.operation.engine + } + + /// Return cloned buffer-pool guards for catalog/table lifecycle work. + #[inline] + pub(crate) fn pool_guards(&self) -> PoolGuards { + self.operation.state.pool_guards().clone() + } + + /// Start one mandatory-owned nested private transaction. + #[inline] + pub(crate) fn begin_private_trx(&self) -> LifecycleResult { + self.operation.begin_private_trx() + } + + /// Borrow the prepared proof used by catalog statement mutation. + #[inline] + pub(crate) fn catalog_write_authority(&self) -> PreparedCatalogWriteAuthority<'_> { + self.locks + .as_ref() + .map(PreparedTableDdlLocks::catalog_write_authority) + .unwrap_or_else(|| { + panic!("accepted table DDL must retain prepared locks during execution") + }) + } + + /// Verify the nested state before returning from accepted execution. + #[inline] + pub(crate) fn mark_terminal_ready(&mut self) { + self.operation.assert_finish_ready(); + self.finish_state = TableDdlFinishState::TerminalReady; + } + + /// Publish normal completion or defensively retain an invalid finish state. + #[inline] + pub(crate) fn finish(&mut self) { + let state = replace(&mut self.finish_state, TableDdlFinishState::FailedRetained); + match state { + TableDdlFinishState::TerminalReady => { + drop(self.locks.take()); + self.operation.finish(); + } + TableDdlFinishState::Executing => { + self.operation.fail_retained(); + let report = Report::new(FatalError::MandatoryTaskPanic) + .attach("accepted table DDL finished without terminal-ready state"); + self.operation.engine.poisoner.poison(report); + drop(self.locks.take()); + } + TableDdlFinishState::FailedRetained => { + drop(self.locks.take()); + } + } + } + + /// Retain unsafe nested ownership before the supervisor publishes poison. + #[inline] + pub(crate) fn handle_panic(&mut self) { + self.operation.fail_retained(); + self.finish_state = TableDdlFinishState::FailedRetained; + } +} + #[derive(Clone, Copy)] enum MaintenanceBoundary { GcHorizon, @@ -443,11 +689,24 @@ impl Session { table_spec: TableSpec, index_specs: Vec, ) -> Result { - let session = self + let validated = ValidatedCreateTable::try_new(table_spec, index_specs).disclose()?; + let operation = self .pin_operation(SessionOperationKind::Ddl) .attach("operation=create_table") .disclose()?; - create_table_for_session(session, table_spec, index_specs).await + let mandatory_runtime = operation.engine.mandatory_runtime.clone(); + let prepared = operation + .prepare_create_table(validated) + .await + .attach("operation=create_table") + .disclose()?; + let observer = mandatory_runtime + .submit(prepared) + .await + .attach("operation=create_table") + .disclose()?; + drop(mandatory_runtime); + observer.wait().await } /// Build and publish a new secondary index for an existing user table. @@ -477,11 +736,24 @@ impl Session { /// Logically drop an existing user table. #[inline] pub async fn drop_table(&mut self, table_id: TableID) -> Result<()> { - let session = self + reject_non_user_table_id(table_id, "drop_table").disclose()?; + let operation = self .pin_operation(SessionOperationKind::Ddl) .attach("operation=drop_table") .disclose()?; - drop_table_for_session(session, table_id).await + let mandatory_runtime = operation.engine.mandatory_runtime.clone(); + let prepared = operation + .prepare_drop_table(table_id) + .await + .attach("operation=drop_table") + .disclose()?; + let observer = mandatory_runtime + .submit(prepared) + .await + .attach("operation=drop_table") + .disclose()?; + drop(mandatory_runtime); + observer.wait().await } /// Run one online catalog checkpoint. @@ -931,16 +1203,13 @@ impl SessionOperationPin { } /// Consume voluntary authority at the exact mandatory ownership handoff. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] + /// + /// The lifecycle slot remains `Active` with this same entry; only the + /// entry's owner label changes from voluntary to mandatory. No later + /// operation can replace that active identity before terminal publication. #[inline] pub(crate) fn into_mandatory(mut self) -> MandatoryOperationGuard { - self.state.accept_mandatory(self.key()); + self.state.accept_mandatory(&self.entry); self.armed = false; MandatoryOperationGuard { engine: self.engine.clone(), @@ -953,29 +1222,47 @@ impl SessionOperationPin { /// Starts one private transaction inside this DDL or maintenance operation. #[inline] pub(crate) fn begin_private_trx(&self) -> LifecycleResult { - let kind = self.kind(); - assert!( - matches!( - kind, - SessionOperationKind::Ddl | SessionOperationKind::Maintenance - ), - "private transaction requires DDL or maintenance authority: key={}, kind={}", - self.key(), - kind.label() - ); - let inner = Box::new(TrxInner::private()); - Ok(self - .engine - .trx_sys - .begin_trx( - &self.engine, - &self.state, - self.key(), - kind, - Some(&self.entry), - inner, - ) - .handle) + begin_private_transaction(&self.engine, &self.entry) + } + + /// Prepare CREATE TABLE while consuming this foreground operation. + async fn prepare_create_table( + self, + validated: ValidatedCreateTable, + ) -> OperationResult { + let table_id = self.engine.catalog().next_table_id(); + let plan = validated.into_plan(table_id); + let scope = + PreparedTableDdlScope::create(self, table_id, create_table_catalog_write_targets()) + .await + .attach_with(|| format!("prepare CREATE TABLE locks: table_id={table_id}"))?; + Ok(PreparedCreateTable::new(scope, plan)) + } + + /// Prepare DROP TABLE while consuming this foreground operation. + async fn prepare_drop_table(self, table_id: TableID) -> OperationResult { + let owner = self.operation_lock_owner(); + self.engine + .lock_manager() + .reject_table_ddl_explicit_session_lock(table_id, owner) + .attach("prepare DROP TABLE explicit-session-lock check")?; + let scope = + PreparedTableDdlScope::drop_table(self, table_id, drop_table_catalog_write_targets()) + .await + .attach_with(|| format!("prepare DROP TABLE locks: table_id={table_id}"))?; + let table = scope + .engine() + .catalog() + .current_live_user_table(table_id) + .ok_or_else(|| { + Report::new(OperationError::TableNotFound).attach(format!( + "drop table current-live lookup: table_id={table_id}" + )) + })?; + Ok(PreparedDropTable::new( + scope, + DropTablePlan::new(table_id, table), + )) } /// Resolve a live user table from authoritative current catalog state. @@ -1053,49 +1340,56 @@ impl Drop for SessionOperationPin { } /// Sole terminal authority for one accepted session operation. -#[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) -)] +/// +/// `state` coordinates session-wide disposition, observation, and terminal +/// slot publication. `entry` is the exact operation retained by that slot. +/// The slot identity stays stable while this guard is armed, although close or +/// abandonment may still change lifecycle disposition or install listeners. +/// Nested private-transaction state can therefore move directly through +/// `entry` without locking the outer lifecycle. pub(crate) struct MandatoryOperationGuard { engine: EngineRef, state: Arc, + /// Intentionally redundant with the `Arc` retained by `Active(entry)`. + /// + /// This direct reference is the guard's exact operation authority. It + /// avoids lifecycle relookup and lets nested transaction state move through + /// the stable entry without taking the outer lifecycle lock. entry: Arc, armed: bool, } impl MandatoryOperationGuard { /// Return the accepted operation key. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] #[inline] pub(crate) fn key(&self) -> SessionOperationKey { self.entry.key() } + /// Starts one private transaction owned by accepted mandatory execution. + /// + /// 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(&self) -> LifecycleResult { + begin_private_transaction(&self.engine, &self.entry) + } + + /// Verify that accepted execution settled every nested transaction. + /// + /// This assertion-bearing check must run only from `AcceptedExecution::execute`. + #[inline] + pub(crate) fn assert_finish_ready(&self) { + self.entry.assert_mandatory_finish_ready(); + } + /// Publish normal terminal state after transferred resources are released. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] #[inline] pub(crate) fn finish(&mut self) { - assert!( - self.armed, - "mandatory operation guard finishes exactly once" - ); - let remove_from_registry = self.state.finish_mandatory(self.key()); + if !self.armed { + return; + } + let remove_from_registry = self.state.finish_mandatory(&self.entry); self.engine .session_registry .remove_if_requested(self.key().session_id(), remove_from_registry); @@ -1103,20 +1397,13 @@ impl MandatoryOperationGuard { } /// Publish retained fatal state after domain-specific panic handling. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] #[inline] pub(crate) fn fail_retained(&mut self) { if !self.armed { return; } self.armed = false; - self.state.fail_mandatory_retained(self.key()); + self.state.fail_mandatory_retained(&self.entry); } } @@ -1139,7 +1426,7 @@ impl Drop for MandatoryOperationGuard { return; } self.armed = false; - self.state.fail_mandatory_retained(self.key()); + self.state.fail_mandatory_retained(&self.entry); let report = Report::new(FatalError::MandatoryTaskPanic).attach(format!( "mandatory operation authority dropped unexpectedly: operation_key={}", self.key() @@ -1553,17 +1840,10 @@ impl SessionState { self.id ) }); - let started = engine.trx_sys.begin_trx( - engine, - self, - key, - SessionOperationKind::PublicTransaction, - None, - inner, - ); + let (trx, entry) = engine.trx_sys.begin_public_trx(engine, key, inner); lifecycle.advance_operation_id(); - lifecycle.slot = SessionOperationSlot::Active(Arc::clone(&started.entry)); - Ok(started.handle) + lifecycle.slot = SessionOperationSlot::Active(entry); + Ok(trx) } /// Recycle a cached public core or directly drop an ephemeral private core. @@ -1679,39 +1959,29 @@ impl SessionState { (remove_from_registry, release.cleanup) } - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] + /// Transfer the retained active entry to mandatory ownership. + /// + /// `entry` is pointer-identical to the lifecycle slot entry by reservation + /// and pin construction. The lifecycle lock serializes the ownership edge + /// and notification; it is not used to resolve the entry again. #[inline] - fn accept_mandatory(&self, key: SessionOperationKey) { + fn accept_mandatory(&self, entry: &Arc) { let lifecycle = self.lifecycle.lock(); - let entry = lifecycle.active_entry(key).unwrap_or_else(|| { - panic!("mandatory acceptance requires exact active operation: operation_key={key}") - }); entry.accept_mandatory(); let notify = lifecycle.change_ev.clone(); drop(lifecycle); Self::notify_operation_change(notify); } - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] + /// Publish the retained mandatory entry and its outer slot atomically. + /// + /// The armed guard supplies the same entry still stored in `Active`. The + /// lifecycle lock orders entry publication with concurrent close or + /// abandonment before changing the slot to `Idle` or `Closed`. #[inline] - fn finish_mandatory(&self, key: SessionOperationKey) -> bool { + fn finish_mandatory(&self, entry: &Arc) -> bool { let mut lifecycle = self.lifecycle.lock(); - let entry = lifecycle.active_entry(key).cloned().unwrap_or_else(|| { - panic!("mandatory completion requires exact active operation: operation_key={key}") - }); - entry.finish_mandatory(); + entry.publish_mandatory_terminal(); let remove_from_registry = lifecycle.finalize_terminal(); let notify = lifecycle.change_ev.clone(); drop(lifecycle); @@ -1722,19 +1992,11 @@ impl SessionState { remove_from_registry } - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] + /// Retain failure on the same stable entry while notifying lifecycle waiters. #[inline] - fn fail_mandatory_retained(&self, key: SessionOperationKey) { + fn fail_mandatory_retained(&self, entry: &Arc) { let lifecycle = self.lifecycle.lock(); - if let Some(entry) = lifecycle.active_entry(key) { - entry.fail_mandatory_retained(); - } + entry.fail_mandatory_retained(); let notify = lifecycle.change_ev.clone(); drop(lifecycle); Self::notify_operation_change(notify); @@ -2033,8 +2295,10 @@ impl SessionDisposition { } } +/// Registry-visible ownership slot for one session's effectful operation. enum SessionOperationSlot { Idle, + /// Exact active entry, stable until its terminal lifecycle publication. Active(Arc), Closed, } @@ -2215,6 +2479,26 @@ impl TrxAttachment { } } +/// Starts one private transaction under an existing DDL or maintenance owner. +#[inline] +fn begin_private_transaction( + engine: &EngineRef, + entry: &Arc, +) -> LifecycleResult { + let kind = entry.kind(); + assert!( + matches!( + kind, + SessionOperationKind::Ddl | SessionOperationKind::Maintenance + ), + "private transaction requires DDL or maintenance authority: key={}, kind={}", + entry.key(), + kind.label() + ); + let inner = Box::new(TrxInner::private()); + Ok(engine.trx_sys.begin_private_trx(engine, entry, inner)) +} + async fn wait_for_checkpoint_retry_in_operation( session: &SessionOperationPin, reason: CheckpointDelayReason, @@ -3452,6 +3736,7 @@ pub(crate) mod tests { entry.inspect().state, SessionOperationState::Mandatory(None) ); + mandatory.assert_finish_ready(); mandatory.finish(); assert_eq!(entry.inspect().state, SessionOperationState::Terminal); drop(mandatory); diff --git a/doradb-storage/src/table/layout.rs b/doradb-storage/src/table/layout.rs index 2a9414b7..3b8953c7 100644 --- a/doradb-storage/src/table/layout.rs +++ b/doradb-storage/src/table/layout.rs @@ -168,6 +168,7 @@ mod tests { }; use crate::id::TrxID; use crate::table::tests::*; + use crate::trx::purge::PurgeTestEvent; use crate::value::ValKind; use std::panic::{AssertUnwindSafe, catch_unwind}; use tempfile::TempDir; @@ -291,7 +292,23 @@ mod tests { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let engine = lightweight_test_engine(&temp_dir, "redo_testsys_lightweight").await; + let (purge_event_tx, purge_event_rx) = flume::unbounded(); + engine + .inner() + .trx_sys + .set_purge_test_observer(purge_event_tx); let table_id = create_table2_for_test(&engine).await; + engine.inner().trx_sys.request_purge_observation(); + let mut create_commit_recorded = false; + loop { + match purge_event_rx.recv_async().await.unwrap() { + PurgeTestEvent::CommittedRecorded { .. } => { + create_commit_recorded = true; + } + PurgeTestEvent::CycleCompleted if create_commit_recorded => break, + _ => {} + } + } let table = table_for_internal_assertion(&engine, table_id); let old_layout = table.layout_snapshot(); assert_eq!(old_layout.metadata().idx.active_index_count(), 1); diff --git a/doradb-storage/src/table/persistence.rs b/doradb-storage/src/table/persistence.rs index 35997385..a631c0ca 100644 --- a/doradb-storage/src/table/persistence.rs +++ b/doradb-storage/src/table/persistence.rs @@ -5078,22 +5078,20 @@ mod tests { release_rx.recv_async().await.unwrap(); }); - let checkpoint = checkpoint_session.checkpoint_table(table_id).fuse(); - futures::pin_mut!(checkpoint); - let entered = entered_rx.recv_async().fuse(); - futures::pin_mut!(entered); + let mut checkpoint = Box::pin(checkpoint_session.checkpoint_table(table_id).fuse()); + let mut entered = Box::pin(entered_rx.recv_async().fuse()); futures::select! { - result = checkpoint => { + result = checkpoint.as_mut() => { panic!("checkpoint completed before transition hook: {result:?}"); } - result = entered => result.unwrap(), + result = entered.as_mut() => result.unwrap(), } + drop(entered); let mut drop_session = engine.new_session().unwrap(); let table = table_for_internal_assertion(&engine, table_id); let checkpoint_redo_cts = { - let drop_table = drop_session.drop_table(table_id).fuse(); - futures::pin_mut!(drop_table); + let mut drop_table = Box::pin(drop_session.drop_table(table_id).fuse()); assert!(matches!( futures::poll!(drop_table.as_mut()), std::task::Poll::Pending diff --git a/doradb-storage/src/trx/mod.rs b/doradb-storage/src/trx/mod.rs index ad2f2b4c..e6c5638f 100644 --- a/doradb-storage/src/trx/mod.rs +++ b/doradb-storage/src/trx/mod.rs @@ -54,10 +54,12 @@ use crate::session::TrxAttachment; use crate::trx::undo::{IndexPurgeEntry, IndexUndoLogs, RowUndoHead, RowUndoLogs, UndoStatus}; use error_stack::{Report, ResultExt}; use event_listener::{Event, EventListener}; +use futures::FutureExt; use parking_lot::Mutex; use std::marker::PhantomData; use std::mem; use std::ops::AsyncFnOnce; +use std::panic::{AssertUnwindSafe, resume_unwind}; use std::ptr::addr_eq; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; @@ -307,6 +309,53 @@ impl Transaction { result } + /// Stages catalog DDL under an accepted operation's prepared logical locks. + /// + /// This is a narrow bridge for the current exact-owner lock manager. + /// Reacquiring the same catalog claims for the nested transaction would be + /// correctness-safe through same-family coverage, but would add duplicate + /// manager grants and owner-cache entries. A future exact-family lock + /// design should unify operation and transaction claims and remove this + /// special authority path while preserving the panic settlement below. + /// + /// 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. + #[inline] + pub(crate) async fn stage_prepared_catalog_statement( + &mut self, + authority: PreparedCatalogWriteAuthority<'_>, + f: F, + ) -> RuntimeResult + where + F: for<'borrow> AsyncFnOnce(&'borrow mut Statement<'_>) -> RuntimeResult, + { + let checkout = self + .checkout() + .change_context(RuntimeError::CatalogAccess) + .attach("operation=stage_prepared_catalog_statement")?; + let mut stmt_state = StmtState::private(checkout); + let outcome = AssertUnwindSafe(async { + let mut stmt = stmt_state.prepared_catalog_statement(authority); + let result = f(&mut stmt).await; + stmt.merge_effects(); + result + }) + .catch_unwind() + .await; + match outcome { + Ok(result) => { + stmt_state.return_ordinary(); + result + } + Err(panic) => { + stmt_state.return_after_mandatory_panic(); + resume_unwind(panic); + } + } + } + /// Commit the transaction. #[inline] pub async fn commit(self) -> Result { @@ -467,14 +516,6 @@ impl Drop for Transaction { } } -/// Transaction begin result with separate public handle and registry entry. -pub(crate) struct StartedTransaction { - /// Public transaction handle returned to the session. - pub(crate) handle: Transaction, - /// Stable session-registry entry for the mutable transaction core. - pub(crate) entry: Arc, -} - /// Shared transaction timestamp state referenced by row undo heads. pub(crate) struct SharedTrxStatus { ts: AtomicU64, @@ -698,22 +739,90 @@ impl TrxContext { } } +/// Borrowed proof that accepted table DDL prepared catalog write locks. +/// +/// The enclosing operation owns metadata-S and data-IX claims for longer than +/// its nested catalog transaction. This temporary capability lets that +/// transaction reuse those covering claims without registering duplicate +/// exact-owner grants. It is deliberately not a general lock-bypass flag: the +/// borrow ties its lifetime to the prepared lock scope and every catalog-table +/// write still asserts exact coverage. +#[derive(Clone, Copy)] +pub(crate) struct PreparedCatalogWriteAuthority<'a> { + locks: &'a OwnerLockState, +} + +impl<'a> PreparedCatalogWriteAuthority<'a> { + /// Create a borrowed proof over one accepted operation's prepared locks. + #[inline] + pub(crate) fn new(locks: &'a OwnerLockState) -> Self { + Self { locks } + } + + /// Assert that the prepared owner covers one catalog-table write. + #[inline] + pub(crate) fn assert_table_write(self, table_id: TableID) { + assert!( + self.covers_table_write(table_id), + "prepared catalog-write authority is incomplete: table_id={table_id}, owner={}", + self.locks.owner() + ); + } + + /// Return whether metadata-S and data-IX are both present. + #[inline] + pub(crate) fn covers_table_write(self, table_id: TableID) -> bool { + self.locks + .cached_covers(LockResource::TableMetadata(table_id), LockMode::Shared) + && self + .locks + .cached_covers(LockResource::TableData(table_id), LockMode::IntentExclusive) + } +} + /// Operation-local transaction runtime view. /// -/// `TrxRuntime` pairs immutable MVCC identity with the checked-out operation's -/// runtime attachment. It is borrowed from scoped foreground statement or -/// terminal work; checked-in transaction state never stores it. +/// Prepared catalog authority is present only for accepted table DDL. Ordinary +/// statements continue proving writes with transaction-owned logical locks. +/// The optional authority exists only to carry the temporary operation-claim +/// proof into lower-level write assertions. #[derive(Clone, Copy)] pub(crate) struct TrxRuntime<'r> { ctx: &'r TrxContext, attachment: &'r TrxAttachment, + #[cfg_attr( + not(debug_assertions), + expect( + dead_code, + reason = "prepared authority participates in debug-only lower-level lock assertions" + ) + )] + prepared_catalog_write: Option>, } impl<'r> TrxRuntime<'r> { /// Create an operation-local runtime view. #[inline] pub(crate) fn new(ctx: &'r TrxContext, attachment: &'r TrxAttachment) -> Self { - Self { ctx, attachment } + Self { + ctx, + attachment, + prepared_catalog_write: None, + } + } + + /// Create a runtime view backed by prepared operation-level catalog locks. + #[inline] + pub(crate) fn new_prepared_catalog( + ctx: &'r TrxContext, + attachment: &'r TrxAttachment, + authority: PreparedCatalogWriteAuthority<'r>, + ) -> Self { + Self { + ctx, + attachment, + prepared_catalog_write: Some(authority), + } } /// Returns this runtime's immutable transaction context. @@ -767,6 +876,12 @@ impl<'r> TrxRuntime<'r> { pub(crate) fn debug_assert_table_write_lock_held(&self, table_id: TableID) { #[cfg(debug_assertions)] { + if self + .prepared_catalog_write + .is_some_and(|authority| authority.covers_table_write(table_id)) + { + return; + } let resource = LockResource::TableData(table_id); let owner = LockOwner::transaction(self.attachment.session_id(), self.ctx.trx_id()); let held = self.engine().lock_manager().owner_holds( @@ -974,13 +1089,6 @@ pub(crate) enum SessionOperationState { /// Caller-owned preparation or foreground execution. Voluntary(Option), /// Runtime-owned accepted execution. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] Mandatory(Option), /// A checked-in abandoned transaction may be claimed for cleanup. CleanupReady, @@ -1170,17 +1278,28 @@ impl SessionOperationEntry { ); let trx_id = trx_inner.trx_id(); 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.outer_foreground_alive - && inner.state == SessionOperationState::Voluntary(None) - && inner.trx_id.is_none() - && inner.trx_inner.is_none(), - "private transaction installation requires an empty foreground entry: key={}, state={}, trx_id={:?}", + inner.trx_id.is_none() && inner.trx_inner.is_none(), + "private transaction installation requires an empty payload slot: key={}, state={}, trx_id={:?}", self.key, inner.state.label(), inner.trx_id ); - inner.state = SessionOperationState::Voluntary(Some(InternalTrxState::Available)); + inner.state = next_state; inner.trx_id = Some(trx_id); inner.trx_inner = Some(trx_inner); } @@ -1214,6 +1333,14 @@ impl SessionOperationEntry { InternalTrxState::Running, ))) } + SessionOperationKind::Ddl | SessionOperationKind::Maintenance + if inner.state + == SessionOperationState::Mandatory(Some(InternalTrxState::Available)) => + { + Some(SessionOperationState::Mandatory(Some( + InternalTrxState::Running, + ))) + } SessionOperationKind::PublicTransaction | SessionOperationKind::Ddl | SessionOperationKind::Maintenance @@ -1262,6 +1389,12 @@ impl SessionOperationEntry { { SessionOperationState::Voluntary(Some(InternalTrxState::Completing)) } + SessionOperationKind::Ddl | SessionOperationKind::Maintenance + if inner.state + == SessionOperationState::Mandatory(Some(InternalTrxState::Available)) => + { + SessionOperationState::Mandatory(Some(InternalTrxState::Completing)) + } SessionOperationKind::PublicTransaction | SessionOperationKind::Ddl | SessionOperationKind::Maintenance @@ -1349,7 +1482,11 @@ impl SessionOperationEntry { inner.state == SessionOperationState::Voluntary(None) } SessionOperationKind::Ddl | SessionOperationKind::Maintenance => { - inner.state == SessionOperationState::Voluntary(Some(InternalTrxState::Running)) + matches!( + inner.state, + SessionOperationState::Voluntary(Some(InternalTrxState::Running)) + | SessionOperationState::Mandatory(Some(InternalTrxState::Running)) + ) } SessionOperationKind::SessionExplicitLock => false, }; @@ -1390,12 +1527,23 @@ impl SessionOperationEntry { if self.kind != SessionOperationKind::PublicTransaction { inner.state = match self.kind { SessionOperationKind::Ddl | SessionOperationKind::Maintenance => { - assert!( - inner.outer_foreground_alive, - "detached private transaction return requires cleanup intent: key={}", - self.key - ); - SessionOperationState::Voluntary(Some(InternalTrxState::Available)) + 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::SessionExplicitLock => { panic!( @@ -1601,6 +1749,12 @@ impl SessionOperationEntry { SessionOperationState::Voluntary(Some(InternalTrxState::Completing)) ) } + SessionOperationKind::Ddl | SessionOperationKind::Maintenance + if inner.state + == SessionOperationState::Mandatory(Some(InternalTrxState::Completing)) => + { + true + } SessionOperationKind::Ddl | SessionOperationKind::Maintenance => { inner.state == SessionOperationState::Completing } @@ -1612,12 +1766,20 @@ impl SessionOperationEntry { inner.trx_inner.take(); inner.trx_id = None; inner.cleanup_requested = false; - if self.kind == SessionOperationKind::PublicTransaction || !inner.outer_foreground_alive { - inner.state = SessionOperationState::Terminal; - Some(true) - } else { - inner.state = SessionOperationState::Voluntary(None); - Some(false) + match inner.state { + SessionOperationState::Mandatory(Some(InternalTrxState::Completing)) => { + inner.state = SessionOperationState::Mandatory(None); + Some(false) + } + SessionOperationState::Voluntary(Some(InternalTrxState::Completing)) => { + inner.state = SessionOperationState::Voluntary(None); + Some(false) + } + SessionOperationState::Completing => { + inner.state = SessionOperationState::Terminal; + Some(true) + } + _ => None, } } @@ -1649,46 +1811,17 @@ impl SessionOperationEntry { /// /// The caller holds the session lifecycle mutex, preserving the global /// `lifecycle -> entry` lock order. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] #[inline] pub(crate) fn accept_mandatory(&self) { - assert!( - self.kind != SessionOperationKind::PublicTransaction, - "public transactions cannot become mandatory operations: key={}", - self.key - ); let mut inner = self.inner.lock(); - assert!( - inner.outer_foreground_alive - && inner.state == SessionOperationState::Voluntary(None) - && inner.trx_id.is_none() - && inner.trx_inner.is_none(), - "mandatory acceptance requires empty voluntary authority: key={}, state={}, trx_id={:?}", - self.key, - inner.state.label(), - inner.trx_id - ); inner.outer_foreground_alive = false; inner.state = SessionOperationState::Mandatory(None); } - /// Publish successful mandatory completion after all nested work is gone. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] + /// Verify successful mandatory completion while execution is supervised. #[inline] - pub(crate) fn finish_mandatory(&self) { - let mut inner = self.inner.lock(); + pub(crate) fn assert_mandatory_finish_ready(&self) { + let inner = self.inner.lock(); assert!( inner.state == SessionOperationState::Mandatory(None) && inner.trx_id.is_none() @@ -1698,17 +1831,15 @@ impl SessionOperationEntry { inner.state.label(), inner.trx_id ); - inner.state = SessionOperationState::Terminal; + } + + /// Publish terminal state after execution-side validation succeeded. + #[inline] + pub(crate) fn publish_mandatory_terminal(&self) { + self.inner.lock().state = SessionOperationState::Terminal; } /// Publish safe fatal retention for an unexpectedly lost mandatory owner. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves production handoff with synthetic adapters" - ) - )] #[inline] pub(crate) fn fail_mandatory_retained(&self) { let mut inner = self.inner.lock(); @@ -3697,6 +3828,63 @@ pub(crate) mod tests { assert_eq!(entry.inspect().state, SessionOperationState::Terminal); } + #[test] + fn test_private_transaction_state_is_nested_under_mandatory_operation() { + let session_id = SessionID::new(102); + let trx_id = MIN_ACTIVE_TRX_ID + 102; + let entry = SessionOperationEntry::new( + SessionOperationKey::new(session_id, OperationID::new(1)), + SessionOperationKind::Ddl, + ); + + entry.accept_mandatory(); + assert_eq!( + entry.inspect().state, + SessionOperationState::Mandatory(None) + ); + entry.install_private_transaction(Box::new(trx_inner( + trx_id, + TrxID::new(102), + 0, + session_id, + ))); + assert_eq!( + entry.inspect().state, + SessionOperationState::Mandatory(Some(InternalTrxState::Available)) + ); + + let inner = entry + .take_for_checkout(trx_id) + .expect("mandatory private transaction can be checked out"); + assert_eq!( + entry.inspect().state, + SessionOperationState::Mandatory(Some(InternalTrxState::Running)) + ); + assert!(!entry.return_inner(inner)); + assert_eq!( + entry.inspect().state, + SessionOperationState::Mandatory(Some(InternalTrxState::Available)) + ); + + let inner = entry + .take_for_terminal(trx_id) + .expect("mandatory private transaction terminal can be claimed"); + assert_eq!( + entry.inspect().state, + SessionOperationState::Mandatory(Some(InternalTrxState::Completing)) + ); + drop(inner); + assert_eq!(entry.finish_transaction(trx_id), Some(false)); + assert_eq!( + entry.inspect().state, + SessionOperationState::Mandatory(None) + ); + + entry.assert_mandatory_finish_ready(); + entry.publish_mandatory_terminal(); + assert_eq!(entry.inspect().state, SessionOperationState::Terminal); + } + #[test] fn test_stale_transaction_identity_cannot_claim_reused_operation_entry() { let session_id = SessionID::new(200); diff --git a/doradb-storage/src/trx/stmt.rs b/doradb-storage/src/trx/stmt.rs index f9297b1d..cd5dfee1 100644 --- a/doradb-storage/src/trx/stmt.rs +++ b/doradb-storage/src/trx/stmt.rs @@ -20,8 +20,8 @@ use crate::trx::undo::{ IndexUndo, IndexUndoKind, IndexUndoLogs, OwnedRowUndo, RowUndoKind, RowUndoLogs, }; use crate::trx::{ - FatalRollbackRetention, SessionOperationCheckout, TableAdmissionRequest, TrxEffects, TrxInner, - TrxRuntime, + FatalRollbackRetention, PreparedCatalogWriteAuthority, SessionOperationCheckout, + TableAdmissionRequest, TrxEffects, TrxInner, TrxRuntime, }; use crate::value::Val; use error_stack::ResultExt; @@ -299,6 +299,23 @@ impl StmtState { /// Lends one direct callback-facing statement facade. #[inline] pub(crate) fn statement(&mut self) -> Statement<'_> { + self.statement_with_authority(None) + } + + /// Lends a callback facade backed by prepared catalog-write authority. + #[inline] + pub(crate) fn prepared_catalog_statement<'a>( + &'a mut self, + authority: PreparedCatalogWriteAuthority<'a>, + ) -> Statement<'a> { + self.statement_with_authority(Some(authority)) + } + + #[inline] + fn statement_with_authority<'a>( + &'a mut self, + prepared_catalog_write: Option>, + ) -> Statement<'a> { let Self { effects, stmt_locks, @@ -315,6 +332,7 @@ impl StmtState { effects, stmt_locks, disable_dml_validation: false, + prepared_catalog_write, } } @@ -337,6 +355,22 @@ 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.release_statement_locks(); + self.checkout = None; + } + #[inline] fn release_statement_locks(&mut self) { if let Some(checkout) = self.checkout.as_ref() { @@ -393,6 +427,7 @@ pub struct Statement<'stmt> { effects: &'stmt mut StmtEffects, stmt_locks: &'stmt mut OwnerLockState, disable_dml_validation: bool, + prepared_catalog_write: Option>, } impl<'stmt> Statement<'stmt> { @@ -413,7 +448,12 @@ impl<'stmt> Statement<'stmt> { /// Returns this statement's operation-local transaction runtime. #[inline] pub(crate) fn runtime(&self) -> TrxRuntime<'_> { - TrxRuntime::new(self.inner.ctx(), self.attachment) + match self.prepared_catalog_write { + Some(authority) => { + TrxRuntime::new_prepared_catalog(self.inner.ctx(), self.attachment, authority) + } + None => TrxRuntime::new(self.inner.ctx(), self.attachment), + } } /// Returns mutable access to this statement's effect accumulator. @@ -424,10 +464,13 @@ impl<'stmt> Statement<'stmt> { #[inline] fn runtime_and_effects_mut(&mut self) -> (TrxRuntime<'_>, &mut StmtEffects) { - ( - TrxRuntime::new(self.inner.ctx(), self.attachment), - self.effects, - ) + let runtime = match self.prepared_catalog_write { + Some(authority) => { + TrxRuntime::new_prepared_catalog(self.inner.ctx(), self.attachment, authority) + } + None => TrxRuntime::new(self.inner.ctx(), self.attachment), + }; + (runtime, self.effects) } /// Acquires transaction-lifetime metadata protection for a table write. @@ -846,18 +889,24 @@ impl<'stmt> Statement<'stmt> { ) -> OperationOrRuntimeResult { const OPERATION: &str = "catalog_insert_mvcc"; let table_id = table.table_id(); - self.acquire_table_write_metadata_lock(table_id) - .await - .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; + if let Some(authority) = self.prepared_catalog_write { + authority.assert_table_write(table_id); + } else { + self.acquire_table_write_metadata_lock(table_id) + .await + .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; + } if !self.disable_dml_validation { DmlValidator::new(table.metadata()) .validate_full_row(&cols) .change_context(OperationError::InvalidDmlInput) .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; } - self.acquire_table_write_data_lock(table_id) - .await - .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; + if self.prepared_catalog_write.is_none() { + self.acquire_table_write_data_lock(table_id) + .await + .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; + } let (rt, effects) = self.runtime_and_effects_mut(); table .insert_mvcc(rt, effects, cols) @@ -894,18 +943,24 @@ impl<'stmt> Statement<'stmt> { ) -> OperationOrRuntimeResult { const OPERATION: &str = "catalog_delete_primary_key_mvcc"; let table_id = table.table_id(); - self.acquire_table_write_metadata_lock(table_id) - .await - .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; + if let Some(authority) = self.prepared_catalog_write { + authority.assert_table_write(table_id); + } else { + self.acquire_table_write_metadata_lock(table_id) + .await + .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; + } if !self.disable_dml_validation { DmlValidator::new(table.metadata()) .validate_primary_key(index_no, key_vals) .change_context(OperationError::InvalidDmlInput) .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; } - self.acquire_table_write_data_lock(table_id) - .await - .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; + if self.prepared_catalog_write.is_none() { + self.acquire_table_write_data_lock(table_id) + .await + .attach_with(|| format!("operation={OPERATION}, table_id={table_id}"))?; + } let (rt, effects) = self.runtime_and_effects_mut(); table .delete_unique_mvcc(rt, effects, index_no, key_vals, log_by_key) diff --git a/doradb-storage/src/trx/sys.rs b/doradb-storage/src/trx/sys.rs index a1cd9c1f..6d0dcdb8 100644 --- a/doradb-storage/src/trx/sys.rs +++ b/doradb-storage/src/trx/sys.rs @@ -14,7 +14,7 @@ use crate::error::{ }; use crate::file::fs::FileSystem; use crate::file::table_file::{MutableTableFile, OldRoot, TableFile}; -use crate::id::{SessionOperationKey, TrxID}; +use crate::id::{SessionID, SessionOperationKey, TrxID}; use crate::log::redo::RedoLogs; use crate::log::{EnqueuePrecommitError, LogFileSealer, LogWriteDriver, RedoLog, RedoLogWriter}; use crate::notify::MonotonicU64; @@ -23,10 +23,8 @@ use crate::poison::EnginePoisoner; use crate::quiescent::{QuiescentBox, QuiescentGuard, SyncQuiescentGuard}; use crate::recovery::RecoveryResources; use crate::recovery::stream::CatalogSafeRedoSegment; -use crate::runtime::mandatory::{ - MandatoryInternalTask, MandatoryRuntime, MandatoryTaskMetadata, submit_internal, -}; -use crate::session::{SessionState, TrxAttachment}; +use crate::runtime::mandatory::{MandatoryInternalTask, MandatoryRuntime, MandatoryTaskMetadata}; +use crate::session::TrxAttachment; use crate::thread; #[cfg(test)] use crate::trx::SessionOperationState; @@ -39,8 +37,7 @@ 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, SessionOperationKind, - StartedTransaction, Transaction, TrxInner, + SessionOperationCompletionClaim, SessionOperationEntry, Transaction, TrxInner, }; use crossbeam_utils::CachePadded; use either::Either::{Left, Right}; @@ -1087,17 +1084,9 @@ impl TransactionSystem { Ok(table_file) } - /// Create a new transaction. + /// Initialize one ready transaction core and register its active snapshot. #[inline] - pub(crate) fn begin_trx( - &self, - engine: &EngineRef, - session_state: &Arc, - operation_key: SessionOperationKey, - kind: SessionOperationKind, - enclosing_entry: Option<&Arc>, - mut inner: Box, - ) -> StartedTransaction { + fn init_trx(&self, session_id: SessionID, inner: &mut TrxInner) -> (TrxID, TrxID) { let gc_no = self.next_gc_no(); let gc_bucket = &self.gc_buckets[gc_no]; // Add to active sts list. @@ -1120,38 +1109,36 @@ impl TransactionSystem { .store(sts.as_u64(), Ordering::Relaxed); } drop(g); // release bucket lock. - inner.init(trx_id, sts, gc_no, session_state.id()); - let entry = match kind { - SessionOperationKind::PublicTransaction => { - assert!( - enclosing_entry.is_none(), - "public transaction must allocate its stable operation entry directly: key={operation_key}" - ); - SessionOperationEntry::new_public_transaction(operation_key, inner) - } - SessionOperationKind::Ddl | SessionOperationKind::Maintenance => { - let entry = enclosing_entry.unwrap_or_else(|| { - panic!( - "private transaction requires enclosing operation entry: key={operation_key}, kind={}", - kind.label() - ) - }); - assert!( - entry.key() == operation_key && entry.kind() == kind, - "private transaction enclosing entry mismatch: expected_key={operation_key}, actual_key={}, expected_kind={}, actual_kind={}", - entry.key(), - kind.label(), - entry.kind().label() - ); - entry.install_private_transaction(inner); - Arc::clone(entry) - } - SessionOperationKind::SessionExplicitLock => { - panic!("explicit-lock operation cannot own a transaction: key={operation_key}") - } - }; + inner.init(trx_id, sts, gc_no, session_id); + (trx_id, sts) + } + + /// Create a public transaction and its new stable session entry. + #[inline] + pub(crate) fn begin_public_trx( + &self, + engine: &EngineRef, + operation_key: SessionOperationKey, + mut inner: Box, + ) -> (Transaction, Arc) { + let (trx_id, sts) = self.init_trx(operation_key.session_id(), inner.as_mut()); + let entry = SessionOperationEntry::new_public_transaction(operation_key, inner); let handle = Transaction::new(engine.downgrade(), operation_key, trx_id, sts); - StartedTransaction { handle, entry } + (handle, entry) + } + + /// Create a private transaction inside an existing stable operation entry. + #[inline] + pub(crate) fn begin_private_trx( + &self, + engine: &EngineRef, + enclosing_entry: &Arc, + mut inner: Box, + ) -> Transaction { + let operation_key = enclosing_entry.key(); + let (trx_id, sts) = self.init_trx(operation_key.session_id(), inner.as_mut()); + enclosing_entry.install_private_transaction(inner); + Transaction::new(engine.downgrade(), operation_key, trx_id, sts) } /// Allocate a timestamp fence for a runtime state transition. @@ -1332,7 +1319,7 @@ impl TransactionSystem { completion: Arc::clone(&completion), operation, }; - if let Err(job) = submit_internal(&self.mandatory_runtime, job) { + if let Err(job) = self.mandatory_runtime.submit_internal(job) { // The returned job owns an already-claimed terminal transaction. // Do not drop, discard, or run it from this caller. A closed // internal admission means the mandatory-runtime lifetime invariant @@ -1656,15 +1643,14 @@ impl TransactionSystem { operation_key: SessionOperationKey, trx_id: TrxID, ) { - let _ = submit_internal( - &self.mandatory_runtime, - SessionOperationCleanupJob { + let _ = self + .mandatory_runtime + .submit_internal(SessionOperationCleanupJob { engine, operation_key, trx_id, claim: None, - }, - ); + }); } /// Submit failed-precommit rollback cleanup. @@ -1680,7 +1666,7 @@ impl TransactionSystem { /// admission, so redo can submit every final failed-precommit job. #[inline] pub(crate) fn request_failed_precommit_cleanup(&self, job: FailedPrecommitCleanupJob) { - if let Err(job) = submit_internal(&self.mandatory_runtime, job) { + if let Err(job) = self.mandatory_runtime.submit_internal(job) { // The returned job may own rollback-capable `PrecommitTrx` payloads. // Do not drop, discard, or run it from this synchronous caller. The // closed internal admission means the runtime-lifetime invariant is