diff --git a/docs/architecture.md b/docs/architecture.md index 716bc1db..94cbc0a9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -138,10 +138,14 @@ data coordination is described in [Lock System](./lock-system.md). ## Mandatory Background Runtime The engine owns one fixed-thread asynchronous executor for obligations that -must reach a supervised terminal outcome after acceptance. Caller preparation -and operation-lock waiting remain outside runtime capacity and are -cancellable. A synchronous consuming acceptance edge transfers all prepared -resources into mandatory ownership before the task is detached. +must reach a supervised terminal outcome after acceptance. Effectful session +maintenance uses this runtime beside table and index DDL: table freeze and +checkpoint, catalog checkpoint, redo truncation, combined catalog/redo +maintenance, and secondary `MemIndex` cleanup all prepare their complete +authority before mandatory admission. Caller preparation and operation-lock +waiting remain outside runtime capacity and are cancellable. A synchronous +consuming acceptance edge transfers all prepared resources into mandatory +ownership before the task is detached. The same executor replaces the former sequential transaction-cleanup thread. Abandoned transactions, explicit terminal rollback, and failed-precommit diff --git a/docs/checkpoint.md b/docs/checkpoint.md index d20fd81b..7900a036 100644 --- a/docs/checkpoint.md +++ b/docs/checkpoint.md @@ -73,10 +73,14 @@ silently changing the batch or its snapshot boundary. One checkpoint attempt follows these conceptual phases: -1. From an idle session, acquire scoped `TableMetadata(S)` and `TableData(IS)` - locks, revalidate the live table, and claim its checkpoint workflow. -2. Hold root-mutation exclusion and verify that the currently active root is no - longer visible to an active snapshot before forking a mutable CoW root. +1. From an idle session, acquire owned `TableMetadata(S)` and `TableData(IS)` + locks, revalidate the live table, claim its checkpoint workflow, and acquire + lifetime-free root-mutation exclusion while the caller future remains + cancellable. +2. After the complete table, workflow, and root authority is prepared, transfer + it synchronously to the mandatory runtime. Accepted execution acquires none + of those resources and verifies that the currently active root is no longer + visible to an active snapshot before forking a mutable CoW root. 3. Use the purge-published GC horizon as the exclusive cutoff for both frozen row images and cold-row delete selection, then allocate `checkpoint_ts`. 4. Convert a ready frozen-page prefix into LWC blocks and collect matching @@ -134,7 +138,12 @@ checkpoint attempt and returns listener-only wait state, so the subsequent sleep owns no table runtime, layout, frozen page, page guard, checkpoint attempt, or logical table lock. Completion only means a retry may be useful. `checkpoint_table_with_wait` retries delayed outcomes and returns published, -cancelled, or error outcomes unchanged. +cancelled, or error outcomes unchanged. Each delayed checkpoint attempt first +reaches its own terminal operation state and releases its mandatory permit, +table runtime, workflow/root authority, and logical locks. The standalone +observer wait then owns only listener state. A useful wake starts a new +checkpoint operation with a fresh operation key and freshly prepared +authority; retries never reuse the completed attempt's operation owner. ### Root Liveness and Reclamation diff --git a/docs/engine-component-lifetime.md b/docs/engine-component-lifetime.md index 2506f194..9d5615c1 100644 --- a/docs/engine-component-lifetime.md +++ b/docs/engine-component-lifetime.md @@ -197,6 +197,19 @@ 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. +Effectful maintenance uses the same contract. Table freeze/checkpoint transfer +the exact live table, owned maintenance locks, workflow attempt, and checkpoint +root-mutation scope. Catalog checkpoint and redo maintenance transfer their +catalog-checkpoint and redo-retention scopes; combined maintenance preserves +both through catalog publication, releases catalog admission before unlink, +and retains redo exclusion until unlink accounting finishes. Secondary +`MemIndex` cleanup transfers its table scope and stores each private +transaction in accepted progress before scanning or awaiting. On normal +completion domain workflow/gate and private-transaction resources release +first, then prepared maintenance locks, and only then the outer session entry +publishes `Terminal`. A dropped result observer owns none of these resources +and cannot cancel accepted work. + 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 8d60fad8..c514834b 100644 --- a/docs/lock-system.md +++ b/docs/lock-system.md @@ -119,7 +119,7 @@ The current implementation uses the following table-level mapping: | Full-table MVCC mutation | transaction `S` | transaction `X` | transaction | | 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 | +| Freeze/checkpoint | prepared owned `S` | prepared owned `IS` | prepared maintenance operation, then mandatory owner | | 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 INDEX | target `X`; catalog slots 0,2,3 `S` | target `X`; catalog slots 0,2,3 `IX` | prepared DDL operation, then mandatory owner | @@ -145,7 +145,10 @@ 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. -Maintenance continues through its scoped lock-manager path. +Effectful maintenance likewise prepares an owned lock scope before mandatory +admission and transfers the exact `OwnerLockState` without a release/reacquire +window. The read-only `total_row_pages` observation remains caller-owned and +uses the borrowed scoped lock-manager path. Recovery, purge, and no-transaction replay do not acquire logical locks. They run at lifecycle boundaries where foreground lock owners do not exist. Logical @@ -327,11 +330,12 @@ global `release_owner()` scan during session cleanup. Transactions, DDL, maintenance, and explicit-lock mutations reserve ids from one plain session-local sequence. One public DDL call retains one -`Operation` owner through its typed `SessionDdlContext`; one public maintenance -workflow retains one `Operation` owner across every -`ScopedTableRuntimeAccess`, bounded recheck, and internal retry. Operation -cleanup still uses fresh-lock and scoped guards rather than an authoritative -`LockScopeState`. +`Operation` owner through its typed `SessionDdlContext`; one effectful public +maintenance call retains one `Operation` owner through its prepared and +accepted owned scope. A delayed checkpoint completes that operation before its +observer-only wait and allocates a fresh operation owner for the next attempt. +The caller-owned `total_row_pages` observation continues to use +`ScopedTableRuntimeAccess`. Maintenance always records its own exact claims even when a covering `SessionExplicit` claim admits it. Releasing maintenance therefore cannot diff --git a/docs/public-error-audit.csv b/docs/public-error-audit.csv index b8195e03..0ed79065 100644 --- a/docs/public-error-audit.csv +++ b/docs/public-error-audit.csv @@ -11,28 +11,26 @@ doradb-storage/src/error.rs,SharedFatalError::disclose,1 doradb-storage/src/log/mod.rs,LogSync::from_str,1 doradb-storage/src/session.rs,Session::begin_trx,4 doradb-storage/src/session.rs,Session::buffer_pool_stats,1 -doradb-storage/src/session.rs,Session::checkpoint_catalog,2 -doradb-storage/src/session.rs,Session::checkpoint_catalog_and_truncate_redo_log,2 -doradb-storage/src/session.rs,Session::checkpoint_table,3 -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::checkpoint_catalog,3 +doradb-storage/src/session.rs,Session::checkpoint_catalog_and_truncate_redo_log,3 +doradb-storage/src/session.rs,Session::checkpoint_table,5 +doradb-storage/src/session.rs,Session::cleanup_secondary_mem_indexes,5 doradb-storage/src/session.rs,Session::close,3 doradb-storage/src/session.rs,Session::create_index,9 doradb-storage/src/session.rs,Session::create_table,4 doradb-storage/src/session.rs,Session::drop_index,8 doradb-storage/src/session.rs,Session::drop_table,4 -doradb-storage/src/session.rs,Session::freeze_table,3 +doradb-storage/src/session.rs,Session::freeze_table,5 doradb-storage/src/session.rs,Session::list_table_ids,1 doradb-storage/src/session.rs,Session::lock_table,2 doradb-storage/src/session.rs,Session::storage_io_stats,1 doradb-storage/src/session.rs,Session::total_row_pages,3 doradb-storage/src/session.rs,Session::transaction_system_stats,1 -doradb-storage/src/session.rs,Session::truncate_redo_log,2 +doradb-storage/src/session.rs,Session::truncate_redo_log,3 doradb-storage/src/session.rs,Session::unlock_table,2 doradb-storage/src/session.rs,Session::wait_for_checkpoint_retry,2 doradb-storage/src/session.rs,Session::wait_for_gc_horizon_after,1 doradb-storage/src/session.rs,Session::wait_for_purge_completion_after,1 -doradb-storage/src/session.rs,wait_for_checkpoint_retry_in_operation,2 doradb-storage/src/session.rs,wait_for_maintenance_boundary,4 doradb-storage/src/table/access.rs,LazyRow::val,2 doradb-storage/src/table/access.rs,UserTableAccessor::delete_known_cold_row,3 diff --git a/docs/rfcs/0026-engine-owned-mandatory-background-runtime.md b/docs/rfcs/0026-engine-owned-mandatory-background-runtime.md index 389eceeb..d2d195aa 100644 --- a/docs/rfcs/0026-engine-owned-mandatory-background-runtime.md +++ b/docs/rfcs/0026-engine-owned-mandatory-background-runtime.md @@ -1238,10 +1238,10 @@ focused validation. completion. - Non-goals: Do not migrate standalone wait/diagnostic APIs, change checkpoint/redo policy or formats, or parallelize recovery/checkpoint. - - Task Doc: `docs/tasks/TBD.md` - - Task Issue: `#0` - - Phase Status: `pending` - - Implementation Summary: `pending` + - Task Doc: `docs/tasks/000251-runtime-owned-mandatory-maintenance.md` + - Task Issue: `#928` + - Phase Status: done + - Implementation Summary: Implemented all six effectful maintenance roots as caller-prepared, mandatory-runtime-owned operations while preserving observer-only waits and finite read-only observations. [Task Resolve Sync: docs/tasks/000251-runtime-owned-mandatory-maintenance.md @ 2026-08-03] - **Phase 5: Lifecycle, Fairness, And Evolution Readiness** - Scope: Remove superseded foreground-handoff transitions and queue paths; diff --git a/docs/tasks/000251-runtime-owned-mandatory-maintenance.md b/docs/tasks/000251-runtime-owned-mandatory-maintenance.md new file mode 100644 index 00000000..674c6c3d --- /dev/null +++ b/docs/tasks/000251-runtime-owned-mandatory-maintenance.md @@ -0,0 +1,283 @@ +--- +id: 000251 +title: Runtime-Owned Mandatory Maintenance +status: implemented # proposal | implemented | superseded +created: 2026-08-02 +github_issue: 928 +--- + +# Task: Runtime-Owned Mandatory Maintenance + +## Summary + +Implemented RFC-0026 Phase 4 by moving every accepted effectful public +maintenance attempt from the caller executor to the engine-owned mandatory +runtime. The migrated roots are table freeze, one-shot table checkpoint, +catalog checkpoint, combined catalog-checkpoint-plus-redo-truncation, +standalone redo truncation, and secondary MemIndex cleanup. + +Each operation now completes authoritative, drop-cancellable preparation before +requesting mandatory capacity. Acceptance synchronously transfers the exact +session operation, logical locks, table/workflow authority, catalog and redo +gates, and operation resources to the runtime. Dropping the public future or +completion observer after acceptance does not cancel work or release its +resources early. + +Checkpoint retry orchestration now runs as separate mandatory attempts with an +operation-free observer wait between them. Finite read-only observations, +standalone progress waits, table listing, and statistics remain caller-owned +and cancellable. + +## Context + +`Issue Labels:` +`- type:task` +`- priority:medium` +`- codex` + +`Parent RFC:` +`- docs/rfcs/0026-engine-owned-mandatory-background-runtime.md` + +This task completed Phase 4 after: + +- task `000248` introduced mandatory runtime admission, supervision, typed + completion, and concurrent cleanup; +- task `000249` migrated table DDL and established lifetime-free prepared + logical-lock authority; +- task `000250` migrated index DDL and established transferable metadata gates + plus executor-neutral test control. + +Before this task, effectful maintenance retained a foreground +`SessionOperationPin` while performing IO, publication, private transactions, +redo cleanup, and retry waits. Borrowed table and maintenance-gate guards could +not cross the mandatory runtime's `Send + 'static` boundary. Accepted work +could therefore still depend on the caller executor, and checkpoint retries +retained one operation across an indefinite wait. + +The durable constraints were: + +- caller preparation must remain cancellable before acceptance; +- accepted execution must own every effect and never reacquire an operation + lock; +- workflow compensation and existing fatal publication boundaries remain + authoritative; +- catalog authority is acquired before redo-retention authority and released + before unlink, while redo authority remains held through cleanup; +- public APIs, error taxonomy, recovery behavior, and on-disk formats remain + unchanged. + +## Goals + +1. Transfer each effectful maintenance attempt from + `Voluntary(None)` to `Mandatory(None)` only after complete preparation. +2. Acquire table metadata `S` then data `IS` before mandatory capacity. +3. Retain the exact current-live table and release it before its logical locks. +4. Make freeze/checkpoint workflow attempts and root-mutation authority + lifetime-free and cancellation-safe. +5. Preserve exact frozen-batch restoration on pre-publication cancellation. +6. Keep checkpoint publication admission and irreversible error policy inside + accepted execution. +7. Prepare catalog checkpoint authority before redo-retention authority. +8. Release catalog authority after root/marker publication and retain redo + authority through obsolete-file cleanup. +9. Keep MemIndex cleanup's active private transaction in supervised accepted + resources and settle it before retry or finish. +10. Make observer drop execution-inert for every migrated operation. +11. Release domain resources, logical locks, and the outer operation in that + order on normal completion. +12. Retain unsafe nested state and poison the engine after an unexpected + accepted-execution panic. +13. Run each delayed checkpoint retry as a fresh operation with no permit or + table owner retained during the wait. +14. Keep non-effectful diagnostics and standalone waits caller-owned. + +## Non-Goals + +1. No checkpoint, catalog, redo-retention, MemIndex, MVCC, or recovery + algorithm change. +2. No public method signature, outcome type, error taxonomy, file format, or + configuration change. +3. No generic lock plan, maintenance command registry, task group, priority + lane, adaptive runtime, or dedicated maintenance pool. +4. No parallelization of an individual checkpoint, cleanup, scan, plan, or + unlink workflow. +5. No fallible rollback, compensation, or storage cleanup from `Drop`, + `finish`, or panic-policy callbacks. +6. No migration of `total_row_pages`, progress waits, table listing, or + statistics to the mandatory runtime. +7. No mandatory permit retained across checkpoint retry waits. +8. No RFC-0026 Phase 5 stress, benchmark, observability, scheduling-policy, or + superseded-RFC cleanup work. + +## Plan + +### Maintenance ownership and admission + +`PreparedMaintenanceLocks` owns operation-scoped logical locks through +`OwnerLockState` and a retained lock-manager guard. +`PreparedMaintenanceScope` owns optional table locks followed by the voluntary +operation pin. Cancellation therefore releases locks before publishing the +foreground terminal edge. + +`AcceptedMaintenanceScope` owns the transferred +`MandatoryOperationGuard`, prepared locks, and an +`Executing`/`TerminalReady`/`FailedRetained` finish state. Successful +execution proves the nested transaction slot returned to `Mandatory(None)`, +releases locks, and then publishes the outer terminal state. Panic handling +retains unsafe mandatory state before engine poison is published. + +The shared `MaintenanceExecutionSpec` supplies each operation's output, +named resource structure, panic label, and execute body. +`PreparedMaintenanceExecution` and `AcceptedMaintenanceExecution` implement +the common prepared/accepted handoff, resource release, finish, and panic +policy once. Operation resources are declared before the maintenance scope so +they drop before logical locks and the outer session operation. + +### Table freeze and checkpoint + +Table preparation acquires metadata `S` then data `IS`, resolves the +authoritative live `Arc`, claims the reversible workflow attempt, and +acquires `TableCheckpointRootMutationScope` before capacity admission. + +`Table::begin_freeze` and `Table::begin_checkpoint` return lifetime-free +attempts. The checkpoint workflow uses one shared admission state machine for +borrowed test/internal attempts and owned production attempts. Both attempt +forms share restoration logic, including returning the exact +`FrozenPageBatch` when an admitted checkpoint is dropped before publication. + +Accepted freeze owns page selection, page-state publication, and fence +allocation. Accepted checkpoint owns analysis, page transition, table-root or +silent-watermark publication, system transaction enqueue, compensation, and +the existing reversible-to-fatal boundary. + +`checkpoint_table_with_wait` calls one mandatory checkpoint attempt at a time. +A delayed result reaches terminal and releases its permit, table, workflow +attempt, root authority, and locks before the caller obtains a detached retry +observation. A later retry receives a new operation key and fresh authority. + +### Catalog checkpoint and redo retention + +`CatalogCheckpointScope` retains lifetime-free catalog checkpoint admission. +`RedoRetentionScope` retains a transaction-system guard and the shared +`ExclusiveGate` admission used for retained-redo observations. Production +preparation always acquires catalog authority first and redo authority second. + +Catalog checkpoint, standalone truncation, and combined maintenance receive +both scopes as named accepted resources. Checkpoint execution no longer +acquires either gate internally. Standalone and combined truncation release +catalog authority through a callback after root or marker publication, while +redo authority remains held through unlink accounting. + +The combined operation preserves projected silent-watermark and dropped-table +floor planning, catalog-safe segment proof, marker-only and combined-root +publication, purge requests, and retryable best-effort unlink results. + +### Secondary MemIndex cleanup + +Accepted cleanup stores its optional active private transaction outside the +panic-caught future and records a phase-specific panic label. Each iteration +starts a fresh private transaction, captures one proof-bound table root, and +rolls the transaction back before returning or retrying. + +The fresh-STS retry loop is intentionally unbounded: transaction starts and +root publication fences use the same monotonic timestamp source, so a retry +observes the raced root unless another publication wins again. Awaited +rollback prevents the loop from becoming a tight busy loop. + +### Supervision and test control + +All migrated fault and phase controls are engine-scoped and thread-neutral. +Accepted resources stay outside the caught future so supervisor panic policy +can restore reversible workflow state, retain unsafe nested ownership, publish +engine poison, complete or detach the observer safely, and release the runtime +permit exactly once. + +Global maintenance health rechecks occur after both gates are acquired but +after they have been packaged into the prepared carrier. An early fatal result +therefore releases catalog authority before redo authority and settles the +still-voluntary operation without crossing the mandatory acceptance boundary. + +## Implementation Notes + +Implemented all six effectful maintenance roots as caller-prepared, mandatory-runtime-owned operations while preserving observer-only waits and finite read-only observations. + +- The implementation consolidated the originally planned operation-specific + prepared/accepted carrier pairs into one generic carrier driven by + `MaintenanceExecutionSpec`. Named resource structs preserve domain meaning + and deterministic drop order; MemIndex cleanup uses a dedicated panic-label + newtype. +- A reusable `ExclusiveGate` replaced redo-retention-specific duplicate gate + state while preserving its precheck, fairness, and RAII release behavior. +- Freeze and checkpoint preparation moved to inherent `Table` methods. + Borrowed and owned checkpoint attempts share one admission state machine and + one restoration helper. +- Production-only wrapper layers and obsolete entry points were removed after + the owned paths became authoritative. Test-only call chains now use the same + production primitives. +- Review verified that pre-acceptance failures remain voluntary rather than + being forced through mandatory ownership. Complete global preparation is + packaged before the post-wait health check so gate release order remains + catalog then redo. +- The MemIndex cleanup contract, fresh-STS retry proof, freeze-fence ordering, + selected-page panic invariant, and catalog/redo gate rationale were retained + as inline documentation after refactoring. +- A dropped-table purge regression test now waits for the asynchronous + operational-state predicate before asserting retained state is empty, + removing a CI scheduling race without changing purge behavior. +- Architecture, checkpoint, transaction-system, lock-system, engine-lifetime, + public-error, and unsafe-usage documentation were synchronized with the + runtime-owned maintenance boundary. +- Final verification passed the branch-diff style audit across 22 Rust files, + strict workspace clippy, 1,629 standard workspace tests, and 1,536 + `libaio` tests. No nextest policy was changed. + +No implementation work was deferred from this task. + +## Impacts + +- `Session` effectful maintenance now pays one mandatory-capacity admission and + executor scheduling hop after preparation. +- Prepared operations may retain logical locks or maintenance gates while + waiting for bounded runtime capacity; accepted execution never waits for + those operation-level authorities. +- Table checkpoint workflow, lifecycle, persistence, page transition, and + MemIndex cleanup now expose lifetime-free preparation suitable for transfer. +- Catalog checkpoint and redo truncation use owned scopes with explicit + catalog-before-redo acquisition and early catalog release. +- Mandatory runtime supervision now covers maintenance private transactions, + publication, compensation, marker update, and unlink completion. +- Public APIs, public outcomes, errors, storage formats, recovery semantics, + dependencies, and configuration remain compatible. + +## Test Cases + +1. Unpolled and partially prepared futures create no effects and release every + observed logical lock, workflow claim, gate, and operation on drop. +2. Capacity saturation proves all operation authority is prepared before a + permit and acceptance transfers it without a release/reacquire gap. +3. Observer-drop tests prove accepted freeze, checkpoint, catalog/redo, and + MemIndex cleanup continue to their normal terminal outcome. +4. Freeze/checkpoint cancellation restores idle or the exact frozen batch; + delayed retry waits retain no operation, permit, table, or workflow owner. +5. Table checkpoint covers active-root and frozen-page delays, changed-root and + silent-watermark publication, system commit, and post-publication poison. +6. Catalog checkpoint covers publish/no-op, durable scan bounds, retention + progress, purge requests, gate serialization, and poison after gate wait. +7. Standalone and combined redo maintenance cover projected floors, marker + advancement, blockers, missing files, retryable unlink failure, early + catalog release, and redo exclusion through cleanup. +8. MemIndex cleanup covers root races, fresh transaction IDs, live delays, + delete-overlay cleanup, per-index statistics, rollback precedence, observer + drop, and panic retention. +9. Shutdown diagnostics distinguish voluntary preparation from accepted + mandatory work and drain observer-dropped execution. +10. Existing DDL, transaction cleanup, table drop, recovery, catalog retention, + and redo truncation suites remain passing. +11. The standard workspace suite passes with 1,629 tests. +12. The alternate `libaio` suite passes with 1,536 tests. + +## Open Questions + +No unresolved questions or deferred follow-ups remain in this task. RFC-0026 +Phase 5 may now rely on complete production migration of DDL and effectful +maintenance to caller preparation plus atomic mandatory submission. diff --git a/docs/tasks/next-id b/docs/tasks/next-id index fd99cd95..5d649a9e 100644 --- a/docs/tasks/next-id +++ b/docs/tasks/next-id @@ -1 +1 @@ -000251 +000252 diff --git a/docs/transaction-system.md b/docs/transaction-system.md index ba9b68cb..b49ecd88 100644 --- a/docs/transaction-system.md +++ b/docs/transaction-system.md @@ -263,15 +263,16 @@ abandonment, and claim the same entry and core through rolls back inline; it records cleanup intent on the exact entry and queues transaction-system cleanup when the engine is still reachable. -DDL and 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, `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 -`Voluntary(None)` only while checked out. +DDL and effectful maintenance start private transactions through their +already-reserved operation authority. A private transaction allocates a new +`TrxID` and boxed core but inherits the outer operation key, installs that box +in the same entry mutex, and does not replace the active slot. During caller +preparation the entry remains `Voluntary(None)`; accepted DDL and maintenance +transfer it to `Mandatory(None)` before starting a child. While mandatory +execution owns that child, `Mandatory(Some(InternalTrxState))` records its +available, checked-out, cleanup, or completion position. Public transactions +use the outer operation states directly and therefore use `Voluntary(None)` +only while checked out. Accepted table and index DDL transfer the same entry to `Mandatory(None)` before the runtime task is detached. Their nested catalog transaction follows @@ -285,12 +286,13 @@ 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. -Maintenance retains the voluntary private-transaction path. Its 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. +Accepted maintenance uses the same mandatory child transitions as accepted +DDL. Secondary `MemIndex` cleanup installs its private transaction into the +stable entry before any hook, scan, or await. A root-capture race settles that +child completely back to `Mandatory(None)` before a retry installs a fresh +`TrxID`. Normal completion releases the prepared maintenance resources before +publishing the outer terminal state; supervised unwind retains unsafe child +state in `FailedRetained`. After explicit rollback claims terminal ownership and publishes `RollingBack`, the claimed transaction core, undo buffers, locks, and session cleanup @@ -371,20 +373,18 @@ readers remain admitted. A transaction that already holds `TableData(IX)` can convert to `X` only when conversion is immediately compatible; otherwise the operation returns `LockUpgradeWouldBlock` before invoking the callback. -Finite session maintenance reserves one outer `Maintenance` operation and uses -that operation's exact lock owner for every scoped runtime admission: -`TableMetadata(S)` followed by `TableData(IS)`, then current live-runtime -resolution. Freeze, checkpoint, hot-row-page counting, secondary `MemIndex` -cleanup, and each bounded checkpoint-retry recheck keep this scope through -their last table/layout/index use. The table runtime owner is explicitly -released before fresh lock guards. These calls preserve ordinary `IX` DML and -explicit `S` table-reader concurrency while excluding same-table DROP and -serializing page freeze/transition against full-table mutation `X`. Grants -admitted by a covering explicit session lock are still recorded under a -distinct `Operation(operation_id)` owner. Returning from the scoped access -releases only that maintenance owner's fresh grants and preserves the exact -`SessionExplicit` claims. Retries reuse the same outer owner rather than -allocating another operation id. +Finite effectful session maintenance reserves one outer `Maintenance` +operation, acquires owned `TableMetadata(S)` followed by `TableData(IS)`, and +resolves the exact live runtime before mandatory admission. Freeze, checkpoint, +and secondary `MemIndex` cleanup transfer that complete scope into accepted +execution and retain it through their last table/layout/index use. Hot-row-page +counting remains a caller-owned, cancellable scoped observation. These calls +preserve ordinary `IX` DML and explicit `S` table-reader concurrency while +excluding same-table DROP and serializing page freeze/transition against +full-table mutation `X`. Grants admitted by a covering explicit session lock +are still recorded under a distinct `Operation(operation_id)` owner. Scope +release consumes only that maintenance owner's fresh grants and preserves the +exact `SessionExplicit` claims. Checkpoint retry never keeps that scope across its indefinite sleep. One recheck registers the relevant lifecycle, transaction-terminal, GC-horizon, @@ -392,7 +392,10 @@ poison, and shutdown listeners and then verifies the predicate again. It returns only detached listener state, releases checkpoint attempts, page 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. +listener carries that change into the next bounded recheck. The completed +checkpoint operation is not retained across this sleep: each retry starts a +new outer operation id and prepares new logical-lock, table, workflow, and +root-mutation authority. `CREATE TABLE` validates metadata before reservation, allocates a distinct gap-tolerant id, and caller-prepares target metadata X plus metadata-S/data-IX diff --git a/docs/unsafe-usage-baseline.md b/docs/unsafe-usage-baseline.md index 41509889..f9562e7b 100644 --- a/docs/unsafe-usage-baseline.md +++ b/docs/unsafe-usage-baseline.md @@ -8,7 +8,7 @@ | module | files | unsafe | transmute | new_unchecked | assume_init | // SAFETY: | |---|---:|---:|---:|---:|---:|---:| | buffer | 13 | 51 | 0 | 0 | 0 | 45 | -| latch | 4 | 40 | 0 | 0 | 0 | 36 | +| latch | 5 | 40 | 0 | 0 | 0 | 36 | | row | 3 | 6 | 0 | 0 | 0 | 6 | | index | 21 | 13 | 0 | 0 | 3 | 7 | | io | 6 | 21 | 0 | 0 | 1 | 18 | @@ -17,7 +17,7 @@ | file | 8 | 11 | 0 | 0 | 2 | 11 | | log | 6 | 0 | 0 | 0 | 0 | 0 | | recovery | 5 | 0 | 0 | 0 | 0 | 0 | -| **total** | **82** | **150** | **0** | **0** | **6** | **130** | +| **total** | **83** | **150** | **0** | **0** | **6** | **130** | ## File Hotspots (top 40) diff --git a/doradb-storage/src/catalog/checkpoint.rs b/doradb-storage/src/catalog/checkpoint.rs index 9a9eb75f..5269427d 100644 --- a/doradb-storage/src/catalog/checkpoint.rs +++ b/doradb-storage/src/catalog/checkpoint.rs @@ -4,14 +4,21 @@ use crate::catalog::{ is_user_table, }; use crate::error::{ - DataIntegrityError, DataIntegrityResult, FatalError, IoError, RuntimeError, - RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, + CompletionErrorBridge, CompletionResult, DataIntegrityError, DataIntegrityResult, FatalError, + IoError, RuntimeError, RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, }; use crate::id::{TableID, TrxID}; use crate::log::discover_redo_log_files; use crate::log::redo::{DDLRedo, RowRedoKind, TableDML}; use crate::obs; +use crate::quiescent::QuiescentGuard; use crate::recovery::stream::{CatalogSafeRedoSegment, RedoReplayPlanner}; +use crate::runtime::mandatory::PreparedExecution; +use crate::session::{ + AcceptedMaintenanceScope, MaintenanceExecutionSpec, PreparedMaintenanceExecution, + PreparedMaintenanceScope, +}; +use crate::trx::RedoRetentionScope; use crate::trx::sys::{CatalogRedoRetentionProgress, TransactionSystem}; use error_stack::{Report, ResultExt}; use event_listener::{Event, listener}; @@ -170,14 +177,14 @@ impl CatalogCheckpointGate { } } - /// Acquire a catalog checkpoint/marker-publish lease. + /// Acquire checkpoint admission without constructing a borrowed lease. /// - /// The lease waits until no catalog metadata change is active or pending, - /// and also serializes overlapping catalog checkpoints or redo marker - /// publishes. It does not protect the retained redo suffix itself; callers - /// that scan retained redo, publish a marker, or unlink obsolete files must - /// also hold the transaction-system redo-retention lease. - pub(crate) async fn begin_checkpoint(&self) -> CatalogCheckpointLease<'_> { + /// Admission waits until no catalog metadata change is active or pending + /// and serializes overlapping catalog checkpoints or redo marker + /// publishers. It does not protect the retained redo suffix itself; + /// retained-redo scans, marker publication, and obsolete-file cleanup must + /// also hold [`RedoRetentionScope`]. + async fn acquire_checkpoint(&self) { loop { { let mut state = self.state.lock(); @@ -185,7 +192,7 @@ impl CatalogCheckpointGate { && !state.checkpoint_active { state.checkpoint_active = true; - return CatalogCheckpointLease { gate: self }; + return; } } listener!(self.changed => listener); @@ -319,86 +326,108 @@ impl Drop for PendingCatalogMetadataChange<'_> { } } -/// RAII guard for one catalog checkpoint scan/apply section. +/// Lifetime-free catalog checkpoint exclusion scope. /// -/// While held, other catalog checkpoints and catalog metadata DDL wait on the -/// catalog checkpoint gate. -pub(crate) struct CatalogCheckpointLease<'a> { - gate: &'a CatalogCheckpointGate, +/// While held, other catalog checkpoints, redo marker publishers, and catalog +/// metadata DDL wait on the catalog checkpoint gate. +pub(crate) struct CatalogCheckpointScope { + catalog: QuiescentGuard, + active: bool, } -impl Drop for CatalogCheckpointLease<'_> { +impl CatalogCheckpointScope { + /// Acquire catalog checkpoint authority for mandatory preparation. + pub(crate) async fn acquire(catalog: QuiescentGuard) -> Self { + catalog.checkpoint_gate.acquire_checkpoint().await; + Self { + catalog, + active: true, + } + } + + /// Release catalog authority before redo-file cleanup. + #[inline] + pub(crate) fn release(&mut self) { + if self.active { + self.active = false; + self.catalog.checkpoint_gate.release_checkpoint(); + } + } +} + +impl Drop for CatalogCheckpointScope { #[inline] fn drop(&mut self) { - self.gate.release_checkpoint(); + self.release(); + } +} + +struct CatalogCheckpointResources { + _catalog_scope: CatalogCheckpointScope, + _redo_scope: RedoRetentionScope, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum CatalogCheckpointTxnAction { + Include, + Skip, + Stop(CatalogCheckpointScanStopReason), +} + +struct CatalogCheckpointExecution; + +impl MaintenanceExecutionSpec for CatalogCheckpointExecution { + type Output = CatalogCheckpointOutcome; + type Resources = CatalogCheckpointResources; + type PanicLabel = &'static str; + + const LABEL: &'static str = "checkpoint_catalog"; + + async fn execute( + scope: &mut AcceptedMaintenanceScope, + _resources: &mut Self::Resources, + _panic_label: &mut Self::PanicLabel, + ) -> CompletionResult { + let engine = scope.engine().clone(); + let result = engine + .catalog() + .checkpoint_prepared(&engine.trx_sys) + .await + .map_err(CompletionErrorBridge::capture_runtime_or_fatal); + scope.mark_terminal_ready(); + result } } +/// Prepare a catalog checkpoint with both exclusion scopes held. +pub(crate) fn prepare_catalog_checkpoint_operation( + catalog_scope: CatalogCheckpointScope, + redo_scope: RedoRetentionScope, + scope: PreparedMaintenanceScope, +) -> impl PreparedExecution { + PreparedMaintenanceExecution::::global( + scope, + CatalogCheckpointResources { + _catalog_scope: catalog_scope, + _redo_scope: redo_scope, + }, + "accepted catalog checkpoint panicked", + ) +} + impl Catalog { - /// Trigger one ad-hoc catalog checkpoint publish. - /// - /// Normal overlap with another catalog checkpoint, catalog metadata DDL, or - /// redo truncation waits through the catalog checkpoint gate and the - /// transaction-system redo-retention gate. + /// Execute one checkpoint with catalog and redo authority already held. /// - /// # Panics - /// - /// A panic indicates an internal invariant violation, such as bypassing the - /// required gates and reaching the shared `CatalogStorage`/`MultiTableFile` - /// with multiple mutable writers. - #[inline] - pub(crate) async fn checkpoint_now( + /// The caller must retain both exclusion scopes for the whole call so the + /// shared catalog root cannot acquire concurrent mutable writers and the + /// retained redo observation remains stable. + async fn checkpoint_prepared( &self, trx_sys: &TransactionSystem, ) -> RuntimeOrFatalResult { obs::info!("event=checkpoint_publish component=catalog action=start result=ok"); - async { - let _checkpoint_lease = self.checkpoint_gate.begin_checkpoint().await; - let _redo_retention_lease = trx_sys.begin_redo_retention().await; - let scan_cfg = trx_sys.catalog_checkpoint_scan_config()?; - let batch = self - .scan_checkpoint_batch(trx_sys.persisted_watermark_cts(), scan_cfg) - .await?; - let publishable_progress = batch.redo_retention_progress(); - match self.apply_checkpoint_batch(batch).await { - Ok(CatalogCheckpointOutcome::Published { - catalog_replay_start_ts, - }) => { - if let Some(progress) = publishable_progress { - debug_assert_eq!(progress.catalog_replay_start_ts, catalog_replay_start_ts); - trx_sys.record_catalog_redo_retention_progress(progress); - } - trx_sys.request_dropped_table_purge(); - Ok(CatalogCheckpointOutcome::Published { - catalog_replay_start_ts, - }) - } - Ok(CatalogCheckpointOutcome::Noop) => Ok(CatalogCheckpointOutcome::Noop), - Err(err) => { - let has_io_source = match &err { - RuntimeOrFatalError::Runtime(report) => { - report.downcast_ref::().is_some() - } - RuntimeOrFatalError::Fatal(report) => { - report.downcast_ref::().is_some() - } - }; - if !has_io_source { - return Err(err); - } - // Preserve the existing policy: any apply failure carrying - // an IO source is poisoned as a checkpoint-write failure. - // An already-Fatal source retains its original Fatal reason. - let report = err - .into_fatal_report(FatalError::CheckpointWrite) - .attach("catalog checkpoint publish IO failure"); - Err(RuntimeOrFatalError::from( - self.poisoner.poison(report).into_report(), - )) - } - } - } - .await + self.checkpoint_prepared_inner(trx_sys) + .await .inspect(|outcome| match outcome { CatalogCheckpointOutcome::Published { catalog_replay_start_ts, @@ -422,6 +451,54 @@ impl Catalog { }) } + async fn checkpoint_prepared_inner( + &self, + trx_sys: &TransactionSystem, + ) -> RuntimeOrFatalResult { + let scan_cfg = trx_sys.catalog_checkpoint_scan_config()?; + let batch = self + .scan_checkpoint_batch(trx_sys.persisted_watermark_cts(), scan_cfg) + .await?; + let publishable_progress = batch.redo_retention_progress(); + match self.apply_checkpoint_batch(batch).await { + Ok(CatalogCheckpointOutcome::Published { + catalog_replay_start_ts, + }) => { + if let Some(progress) = publishable_progress { + debug_assert_eq!(progress.catalog_replay_start_ts, catalog_replay_start_ts); + trx_sys.record_catalog_redo_retention_progress(progress); + } + trx_sys.request_dropped_table_purge(); + Ok(CatalogCheckpointOutcome::Published { + catalog_replay_start_ts, + }) + } + Ok(CatalogCheckpointOutcome::Noop) => Ok(CatalogCheckpointOutcome::Noop), + Err(err) => { + let has_io_source = match &err { + RuntimeOrFatalError::Runtime(report) => { + report.downcast_ref::().is_some() + } + RuntimeOrFatalError::Fatal(report) => { + report.downcast_ref::().is_some() + } + }; + if !has_io_source { + return Err(err); + } + // Preserve the existing policy: any apply failure carrying + // an IO source is poisoned as a checkpoint-write failure. + // An already-Fatal source retains its original Fatal reason. + let report = err + .into_fatal_report(FatalError::CheckpointWrite) + .attach("catalog checkpoint publish IO failure"); + Err(RuntimeOrFatalError::from( + self.poisoner.poison(report).into_report(), + )) + } + } + } + /// Scan persisted redo logs and collect one safe catalog checkpoint batch. /// /// The caller owns selecting the durable upper CTS and base redo scan @@ -637,13 +714,6 @@ impl Catalog { } } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum CatalogCheckpointTxnAction { - Include, - Skip, - Stop(CatalogCheckpointScanStopReason), -} - fn drop_table_has_catalog_table_delete( table_id: TableID, dml: &BTreeMap, @@ -779,7 +849,7 @@ mod tests { fn test_catalog_metadata_change_waits_for_active_checkpoint() { smol::block_on(async { let gate = CatalogCheckpointGate::new(); - let checkpoint_lease = gate.begin_checkpoint().await; + gate.acquire_checkpoint().await; let mut metadata_fut = Box::pin(gate.acquire_metadata_change()); assert!(matches!( @@ -787,16 +857,17 @@ mod tests { std::task::Poll::Pending )); - drop(checkpoint_lease); + gate.release_checkpoint(); metadata_fut.await; - let mut checkpoint_fut = Box::pin(gate.begin_checkpoint()); + let mut checkpoint_fut = Box::pin(gate.acquire_checkpoint()); assert!(matches!( futures::poll!(checkpoint_fut.as_mut()), std::task::Poll::Pending )); gate.release_metadata_change(); - let _checkpoint_lease = checkpoint_fut.await; + checkpoint_fut.await; + gate.release_checkpoint(); }); } @@ -805,7 +876,7 @@ mod tests { smol::block_on(async { let gate = CatalogCheckpointGate::new(); gate.acquire_metadata_change().await; - let mut checkpoint_fut = Box::pin(gate.begin_checkpoint()); + let mut checkpoint_fut = Box::pin(gate.acquire_checkpoint()); assert!(matches!( futures::poll!(checkpoint_fut.as_mut()), @@ -813,7 +884,8 @@ mod tests { )); gate.release_metadata_change(); - let _checkpoint_lease = checkpoint_fut.await; + checkpoint_fut.await; + gate.release_checkpoint(); }); } @@ -821,16 +893,17 @@ mod tests { fn test_catalog_checkpoint_waits_for_active_checkpoint() { smol::block_on(async { let gate = CatalogCheckpointGate::new(); - let checkpoint_lease = gate.begin_checkpoint().await; - let mut checkpoint_fut = Box::pin(gate.begin_checkpoint()); + gate.acquire_checkpoint().await; + let mut checkpoint_fut = Box::pin(gate.acquire_checkpoint()); assert!(matches!( futures::poll!(checkpoint_fut.as_mut()), std::task::Poll::Pending )); - drop(checkpoint_lease); - let _checkpoint_lease = checkpoint_fut.await; + gate.release_checkpoint(); + checkpoint_fut.await; + gate.release_checkpoint(); }); } @@ -838,20 +911,20 @@ mod tests { fn test_catalog_checkpoint_waits_behind_pending_metadata_change() { smol::block_on(async { let gate = CatalogCheckpointGate::new(); - let checkpoint_lease = gate.begin_checkpoint().await; + gate.acquire_checkpoint().await; let mut metadata_fut = Box::pin(gate.acquire_metadata_change()); assert!(matches!( futures::poll!(metadata_fut.as_mut()), std::task::Poll::Pending )); - let mut checkpoint_fut = Box::pin(gate.begin_checkpoint()); + let mut checkpoint_fut = Box::pin(gate.acquire_checkpoint()); assert!(matches!( futures::poll!(checkpoint_fut.as_mut()), std::task::Poll::Pending )); - drop(checkpoint_lease); + gate.release_checkpoint(); metadata_fut.await; assert!(matches!( futures::poll!(checkpoint_fut.as_mut()), @@ -859,7 +932,8 @@ mod tests { )); gate.release_metadata_change(); - let _checkpoint_lease = checkpoint_fut.await; + checkpoint_fut.await; + gate.release_checkpoint(); }); } @@ -867,7 +941,7 @@ mod tests { fn test_catalog_pending_metadata_change_cancellation_reopens_checkpoint() { smol::block_on(async { let gate = CatalogCheckpointGate::new(); - let checkpoint_lease = gate.begin_checkpoint().await; + gate.acquire_checkpoint().await; let mut metadata_fut = Box::pin(gate.acquire_metadata_change()); assert!(matches!( @@ -876,8 +950,9 @@ mod tests { )); drop(metadata_fut); - drop(checkpoint_lease); - let _checkpoint_lease = gate.begin_checkpoint().await; + gate.release_checkpoint(); + gate.acquire_checkpoint().await; + gate.release_checkpoint(); }); } @@ -962,7 +1037,7 @@ mod tests { fn test_catalog_second_metadata_change_completes_after_pending_waiter_cancelled() { smol::block_on(async { let gate = CatalogCheckpointGate::new(); - let checkpoint_lease = gate.begin_checkpoint().await; + gate.acquire_checkpoint().await; let mut pending_owner = Box::pin(gate.acquire_metadata_change()); let mut second_waiter = Box::pin(gate.acquire_metadata_change()); @@ -976,7 +1051,7 @@ mod tests { )); drop(pending_owner); - drop(checkpoint_lease); + gate.release_checkpoint(); let waiter = async { second_waiter.await; diff --git a/doradb-storage/src/catalog/index.rs b/doradb-storage/src/catalog/index.rs index c04bf61a..10dafb19 100644 --- a/doradb-storage/src/catalog/index.rs +++ b/doradb-storage/src/catalog/index.rs @@ -3530,8 +3530,9 @@ pub(crate) mod tests { assert_eq!(catalog_indexes.len(), 1); assert_eq!(catalog_indexes[0].index_no, 0); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); drop(session); diff --git a/doradb-storage/src/catalog/mod.rs b/doradb-storage/src/catalog/mod.rs index 6965bcd4..79f38e76 100644 --- a/doradb-storage/src/catalog/mod.rs +++ b/doradb-storage/src/catalog/mod.rs @@ -601,12 +601,6 @@ impl Catalog { ) } - /// Acquires the catalog checkpoint side of the catalog metadata gate. - #[inline] - pub(crate) async fn begin_checkpoint(&self) -> CatalogCheckpointLease<'_> { - self.checkpoint_gate.begin_checkpoint().await - } - /// Acquires transferable index-DDL metadata admission. #[inline] pub(crate) async fn acquire_index_metadata_change(&self) { @@ -1304,6 +1298,31 @@ pub(crate) mod tests { assert_dropped_table_floor(engine.catalog(), table_id); } + /// Waits for targeted purge completion after dropped-table file cleanup becomes eligible. + pub(crate) async fn wait_for_no_dropped_table_operational_state( + engine: &Engine, + table_id: TableID, + ) { + let (event_tx, event_rx) = flume::unbounded(); + engine.inner().trx_sys.set_purge_test_observer(event_tx); + while engine + .catalog() + .retained_dropped_table_ids_now() + .contains(&table_id) + { + engine.inner().trx_sys.request_dropped_table_purge(); + let mut dropped_table_started = false; + loop { + match event_rx.recv_async().await.unwrap() { + PurgeTestEvent::DroppedTableStarted => dropped_table_started = true, + PurgeTestEvent::CycleCompleted if dropped_table_started => break, + _ => {} + } + } + } + assert_no_dropped_table_operational_state(engine.catalog(), table_id); + } + #[inline] pub(crate) fn assert_no_dropped_table_operational_state(catalog: &Catalog, table_id: TableID) { assert!(!catalog.retained_dropped_table_ids_now().contains(&table_id)); @@ -1785,8 +1804,9 @@ pub(crate) mod tests { ); drop(table); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); drop(engine); @@ -1834,7 +1854,7 @@ pub(crate) mod tests { } #[test] - fn test_catalog_checkpoint_now_publish_and_noop() { + fn test_session_catalog_checkpoint_publish_and_noop() { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let main_dir = temp_dir.path().to_path_buf(); @@ -1853,8 +1873,9 @@ pub(crate) mod tests { let _ = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let snap1 = engine.catalog().storage.checkpoint_snapshot(); @@ -1879,8 +1900,9 @@ pub(crate) mod tests { ); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let snap2 = engine.catalog().storage.checkpoint_snapshot(); @@ -1903,8 +1925,9 @@ pub(crate) mod tests { let _ = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -1964,8 +1987,9 @@ pub(crate) mod tests { let _ = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2015,7 +2039,7 @@ pub(crate) mod tests { } #[test] - fn test_catalog_checkpoint_now_heartbeat_without_catalog_ops() { + fn test_session_catalog_checkpoint_heartbeat_without_catalog_ops() { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let main_dir = temp_dir.path().to_path_buf(); @@ -2025,8 +2049,9 @@ pub(crate) mod tests { let table_id = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let snap1 = engine.catalog().storage.checkpoint_snapshot(); @@ -2044,8 +2069,9 @@ pub(crate) mod tests { trx.commit().await.unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let snap2 = engine.catalog().storage.checkpoint_snapshot(); @@ -2112,7 +2138,7 @@ pub(crate) mod tests { } #[test] - fn test_catalog_checkpoint_now_heartbeat_with_mixed_user_table_checkpoint_states() { + fn test_session_catalog_checkpoint_heartbeat_with_mixed_user_table_checkpoint_states() { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let main_dir = temp_dir.path().to_path_buf(); @@ -2125,8 +2151,9 @@ pub(crate) mod tests { let replay_only_table_id = table2(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let snap1 = engine.catalog().storage.checkpoint_snapshot(); @@ -2207,8 +2234,9 @@ pub(crate) mod tests { ); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let snap2 = engine.catalog().storage.checkpoint_snapshot(); diff --git a/doradb-storage/src/catalog/storage/mod.rs b/doradb-storage/src/catalog/storage/mod.rs index c90868b9..e4d46190 100644 --- a/doradb-storage/src/catalog/storage/mod.rs +++ b/doradb-storage/src/catalog/storage/mod.rs @@ -1961,8 +1961,9 @@ pub(crate) mod tests { let _ = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -1998,8 +1999,9 @@ pub(crate) mod tests { let _ = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2212,8 +2214,9 @@ pub(crate) mod tests { let table_id = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2348,8 +2351,9 @@ pub(crate) mod tests { let _ = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2394,8 +2398,9 @@ pub(crate) mod tests { let _ = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2449,8 +2454,9 @@ pub(crate) mod tests { let table1_id = table1(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2461,8 +2467,9 @@ pub(crate) mod tests { let table2_id = table2(&engine).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); diff --git a/doradb-storage/src/catalog/table.rs b/doradb-storage/src/catalog/table.rs index 924e6392..83c7ff54 100644 --- a/doradb-storage/src/catalog/table.rs +++ b/doradb-storage/src/catalog/table.rs @@ -2167,6 +2167,7 @@ pub(crate) mod tests { use crate::catalog::tests::{ assert_dropped_table_floor, assert_dropped_table_runtime, assert_no_dropped_table_operational_state, wait_for_dropped_table_floor, + wait_for_no_dropped_table_operational_state, }; use crate::catalog::{ CatalogCheckpointScanStopReason, ColumnAttributes, ColumnSpec, CurrentTableState, @@ -4821,11 +4822,13 @@ pub(crate) mod tests { assert!(Path::new(&table_file_path).exists()); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); - wait_path_exists(&table_file_path, false).await; + wait_for_no_dropped_table_operational_state(&engine, table_id).await; + assert!(!Path::new(&table_file_path).exists()); assert!(engine.catalog().retained_dropped_table_ids_now().is_empty()); assert_no_dropped_table_operational_state(engine.catalog(), table_id); assert!( @@ -5071,8 +5074,9 @@ pub(crate) mod tests { session.drop_table(table_id).await.unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); assert!( diff --git a/doradb-storage/src/engine.rs b/doradb-storage/src/engine.rs index 4ce7f78b..7831f5a6 100644 --- a/doradb-storage/src/engine.rs +++ b/doradb-storage/src/engine.rs @@ -29,6 +29,8 @@ use crate::root::{StorageRootLease, StorageRootLeaseAttempt}; use crate::runtime::block_on; use crate::runtime::mandatory::{MandatoryRuntime, MandatoryRuntimeWorkers}; use crate::session::{Session, SessionRegistry}; +#[cfg(test)] +use crate::table::tests::MaintenanceTestController; use crate::trx::sys::{TransactionPurgeWorkers, TransactionRedoWorkers, TransactionSystem}; use crate::{DiskPool, IndexPool, MemPool, MetaPool}; use error_stack::{Report, ResultExt}; @@ -711,6 +713,9 @@ pub(crate) struct EngineInner { /// Per-engine index-DDL fault and phase controller. #[cfg(test)] pub(crate) index_ddl_test: IndexDdlTestController, + /// Per-engine maintenance fault and phase controller. + #[cfg(test)] + pub(crate) maintenance_test: MaintenanceTestController, lifecycle: EngineLifecycle, } @@ -941,6 +946,8 @@ async fn bootstrap_inner(config: EngineConfig) -> Result { table_ddl_test: TableDdlTestController::default(), #[cfg(test)] index_ddl_test: IndexDdlTestController::default(), + #[cfg(test)] + maintenance_test: MaintenanceTestController::default(), lifecycle: EngineLifecycle::new(), }; Ok(Engine { diff --git a/doradb-storage/src/latch/gate.rs b/doradb-storage/src/latch/gate.rs new file mode 100644 index 00000000..827daaed --- /dev/null +++ b/doradb-storage/src/latch/gate.rs @@ -0,0 +1,109 @@ +use event_listener::{Event, listener}; +use parking_lot::Mutex; + +/// Async exclusive admission with explicit release. +/// +/// Unlike a mutex guard, acquisition returns no borrowed token. Callers must +/// pair every successful [`Self::acquire`] with exactly one [`Self::release`]. +/// This permits a higher-level owned scope to transfer the admission across +/// runtime threads while retaining its own resource-lifetime proof. +pub(crate) struct ExclusiveGate { + active: Mutex, + changed: Event, +} + +impl ExclusiveGate { + /// Create an inactive gate. + #[inline] + pub(crate) fn new() -> Self { + Self { + active: Mutex::new(false), + changed: Event::new(), + } + } + + /// Wait for and acquire exclusive admission. + /// + /// Cancelling while this future is pending does not change gate state. + /// Once this method returns, the caller owns admission and must release it. + pub(crate) async fn acquire(&self) { + if self.try_acquire() { + return; + } + loop { + listener!(self.changed => changed); + if self.try_acquire() { + return; + } + changed.await; + } + } + + #[inline] + fn try_acquire(&self) -> bool { + let mut active = self.active.lock(); + if *active { + return false; + } + *active = true; + true + } + + /// Release one successful acquisition and wake current waiters. + #[inline] + pub(crate) fn release(&self) { + let mut active = self.active.lock(); + assert!(*active, "exclusive gate release without active acquisition"); + *active = false; + drop(active); + self.changed.notify(usize::MAX); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_exclusive_gate_serializes_acquisitions() { + smol::block_on(async { + let gate = ExclusiveGate::new(); + gate.acquire().await; + let mut waiter = Box::pin(gate.acquire()); + + assert!(matches!( + futures::poll!(waiter.as_mut()), + std::task::Poll::Pending + )); + + gate.release(); + waiter.await; + gate.release(); + }); + } + + #[test] + fn test_exclusive_gate_pending_cancellation_does_not_consume_admission() { + smol::block_on(async { + let gate = ExclusiveGate::new(); + gate.acquire().await; + let mut cancelled = Box::pin(gate.acquire()); + + assert!(matches!( + futures::poll!(cancelled.as_mut()), + std::task::Poll::Pending + )); + + drop(cancelled); + gate.release(); + gate.acquire().await; + gate.release(); + }); + } + + #[test] + #[should_panic(expected = "exclusive gate release without active acquisition")] + fn test_exclusive_gate_rejects_unmatched_release() { + ExclusiveGate::new().release(); + } +} diff --git a/doradb-storage/src/latch/mod.rs b/doradb-storage/src/latch/mod.rs index 51dc03b2..ade80392 100644 --- a/doradb-storage/src/latch/mod.rs +++ b/doradb-storage/src/latch/mod.rs @@ -1,6 +1,8 @@ +mod gate; mod hybrid; mod mutex; mod rwlock; +pub(crate) use gate::ExclusiveGate; pub(crate) use hybrid::RawHybridGuard; pub(crate) use hybrid::*; diff --git a/doradb-storage/src/recovery/mod.rs b/doradb-storage/src/recovery/mod.rs index d5e88a09..36e78e2f 100644 --- a/doradb-storage/src/recovery/mod.rs +++ b/doradb-storage/src/recovery/mod.rs @@ -1486,8 +1486,9 @@ mod tests { durability_trx.commit().await.unwrap(); drop(session); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); publish_first_redo_log_seq_for_test(&engine.catalog().storage, 1) @@ -1505,8 +1506,9 @@ mod tests { session.drop_table(table_id).await.unwrap(); drop(session); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let replay_floor = engine @@ -2148,8 +2150,9 @@ mod tests { let table_id = create_index_ddl_base_table(&engine, vec![base_unique_index_spec()]).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2181,8 +2184,9 @@ mod tests { let table_id = create_index_ddl_base_table(&engine, vec![base_unique_index_spec()]).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2218,8 +2222,9 @@ mod tests { ) .await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2252,8 +2257,9 @@ mod tests { let table_id = create_index_ddl_base_table(&engine, vec![base_unique_index_spec()]).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2288,8 +2294,9 @@ mod tests { .await .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2323,8 +2330,9 @@ mod tests { .await .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -2363,15 +2371,17 @@ mod tests { let table_id = create_index_ddl_base_table(&engine, vec![base_unique_index_spec()]).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let ddl_cts = commit_create_index_catalog_ddl(&engine, table_id).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let snapshot = engine.catalog().storage.checkpoint_snapshot(); @@ -2403,16 +2413,18 @@ mod tests { let table_id = create_index_ddl_base_table(&engine, vec![base_unique_index_spec()]).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let ddl_cts = commit_create_index_catalog_ddl(&engine, table_id).await; publish_index_metadata_root(&engine, table_id, created_index_metadata(), ddl_cts).await; engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let snapshot = engine.catalog().storage.checkpoint_snapshot(); @@ -2886,8 +2898,9 @@ mod tests { .await .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let snap = engine.catalog().storage.checkpoint_snapshot(); @@ -3073,8 +3086,9 @@ mod tests { .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let catalog_replay_start_ts = engine @@ -3202,8 +3216,9 @@ mod tests { .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -3339,8 +3354,9 @@ mod tests { .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -3482,8 +3498,9 @@ mod tests { .await .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -3623,8 +3640,9 @@ mod tests { .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let catalog_replay_start_ts = engine @@ -3747,8 +3765,9 @@ mod tests { .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -3909,8 +3928,9 @@ mod tests { .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let baseline_catalog_replay_start_ts = engine @@ -3984,8 +4004,9 @@ mod tests { ); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); let final_catalog_replay_start_ts = engine @@ -4102,8 +4123,9 @@ mod tests { .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); @@ -4226,8 +4248,9 @@ mod tests { .unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); diff --git a/doradb-storage/src/runtime/mandatory.rs b/doradb-storage/src/runtime/mandatory.rs index 1303c436..234b1c67 100644 --- a/doradb-storage/src/runtime/mandatory.rs +++ b/doradb-storage/src/runtime/mandatory.rs @@ -54,13 +54,6 @@ pub(crate) struct MandatoryTaskMetadata { impl MandatoryTaskMetadata { /// Build caller-operation metadata. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Phase 1 proves caller operation adapters synthetically" - ) - )] #[inline] pub(crate) const fn operation( label: &'static str, diff --git a/doradb-storage/src/session.rs b/doradb-storage/src/session.rs index 93d51b23..91738c09 100644 --- a/doradb-storage/src/session.rs +++ b/doradb-storage/src/session.rs @@ -1,17 +1,18 @@ use crate::buffer::page::VersionedPageID; use crate::buffer::{BufferPool, PoolGuards}; use crate::catalog::{ - CatalogCheckpointOutcome, CreateIndexPlan, DropIndexPlan, DropTablePlan, IndexDdlGateScope, - IndexNo, IndexSpec, PreparedCreateIndex, PreparedCreateTable, PreparedDropIndex, - PreparedDropTable, TableSpec, ValidatedCreateTable, create_index_catalog_write_targets, - create_table_catalog_write_targets, drop_index_catalog_write_targets, - drop_table_catalog_write_targets, reject_non_user_table_id, + CatalogCheckpointOutcome, CatalogCheckpointScope, CreateIndexPlan, DropIndexPlan, + DropTablePlan, IndexDdlGateScope, IndexNo, IndexSpec, PreparedCreateIndex, PreparedCreateTable, + PreparedDropIndex, PreparedDropTable, TableSpec, ValidatedCreateTable, + create_index_catalog_write_targets, create_table_catalog_write_targets, + drop_index_catalog_write_targets, drop_table_catalog_write_targets, + prepare_catalog_checkpoint_operation, reject_non_user_table_id, reject_user_table_primary_key_index, validated_index_ddl_target, }; use crate::engine::{EngineInner, EngineRef, WeakEngineRef}; use crate::error::{ - DiscloseError, DiscloseResultExt, FatalError, LifecycleError, LifecycleResult, - MultiDomainResultExt, OperationError, OperationResult, Result, + CompletionErrorBridge, CompletionResult, DiscloseError, DiscloseResultExt, FatalError, + LifecycleError, LifecycleResult, MultiDomainResultExt, OperationError, OperationResult, Result, }; use crate::id::{OperationID, SessionID, SessionOperationKey, TableID, TrxID}; use crate::lock::{ @@ -20,23 +21,30 @@ use crate::lock::{ use crate::map::{FastDashMap, FastHashMap}; use crate::notify::EventNotifyOnDrop; use crate::quiescent::QuiescentGuard; +use crate::runtime::mandatory::{AcceptedExecution, MandatoryTaskMetadata, PreparedExecution}; use crate::stats::{ BufferPoolStats, StorageIoStats, TransactionSystemStats, buffer_pool_runtime_stats_snapshot, storage_io_stats_snapshot, transaction_system_stats_snapshot, }; use crate::table::{ CheckpointDelayReason, CheckpointOutcome, CheckpointRetryObservation, FreezeOutcome, - MemIndexCleanupOutcome, Table, + MemIndexCleanupOutcome, Table, prepare_checkpoint_table_operation, + prepare_freeze_table_operation, prepare_mem_index_cleanup_operation, }; use crate::trx::{ - PreparedCatalogWriteAuthority, ReleasedTransactionLocks, SessionOperationEntry, - SessionOperationKind, SessionOperationState, Transaction, TrxInner, + PreparedCatalogWriteAuthority, RedoRetentionScope, ReleasedTransactionLocks, + SessionOperationEntry, SessionOperationKind, SessionOperationState, Transaction, TrxInner, + prepare_catalog_redo_maintenance_operation, prepare_redo_truncation_operation, }; use error_stack::{Report, ResultExt}; use event_listener::EventListener; use futures::future::select_all; use parking_lot::Mutex; +use std::any::Any; use std::cell::Cell; +use std::fmt::Display; +use std::future::Future; +use std::marker::PhantomData; use std::mem::replace; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Weak}; @@ -375,6 +383,352 @@ impl AcceptedDdlScope { } } +/// Lifetime-free logical-lock scope prepared for one maintenance operation. +pub(crate) struct PreparedMaintenanceLocks { + lock_manager: QuiescentGuard, + locks: OwnerLockState, +} + +impl PreparedMaintenanceLocks { + #[inline] + fn new(operation: &SessionOperationPin) -> Self { + Self { + lock_manager: operation.engine.lock_manager().clone(), + locks: OwnerLockState::new(operation.operation_lock_owner()), + } + } + + /// Acquire table metadata S followed by table data IS. + async fn acquire_table(&mut self, table_id: TableID) -> OperationResult<()> { + self.locks + .acquire( + &self.lock_manager, + LockResource::TableMetadata(table_id), + LockMode::Shared, + ) + .await?; + self.locks + .acquire( + &self.lock_manager, + LockResource::TableData(table_id), + LockMode::IntentShared, + ) + .await + } +} + +impl Drop for PreparedMaintenanceLocks { + #[inline] + fn drop(&mut self) { + self.locks.release_all(&self.lock_manager); + } +} + +/// Caller-owned maintenance preparation transferred atomically at acceptance. +/// +/// Prepared locks precede the voluntary operation pin so cancellation releases +/// every logical-lock claim before publishing the foreground terminal edge. +pub(crate) struct PreparedMaintenanceScope { + locks: Option, + operation: SessionOperationPin, +} + +impl PreparedMaintenanceScope { + /// Prepare one table-scoped maintenance lock set. + pub(crate) async fn table( + operation: SessionOperationPin, + table_id: TableID, + ) -> OperationResult { + let mut locks = PreparedMaintenanceLocks::new(&operation); + locks.acquire_table(table_id).await?; + Ok(Self { + locks: Some(locks), + operation, + }) + } + + /// Prepare one catalog/redo-wide maintenance operation. + #[inline] + pub(crate) fn global(operation: SessionOperationPin) -> Self { + Self { + locks: None, + 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 owns the scope. + #[inline] + pub(crate) fn engine(&self) -> &EngineRef { + &self.operation.engine + } + + /// Resolve and retain the authoritative current-live table under locks. + pub(crate) async fn resolve_user_table( + &self, + table_id: TableID, + ) -> OperationResult> { + let table = self + .operation + .engine + .catalog() + .validate_user_table_live(table_id) + .await?; + self.operation.state.cache_user_table(&table); + Ok(table) + } + + /// Synchronously consume caller preparation into accepted authority. + #[inline] + pub(crate) fn accept(self) -> AcceptedMaintenanceScope { + let Self { locks, operation } = self; + AcceptedMaintenanceScope { + operation: operation.into_mandatory(), + locks, + finish_state: MaintenanceFinishState::Executing, + } + } +} + +enum MaintenanceFinishState { + Executing, + TerminalReady, + FailedRetained, +} + +/// Runtime-owned maintenance operation and its transferred logical locks. +pub(crate) struct AcceptedMaintenanceScope { + operation: MandatoryOperationGuard, + locks: Option, + finish_state: MaintenanceFinishState, +} + +impl AcceptedMaintenanceScope { + /// Return the retained engine runtime. + #[inline] + pub(crate) fn engine(&self) -> &EngineRef { + &self.operation.engine + } + + /// Return cloned buffer-pool guards for maintenance 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() + } + + /// Verify nested state before returning from accepted execution. + #[inline] + pub(crate) fn mark_terminal_ready(&mut self) { + self.operation.assert_finish_ready(); + self.finish_state = MaintenanceFinishState::TerminalReady; + } + + /// Publish normal completion or retain an invalid finish state. + #[inline] + pub(crate) fn finish(&mut self) { + let state = replace( + &mut self.finish_state, + MaintenanceFinishState::FailedRetained, + ); + match state { + MaintenanceFinishState::TerminalReady => { + drop(self.locks.take()); + self.operation.finish(); + } + MaintenanceFinishState::Executing => { + self.operation.fail_retained(); + let report = Report::new(FatalError::MandatoryTaskPanic) + .attach("accepted maintenance finished without terminal-ready state"); + self.operation.engine.poisoner.poison(report); + drop(self.locks.take()); + } + MaintenanceFinishState::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 = MaintenanceFinishState::FailedRetained; + } +} + +impl SessionRuntimeAccess for AcceptedMaintenanceScope { + #[inline] + fn engine(&self) -> &EngineRef { + &self.operation.engine + } + + #[inline] + fn state(&self) -> &Arc { + &self.operation.state + } +} + +/// Operation-specific execution specification used by the shared maintenance carrier. +pub(crate) trait MaintenanceExecutionSpec: Send + 'static { + /// Terminal output delivered to the maintenance observer. + type Output: Send + 'static; + /// Domain resources retained outside the panic-caught execution future. + type Resources: Send + 'static; + /// Mutable diagnostic data attached after an unexpected execution panic. + type PanicLabel: Display + Send + 'static; + + /// Stable mandatory-runtime diagnostic label. + const LABEL: &'static str; + + /// Execute one accepted operation while borrowing its retained resources. + fn execute( + scope: &mut AcceptedMaintenanceScope, + resources: &mut Self::Resources, + panic_label: &mut Self::PanicLabel, + ) -> impl Future> + Send; +} + +/// Shared caller-prepared carrier for one maintenance execution body. +/// +/// Resource declaration before the maintenance scope preserves domain-resource +/// release before logical locks and the voluntary operation terminal edge. +pub(crate) struct PreparedMaintenanceExecution +where + S: MaintenanceExecutionSpec, +{ + resources: S::Resources, + scope: PreparedMaintenanceScope, + panic_label: S::PanicLabel, + metadata: MandatoryTaskMetadata, + spec: PhantomData, +} + +impl PreparedMaintenanceExecution +where + S: MaintenanceExecutionSpec, +{ + /// Build one global catalog/redo maintenance operation. + #[inline] + pub(crate) fn global( + scope: PreparedMaintenanceScope, + resources: S::Resources, + panic_label: S::PanicLabel, + ) -> Self { + let metadata = MandatoryTaskMetadata::operation(S::LABEL, Some(scope.key())); + Self { + resources, + scope, + panic_label, + metadata, + spec: PhantomData, + } + } + + /// Build one table-scoped maintenance operation. + #[inline] + pub(crate) fn table( + scope: PreparedMaintenanceScope, + resources: S::Resources, + panic_label: S::PanicLabel, + table_id: TableID, + ) -> Self { + let metadata = MandatoryTaskMetadata::table_operation(S::LABEL, scope.key(), table_id); + Self { + resources, + scope, + panic_label, + metadata, + spec: PhantomData, + } + } +} + +impl PreparedExecution for PreparedMaintenanceExecution +where + S: MaintenanceExecutionSpec, +{ + type Output = S::Output; + type Accepted = AcceptedMaintenanceExecution; + + const LABEL: &'static str = S::LABEL; + + #[inline] + fn metadata(&self) -> MandatoryTaskMetadata { + self.metadata.clone() + } + + #[inline] + fn accept(self) -> Self::Accepted { + let Self { + resources, + scope, + panic_label, + metadata: _, + spec: _, + } = self; + AcceptedMaintenanceExecution { + resources: Some(resources), + scope: scope.accept(), + panic_label, + spec: PhantomData, + } + } +} + +/// Shared mandatory-runtime owner for one accepted maintenance execution. +pub(crate) struct AcceptedMaintenanceExecution +where + S: MaintenanceExecutionSpec, +{ + resources: Option, + scope: AcceptedMaintenanceScope, + panic_label: S::PanicLabel, + spec: PhantomData, +} + +impl AcceptedExecution for AcceptedMaintenanceExecution +where + S: MaintenanceExecutionSpec, +{ + type Output = S::Output; + + #[inline] + fn execute(&mut self) -> impl Future> + Send { + S::execute( + &mut self.scope, + self.resources + .as_mut() + .unwrap_or_else(|| panic!("accepted maintenance resources are missing")), + &mut self.panic_label, + ) + } + + #[inline] + fn finish(&mut self) { + drop(self.resources.take()); + self.scope.finish(); + } + + #[inline] + async fn handle_panic(&mut self, _panic: Box) -> CompletionErrorBridge { + self.scope.handle_panic(); + CompletionErrorBridge::capture( + Report::new(FatalError::MandatoryTaskPanic).attach(self.panic_label.to_string()), + ) + } +} + #[derive(Clone, Copy)] enum MaintenanceBoundary { GcHorizon, @@ -442,27 +796,6 @@ impl<'lock> ScopedTableRuntimeAccess<'lock> { }) } - /// Acquires retry-recheck access, treating absent or terminal state as obsolete. - async fn acquire_for_retry( - session: &'lock SessionOperationPin, - table_id: TableID, - ) -> OperationResult> { - let owner = session.operation_lock_owner(); - let (metadata_lock, data_lock) = Self::acquire_locks(session, table_id, owner).await?; - let Some(table) = session.engine.catalog().current_live_user_table(table_id) else { - return Ok(None); - }; - if table.check_foreground_live().is_err() { - return Ok(None); - } - session.state.cache_user_table(&table); - Ok(Some(Self { - table: Some(table), - metadata_lock, - data_lock, - })) - } - /// Acquires logical locks in the repository-wide table resource order. async fn acquire_locks( session: &'lock SessionOperationPin, @@ -833,17 +1166,24 @@ impl Session { /// truncation planning. #[inline] pub async fn checkpoint_catalog(&mut self) -> Result<()> { - let session = self + let operation = self .pin_operation(SessionOperationKind::Maintenance) .attach("operation=checkpoint_catalog") .disclose()?; - session - .engine - .catalog() - .checkpoint_now(&session.engine.trx_sys) + let mandatory_runtime = operation.engine.mandatory_runtime.clone(); + let scope = PreparedMaintenanceScope::global(operation); + let engine = scope.engine().clone(); + let catalog_scope = CatalogCheckpointScope::acquire(engine.catalog_guard()).await; + let redo_scope = RedoRetentionScope::acquire(engine.trx_sys.clone()).await; + let prepared = prepare_catalog_checkpoint_operation(catalog_scope, redo_scope, scope); + engine.poisoner.ensure_healthy().disclose()?; + let observer = mandatory_runtime + .submit(prepared) .await - .disclose() - .map(|_| ()) + .attach("operation=checkpoint_catalog") + .disclose()?; + drop(mandatory_runtime); + observer.wait().await.map(|_| ()) } /// Run catalog checkpoint and redo-log truncation as one maintenance operation. @@ -855,16 +1195,24 @@ impl Session { pub async fn checkpoint_catalog_and_truncate_redo_log( &mut self, ) -> Result { - let session = self + let operation = self .pin_operation(SessionOperationKind::Maintenance) .attach("operation=checkpoint_catalog_and_truncate_redo_log") .disclose()?; - session - .engine - .trx_sys - .checkpoint_catalog_and_truncate_redo_log() + let mandatory_runtime = operation.engine.mandatory_runtime.clone(); + let scope = PreparedMaintenanceScope::global(operation); + let engine = scope.engine().clone(); + let catalog_scope = CatalogCheckpointScope::acquire(engine.catalog_guard()).await; + let redo_scope = RedoRetentionScope::acquire(engine.trx_sys.clone()).await; + let prepared = prepare_catalog_redo_maintenance_operation(catalog_scope, redo_scope, scope); + engine.poisoner.ensure_healthy().disclose()?; + let observer = mandatory_runtime + .submit(prepared) .await - .disclose() + .attach("operation=checkpoint_catalog_and_truncate_redo_log") + .disclose()?; + drop(mandatory_runtime); + observer.wait().await } /// Physically remove recovery-obsolete sealed redo prefix files. @@ -875,11 +1223,24 @@ impl Session { /// summarized in the returned outcome and can be retried by a later call. #[inline] pub async fn truncate_redo_log(&mut self) -> Result { - let session = self + let operation = self .pin_operation(SessionOperationKind::Maintenance) .attach("operation=truncate_redo_log") .disclose()?; - session.engine.trx_sys.truncate_redo_log().await.disclose() + let mandatory_runtime = operation.engine.mandatory_runtime.clone(); + let scope = PreparedMaintenanceScope::global(operation); + let engine = scope.engine().clone(); + let catalog_scope = CatalogCheckpointScope::acquire(engine.catalog_guard()).await; + let redo_scope = RedoRetentionScope::acquire(engine.trx_sys.clone()).await; + let prepared = prepare_redo_truncation_operation(catalog_scope, redo_scope, scope); + engine.poisoner.ensure_healthy().disclose()?; + let observer = mandatory_runtime + .submit(prepared) + .await + .attach("operation=truncate_redo_log") + .disclose()?; + drop(mandatory_runtime); + observer.wait().await } /// Return a monotonic transaction-system statistics snapshot. @@ -963,39 +1324,63 @@ impl Session { table_id: TableID, max_rows: usize, ) -> Result { - let session = self + let operation = self .pin_operation(SessionOperationKind::Maintenance) .attach("operation=freeze_table") .disclose()?; - let access = ScopedTableRuntimeAccess::acquire(&session, table_id) + let mandatory_runtime = operation.engine.mandatory_runtime.clone(); + let scope = PreparedMaintenanceScope::table(operation, table_id) .await .attach_with(|| format!("operation=freeze_table, table_id={table_id}")) .disclose()?; - access - .table() - .freeze(&session, max_rows) + let table = scope + .resolve_user_table(table_id) .await .attach_with(|| format!("operation=freeze_table, table_id={table_id}")) - .disclose() + .disclose()?; + scope.engine().poisoner.ensure_healthy().disclose()?; + let prepared = match prepare_freeze_table_operation(scope, table, max_rows) { + Ok(prepared) => prepared, + Err(outcome) => return Ok(outcome), + }; + let observer = mandatory_runtime + .submit(prepared) + .await + .attach_with(|| format!("operation=freeze_table, table_id={table_id}")) + .disclose()?; + drop(mandatory_runtime); + observer.wait().await } /// Persist eligible state using the table-owned canonical frozen batch. #[inline] pub async fn checkpoint_table(&mut self, table_id: TableID) -> Result { - let session = self + let operation = self .pin_operation(SessionOperationKind::Maintenance) .attach("operation=checkpoint_table") .disclose()?; - let access = ScopedTableRuntimeAccess::acquire(&session, table_id) + let mandatory_runtime = operation.engine.mandatory_runtime.clone(); + let scope = PreparedMaintenanceScope::table(operation, table_id) .await .attach_with(|| format!("operation=checkpoint_table, table_id={table_id}")) .disclose()?; - access - .table() - .checkpoint(&session) + let table = scope + .resolve_user_table(table_id) .await .attach_with(|| format!("operation=checkpoint_table, table_id={table_id}")) - .disclose() + .disclose()?; + scope.engine().poisoner.ensure_healthy().disclose()?; + let prepared = match prepare_checkpoint_table_operation(scope, table) { + Ok(prepared) => prepared, + Err(outcome) => return Ok(outcome), + }; + let observer = mandatory_runtime + .submit(prepared) + .await + .attach_with(|| format!("operation=checkpoint_table, table_id={table_id}")) + .disclose()?; + drop(mandatory_runtime); + observer.wait().await } /// Wait until retry may be useful for one self-identifying checkpoint delay. @@ -1041,29 +1426,11 @@ impl Session { &mut self, table_id: TableID, ) -> Result { - let session = self - .pin_operation(SessionOperationKind::Maintenance) - .attach("operation=checkpoint_table_with_wait") - .disclose()?; loop { - let access = ScopedTableRuntimeAccess::acquire(&session, table_id) - .await - .attach_with(|| { - format!("operation=checkpoint_table_with_wait, table_id={table_id}") - }) - .disclose()?; - let outcome = access - .table() - .checkpoint(&session) - .await - .attach_with(|| { - format!("operation=checkpoint_table_with_wait, table_id={table_id}") - }) - .disclose()?; - drop(access); + let outcome = self.checkpoint_table(table_id).await?; match outcome { CheckpointOutcome::Delayed { reason } => { - wait_for_checkpoint_retry_in_operation(&session, reason).await?; + self.wait_for_checkpoint_retry(reason).await?; } outcome => return Ok(outcome), } @@ -1127,20 +1494,32 @@ impl Session { table_id: TableID, clean_live_entries: bool, ) -> Result { - let session = self + let operation = self .pin_operation(SessionOperationKind::Maintenance) .attach("operation=cleanup_secondary_mem_indexes") .disclose()?; - let access = ScopedTableRuntimeAccess::acquire(&session, table_id) + let mandatory_runtime = operation.engine.mandatory_runtime.clone(); + let scope = PreparedMaintenanceScope::table(operation, table_id) .await .attach("operation=cleanup_secondary_mem_indexes") .disclose()?; - access - .table() - .cleanup_secondary_mem_indexes(&session, clean_live_entries) + let table = scope + .resolve_user_table(table_id) .await .attach_with(|| format!("operation=cleanup_secondary_mem_indexes, table_id={table_id}")) - .disclose() + .disclose()?; + scope.engine().poisoner.ensure_healthy().disclose()?; + let observer = mandatory_runtime + .submit(prepare_mem_index_cleanup_operation( + scope, + table, + clean_live_entries, + )) + .await + .attach_with(|| format!("operation=cleanup_secondary_mem_indexes, table_id={table_id}")) + .disclose()?; + drop(mandatory_runtime); + observer.wait().await } /// Acquires an explicit session-lifetime table lock. @@ -1288,12 +1667,6 @@ impl SessionOperationPin { } } - /// Starts one private transaction inside this DDL or maintenance operation. - #[inline] - pub(crate) fn begin_private_trx(&self) -> LifecycleResult { - begin_private_transaction(&self.engine, &self.entry) - } - /// Prepare CREATE TABLE while consuming this foreground operation. async fn prepare_create_table( self, @@ -2567,34 +2940,6 @@ fn begin_private_transaction( Ok(engine.trx_sys.begin_private_trx(engine, entry, inner)) } -async fn wait_for_checkpoint_retry_in_operation( - session: &SessionOperationPin, - reason: CheckpointDelayReason, -) -> Result<()> { - let table_id = match reason { - CheckpointDelayReason::ActiveRoot { table_id, .. } - | CheckpointDelayReason::FrozenPageCutoff { table_id, .. } => table_id, - }; - loop { - let Some(access) = ScopedTableRuntimeAccess::acquire_for_retry(session, table_id) - .await - .disclose()? - else { - return Ok(()); - }; - let observation = access - .table() - .checkpoint_retry_observation(session, reason) - .await - .disclose()?; - drop(access); - match observation { - CheckpointRetryObservation::Ready => return Ok(()), - CheckpointRetryObservation::Wait(waiter) => waiter.wait().await, - } - } -} - async fn wait_for_maintenance_boundary( session: &SessionObserverPin, ts: TrxID, @@ -3882,7 +4227,7 @@ pub(crate) mod tests { } #[test] - fn test_private_transaction_preserves_stable_entry_and_public_cache() { + fn test_mandatory_private_transaction_preserves_stable_entry_and_public_cache() { smol::block_on(async { let root = TempDir::new().unwrap(); let engine = Engine::bootstrap(EngineConfig::default().storage_root(root.path())) @@ -3903,6 +4248,11 @@ pub(crate) mod tests { .as_deref() .map(|inner| inner as *const TrxInner as usize) .expect("session must retain its public transaction cache"); + let mut operation = operation.into_mandatory(); + assert_eq!( + entry.inspect().state, + SessionOperationState::Mandatory(None) + ); let trx = operation.begin_private_trx().unwrap(); let first_inner = entry .inner_ptr_for_test() @@ -3932,7 +4282,7 @@ pub(crate) mod tests { trx.rollback().await.unwrap(); let snapshot = entry.inspect(); - assert_eq!(snapshot.state, SessionOperationState::Voluntary(None)); + assert_eq!(snapshot.state, SessionOperationState::Mandatory(None)); assert_eq!(snapshot.trx_id, None); { let lifecycle = state.lifecycle.lock(); @@ -3972,6 +4322,9 @@ pub(crate) mod tests { ); replacement.rollback().await.unwrap(); + operation.assert_finish_ready(); + operation.finish(); + assert_eq!(entry.inspect().state, SessionOperationState::Terminal); drop(operation); assert!(matches!( state.lifecycle.lock().slot, @@ -4578,14 +4931,15 @@ pub(crate) mod tests { let catalog_path = engine.inner().table_fs.catalog_mtb_file_path(); let publish_hook = Arc::new(FailingFirstWriteHook::new(catalog_path)); let _publish_hook_guard = install_storage_backend_test_hook(publish_hook.clone()); - let _cleanup_hook_guard = install_redo_cleanup_before_unlink_hook(Arc::new( - |file_seq, path| { + let _cleanup_hook_guard = install_redo_cleanup_before_unlink_hook( + &engine.inner().maintenance_test, + Arc::new(|file_seq, path| { panic!( "redo cleanup must not run after combined checkpoint failure: file_seq={file_seq}, path={}", path.display() ); - }, - )); + }), + ); let err = session .checkpoint_catalog_and_truncate_redo_log() @@ -4651,14 +5005,15 @@ pub(crate) mod tests { let catalog_path = engine.inner().table_fs.catalog_mtb_file_path(); let publish_hook = Arc::new(FailingFirstWriteHook::new(catalog_path)); let _publish_hook_guard = install_storage_backend_test_hook(publish_hook.clone()); - let _cleanup_hook_guard = install_redo_cleanup_before_unlink_hook(Arc::new( - |file_seq, path| { + let _cleanup_hook_guard = install_redo_cleanup_before_unlink_hook( + &engine.inner().maintenance_test, + Arc::new(|file_seq, path| { panic!( "redo cleanup must not run after combined marker failure: file_seq={file_seq}, path={}", path.display() ); - }, - )); + }), + ); let err = session .checkpoint_catalog_and_truncate_redo_log() @@ -4711,8 +5066,9 @@ pub(crate) mod tests { let hook_called = Arc::new(AtomicBool::new(false)); let hook_flag = Arc::clone(&hook_called); let hook_engine = engine.new_ref().unwrap(); - let hook_guard = - install_redo_cleanup_before_unlink_hook(Arc::new(move |file_seq, _path| { + let hook_guard = install_redo_cleanup_before_unlink_hook( + &engine.inner().maintenance_test, + Arc::new(move |file_seq, _path| { if file_seq != 0 { return; } @@ -4728,7 +5084,8 @@ pub(crate) mod tests { } } catalog.release_index_metadata_change(); - })); + }), + ); let mut session = engine.new_session().unwrap(); let outcome = session @@ -4769,20 +5126,34 @@ pub(crate) mod tests { let obsolete_path = redo_file_path(&main_dir, log_file_stem, 0); assert!(obsolete_path.exists()); - let redo_retention_lease = engine.inner().trx_sys.begin_redo_retention().await; + let redo_retention_scope = + RedoRetentionScope::acquire(engine.inner().trx_sys.clone()).await; let mut session = engine.new_session().unwrap(); + let session_id = session.id(); + let state = engine + .inner() + .session_registry + .session_state(session_id) + .unwrap(); let mut maintenance_fut = Box::pin(session.checkpoint_catalog_and_truncate_redo_log()); assert!(matches!( futures::poll!(maintenance_fut.as_mut()), std::task::Poll::Pending )); + let entry = + active_operation_entry_for_test(&engine.inner().session_registry, session_id); + assert_eq!( + entry.inspect().state, + SessionOperationState::Voluntary(None) + ); + assert_eq!(engine.inner().mandatory_runtime.blocker_counts(), (0, 0)); let _ = engine .inner() .poisoner .poison(Report::new(FatalError::RedoWrite).attach("test redo write failure")); - drop(redo_retention_lease); + drop(redo_retention_scope); let err = maintenance_fut.await.unwrap_err(); assert_eq!(err.kind(), ErrorKind::Fatal); @@ -4790,6 +5161,25 @@ pub(crate) mod tests { err.report().downcast_ref::().copied(), Some(FatalError::RedoWrite) ); + assert_eq!(entry.inspect().state, SessionOperationState::Terminal); + assert!(matches!( + state.lifecycle.lock().slot, + SessionOperationSlot::Idle + )); + assert_eq!(engine.inner().mandatory_runtime.blocker_counts(), (0, 0)); + let mut catalog_acquire = Box::pin(CatalogCheckpointScope::acquire( + engine.inner().catalog.clone(), + )); + let Poll::Ready(catalog_scope) = futures::poll!(catalog_acquire.as_mut()) else { + panic!("failed preparation must release catalog checkpoint authority") + }; + let mut redo_acquire = + Box::pin(RedoRetentionScope::acquire(engine.inner().trx_sys.clone())); + let Poll::Ready(redo_scope) = futures::poll!(redo_acquire.as_mut()) else { + panic!("failed preparation must release redo retention authority") + }; + drop(catalog_scope); + drop(redo_scope); assert!( obsolete_path.exists(), "obsolete redo file should not be removed after poison" @@ -5011,14 +5401,15 @@ pub(crate) mod tests { let catalog_path = engine.inner().table_fs.catalog_mtb_file_path(); let publish_hook = Arc::new(FailingFirstWriteHook::new(catalog_path)); let _publish_hook_guard = install_storage_backend_test_hook(publish_hook.clone()); - let _cleanup_hook_guard = install_redo_cleanup_before_unlink_hook(Arc::new( - |file_seq, path| { + let _cleanup_hook_guard = install_redo_cleanup_before_unlink_hook( + &engine.inner().maintenance_test, + Arc::new(|file_seq, path| { panic!( "redo cleanup must not run after marker publication failure: file_seq={file_seq}, path={}", path.display() ); - }, - )); + }), + ); let err = session.truncate_redo_log().await.unwrap_err(); @@ -5069,8 +5460,9 @@ pub(crate) mod tests { let hook_called = Arc::new(AtomicBool::new(false)); let hook_flag = Arc::clone(&hook_called); let hook_engine = engine.new_ref().unwrap(); - let hook_guard = - install_redo_cleanup_before_unlink_hook(Arc::new(move |file_seq, _path| { + let hook_guard = install_redo_cleanup_before_unlink_hook( + &engine.inner().maintenance_test, + Arc::new(move |file_seq, _path| { if file_seq != 0 { return; } @@ -5086,7 +5478,8 @@ pub(crate) mod tests { } } catalog.release_index_metadata_change(); - })); + }), + ); let mut session = engine.new_session().unwrap(); let outcome = session.truncate_redo_log().await.unwrap(); @@ -5102,6 +5495,54 @@ pub(crate) mod tests { }); } + #[test] + fn test_dropped_redo_truncation_observer_does_not_cancel_unlink() { + smol::block_on(async { + let root = TempDir::new().unwrap(); + let main_dir = root.path().to_path_buf(); + let log_file_stem = "redo_truncate_observer_drop"; + let engine = Engine::bootstrap(redo_truncation_engine_config(&main_dir, log_file_stem)) + .await + .unwrap(); + create_rotated_redo_table(&engine, &main_dir, log_file_stem, 1).await; + engine + .catalog() + .storage + .publish_first_redo_log_seq(1) + .await + .unwrap(); + let obsolete_path = redo_file_path(&main_dir, log_file_stem, 0); + assert!(obsolete_path.exists()); + + let (entered_tx, entered_rx) = flume::bounded(1); + let (release_tx, release_rx) = flume::bounded(1); + let hook_guard = install_redo_cleanup_before_unlink_hook( + &engine.inner().maintenance_test, + Arc::new(move |file_seq, _path| { + if file_seq == 0 { + entered_tx.send(()).unwrap(); + release_rx.recv().unwrap(); + } + }), + ); + + let mut session = engine.new_session().unwrap(); + let session_id = session.id(); + let mut truncate = Box::pin(session.truncate_redo_log()); + assert!(matches!( + futures::poll!(truncate.as_mut()), + std::task::Poll::Pending + )); + entered_rx.recv_async().await.unwrap(); + drop(truncate); + + release_tx.send_async(()).await.unwrap(); + wait_for_session_idle(&engine.inner().session_registry, session_id).await; + assert!(!obsolete_path.exists()); + drop(hook_guard); + }); + } + #[test] fn test_session_truncate_redo_log_rechecks_poison_after_gate_wait() { smol::block_on(async { @@ -5121,7 +5562,8 @@ pub(crate) mod tests { let obsolete_path = redo_file_path(&main_dir, log_file_stem, 0); assert!(obsolete_path.exists()); - let redo_retention_lease = engine.inner().trx_sys.begin_redo_retention().await; + let redo_retention_scope = + RedoRetentionScope::acquire(engine.inner().trx_sys.clone()).await; let mut session = engine.new_session().unwrap(); let mut truncate_fut = Box::pin(session.truncate_redo_log()); @@ -5134,7 +5576,7 @@ pub(crate) mod tests { .inner() .poisoner .poison(Report::new(FatalError::RedoWrite).attach("test redo write failure")); - drop(redo_retention_lease); + drop(redo_retention_scope); let err = truncate_fut.await.unwrap_err(); assert_eq!(err.kind(), ErrorKind::Fatal); @@ -5169,13 +5611,15 @@ pub(crate) mod tests { let hook_removed_file = Arc::new(AtomicBool::new(false)); let hook_flag = Arc::clone(&hook_removed_file); let hook_path = obsolete_path.clone(); - let hook_guard = - install_redo_cleanup_before_unlink_hook(Arc::new(move |file_seq, path| { + let hook_guard = install_redo_cleanup_before_unlink_hook( + &engine.inner().maintenance_test, + Arc::new(move |file_seq, path| { if file_seq == 0 && path == hook_path && !hook_flag.swap(true, Ordering::SeqCst) { fs::remove_file(path).unwrap(); } - })); + }), + ); let mut session = engine.new_session().unwrap(); let missing = session.truncate_redo_log().await.unwrap(); diff --git a/doradb-storage/src/table/access.rs b/doradb-storage/src/table/access.rs index 2782ce5c..ddcb5709 100644 --- a/doradb-storage/src/table/access.rs +++ b/doradb-storage/src/table/access.rs @@ -4614,6 +4614,7 @@ mod tests { use crate::table::hot::{ DeleteInternal, HotRowMutator, InsertRowIntoPage, RowInserter, UpdateRowInplace, }; + use crate::table::lifecycle::TableCheckpointRootMutationScope; use crate::table::tests::*; use crate::table::{CheckpointOutcome, FreezeOutcome}; use crate::table::{ColumnDeletionBuffer, DeleteMarker}; @@ -8760,7 +8761,8 @@ mod tests { .checkpoint_workflow .begin_checkpoint(&table.lifecycle) .unwrap(); - let root_lease = table.try_begin_checkpoint_root_mutation().unwrap(); + let root_lease = + TableCheckpointRootMutationScope::acquire(Arc::clone(&table)).unwrap(); let frozen_pages = checkpoint_attempt.batch().unwrap().pages.clone(); let transition_pages = table .load_frozen_pages_for_transition(&session.pool_guards(), &frozen_pages) @@ -8770,6 +8772,7 @@ mod tests { &transition_pages, checkpoint_attempt.batch_mut().unwrap(), stmt.runtime().sts(), + &engine.inner().maintenance_test, ); assert!(delay.is_none()); let transition_lease = table @@ -8780,6 +8783,7 @@ mod tests { &transition_pages, checkpoint_attempt.batch_mut().unwrap(), stmt.runtime().sts(), + &engine.inner().maintenance_test, ); let marker = table_for_internal_assertion(&engine, table_id) diff --git a/doradb-storage/src/table/checkpoint_workflow.rs b/doradb-storage/src/table/checkpoint_workflow.rs index 20017f15..3a7cb901 100644 --- a/doradb-storage/src/table/checkpoint_workflow.rs +++ b/doradb-storage/src/table/checkpoint_workflow.rs @@ -1,6 +1,6 @@ -use super::CheckpointCancelReason; use super::deletion_buffer::DeleteMarker; use super::lifecycle::{CheckpointPublishLease, TableLifecycle, TableTerminal}; +use super::{CheckpointCancelReason, Table}; use crate::id::{PageID, RowID, TableID, TrxID}; use crate::trx::SharedTrxStatus; use parking_lot::Mutex; @@ -276,43 +276,10 @@ impl TableCheckpointWorkflow { } } - pub(super) fn begin_freeze<'a>( - &'a self, - lifecycle: &TableLifecycle, - ) -> StdResult, FreezeOutcome> { - let mut state = self.state.lock(); - if let Err(reason) = checkpoint_lifecycle(lifecycle.inspect_terminal()) { - return Err(FreezeOutcome::Cancelled { reason }); - } - match &*state { - TableCheckpointWorkflowState::Idle => { - *state = TableCheckpointWorkflowState::Freezing; - Ok(FreezeAttempt { - workflow: self, - restore_idle: true, - }) - } - TableCheckpointWorkflowState::Freezing => Err(FreezeOutcome::Cancelled { - reason: CheckpointCancelReason::FreezeInProgress, - }), - TableCheckpointWorkflowState::Frozen(batch) => Err(FreezeOutcome::AlreadyFrozen { - batch: batch.info(), - }), - TableCheckpointWorkflowState::Checkpointing { .. } - | TableCheckpointWorkflowState::Publishing - | TableCheckpointWorkflowState::Transition => Err(FreezeOutcome::Cancelled { - reason: CheckpointCancelReason::CheckpointInProgress, - }), - TableCheckpointWorkflowState::Closed => Err(FreezeOutcome::Cancelled { - reason: terminal_checkpoint_cancel(lifecycle.inspect_terminal()), - }), - } - } - - pub(super) fn begin_checkpoint<'a>( - &'a self, + fn begin_checkpoint_inner( + &self, lifecycle: &TableLifecycle, - ) -> StdResult, CheckpointCancelReason> { + ) -> StdResult<(CheckpointSource, Option), CheckpointCancelReason> { let mut state = self.state.lock(); checkpoint_lifecycle(lifecycle.inspect_terminal())?; let (source, batch) = match &*state { @@ -338,6 +305,14 @@ impl TableCheckpointWorkflow { } }; *state = TableCheckpointWorkflowState::Checkpointing { source }; + Ok((source, batch)) + } + + pub(super) fn begin_checkpoint<'a>( + &'a self, + lifecycle: &TableLifecycle, + ) -> StdResult, CheckpointCancelReason> { + let (source, batch) = self.begin_checkpoint_inner(lifecycle)?; Ok(CheckpointAttempt { workflow: self, source, @@ -488,15 +463,70 @@ impl TableCheckpointWorkflow { } } -pub(super) struct FreezeAttempt<'a> { - workflow: &'a TableCheckpointWorkflow, +impl Table { + pub(super) fn begin_freeze(self: Arc) -> StdResult { + let mut state = self.checkpoint_workflow.state.lock(); + if let Err(reason) = checkpoint_lifecycle(self.lifecycle.inspect_terminal()) { + return Err(FreezeOutcome::Cancelled { reason }); + } + match &*state { + TableCheckpointWorkflowState::Idle => { + *state = TableCheckpointWorkflowState::Freezing; + } + TableCheckpointWorkflowState::Freezing => { + return Err(FreezeOutcome::Cancelled { + reason: CheckpointCancelReason::FreezeInProgress, + }); + } + TableCheckpointWorkflowState::Frozen(batch) => { + return Err(FreezeOutcome::AlreadyFrozen { + batch: batch.info(), + }); + } + TableCheckpointWorkflowState::Checkpointing { .. } + | TableCheckpointWorkflowState::Publishing + | TableCheckpointWorkflowState::Transition => { + return Err(FreezeOutcome::Cancelled { + reason: CheckpointCancelReason::CheckpointInProgress, + }); + } + TableCheckpointWorkflowState::Closed => { + return Err(FreezeOutcome::Cancelled { + reason: terminal_checkpoint_cancel(self.lifecycle.inspect_terminal()), + }); + } + } + drop(state); + Ok(PreparedFreezeAttempt { + table: self, + restore_idle: true, + }) + } + + pub(super) fn begin_checkpoint( + self: Arc, + ) -> StdResult { + let (source, batch) = self + .checkpoint_workflow + .begin_checkpoint_inner(&self.lifecycle)?; + Ok(PreparedCheckpointAttempt { + table: self, + source, + batch, + }) + } +} + +/// Lifetime-free reversible freeze attempt prepared by the caller. +pub(super) struct PreparedFreezeAttempt { + table: Arc
, restore_idle: bool, } -impl FreezeAttempt<'_> { +impl PreparedFreezeAttempt { #[inline] pub(super) fn begin_page_publication(&mut self) -> bool { - let state = self.workflow.state.lock(); + let state = self.table.checkpoint_workflow.state.lock(); match *state { TableCheckpointWorkflowState::Freezing => { self.restore_idle = false; @@ -511,43 +541,40 @@ impl FreezeAttempt<'_> { } #[inline] - pub(super) fn cancelled(mut self, lifecycle: &TableLifecycle) -> FreezeOutcome { + pub(super) fn cancelled(mut self) -> FreezeOutcome { self.restore_idle = false; FreezeOutcome::Cancelled { - reason: terminal_checkpoint_cancel(lifecycle.inspect_terminal()), + reason: terminal_checkpoint_cancel(self.table.lifecycle.inspect_terminal()), } } - pub(super) fn finish( - mut self, - batch: FrozenPageBatch, - lifecycle: &TableLifecycle, - ) -> FreezeOutcome { + pub(super) fn finish(mut self, batch: FrozenPageBatch) -> FreezeOutcome { let info = batch.info(); - let mut state = self.workflow.state.lock(); - let outcome = if let Err(reason) = checkpoint_lifecycle(lifecycle.inspect_terminal()) { - *state = TableCheckpointWorkflowState::Closed; - FreezeOutcome::Cancelled { reason } - } else { - assert!( - matches!(*state, TableCheckpointWorkflowState::Freezing), - "freeze completion requires Freezing workflow state" - ); - *state = TableCheckpointWorkflowState::Frozen(batch); - FreezeOutcome::Frozen { batch: info } - }; + let mut state = self.table.checkpoint_workflow.state.lock(); + let outcome = + if let Err(reason) = checkpoint_lifecycle(self.table.lifecycle.inspect_terminal()) { + *state = TableCheckpointWorkflowState::Closed; + FreezeOutcome::Cancelled { reason } + } else { + assert!( + matches!(*state, TableCheckpointWorkflowState::Freezing), + "freeze completion requires Freezing workflow state" + ); + *state = TableCheckpointWorkflowState::Frozen(batch); + FreezeOutcome::Frozen { batch: info } + }; self.restore_idle = false; outcome } } -impl Drop for FreezeAttempt<'_> { +impl Drop for PreparedFreezeAttempt { #[inline] fn drop(&mut self) { if !self.restore_idle { return; } - let mut state = self.workflow.state.lock(); + let mut state = self.table.checkpoint_workflow.state.lock(); if matches!(*state, TableCheckpointWorkflowState::Freezing) { *state = TableCheckpointWorkflowState::Idle; } @@ -561,6 +588,32 @@ pub(super) struct CheckpointAttempt<'a> { } impl CheckpointAttempt<'_> { + #[inline] + pub(super) fn batch(&self) -> Option<&FrozenPageBatch> { + self.batch.as_ref() + } + + #[inline] + pub(super) fn batch_mut(&mut self) -> Option<&mut FrozenPageBatch> { + self.batch.as_mut() + } +} + +impl Drop for CheckpointAttempt<'_> { + #[inline] + fn drop(&mut self) { + restore_checkpoint_attempt(self.workflow, self.source, &mut self.batch); + } +} + +/// Lifetime-free reversible checkpoint attempt prepared by the caller. +pub(super) struct PreparedCheckpointAttempt { + table: Arc
, + source: CheckpointSource, + batch: Option, +} + +impl PreparedCheckpointAttempt { #[inline] pub(super) fn source(&self) -> CheckpointSource { self.source @@ -577,27 +630,40 @@ impl CheckpointAttempt<'_> { } } -impl Drop for CheckpointAttempt<'_> { +impl Drop for PreparedCheckpointAttempt { + #[inline] fn drop(&mut self) { - let mut state = self.workflow.state.lock(); - if !matches!( - *state, - TableCheckpointWorkflowState::Checkpointing { source } - if source == self.source - ) { - return; + restore_checkpoint_attempt( + &self.table.checkpoint_workflow, + self.source, + &mut self.batch, + ); + } +} + +fn restore_checkpoint_attempt( + workflow: &TableCheckpointWorkflow, + source: CheckpointSource, + batch: &mut Option, +) { + let mut state = workflow.state.lock(); + if !matches!( + *state, + TableCheckpointWorkflowState::Checkpointing { source: current } + if current == source + ) { + return; + } + match source { + CheckpointSource::Idle => { + debug_assert!(batch.is_none()); + *state = TableCheckpointWorkflowState::Idle; } - match self.source { - CheckpointSource::Idle => { - debug_assert!(self.batch.is_none()); - *state = TableCheckpointWorkflowState::Idle; - } - CheckpointSource::Frozen => { - let Some(batch) = self.batch.take() else { - panic!("frozen checkpoint attempt must retain its batch") - }; - *state = TableCheckpointWorkflowState::Frozen(batch); - } + CheckpointSource::Frozen => { + let Some(batch) = batch.take() else { + panic!("frozen checkpoint attempt must retain its batch") + }; + *state = TableCheckpointWorkflowState::Frozen(batch); } } } @@ -679,66 +745,14 @@ mod tests { } #[test] - fn test_repeated_freeze_keeps_original_batch_and_validation_cache() { + fn test_reversible_checkpoint_attempt_restores_admitted_state() { let lifecycle = TableLifecycle::new(); let workflow = TableCheckpointWorkflow::new(); - let attempt = workflow.begin_freeze(&lifecycle).unwrap(); - let frozen_ts = TrxID::new(101); - let outcome = attempt.finish(one_page_batch(frozen_ts), &lifecycle); - let FreezeOutcome::Frozen { batch } = outcome else { - panic!("first freeze should install a batch: {outcome:?}"); - }; - assert_eq!(batch.table_id(), TABLE_ID); - assert_eq!(batch.frozen_ts(), frozen_ts); - assert_eq!(batch.approximate_rows(), 7); - assert_eq!(batch.page_count(), 1); - assert_eq!(batch.stable_page_count(), 0); - - let mut checkpoint = workflow.begin_checkpoint(&lifecycle).unwrap(); - checkpoint.batch_mut().unwrap().validation[0] = FrozenPageValidationState::Stable { - required_cutoff_ts: Some(TrxID::new(88)), - }; - drop(checkpoint); - - let repeated = workflow.begin_freeze(&lifecycle).err().unwrap(); - let FreezeOutcome::AlreadyFrozen { batch } = repeated else { - panic!("repeated freeze should return the canonical batch: {repeated:?}"); - }; - assert_eq!(batch.frozen_ts(), frozen_ts); - assert_eq!(batch.approximate_rows(), 7); - assert_eq!(batch.page_count(), 1); - assert_eq!(batch.stable_page_count(), 1); - assert_eq!(workflow.state_name(), "Frozen"); - } - - #[test] - fn test_reversible_attempt_guards_restore_admitted_state() { - let lifecycle = TableLifecycle::new(); - let workflow = TableCheckpointWorkflow::new(); - - let freeze = workflow.begin_freeze(&lifecycle).unwrap(); - assert_eq!(workflow.state_name(), "Freezing"); - assert_eq!( - workflow.begin_checkpoint(&lifecycle).err().unwrap(), - CheckpointCancelReason::FreezeInProgress - ); - let repeated = workflow.begin_freeze(&lifecycle).err().unwrap(); - assert_eq!( - repeated, - FreezeOutcome::Cancelled { - reason: CheckpointCancelReason::FreezeInProgress, - } - ); - drop(freeze); - assert_eq!(workflow.state_name(), "Idle"); let checkpoint = workflow.begin_checkpoint(&lifecycle).unwrap(); - let freeze = workflow.begin_freeze(&lifecycle).err().unwrap(); assert_eq!( - freeze, - FreezeOutcome::Cancelled { - reason: CheckpointCancelReason::CheckpointInProgress, - } + workflow.begin_checkpoint(&lifecycle).err().unwrap(), + CheckpointCancelReason::CheckpointInProgress ); drop(checkpoint); assert_eq!(workflow.state_name(), "Idle"); @@ -759,16 +773,10 @@ mod tests { workflow.begin_checkpoint(&lifecycle).err().unwrap(), CheckpointCancelReason::TableDropping ); - assert_eq!( - workflow.begin_freeze(&lifecycle).err().unwrap(), - FreezeOutcome::Cancelled { - reason: CheckpointCancelReason::TableDropping, - } - ); } #[test] - fn test_publish_states_reject_concurrent_freeze_and_checkpoint() { + fn test_publish_states_reject_concurrent_checkpoint() { fn assert_checkpoint_conflicts( workflow: &TableCheckpointWorkflow, lifecycle: &TableLifecycle, @@ -777,42 +785,25 @@ mod tests { workflow.begin_checkpoint(lifecycle).err().unwrap(), CheckpointCancelReason::CheckpointInProgress ); - assert_eq!( - workflow.begin_freeze(lifecycle).err().unwrap(), - FreezeOutcome::Cancelled { - reason: CheckpointCancelReason::CheckpointInProgress, - } - ); } let lifecycle = TableLifecycle::new(); let workflow = TableCheckpointWorkflow::new(); let publishing_attempt = workflow.begin_checkpoint(&lifecycle).unwrap(); - let publishing_root = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); - let publishing_lease = workflow - .try_begin_publishing(&lifecycle, publishing_attempt.source()) - .unwrap(); + *workflow.state.lock() = TableCheckpointWorkflowState::Publishing; assert_eq!(workflow.state_name(), "Publishing"); assert_checkpoint_conflicts(&workflow, &lifecycle); workflow.finish_publication(); - drop(publishing_lease); - drop(publishing_root); drop(publishing_attempt); - let freeze = workflow.begin_freeze(&lifecycle).unwrap(); - assert!(matches!( - freeze.finish(one_page_batch(TrxID::new(102)), &lifecycle), - FreezeOutcome::Frozen { .. } - )); + *workflow.state.lock() = + TableCheckpointWorkflowState::Frozen(one_page_batch(TrxID::new(102))); let transition_attempt = workflow.begin_checkpoint(&lifecycle).unwrap(); - let transition_root = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); - let transition_lease = workflow.try_begin_transition(&lifecycle).unwrap(); + *workflow.state.lock() = TableCheckpointWorkflowState::Transition; assert_eq!(workflow.state_name(), "Transition"); assert_checkpoint_conflicts(&workflow, &lifecycle); workflow.finish_publication(); - drop(transition_lease); - drop(transition_root); drop(transition_attempt); assert_eq!(workflow.state_name(), "Idle"); } diff --git a/doradb-storage/src/table/gc.rs b/doradb-storage/src/table/gc.rs index 5a75f27a..18dbb5fb 100644 --- a/doradb-storage/src/table/gc.rs +++ b/doradb-storage/src/table/gc.rs @@ -2,7 +2,8 @@ use super::{Table, TableRootSnapshot, TableRuntimeLayout}; use crate::buffer::{BufferPool, EvictableBufferPool, PoolGuard, PoolGuards}; use crate::catalog::TableMetadata; use crate::error::{ - DataIntegrityError, RuntimeError, RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, + CompletionErrorBridge, CompletionResult, DataIntegrityError, RuntimeError, RuntimeOrFatalError, + RuntimeOrFatalResult, RuntimeResult, }; use crate::file::cow_file::SUPER_BLOCK_ID; use crate::id::{BlockID, RowID, TableID, TrxID}; @@ -10,10 +11,15 @@ use crate::index::{ ColumnBlockIndex, MemIndexEntry, NonUniqueMemIndex, ResolvedColumnRow, SecondaryIndex, UniqueMemIndex, }; -use crate::session::SessionOperationPin; -use crate::trx::TrxReadProof; +use crate::runtime::mandatory::PreparedExecution; +use crate::session::{ + AcceptedMaintenanceScope, MaintenanceExecutionSpec, PreparedMaintenanceExecution, + PreparedMaintenanceScope, +}; +use crate::trx::{Transaction, TrxReadProof}; use crate::value::Val; use error_stack::{Report, ResultExt}; +use std::fmt; use std::sync::Arc; /// Aggregate result for a full-scan user-table secondary MemIndex cleanup pass. @@ -177,67 +183,150 @@ enum DeleteOverlayProof { ColdRowValues(Vec), } -impl Table { - /// Full-scan cleanup for user-table secondary MemIndex entries. - /// - /// This pass removes only entries proven redundant or obsolete against one - /// captured table-file root. It never mutates DiskTree state, and it treats - /// missing delete proof as a retention decision for delete overlays. - /// - /// When `clean_live_entries` is `true`, redundant live MemIndex entries are - /// removed only after the captured root is older than every active snapshot. - /// Otherwise live entries are retained and [`MemIndexCleanupOutcome::live_delay`] - /// reports the retry boundary. When `false`, live MemIndex cache entries are - /// retained by policy and no live delay is reported. Obsolete delete overlays - /// are cleaned independently in either case. - pub(crate) async fn cleanup_secondary_mem_indexes( - &self, - session: &SessionOperationPin, - clean_live_entries: bool, - ) -> RuntimeOrFatalResult { - let trx_sys = session.engine.trx_sys.clone(); - let pool_guards = session.pool_guards(); - loop { - let mut trx = session - .begin_private_trx() +#[derive(Clone, Copy, Debug)] +enum MemIndexCleanupPhase { + Starting, + TransactionActive, + Scanning, + RollingBack, + Finished, +} + +struct MemIndexCleanupPanicLabel(MemIndexCleanupPhase); + +impl fmt::Display for MemIndexCleanupPanicLabel { + #[inline] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "accepted secondary MemIndex cleanup panicked: phase={:?}", + self.0 + ) + } +} + +enum CleanupIteration { + Retry, + Finished(RuntimeResult), +} + +struct MemIndexCleanupResources { + active_trx: Option, + table: Arc
, + clean_live_entries: bool, +} + +struct MemIndexCleanupExecution; + +impl MaintenanceExecutionSpec for MemIndexCleanupExecution { + type Output = MemIndexCleanupOutcome; + type Resources = MemIndexCleanupResources; + type PanicLabel = MemIndexCleanupPanicLabel; + + const LABEL: &'static str = "cleanup_secondary_mem_indexes"; + + async fn execute( + scope: &mut AcceptedMaintenanceScope, + resources: &mut Self::Resources, + panic_label: &mut Self::PanicLabel, + ) -> CompletionResult { + let result = execute_mem_index_cleanup_inner(scope, resources, &mut panic_label.0) + .await + .map_err(CompletionErrorBridge::capture_runtime_or_fatal); + scope.mark_terminal_ready(); + panic_label.0 = MemIndexCleanupPhase::Finished; + result + } +} + +/// Prepare secondary MemIndex cleanup for an exact live table runtime. +pub(crate) fn prepare_mem_index_cleanup_operation( + scope: PreparedMaintenanceScope, + table: Arc
, + clean_live_entries: bool, +) -> impl PreparedExecution { + let table_id = table.table_id(); + PreparedMaintenanceExecution::::table( + scope, + MemIndexCleanupResources { + active_trx: None, + table, + clean_live_entries, + }, + MemIndexCleanupPanicLabel(MemIndexCleanupPhase::Starting), + table_id, + ) +} + +/// Full-scan cleanup for user-table secondary MemIndex entries. +/// +/// This pass removes only entries proven redundant or obsolete against one +/// captured table-file root. It never mutates DiskTree state, and it treats +/// missing delete proof as a retention decision for delete overlays. +/// +/// When `clean_live_entries` is `true`, redundant live MemIndex entries are +/// removed only after the captured root is older than every active snapshot. +/// Otherwise live entries are retained and [`MemIndexCleanupOutcome::live_delay`] +/// reports the retry boundary. When `false`, live MemIndex cache entries are +/// retained by policy and no live delay is reported. Obsolete delete overlays +/// are cleaned independently in either case. +async fn execute_mem_index_cleanup_inner( + scope: &mut AcceptedMaintenanceScope, + resources: &mut MemIndexCleanupResources, + phase: &mut MemIndexCleanupPhase, +) -> RuntimeOrFatalResult { + let table = Arc::clone(&resources.table); + let clean_live_entries = resources.clean_live_entries; + let trx_sys = scope.engine().trx_sys.clone(); + let pool_guards = scope.pool_guards(); + loop { + let trx = scope + .begin_private_trx() + .change_context(RuntimeError::TableAccess) + .attach_with(|| { + format!( + "operation=cleanup_secondary_mem_indexes, table_id={}, phase=begin_transaction", + table.table_id() + ) + })?; + resources.active_trx = Some(trx); + *phase = MemIndexCleanupPhase::TransactionActive; + let cleanup_sts = resources + .active_trx + .as_ref() + .unwrap_or_else(|| panic!("cleanup transaction disappeared after installation")) + .sts(); + let min_active_sts = trx_sys.calc_min_active_sts_for_gc(); + #[cfg(test)] + scope + .engine() + .maintenance_test + .run_cleanup_after_trx_start_hook() + .await; + *phase = MemIndexCleanupPhase::Scanning; + let iteration = { + let trx = resources + .active_trx + .as_mut() + .unwrap_or_else(|| panic!("cleanup transaction disappeared before checkout")); + let checkout = trx + .checkout() .change_context(RuntimeError::TableAccess) .attach_with(|| { format!( - "operation=cleanup_secondary_mem_indexes, table_id={}, phase=begin_transaction", - self.table_id() + "operation=cleanup_secondary_mem_indexes, table_id={}, phase=checkout_transaction", + table.table_id() ) }) .map_err(RuntimeOrFatalError::from)?; - let cleanup_sts = trx.sts(); - let min_active_sts = trx_sys.calc_min_active_sts_for_gc(); - #[cfg(test)] - tests::run_test_cleanup_after_trx_start_hook().await; - let cleanup_res = { - let checkout = trx - .checkout() - .change_context(RuntimeError::TableAccess) - .attach_with(|| { - format!( - "operation=cleanup_secondary_mem_indexes, table_id={}, phase=checkout_transaction", - self.table_id() - ) - }) - .map_err(RuntimeOrFatalError::from)?; - let proof = checkout.inner().ctx().read_proof(); - let snapshot = self.capture_mem_index_cleanup_snapshot(min_active_sts, &proof); - if !snapshot.is_visible_to(cleanup_sts) { - drop(snapshot); - drop(checkout); - trx.rollback_table_maintenance().await?; - // The captured root was published after this transaction - // started, so retry with a fresh STS. Transaction starts and - // root fences share one monotonic timestamp source, making - // the next STS newer than this fence unless another root - // publication races again. The awaited rollback above keeps - // this retry from becoming a tight busy loop. - continue; - } - let cleanup_res = self + let proof = checkout.inner().ctx().read_proof(); + let snapshot = table.capture_mem_index_cleanup_snapshot(min_active_sts, &proof); + if !snapshot.is_visible_to(cleanup_sts) { + drop(snapshot); + drop(checkout); + CleanupIteration::Retry + } else { + let cleanup_res = table .cleanup_secondary_mem_indexes_at_snapshot( &pool_guards, &snapshot, @@ -246,13 +335,34 @@ impl Table { .await; drop(snapshot); drop(checkout); - cleanup_res - }; - let rollback_res = trx.rollback_table_maintenance().await; - return finish_secondary_mem_index_cleanup(cleanup_res, rollback_res); + CleanupIteration::Finished(cleanup_res) + } + }; + *phase = MemIndexCleanupPhase::RollingBack; + let trx = resources + .active_trx + .take() + .unwrap_or_else(|| panic!("cleanup transaction missing before rollback")); + let rollback_res = trx.rollback_table_maintenance().await; + match iteration { + CleanupIteration::Retry => { + rollback_res?; + // This retry is intentionally unbounded. The captured root was + // published after the transaction started, so retry with a + // fresh STS. Transaction starts and root fences share one + // monotonic timestamp source, making the next STS newer than + // this fence unless another root publication races again. The + // awaited rollback above keeps retries from becoming a tight + // busy loop. + } + CleanupIteration::Finished(cleanup_res) => { + return finish_secondary_mem_index_cleanup(cleanup_res, rollback_res); + } } } +} +impl Table { #[inline] fn capture_mem_index_cleanup_snapshot<'ctx>( &self, @@ -706,54 +816,33 @@ mod tests { use super::finish_secondary_mem_index_cleanup; use crate::catalog::IndexNo; use crate::catalog::tests::wait_for_dropped_table_floor; + use crate::engine::Engine; use crate::error::{DataIntegrityError, LifecycleError, RuntimeError, RuntimeOrFatalError}; use crate::id::{RowID, TrxID}; use crate::index::IndexMask; - use crate::session::Session; use crate::session::tests::{ SessionTestExt, assert_checkpoint_published, wait_for_checkpoint_purge, + wait_for_session_idle, }; use crate::table::CheckpointOutcome; use crate::table::persistence::test_hooks::set_test_checkpoint_after_trx_start_hook; use crate::table::tests::*; - use crate::trx::{MAX_SNAPSHOT_TS, Transaction}; + use crate::trx::MAX_SNAPSHOT_TS; use crate::value::Val; use error_stack::Report; - use std::cell::{Cell, RefCell}; use std::future::Future; - use std::pin::Pin; - use std::rc::Rc; + use std::sync::Arc; use tempfile::TempDir; - type CleanupAfterTrxStartHook = - Box Pin + 'static>> + 'static>; - - thread_local! { - static TEST_CLEANUP_AFTER_TRX_START_HOOK: - RefCell> = RefCell::new(None); - } - - fn set_test_cleanup_after_trx_start_hook(hook: F) + fn set_test_cleanup_after_trx_start_hook(engine: &Engine, hook: F) where - F: FnOnce() -> Fut + 'static, - Fut: Future + 'static, + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, { - TEST_CLEANUP_AFTER_TRX_START_HOOK.with(|slot| { - let old = slot - .borrow_mut() - .replace(Box::new(move || Box::pin(hook()))); - assert!( - old.is_none(), - "MemIndex cleanup transaction-start hook already installed" - ); - }); - } - - pub(super) async fn run_test_cleanup_after_trx_start_hook() { - let hook = TEST_CLEANUP_AFTER_TRX_START_HOOK.with(|slot| slot.borrow_mut().take()); - if let Some(hook) = hook { - hook().await; - } + engine + .inner() + .maintenance_test + .install_cleanup_after_trx_start_hook(hook); } #[test] @@ -794,7 +883,7 @@ mod tests { ); let mut checkpoint_session = engine.new_session().unwrap(); - set_test_cleanup_after_trx_start_hook(move || async move { + set_test_cleanup_after_trx_start_hook(&engine, move || async move { assert_checkpoint_published(&mut checkpoint_session, table_id).await; }); @@ -811,7 +900,7 @@ mod tests { } #[test] - fn test_secondary_mem_index_cleanup_scoped_access_blocks_drop() { + fn test_dropped_mem_index_cleanup_observer_still_blocks_drop_until_terminal() { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let engine = @@ -820,7 +909,7 @@ mod tests { let mut cleanup_session = engine.new_session().unwrap(); let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); - set_test_cleanup_after_trx_start_hook(move || async move { + set_test_cleanup_after_trx_start_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -829,13 +918,15 @@ mod tests { Box::pin(cleanup_session.cleanup_secondary_mem_indexes(table_id, true)); assert!(futures::poll!(cleanup.as_mut()).is_pending()); entered_rx.recv_async().await.unwrap(); + drop(cleanup); let mut drop_session = engine.new_session().unwrap(); let mut drop_table = Box::pin(drop_session.drop_table(table_id)); assert!(futures::poll!(drop_table.as_mut()).is_pending()); release_tx.send_async(()).await.unwrap(); - cleanup.await.unwrap(); + wait_for_session_idle(&engine.inner().session_registry, cleanup_session.id()).await; + assert!(!cleanup_session.in_trx().unwrap()); drop_table.await.unwrap(); wait_for_dropped_table_floor(&engine, table_id).await; @@ -876,17 +967,16 @@ mod tests { .0; assert_freeze_created(session.freeze_table(table_id, usize::MAX).await.unwrap()); - let reader_holder: Rc>> = - Rc::new(RefCell::new(None)); - let reader_sts = Rc::new(Cell::new(TrxID::new(0))); - let hook_reader_holder = Rc::clone(&reader_holder); - let hook_reader_sts = Rc::clone(&reader_sts); + let reader_holder = Arc::new(parking_lot::Mutex::new(None)); + let reader_sts = Arc::new(parking_lot::Mutex::new(TrxID::new(0))); + let hook_reader_holder = Arc::clone(&reader_holder); + let hook_reader_sts = Arc::clone(&reader_sts); let hook_engine = engine.new_ref().unwrap(); - set_test_checkpoint_after_trx_start_hook(move || async move { + set_test_checkpoint_after_trx_start_hook(&engine, move || async move { let mut reader_session = hook_engine.new_session().unwrap(); let reader = reader_session.begin_trx().unwrap(); - hook_reader_sts.set(reader.sts()); - *hook_reader_holder.borrow_mut() = Some((reader_session, reader)); + *hook_reader_sts.lock() = reader.sts(); + *hook_reader_holder.lock() = Some((reader_session, reader)); }); let checkpoint = session.checkpoint_table(table_id).await.unwrap(); @@ -895,8 +985,8 @@ mod tests { }; let published_root = table.file().active_root_unchecked().clone(); let effective_ts = published_root.effective_ts(); - assert!(checkpoint_ts < reader_sts.get()); - assert!(reader_sts.get() < effective_ts); + assert!(checkpoint_ts < *reader_sts.lock()); + assert!(*reader_sts.lock() < effective_ts); let delayed = session .cleanup_secondary_mem_indexes(table_id, true) @@ -907,7 +997,7 @@ mod tests { .expect("old root reader must delay live-entry cleanup"); assert_eq!(delay.table_id, table_id); assert_eq!(delay.effective_ts, effective_ts); - assert!(delay.min_active_sts <= reader_sts.get()); + assert!(delay.min_active_sts <= *reader_sts.lock()); assert_eq!(delayed.stats.indexes.len(), 2); for index_stats in &delayed.stats.indexes { assert_eq!(index_stats.scanned, 0); @@ -932,7 +1022,7 @@ mod tests { assert_eq!(old_root_rows, vec![row_id]); let (_, reader) = reader_holder - .borrow_mut() + .lock() .take() .expect("checkpoint hook must retain the old-root reader"); reader.commit().await.unwrap(); diff --git a/doradb-storage/src/table/lifecycle.rs b/doradb-storage/src/table/lifecycle.rs index 1a6bb1b7..c79a298c 100644 --- a/doradb-storage/src/table/lifecycle.rs +++ b/doradb-storage/src/table/lifecycle.rs @@ -1,9 +1,11 @@ +use super::Table; use crate::error::{OperationError, OperationResult}; use crate::id::TableID; use error_stack::Report; use event_listener::{Event, EventListener, listener}; use std::fmt::{Debug, Formatter, Result as FmtResult}; use std::result::Result as StdResult; +use std::sync::Arc; use std::sync::atomic::{AtomicU32, Ordering}; const TERMINAL_MASK: u32 = 0b11; @@ -458,11 +460,8 @@ impl TableLifecycle { } } - /// Attempts to enter the checkpoint root-mutation section. - #[inline] - pub(crate) fn try_begin_checkpoint_root_mutation( - &self, - ) -> StdResult, CheckpointCancelReason> { + /// Acquires checkpoint root-mutation admission without borrowing a lease. + fn try_acquire_checkpoint_root_mutation(&self) -> StdResult<(), CheckpointCancelReason> { loop { let (raw, state) = self.inspect_state("begin checkpoint root mutation enter"); check_terminal_for_checkpoint(state.terminal)?; @@ -477,7 +476,7 @@ impl TableLifecycle { next.root_mutation_active = true; next.debug_assert_valid("begin checkpoint root mutation exit"); if self.compare_exchange_state(raw, next) { - return Ok(TableCheckpointRootMutationLease { lifecycle: self }); + return Ok(()); } } } @@ -660,23 +659,42 @@ impl Drop for CheckpointPublishLease<'_> { } } -/// RAII guard for checkpoint table-root mutation before root publication. -pub(crate) struct TableCheckpointRootMutationLease<'a> { - lifecycle: &'a TableLifecycle, +/// Lifetime-free checkpoint root-mutation authority. +/// +/// The retained table keeps the lifecycle owner alive until admission is +/// released, allowing this scope to cross mandatory acceptance. +pub(crate) struct TableCheckpointRootMutationScope { + table: Arc
, + active: bool, } -impl Debug for TableCheckpointRootMutationLease<'_> { +impl TableCheckpointRootMutationScope { + /// Attempt to acquire root-mutation authority for an owned table runtime. + pub(crate) fn acquire(table: Arc
) -> StdResult { + table.lifecycle.try_acquire_checkpoint_root_mutation()?; + Ok(Self { + table, + active: true, + }) + } +} + +impl Debug for TableCheckpointRootMutationScope { #[inline] fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { - f.debug_struct("TableCheckpointRootMutationLease") - .finish_non_exhaustive() + f.debug_struct("TableCheckpointRootMutationScope") + .field("table_id", &self.table.table_id()) + .finish() } } -impl Drop for TableCheckpointRootMutationLease<'_> { +impl Drop for TableCheckpointRootMutationScope { #[inline] fn drop(&mut self) { - self.lifecycle.release_checkpoint_root_mutation(); + if self.active { + self.active = false; + self.table.lifecycle.release_checkpoint_root_mutation(); + } } } @@ -752,7 +770,7 @@ mod tests { fn test_drop_gate_waits_for_active_publish_lease_before_completing() { smol::block_on(async { let lifecycle = Arc::new(TableLifecycle::new()); - let root_lease = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); + lifecycle.try_acquire_checkpoint_root_mutation().unwrap(); let lease = lifecycle.try_begin_checkpoint_publish().unwrap(); let drain = lifecycle.start_drop(TABLE_ID).unwrap(); let mut drop_fut = Box::pin(drain.wait()); @@ -766,8 +784,8 @@ mod tests { Ok(_lease) => panic!("publish lease should be blocked by drop gate"), Err(reason) => assert_eq!(reason, CheckpointCancelReason::TableDropping), } - match lifecycle.try_begin_checkpoint_root_mutation() { - Ok(_lease) => panic!("root mutation should be blocked by drop gate"), + match lifecycle.try_acquire_checkpoint_root_mutation() { + Ok(()) => panic!("root mutation should be blocked by drop gate"), Err(reason) => assert_eq!(reason, CheckpointCancelReason::TableDropping), } let err = lifecycle @@ -782,7 +800,7 @@ mod tests { drop(lease); drop_fut.await; assert_eq!(lifecycle.inspect_terminal(), TableTerminal::Dropping); - drop(root_lease); + lifecycle.release_checkpoint_root_mutation(); }); } @@ -791,7 +809,7 @@ mod tests { fn test_mark_dropped_requires_drained_publish_gate() { smol::block_on(async { let lifecycle = Arc::new(TableLifecycle::new()); - let root_lease = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); + lifecycle.try_acquire_checkpoint_root_mutation().unwrap(); let publish_lease = lifecycle.try_begin_checkpoint_publish().unwrap(); let drain = lifecycle.start_drop(TABLE_ID).unwrap(); let mut drop_fut = Box::pin(drain.wait()); @@ -801,7 +819,7 @@ mod tests { std::task::Poll::Pending )); lifecycle.mark_dropped(TABLE_ID); - drop((publish_lease, drop_fut, root_lease)); + drop((publish_lease, drop_fut)); }); } @@ -847,56 +865,59 @@ mod tests { let lifecycle = TableLifecycle::new(); lifecycle.acquire_metadata_change(TABLE_ID).await.unwrap(); - match lifecycle.try_begin_checkpoint_root_mutation() { - Ok(_lease) => panic!("checkpoint root mutation should be cancelled"), + match lifecycle.try_acquire_checkpoint_root_mutation() { + Ok(()) => panic!("checkpoint root mutation should be cancelled"), Err(reason) => assert_eq!(reason, CheckpointCancelReason::TableMetadataChanging), } lifecycle.release_metadata_change(); - let _root_lease = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); + lifecycle.try_acquire_checkpoint_root_mutation().unwrap(); + lifecycle.release_checkpoint_root_mutation(); }); } #[test] fn test_active_checkpoint_root_mutation_blocks_concurrent_checkpoint() { let lifecycle = TableLifecycle::new(); - let root_lease = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); + lifecycle.try_acquire_checkpoint_root_mutation().unwrap(); - match lifecycle.try_begin_checkpoint_root_mutation() { - Ok(_lease) => panic!("concurrent checkpoint root mutation should be cancelled"), + match lifecycle.try_acquire_checkpoint_root_mutation() { + Ok(()) => panic!("concurrent checkpoint root mutation should be cancelled"), Err(reason) => assert_eq!(reason, CheckpointCancelReason::CheckpointInProgress), } - drop(root_lease); - let _root_lease = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); + lifecycle.release_checkpoint_root_mutation(); + lifecycle.try_acquire_checkpoint_root_mutation().unwrap(); + lifecycle.release_checkpoint_root_mutation(); } #[test] fn test_metadata_change_waits_for_active_checkpoint_root_mutation() { smol::block_on(async { let lifecycle = TableLifecycle::new(); - let root_lease = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); + lifecycle.try_acquire_checkpoint_root_mutation().unwrap(); let mut metadata_fut = Box::pin(lifecycle.acquire_metadata_change(TABLE_ID)); assert!(matches!( futures::poll!(metadata_fut.as_mut()), std::task::Poll::Pending )); - match lifecycle.try_begin_checkpoint_root_mutation() { - Ok(_lease) => panic!("pending metadata change should block new checkpoint roots"), + match lifecycle.try_acquire_checkpoint_root_mutation() { + Ok(()) => panic!("pending metadata change should block new checkpoint roots"), Err(reason) => assert_eq!(reason, CheckpointCancelReason::TableMetadataChanging), } let publish_lease = lifecycle.try_begin_checkpoint_publish().unwrap(); drop(publish_lease); - drop(root_lease); + lifecycle.release_checkpoint_root_mutation(); metadata_fut.await.unwrap(); - match lifecycle.try_begin_checkpoint_root_mutation() { - Ok(_lease) => panic!("active metadata change should block checkpoint roots"), + match lifecycle.try_acquire_checkpoint_root_mutation() { + Ok(()) => panic!("active metadata change should block checkpoint roots"), Err(reason) => assert_eq!(reason, CheckpointCancelReason::TableMetadataChanging), } lifecycle.release_metadata_change(); - let _root_lease = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); + lifecycle.try_acquire_checkpoint_root_mutation().unwrap(); + lifecycle.release_checkpoint_root_mutation(); }); } @@ -904,7 +925,7 @@ mod tests { fn test_pending_metadata_change_cancellation_reopens_checkpoint_root_mutation() { smol::block_on(async { let lifecycle = TableLifecycle::new(); - let root_lease = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); + lifecycle.try_acquire_checkpoint_root_mutation().unwrap(); let mut metadata_fut = Box::pin(lifecycle.acquire_metadata_change(TABLE_ID)); assert!(matches!( @@ -913,8 +934,9 @@ mod tests { )); drop(metadata_fut); - drop(root_lease); - let _root_lease = lifecycle.try_begin_checkpoint_root_mutation().unwrap(); + lifecycle.release_checkpoint_root_mutation(); + lifecycle.try_acquire_checkpoint_root_mutation().unwrap(); + lifecycle.release_checkpoint_root_mutation(); }); } diff --git a/doradb-storage/src/table/mod.rs b/doradb-storage/src/table/mod.rs index 5cf4f4c8..c2059972 100644 --- a/doradb-storage/src/table/mod.rs +++ b/doradb-storage/src/table/mod.rs @@ -15,9 +15,10 @@ mod storage; pub use access::LazyRow; pub(crate) use access::*; pub use checkpoint_workflow::{FreezeOutcome, FrozenPageBatchInfo}; -use checkpoint_workflow::{FrozenPage, FrozenPageBatch, TableCheckpointWorkflow}; +use checkpoint_workflow::{FrozenPage, TableCheckpointWorkflow}; pub(crate) use deletion_buffer::*; pub(crate) use dml_validator::*; +pub(crate) use gc::prepare_mem_index_cleanup_operation; pub use gc::{ MemIndexCleanupDelay, MemIndexCleanupOutcome, MemIndexCleanupStats, SecondaryMemIndexCleanupIndexStats, @@ -26,7 +27,7 @@ pub(crate) use layout::{RetiredSecondaryIndex, TableRuntimeLayout}; pub use lifecycle::CheckpointCancelReason; #[cfg(test)] pub(crate) use lifecycle::TableTerminal; -pub(crate) use lifecycle::{TableCheckpointRootMutationLease, TableDropDrain, TableLifecycle}; +pub(crate) use lifecycle::{TableDropDrain, TableLifecycle}; pub(crate) use mem_table::{MemTable, NoTrxUpsertChange, RowPageDescriptor}; pub use persistence::*; pub(crate) use rollback::IndexRollback; @@ -58,7 +59,6 @@ use error_stack::{Report, ResultExt}; use parking_lot::Mutex; use std::marker::PhantomData; use std::mem::take; -use std::result::Result as StdResult; use std::sync::Arc; /// Copied replay floor fields from one user-table active root. @@ -192,14 +192,6 @@ impl Table { self.lifecycle.release_metadata_change(); } - /// Attempts to enter the checkpoint table-root mutation section. - #[inline] - pub(crate) fn try_begin_checkpoint_root_mutation( - &self, - ) -> StdResult, CheckpointCancelReason> { - self.lifecycle.try_begin_checkpoint_root_mutation() - } - /// Starts terminal drop admission and closes the checkpoint workflow. #[inline] pub(crate) fn start_drop_lifecycle(&self) -> OperationResult> { @@ -1140,7 +1132,7 @@ fn unique_key_from_full_row( #[cfg(test)] pub(crate) mod tests { - use super::lifecycle::CheckpointPublishLease; + use super::lifecycle::{CheckpointPublishLease, TableCheckpointRootMutationScope}; use crate::buffer::page::PAGE_SIZE; use crate::buffer::{PoolGuard, PoolGuards, PoolRole, ReadonlyBufferPool}; use crate::catalog::tests::table2; @@ -1173,231 +1165,627 @@ pub(crate) mod tests { use crate::session::{Session, tests::SessionTestExt}; use crate::table::{ DeleteMarker, DmlValidationError, FreezeOutcome, FrozenPageBatchInfo, Table, - TableCheckpointRootMutationLease, TableRuntimeLayout, + TableRuntimeLayout, }; use crate::trx::Transaction; use crate::trx::stmt::Statement; use crate::value::{Val, ValKind}; use smol::Timer; use std::fs::OpenOptions; + use std::future::Future; use std::io::{Error as IoError, Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; + use std::pin::Pin; use std::sync::Arc; - use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::time::Duration; use tempfile::TempDir; - pub(crate) mod test_hooks { - use crate::error::{InternalError, RuntimeError, RuntimeResult}; - use crate::id::PageID; - use error_stack::Report; - use std::cell::{Cell, RefCell}; - - type FreezePageHook = Box; - type FrozenPageScanHook = Box; - type FrozenPageRowScanHook = Box; - type OptimisticPagePlanComparisonHook = Box; - type FrozenPagePhaseHook = Box; - type TransitionPageHook = Box; - type HotRowWriteHook = Box; + type MaintenanceAsyncHook = + Box Pin + Send + 'static>> + Send + 'static>; + type MaintenanceFallibleAsyncHook = Box< + dyn FnOnce() -> Pin> + Send + 'static>> + + Send + + 'static, + >; + pub(crate) type RedoCleanupBeforeUnlinkHook = Arc; + type FreezePageHook = Box; + type FrozenPageScanHook = Box; + type FrozenPageRowScanHook = Box; + type OptimisticPagePlanComparisonHook = Box; + type FrozenPagePhaseHook = Box; + type TransitionPageHook = Box; + + #[derive(Default)] + struct MaintenanceTestState { + force_secondary_sidecar_error: AtomicBool, + force_post_publish_checkpoint_error: AtomicBool, + force_checkpoint_commit_error: AtomicBool, + freeze_after_loading_hook: parking_lot::Mutex>, + checkpoint_after_trx_start_hook: parking_lot::Mutex>, + checkpoint_after_publish_admission_hook: parking_lot::Mutex>, + checkpoint_retry_after_listener_registration_hook: + parking_lot::Mutex>, + silent_watermark_mutation_hook: parking_lot::Mutex>, + cleanup_after_trx_start_hook: parking_lot::Mutex>, + redo_cleanup_before_unlink_hook: parking_lot::Mutex>, + force_lwc_build_error: AtomicBool, + freeze_page_state_locked_hook: parking_lot::Mutex>, + frozen_page_scan_hook: parking_lot::Mutex>, + frozen_page_row_scan_hook: parking_lot::Mutex>, + optimistic_page_plan_comparison_hook: + parking_lot::Mutex>, + frozen_pages_ready_hook: parking_lot::Mutex>, + stable_page_plans_refreshed_hook: parking_lot::Mutex>, + locked_page_plan_rebuild_hook: parking_lot::Mutex>, + transition_page_published_hook: parking_lot::Mutex>, + } + + /// Per-engine maintenance fault and phase controller shared across runners. + #[derive(Clone, Default)] + pub(crate) struct MaintenanceTestController { + state: Arc, + } + + impl MaintenanceTestController { + #[inline] + pub(crate) fn set_force_secondary_sidecar_error(&self, enabled: bool) { + self.state + .force_secondary_sidecar_error + .store(enabled, Ordering::Relaxed); + } - thread_local! { - static TEST_FORCE_LWC_BUILD_ERROR: Cell = const { Cell::new(false) }; - static TEST_FREEZE_PAGE_STATE_LOCKED_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_FROZEN_PAGE_SCAN_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_FROZEN_PAGE_ROW_SCAN_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_OPTIMISTIC_PAGE_PLAN_COMPARISON_HOOK: - RefCell> = const { RefCell::new(None) }; - static TEST_FROZEN_PAGES_READY_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_STABLE_PAGE_PLANS_REFRESHED_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_LOCKED_PAGE_PLAN_REBUILD_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_TRANSITION_PAGE_PUBLISHED_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_HOT_ROW_WRITE_BEFORE_STATE_LOCK_HOOK: RefCell> = - const { RefCell::new(None) }; + #[inline] + pub(crate) fn force_secondary_sidecar_error(&self) -> bool { + self.state + .force_secondary_sidecar_error + .load(Ordering::Relaxed) } - pub(crate) fn set_test_force_lwc_build_error(enabled: bool) { - TEST_FORCE_LWC_BUILD_ERROR.with(|flag| flag.set(enabled)); + #[inline] + pub(crate) fn set_force_post_publish_checkpoint_error(&self, enabled: bool) { + self.state + .force_post_publish_checkpoint_error + .store(enabled, Ordering::Relaxed); } - pub(crate) struct ForceLwcBuildErrorGuard; + #[inline] + pub(crate) fn force_post_publish_checkpoint_error(&self) -> bool { + self.state + .force_post_publish_checkpoint_error + .load(Ordering::Relaxed) + } - impl ForceLwcBuildErrorGuard { - pub(crate) fn new() -> Self { - set_test_force_lwc_build_error(true); - Self + #[inline] + pub(crate) fn set_force_checkpoint_commit_error(&self, enabled: bool) { + self.state + .force_checkpoint_commit_error + .store(enabled, Ordering::Relaxed); + } + + #[inline] + pub(crate) fn force_checkpoint_commit_error(&self) -> bool { + self.state + .force_checkpoint_commit_error + .load(Ordering::Relaxed) + } + + pub(crate) fn install_freeze_after_loading_hook(&self, hook: F) + where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, + { + let old = self + .state + .freeze_after_loading_hook + .lock() + .replace(Box::new(move || Box::pin(hook()))); + assert!(old.is_none(), "freeze loading hook already installed"); + } + + pub(crate) fn install_checkpoint_after_trx_start_hook(&self, hook: F) + where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, + { + let old = self + .state + .checkpoint_after_trx_start_hook + .lock() + .replace(Box::new(move || Box::pin(hook()))); + assert!( + old.is_none(), + "checkpoint transaction-start hook already installed" + ); + } + + pub(crate) fn install_checkpoint_after_publish_admission_hook(&self, hook: F) + where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, + { + let old = self + .state + .checkpoint_after_publish_admission_hook + .lock() + .replace(Box::new(move || Box::pin(hook()))); + assert!( + old.is_none(), + "checkpoint publish-admission hook already installed" + ); + } + + pub(crate) fn install_checkpoint_retry_after_listener_registration_hook( + &self, + hook: F, + ) where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, + { + let old = self + .state + .checkpoint_retry_after_listener_registration_hook + .lock() + .replace(Box::new(move || Box::pin(hook()))); + assert!( + old.is_none(), + "checkpoint retry listener-registration hook already installed" + ); + } + + pub(crate) fn install_silent_watermark_mutation_hook(&self, hook: F) + where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future> + Send + 'static, + { + let old = self + .state + .silent_watermark_mutation_hook + .lock() + .replace(Box::new(move || Box::pin(hook()))); + assert!( + old.is_none(), + "silent watermark mutation hook already installed" + ); + } + + pub(crate) fn install_cleanup_after_trx_start_hook(&self, hook: F) + where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, + { + let old = self + .state + .cleanup_after_trx_start_hook + .lock() + .replace(Box::new(move || Box::pin(hook()))); + assert!( + old.is_none(), + "MemIndex cleanup transaction-start hook already installed" + ); + } + + pub(crate) async fn run_freeze_after_loading_hook(&self) { + let hook = self.state.freeze_after_loading_hook.lock().take(); + if let Some(hook) = hook { + hook().await; } } - impl Drop for ForceLwcBuildErrorGuard { - fn drop(&mut self) { - set_test_force_lwc_build_error(false); + pub(crate) async fn run_checkpoint_after_trx_start_hook(&self) { + let hook = self.state.checkpoint_after_trx_start_hook.lock().take(); + if let Some(hook) = hook { + hook().await; } } - pub(crate) fn maybe_force_lwc_build_error() -> RuntimeResult<()> { - if TEST_FORCE_LWC_BUILD_ERROR.with(|flag| flag.get()) { - return Err(Report::new(InternalError::LwcBuilderMisuse) - .attach("test LWC build failure") - .change_context(RuntimeError::TableAccess)); + pub(crate) async fn run_checkpoint_after_publish_admission_hook(&self) { + let hook = self + .state + .checkpoint_after_publish_admission_hook + .lock() + .take(); + if let Some(hook) = hook { + hook().await; + } + } + + pub(crate) async fn run_checkpoint_retry_after_listener_registration_hook(&self) { + let hook = self + .state + .checkpoint_retry_after_listener_registration_hook + .lock() + .take(); + if let Some(hook) = hook { + hook().await; + } + } + + pub(crate) async fn run_silent_watermark_mutation_hook(&self) -> RuntimeResult<()> { + let hook = self.state.silent_watermark_mutation_hook.lock().take(); + match hook { + Some(hook) => hook().await, + None => Ok(()), + } + } + + pub(crate) async fn run_cleanup_after_trx_start_hook(&self) { + let hook = self.state.cleanup_after_trx_start_hook.lock().take(); + if let Some(hook) = hook { + hook().await; + } + } + + pub(crate) fn install_redo_cleanup_before_unlink_hook( + &self, + hook: RedoCleanupBeforeUnlinkHook, + ) { + let mut slot = self.state.redo_cleanup_before_unlink_hook.lock(); + assert!( + slot.is_none(), + "redo cleanup before-unlink hook already installed" + ); + *slot = Some(hook); + } + + pub(crate) fn clear_redo_cleanup_before_unlink_hook(&self) { + self.state.redo_cleanup_before_unlink_hook.lock().take(); + } + + pub(crate) fn run_redo_cleanup_before_unlink_hook(&self, file_seq: u32, path: &Path) { + let hook = self.state.redo_cleanup_before_unlink_hook.lock().clone(); + if let Some(hook) = hook { + hook(file_seq, path); } - Ok(()) } - pub(crate) fn set_test_freeze_page_state_locked_hook(hook: F) + pub(crate) fn set_force_lwc_build_error(&self, enabled: bool) { + self.state + .force_lwc_build_error + .store(enabled, Ordering::Relaxed); + } + + pub(crate) fn force_lwc_build_error(&self) -> bool { + self.state.force_lwc_build_error.load(Ordering::Relaxed) + } + + pub(crate) fn install_freeze_page_state_locked_hook(&self, hook: F) where - F: FnOnce(PageID) + 'static, + F: FnOnce(PageID) + Send + 'static, { - TEST_FREEZE_PAGE_STATE_LOCKED_HOOK.with(|slot| { - let old = slot.borrow_mut().replace(Box::new(hook)); - assert!(old.is_none(), "freeze page-state hook already installed"); - }); + let old = self + .state + .freeze_page_state_locked_hook + .lock() + .replace(Box::new(hook)); + assert!(old.is_none(), "freeze page-state hook already installed"); } - pub(crate) fn run_test_freeze_page_state_locked_hook(page_id: PageID) { - let hook = TEST_FREEZE_PAGE_STATE_LOCKED_HOOK.with(|slot| slot.borrow_mut().take()); + pub(crate) fn run_freeze_page_state_locked_hook(&self, page_id: PageID) { + let hook = self.state.freeze_page_state_locked_hook.lock().take(); if let Some(hook) = hook { hook(page_id); } } - pub(crate) fn set_test_locked_page_plan_rebuild_hook(hook: F) + pub(crate) fn install_locked_page_plan_rebuild_hook(&self, hook: F) where - F: FnOnce(PageID) + 'static, + F: FnOnce(PageID) + Send + 'static, { - TEST_LOCKED_PAGE_PLAN_REBUILD_HOOK.with(|slot| { - let old = slot.borrow_mut().replace(Box::new(hook)); - assert!(old.is_none(), "locked page-plan hook already installed"); - }); + let old = self + .state + .locked_page_plan_rebuild_hook + .lock() + .replace(Box::new(hook)); + assert!(old.is_none(), "locked page-plan hook already installed"); } - pub(crate) fn run_test_locked_page_plan_rebuild_hook(page_id: PageID) { - let hook = TEST_LOCKED_PAGE_PLAN_REBUILD_HOOK.with(|slot| slot.borrow_mut().take()); + pub(crate) fn run_locked_page_plan_rebuild_hook(&self, page_id: PageID) { + let hook = self.state.locked_page_plan_rebuild_hook.lock().take(); if let Some(hook) = hook { hook(page_id); } } - pub(crate) fn set_test_transition_page_published_hook(hook: F) + pub(crate) fn install_transition_page_published_hook(&self, hook: F) where - F: FnOnce(PageID) + 'static, + F: FnOnce(PageID) + Send + 'static, { - TEST_TRANSITION_PAGE_PUBLISHED_HOOK.with(|slot| { - let old = slot.borrow_mut().replace(Box::new(hook)); - assert!(old.is_none(), "transition page hook already installed"); - }); + let old = self + .state + .transition_page_published_hook + .lock() + .replace(Box::new(hook)); + assert!(old.is_none(), "transition page hook already installed"); + } + + pub(crate) fn run_transition_page_published_hook(&self, page_id: PageID) { + let hook = self.state.transition_page_published_hook.lock().take(); + if let Some(hook) = hook { + hook(page_id); + } } - pub(crate) fn set_test_frozen_page_scan_hook(hook: F) + pub(crate) fn install_frozen_page_scan_hook(&self, hook: F) where - F: FnMut(PageID) + 'static, + F: FnMut(PageID) + Send + 'static, { - TEST_FROZEN_PAGE_SCAN_HOOK.with(|slot| { - let old = slot.borrow_mut().replace(Box::new(hook)); - assert!(old.is_none(), "frozen-page scan hook already installed"); - }); + let old = self + .state + .frozen_page_scan_hook + .lock() + .replace(Box::new(hook)); + assert!(old.is_none(), "frozen-page scan hook already installed"); } - pub(crate) fn run_test_frozen_page_scan_hook(page_id: PageID) { - TEST_FROZEN_PAGE_SCAN_HOOK.with(|slot| { - if let Some(hook) = slot.borrow_mut().as_mut() { - hook(page_id); - } - }); + pub(crate) fn run_frozen_page_scan_hook(&self, page_id: PageID) { + if let Some(hook) = self.state.frozen_page_scan_hook.lock().as_mut() { + hook(page_id); + } } - pub(crate) fn set_test_frozen_page_row_scan_hook(hook: F) + pub(crate) fn install_frozen_page_row_scan_hook(&self, hook: F) where - F: FnMut(PageID, usize) + 'static, + F: FnMut(PageID, usize) + Send + 'static, { - TEST_FROZEN_PAGE_ROW_SCAN_HOOK.with(|slot| { - let old = slot.borrow_mut().replace(Box::new(hook)); - assert!(old.is_none(), "frozen-page row-scan hook already installed"); - }); + let old = self + .state + .frozen_page_row_scan_hook + .lock() + .replace(Box::new(hook)); + assert!(old.is_none(), "frozen-page row-scan hook already installed"); } - pub(crate) fn run_test_frozen_page_row_scan_hook(page_id: PageID, row_idx: usize) { - TEST_FROZEN_PAGE_ROW_SCAN_HOOK.with(|slot| { - if let Some(hook) = slot.borrow_mut().as_mut() { - hook(page_id, row_idx); - } - }); + pub(crate) fn run_frozen_page_row_scan_hook(&self, page_id: PageID, row_idx: usize) { + if let Some(hook) = self.state.frozen_page_row_scan_hook.lock().as_mut() { + hook(page_id, row_idx); + } } - pub(crate) fn set_test_optimistic_page_plan_comparison_hook(hook: F) + pub(crate) fn install_optimistic_page_plan_comparison_hook(&self, hook: F) where - F: FnMut(PageID, u64, u64, bool) + 'static, + F: FnMut(PageID, u64, u64, bool) + Send + 'static, { - TEST_OPTIMISTIC_PAGE_PLAN_COMPARISON_HOOK.with(|slot| { - let old = slot.borrow_mut().replace(Box::new(hook)); - assert!( - old.is_none(), - "optimistic page-plan comparison hook already installed" - ); - }); + let old = self + .state + .optimistic_page_plan_comparison_hook + .lock() + .replace(Box::new(hook)); + assert!( + old.is_none(), + "optimistic page-plan comparison hook already installed" + ); } - pub(crate) fn run_test_optimistic_page_plan_comparison_hook( + pub(crate) fn run_optimistic_page_plan_comparison_hook( + &self, page_id: PageID, version_before: u64, version_after: u64, retained: bool, ) { - TEST_OPTIMISTIC_PAGE_PLAN_COMPARISON_HOOK.with(|slot| { - if let Some(hook) = slot.borrow_mut().as_mut() { - hook(page_id, version_before, version_after, retained); - } - }); + if let Some(hook) = self + .state + .optimistic_page_plan_comparison_hook + .lock() + .as_mut() + { + hook(page_id, version_before, version_after, retained); + } } - pub(crate) fn set_test_frozen_pages_ready_hook(hook: F) + pub(crate) fn install_frozen_pages_ready_hook(&self, hook: F) where - F: FnOnce() + 'static, + F: FnOnce() + Send + 'static, { - TEST_FROZEN_PAGES_READY_HOOK.with(|slot| { - let old = slot.borrow_mut().replace(Box::new(hook)); - assert!(old.is_none(), "frozen-pages-ready hook already installed"); - }); + let old = self + .state + .frozen_pages_ready_hook + .lock() + .replace(Box::new(hook)); + assert!(old.is_none(), "frozen-pages-ready hook already installed"); } - pub(crate) fn run_test_frozen_pages_ready_hook() { - let hook = TEST_FROZEN_PAGES_READY_HOOK.with(|slot| slot.borrow_mut().take()); + pub(crate) fn run_frozen_pages_ready_hook(&self) { + let hook = self.state.frozen_pages_ready_hook.lock().take(); if let Some(hook) = hook { hook(); } } - pub(crate) fn set_test_stable_page_plans_refreshed_hook(hook: F) + pub(crate) fn install_stable_page_plans_refreshed_hook(&self, hook: F) where - F: FnOnce() + 'static, + F: FnOnce() + Send + 'static, { - TEST_STABLE_PAGE_PLANS_REFRESHED_HOOK.with(|slot| { - let old = slot.borrow_mut().replace(Box::new(hook)); - assert!(old.is_none(), "stable page-plans hook already installed"); - }); + let old = self + .state + .stable_page_plans_refreshed_hook + .lock() + .replace(Box::new(hook)); + assert!(old.is_none(), "stable page-plans hook already installed"); } - pub(crate) fn run_test_stable_page_plans_refreshed_hook() { - let hook = TEST_STABLE_PAGE_PLANS_REFRESHED_HOOK.with(|slot| slot.borrow_mut().take()); + pub(crate) fn run_stable_page_plans_refreshed_hook(&self) { + let hook = self.state.stable_page_plans_refreshed_hook.lock().take(); if let Some(hook) = hook { hook(); } } + } - pub(crate) fn run_test_transition_page_published_hook(page_id: PageID) { - let hook = TEST_TRANSITION_PAGE_PUBLISHED_HOOK.with(|slot| slot.borrow_mut().take()); - if let Some(hook) = hook { - hook(page_id); + pub(crate) mod test_hooks { + use super::MaintenanceTestController; + use crate::engine::Engine; + use crate::error::{InternalError, RuntimeError, RuntimeResult}; + use crate::id::PageID; + use error_stack::Report; + use std::cell::RefCell; + + type HotRowWriteHook = Box; + + thread_local! { + static TEST_HOT_ROW_WRITE_BEFORE_STATE_LOCK_HOOK: RefCell> = + const { RefCell::new(None) }; + } + + pub(crate) struct ForceLwcBuildErrorGuard { + test: MaintenanceTestController, + } + + impl ForceLwcBuildErrorGuard { + pub(crate) fn new(engine: &Engine) -> Self { + let test = engine.inner().maintenance_test.clone(); + test.set_force_lwc_build_error(true); + Self { test } } } + impl Drop for ForceLwcBuildErrorGuard { + fn drop(&mut self) { + self.test.set_force_lwc_build_error(false); + } + } + + pub(crate) fn maybe_force_lwc_build_error( + test: &MaintenanceTestController, + ) -> RuntimeResult<()> { + if test.force_lwc_build_error() { + return Err(Report::new(InternalError::LwcBuilderMisuse) + .attach("test LWC build failure") + .change_context(RuntimeError::TableAccess)); + } + Ok(()) + } + + pub(crate) fn run_test_freeze_page_state_locked_hook( + test: &MaintenanceTestController, + page_id: PageID, + ) { + test.run_freeze_page_state_locked_hook(page_id); + } + + pub(crate) fn set_test_locked_page_plan_rebuild_hook(engine: &Engine, hook: F) + where + F: FnOnce(PageID) + Send + 'static, + { + engine + .inner() + .maintenance_test + .install_locked_page_plan_rebuild_hook(hook); + } + + pub(crate) fn run_test_locked_page_plan_rebuild_hook( + test: &MaintenanceTestController, + page_id: PageID, + ) { + test.run_locked_page_plan_rebuild_hook(page_id); + } + + pub(crate) fn set_test_transition_page_published_hook(engine: &Engine, hook: F) + where + F: FnOnce(PageID) + Send + 'static, + { + engine + .inner() + .maintenance_test + .install_transition_page_published_hook(hook); + } + + pub(crate) fn set_test_frozen_page_scan_hook(engine: &Engine, hook: F) + where + F: FnMut(PageID) + Send + 'static, + { + engine + .inner() + .maintenance_test + .install_frozen_page_scan_hook(hook); + } + + pub(crate) fn run_test_frozen_page_scan_hook( + test: &MaintenanceTestController, + page_id: PageID, + ) { + test.run_frozen_page_scan_hook(page_id); + } + + pub(crate) fn set_test_frozen_page_row_scan_hook(engine: &Engine, hook: F) + where + F: FnMut(PageID, usize) + Send + 'static, + { + engine + .inner() + .maintenance_test + .install_frozen_page_row_scan_hook(hook); + } + + pub(crate) fn run_test_frozen_page_row_scan_hook( + test: &MaintenanceTestController, + page_id: PageID, + row_idx: usize, + ) { + test.run_frozen_page_row_scan_hook(page_id, row_idx); + } + + pub(crate) fn set_test_optimistic_page_plan_comparison_hook(engine: &Engine, hook: F) + where + F: FnMut(PageID, u64, u64, bool) + Send + 'static, + { + engine + .inner() + .maintenance_test + .install_optimistic_page_plan_comparison_hook(hook); + } + + pub(crate) fn run_test_optimistic_page_plan_comparison_hook( + test: &MaintenanceTestController, + page_id: PageID, + version_before: u64, + version_after: u64, + retained: bool, + ) { + test.run_optimistic_page_plan_comparison_hook( + page_id, + version_before, + version_after, + retained, + ); + } + + pub(crate) fn set_test_frozen_pages_ready_hook(engine: &Engine, hook: F) + where + F: FnOnce() + Send + 'static, + { + engine + .inner() + .maintenance_test + .install_frozen_pages_ready_hook(hook); + } + + pub(crate) fn run_test_frozen_pages_ready_hook(test: &MaintenanceTestController) { + test.run_frozen_pages_ready_hook(); + } + + pub(crate) fn set_test_stable_page_plans_refreshed_hook(engine: &Engine, hook: F) + where + F: FnOnce() + Send + 'static, + { + engine + .inner() + .maintenance_test + .install_stable_page_plans_refreshed_hook(hook); + } + + pub(crate) fn run_test_stable_page_plans_refreshed_hook(test: &MaintenanceTestController) { + test.run_stable_page_plans_refreshed_hook(); + } + + pub(crate) fn run_test_transition_page_published_hook( + test: &MaintenanceTestController, + page_id: PageID, + ) { + test.run_transition_page_published_hook(page_id); + } + pub(crate) fn set_test_hot_row_write_before_state_lock_hook(hook: F) where F: FnOnce() + 'static, @@ -1429,12 +1817,9 @@ pub(crate) mod tests { } pub(crate) fn begin_checkpoint_publish_for_test( - table: &Table, - ) -> ( - TableCheckpointRootMutationLease<'_>, - CheckpointPublishLease<'_>, - ) { - let root_lease = table.try_begin_checkpoint_root_mutation().unwrap(); + table: &Arc
, + ) -> (TableCheckpointRootMutationScope, CheckpointPublishLease<'_>) { + let root_lease = TableCheckpointRootMutationScope::acquire(Arc::clone(table)).unwrap(); let publish_lease = table.lifecycle.try_begin_checkpoint_publish().unwrap(); (root_lease, publish_lease) } diff --git a/doradb-storage/src/table/page_transition.rs b/doradb-storage/src/table/page_transition.rs index 243e1ec1..004d1e72 100644 --- a/doradb-storage/src/table/page_transition.rs +++ b/doradb-storage/src/table/page_transition.rs @@ -1,6 +1,9 @@ use super::checkpoint_workflow::{ - FreezeAttempt, FrozenPage, FrozenPageBatch, FrozenPageValidationState, PreparedTransitionPage, + FrozenPage, FrozenPageBatch, FrozenPageValidationState, PreparedFreezeAttempt, + PreparedTransitionPage, }; +#[cfg(test)] +use super::tests::MaintenanceTestController; use super::{DeleteMarker, Table}; use crate::bitmap::Bitmap; use crate::buffer::PoolGuards; @@ -126,7 +129,8 @@ impl Table { &self, page_guards: &[PageSharedGuard], pages: &[FrozenPage], - attempt: &mut FreezeAttempt<'_>, + attempt: &mut PreparedFreezeAttempt, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) -> bool { #[cfg(test)] use super::test_hooks; @@ -182,7 +186,7 @@ impl Table { page_info.page_id ); #[cfg(test)] - test_hooks::run_test_freeze_page_state_locked_hook(page_info.page_id); + test_hooks::run_test_freeze_page_state_locked_hook(maintenance_test, page_info.page_id); *state = RowPageState::Frozen; } true @@ -193,6 +197,7 @@ impl Table { page_guards: &[PageSharedGuard], batch: &mut FrozenPageBatch, cutoff_ts: TrxID, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) -> Option { #[cfg(test)] use super::test_hooks; @@ -205,18 +210,30 @@ impl Table { ); // Phase 1 advances the cached ready prefix only. A delayed attempt // returns before doing work on pages beyond its first blocker. - if let Some(delay) = validate_frozen_pages_incrementally(page_guards, batch, cutoff_ts) { + if let Some(delay) = validate_frozen_pages_incrementally( + page_guards, + batch, + cutoff_ts, + #[cfg(test)] + maintenance_test, + ) { return Some(delay); } #[cfg(test)] - test_hooks::run_test_frozen_pages_ready_hook(); + test_hooks::run_test_frozen_pages_ready_hook(maintenance_test); // Phase 2 refreshes cutoff- and version-specific plans for the entire // ready batch. Readiness is now monotonic: later frozen-page ownership // changes may invalidate plans but cannot reintroduce a cutoff delay. - refresh_frozen_page_plans_optimistically(page_guards, batch, cutoff_ts); + refresh_frozen_page_plans_optimistically( + page_guards, + batch, + cutoff_ts, + #[cfg(test)] + maintenance_test, + ); #[cfg(test)] - test_hooks::run_test_stable_page_plans_refreshed_hook(); + test_hooks::run_test_stable_page_plans_refreshed_hook(maintenance_test); None } @@ -227,6 +244,7 @@ impl Table { batch: &mut FrozenPageBatch, page_idx: usize, cutoff_ts: TrxID, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) -> FrozenPageValidationState { let page_info = batch.pages[page_idx]; match batch.validation[page_idx] { @@ -240,7 +258,14 @@ impl Table { state } FrozenPageValidationState::Unchecked | FrozenPageValidationState::Blocked { .. } => { - analyze_frozen_page_readiness(page_guard, batch, page_idx, cutoff_ts) + analyze_frozen_page_readiness( + page_guard, + batch, + page_idx, + cutoff_ts, + #[cfg(test)] + maintenance_test, + ) } } } @@ -250,6 +275,7 @@ impl Table { page_guards: &[PageSharedGuard], batch: &mut FrozenPageBatch, cutoff_ts: TrxID, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) { #[cfg(test)] use super::test_hooks; @@ -300,13 +326,18 @@ impl Table { Some(plan) => plan, None => { #[cfg(test)] - test_hooks::run_test_locked_page_plan_rebuild_hook(page_info.page_id); + test_hooks::run_test_locked_page_plan_rebuild_hook( + maintenance_test, + page_info.page_id, + ); scan_frozen_page( page, map, page_info, FrozenPageScanMode::RefreshStablePlan, cutoff_ts, + #[cfg(test)] + maintenance_test, ) .into_plan(cutoff_ts, version) .expect("stable frozen page must yield a transition plan under its state lock") @@ -319,7 +350,10 @@ impl Table { // suffix are not stalled behind the whole batch transition. drop(state); #[cfg(test)] - test_hooks::run_test_transition_page_published_hook(page_info.page_id); + test_hooks::run_test_transition_page_published_hook( + maintenance_test, + page_info.page_id, + ); } } @@ -367,6 +401,7 @@ fn validate_frozen_pages_incrementally( page_guards: &[PageSharedGuard], batch: &mut FrozenPageBatch, cutoff_ts: TrxID, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) -> Option { // Stop at the first delay. Stable pages in the preceding prefix retain // their readiness proof, while the unchecked suffix is deferred until a @@ -391,7 +426,14 @@ fn validate_frozen_pages_incrementally( // Unchecked pages need their first proof; blocked pages must be // revisited because transaction resolution may have made the // image checkpointable since the previous attempt. - analyze_frozen_page_readiness(page_guard, batch, page_idx, cutoff_ts) + analyze_frozen_page_readiness( + page_guard, + batch, + page_idx, + cutoff_ts, + #[cfg(test)] + maintenance_test, + ) } }; record_frozen_page_readiness( @@ -414,13 +456,21 @@ fn refresh_frozen_page_plans_optimistically( page_guards: &[PageSharedGuard], batch: &mut FrozenPageBatch, cutoff_ts: TrxID, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) { // Once cached readiness says the whole batch can proceed, refresh every // cutoff-specific plan before acquiring the first page state write lock. // Later mutations observed the Frozen state, so they can change bitmap or // overlay output but cannot invalidate the established image proof. for (page_idx, page_guard) in page_guards.iter().enumerate() { - refresh_stable_page_plan(page_guard, batch, page_idx, cutoff_ts); + refresh_stable_page_plan( + page_guard, + batch, + page_idx, + cutoff_ts, + #[cfg(test)] + maintenance_test, + ); } } @@ -429,6 +479,7 @@ fn analyze_frozen_page_readiness( batch: &mut FrozenPageBatch, page_idx: usize, cutoff_ts: TrxID, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) -> FrozenPageValidationState { let page_info = batch.pages[page_idx]; let page = page_guard.page(); @@ -445,6 +496,8 @@ fn analyze_frozen_page_readiness( frozen_ts: batch.frozen_ts, }, cutoff_ts, + #[cfg(test)] + maintenance_test, ) .into_readiness_analysis(cutoff_ts, version_before); // A mutation that overlaps analysis can start and finish before the final @@ -475,6 +528,8 @@ fn analyze_frozen_page_readiness( version_before, version_after, analysis.plan, + #[cfg(test)] + maintenance_test, ); analysis.validation } @@ -484,6 +539,7 @@ fn refresh_stable_page_plan( batch: &mut FrozenPageBatch, page_idx: usize, cutoff_ts: TrxID, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) { let page_info = batch.pages[page_idx]; let page = page_guard.page(); @@ -512,11 +568,21 @@ fn refresh_stable_page_plan( page_info, FrozenPageScanMode::RefreshStablePlan, cutoff_ts, + #[cfg(test)] + maintenance_test, ) .into_plan(cutoff_ts, version_before) }); let version_after = map.frozen_mutation_version(); - retain_optimistic_page_plan(batch, page_idx, version_before, version_after, plan); + retain_optimistic_page_plan( + batch, + page_idx, + version_before, + version_after, + plan, + #[cfg(test)] + maintenance_test, + ); } fn retain_optimistic_page_plan( @@ -525,6 +591,7 @@ fn retain_optimistic_page_plan( version_before: u64, version_after: u64, plan: Option, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) { batch.prepared[page_idx] = (version_before == version_after).then_some(plan).flatten(); #[cfg(test)] @@ -532,6 +599,7 @@ fn retain_optimistic_page_plan( use super::test_hooks::run_test_optimistic_page_plan_comparison_hook; run_test_optimistic_page_plan_comparison_hook( + maintenance_test, batch.pages[page_idx].page_id, version_before, version_after, @@ -583,12 +651,13 @@ fn scan_frozen_page( page_info: FrozenPage, mode: FrozenPageScanMode, cutoff_ts: TrxID, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) -> FrozenPageScan { #[cfg(test)] use super::test_hooks; #[cfg(test)] - test_hooks::run_test_frozen_page_scan_hook(page_info.page_id); + test_hooks::run_test_frozen_page_scan_hook(maintenance_test, page_info.page_id); let row_count = page.header.row_count(); // Start from the current physical image. Undo inspection below only // adjusts rows whose cutoff visibility differs from that latest image. @@ -602,7 +671,11 @@ fn scan_frozen_page( let Some(head) = undo_guard.as_ref() else { drop(undo_guard); #[cfg(test)] - test_hooks::run_test_frozen_page_row_scan_hook(page_info.page_id, row_idx); + test_hooks::run_test_frozen_page_row_scan_hook( + maintenance_test, + page_info.page_id, + row_idx, + ); continue; }; // Walk the main branch from newest to oldest. Leading locks and deletes @@ -701,7 +774,11 @@ fn scan_frozen_page( } drop(undo_guard); #[cfg(test)] - test_hooks::run_test_frozen_page_row_scan_hook(page_info.page_id, row_idx); + test_hooks::run_test_frozen_page_row_scan_hook( + maintenance_test, + page_info.page_id, + row_idx, + ); } FrozenPageScan { page_info, @@ -759,7 +836,6 @@ mod tests { use crate::bitmap::Bitmap; use crate::catalog::{ColumnAttributes, ColumnSpec, TableMetadata}; use crate::id::RowID; - use crate::table::test_hooks; use crate::trx::row::tests::test_row_write_access; use crate::trx::tests::{commit_shared_trx_status, shared_trx_status}; use crate::trx::undo::{ @@ -850,12 +926,14 @@ mod tests { cutoff_ts: TrxID, ) -> FrozenPageReadinessAnalysis { let observed_version = fixture.map.frozen_mutation_version(); + let maintenance_test = MaintenanceTestController::default(); scan_frozen_page( &fixture.page, &fixture.map, fixture.page_info, FrozenPageScanMode::EstablishReadiness { frozen_ts }, cutoff_ts, + &maintenance_test, ) .into_readiness_analysis(cutoff_ts, observed_version) } @@ -865,12 +943,14 @@ mod tests { cutoff_ts: TrxID, ) -> Option { let observed_version = fixture.map.frozen_mutation_version(); + let maintenance_test = MaintenanceTestController::default(); scan_frozen_page( &fixture.page, &fixture.map, fixture.page_info, FrozenPageScanMode::RefreshStablePlan, cutoff_ts, + &maintenance_test, ) .into_plan(cutoff_ts, observed_version) } @@ -905,6 +985,7 @@ mod tests { }; let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); + let maintenance_test = MaintenanceTestController::default(); thread::scope(|scope| { let writer = scope.spawn(|| { @@ -917,7 +998,7 @@ mod tests { entered_rx.recv().unwrap(); let version_before = row_ver.frozen_mutation_version(); assert_eq!(version_before, 1); - test_hooks::set_test_frozen_page_scan_hook(move |_| { + maintenance_test.install_frozen_page_scan_hook(move |_| { release_tx.send(()).unwrap(); }); let analysis = scan_frozen_page( @@ -928,6 +1009,7 @@ mod tests { frozen_ts: TrxID::new(20), }, TrxID::new(20), + &maintenance_test, ) .into_readiness_analysis(TrxID::new(20), version_before); writer.join().unwrap(); diff --git a/doradb-storage/src/table/persistence.rs b/doradb-storage/src/table/persistence.rs index fee2da19..c35db113 100644 --- a/doradb-storage/src/table/persistence.rs +++ b/doradb-storage/src/table/persistence.rs @@ -1,14 +1,15 @@ use super::checkpoint_workflow::{ - CheckpointAttempt, FrozenPageValidationState, PreparedTransitionPage, + CheckpointAttempt, FrozenPageBatch, FrozenPageValidationState, PreparedCheckpointAttempt, + PreparedFreezeAttempt, PreparedTransitionPage, }; -use super::lifecycle::{CheckpointPublishLease, TableCheckpointRootMutationLease, TableTerminal}; +use super::lifecycle::{CheckpointPublishLease, TableCheckpointRootMutationScope, TableTerminal}; use crate::buffer::PoolGuards; use crate::buffer::guard::PageGuard; use crate::catalog::{IndexSpec, SilentWatermarkObject, TableColumnLayout, TableMetadata}; use crate::error::{ - DataIntegrityError, DataIntegrityResult, FatalError, InternalError, LifecycleError, - MultiDomainResultExt, RuntimeError, RuntimeOrFatalError, RuntimeOrFatalResult, - RuntimeOrFatalResultExt, RuntimeResult, + CompletionErrorBridge, CompletionResult, DataIntegrityError, DataIntegrityResult, FatalError, + InternalError, LifecycleError, MultiDomainResultExt, RuntimeError, RuntimeOrFatalError, + RuntimeOrFatalResult, RuntimeOrFatalResultExt, RuntimeResult, }; use crate::file::cow_file::SUPER_BLOCK_ID; use crate::file::table_file::{ActiveRoot, LwcBlockPersist, MutableTableFile}; @@ -23,10 +24,16 @@ use crate::index::{ use crate::lwc::LwcBuilder; use crate::obs; use crate::row::RowPage; -use crate::session::{SessionOperationPin, SessionRuntimeAccess}; +use crate::runtime::mandatory::PreparedExecution; +use crate::session::{ + AcceptedMaintenanceScope, MaintenanceExecutionSpec, PreparedMaintenanceExecution, + PreparedMaintenanceScope, SessionRuntimeAccess, +}; +#[cfg(test)] +use crate::table::tests::MaintenanceTestController; use crate::table::{ - CheckpointCancelReason, FreezeOutcome, FrozenPage, FrozenPageBatch, Table, - TableRedoReplayFloor, TableRuntimeLayout, + CheckpointCancelReason, FreezeOutcome, FrozenPage, Table, TableRedoReplayFloor, + TableRuntimeLayout, }; use crate::trx::RetiredRowPageBatch; use crate::value::{Val, ValKind, ValType}; @@ -35,6 +42,7 @@ use event_listener::EventListener; use futures::future::select_all; use std::collections::BTreeSet; use std::result::Result as StdResult; +use std::sync::Arc; #[cfg(test)] pub(crate) use tests::test_hooks; @@ -118,30 +126,95 @@ impl DetachedCheckpointRetryWait { } } +struct FreezeTableResources { + attempt: Option, + _root_mutation: TableCheckpointRootMutationScope, + table: Arc
, + max_rows: usize, +} + +struct FreezeTableExecution; + +impl MaintenanceExecutionSpec for FreezeTableExecution { + type Output = FreezeOutcome; + type Resources = FreezeTableResources; + type PanicLabel = &'static str; + + const LABEL: &'static str = "freeze_table"; + + async fn execute( + scope: &mut AcceptedMaintenanceScope, + resources: &mut Self::Resources, + _panic_label: &mut Self::PanicLabel, + ) -> CompletionResult { + let attempt = resources + .attempt + .take() + .unwrap_or_else(|| panic!("accepted freeze attempt is missing")); + let result = resources + .table + .freeze_prepared(scope, resources.max_rows, attempt) + .await + .map_err(CompletionErrorBridge::capture); + scope.mark_terminal_ready(); + result + } +} + +struct CheckpointTableResources { + attempt: Option, + _root_mutation: TableCheckpointRootMutationScope, + table: Arc
, +} + +struct CheckpointTableExecution; + +impl MaintenanceExecutionSpec for CheckpointTableExecution { + type Output = CheckpointOutcome; + type Resources = CheckpointTableResources; + type PanicLabel = &'static str; + + const LABEL: &'static str = "checkpoint_table"; + + async fn execute( + scope: &mut AcceptedMaintenanceScope, + resources: &mut Self::Resources, + _panic_label: &mut Self::PanicLabel, + ) -> CompletionResult { + let attempt = resources + .attempt + .take() + .unwrap_or_else(|| panic!("accepted checkpoint attempt is missing")); + let result = resources + .table + .checkpoint_prepared(scope, attempt) + .await + .map_err(CompletionErrorBridge::capture_runtime_or_fatal); + scope.mark_terminal_ready(); + result + } +} + /// Owns one table checkpoint attempt and its reversible-to-fatal boundary. -struct TableCheckpointer<'table, 'session> { +struct TableCheckpointer<'table, 'session, S: SessionRuntimeAccess + ?Sized> { table: &'table Table, - session: &'session SessionOperationPin, - // Declaration order preserves publication -> root mutation -> attempt - // release ordering when this owner is dropped. + session: &'session S, + // Declaration order preserves publication -> attempt release ordering. publish_lease: Option>, - root_mutation_lease: Option>, - attempt: CheckpointAttempt<'table>, + attempt: PreparedCheckpointAttempt, irreversible: Option, } -impl<'table, 'session> TableCheckpointer<'table, 'session> { +impl<'table, 'session, S> TableCheckpointer<'table, 'session, S> +where + S: SessionRuntimeAccess + ?Sized, +{ #[inline] - fn new( - table: &'table Table, - session: &'session SessionOperationPin, - attempt: CheckpointAttempt<'table>, - ) -> Self { + fn new(table: &'table Table, session: &'session S, attempt: PreparedCheckpointAttempt) -> Self { Self { table, session, publish_lease: None, - root_mutation_lease: None, attempt, irreversible: None, } @@ -153,13 +226,9 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { let table_id = table.table_id(); let table_file = table.file(); let disk_pool = table.disk_pool(); - let trx_sys = session.engine.trx_sys.clone(); - let table_writes = session.engine.table_fs.background_writes().clone(); + let trx_sys = session.engine().trx_sys.clone(); + let table_writes = session.engine().table_fs.background_writes().clone(); let pool_guards = session.pool_guards(); - match table.try_begin_checkpoint_root_mutation() { - Ok(lease) => self.root_mutation_lease = Some(lease), - Err(reason) => return Ok(CheckpointOutcome::Cancelled { reason }), - } if let Some(reason) = table.active_root_checkpoint_delay(session) { return Ok(CheckpointOutcome::Delayed { reason }); } @@ -189,7 +258,8 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { let checkpoint_ts = trx_sys.allocate_checkpoint_ts(); let mut sys_trx = trx_sys.begin_sys_trx(); #[cfg(test)] - test_hooks::run_test_checkpoint_after_trx_start_hook().await; + test_hooks::run_test_checkpoint_after_trx_start_hook(&session.engine().maintenance_test) + .await; // If freeze did not observe a successor page, resolve the current hot // boundary only after allocating checkpoint STS. A page appended after // an empty boundary scan must then have create_cts above the fallback. @@ -229,7 +299,13 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { let Some(batch) = self.attempt.batch_mut() else { panic!("non-empty checkpoint page list requires frozen source") }; - table.prepare_page_transition(&transition_pages, batch, cutoff_ts) + table.prepare_page_transition( + &transition_pages, + batch, + cutoff_ts, + #[cfg(test)] + &session.engine().maintenance_test, + ) }; if let Some(delay) = delay { return Ok(CheckpointOutcome::Delayed { @@ -252,11 +328,20 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { let Some(batch) = self.attempt.batch_mut() else { panic!("non-empty checkpoint page list requires frozen source") }; - table.apply_page_transition(&transition_pages, batch, cutoff_ts); + table.apply_page_transition( + &transition_pages, + batch, + cutoff_ts, + #[cfg(test)] + &session.engine().maintenance_test, + ); } #[cfg(test)] if !pages.is_empty() { - test_hooks::run_test_checkpoint_after_publish_admission_hook().await; + test_hooks::run_test_checkpoint_after_publish_admission_hook( + &session.engine().maintenance_test, + ) + .await; } // Step 4: build LWC blocks from transition pages using the cutoff @@ -283,6 +368,8 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { .map(|batch| batch.prepared.as_slice()) .unwrap_or_default(), collect_visible_row, + #[cfg(test)] + &session.engine().maintenance_test, ) .await .change_context(RuntimeError::CheckpointExecution) @@ -342,6 +429,8 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { &layout, &mut secondary_sidecar, checkpoint_ts, + #[cfg(test)] + &session.engine().maintenance_test, ) .await .change_runtime_context(RuntimeError::CheckpointExecution) @@ -369,7 +458,10 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { match self.begin_publishing() { Ok(true) => { #[cfg(test)] - test_hooks::run_test_checkpoint_after_publish_admission_hook().await; + test_hooks::run_test_checkpoint_after_publish_admission_hook( + &session.engine().maintenance_test, + ) + .await; } Ok(false) => {} Err(reason) => return Ok(CheckpointOutcome::Cancelled { reason }), @@ -381,7 +473,9 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { }; self.set_irreversible(FatalError::CatalogWrite); #[cfg(test)] - test_hooks::run_test_silent_watermark_mutation_hook() + test_hooks::run_test_silent_watermark_mutation_hook( + &session.engine().maintenance_test, + ) .await .change_context(RuntimeError::CheckpointExecution) .attach_with(|| { @@ -390,7 +484,7 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { ) })?; sys_trx - .upsert_silent_watermark(session.engine.catalog(), &pool_guards, watermark) + .upsert_silent_watermark(session.engine().catalog(), &pool_guards, watermark) .await .change_context(RuntimeError::CheckpointExecution) .attach_with(|| { @@ -400,7 +494,7 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { })?; self.set_irreversible(FatalError::CheckpointWrite); #[cfg(test)] - test_hooks::maybe_force_checkpoint_commit_error()?; + test_hooks::maybe_force_checkpoint_commit_error(&session.engine().maintenance_test)?; let redo_cts = trx_sys .commit_sys(sys_trx) .change_runtime_context(RuntimeError::CheckpointExecution) @@ -439,7 +533,10 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { match self.begin_publishing() { Ok(true) => { #[cfg(test)] - test_hooks::run_test_checkpoint_after_publish_admission_hook().await; + test_hooks::run_test_checkpoint_after_publish_admission_hook( + &session.engine().maintenance_test, + ) + .await; } Ok(false) => {} Err(reason) => return Ok(CheckpointOutcome::Cancelled { reason }), @@ -461,10 +558,10 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { .update_column_root(published_pivot_row_id, published_column_root) .await; #[cfg(test)] - test_hooks::maybe_force_post_publish_checkpoint_error()?; + test_hooks::maybe_force_post_publish_checkpoint_error(&session.engine().maintenance_test)?; #[cfg(test)] - test_hooks::maybe_force_checkpoint_commit_error()?; + test_hooks::maybe_force_checkpoint_commit_error(&session.engine().maintenance_test)?; let redo_cts = trx_sys .commit_sys(sys_trx) .change_runtime_context(RuntimeError::CheckpointExecution) @@ -549,7 +646,7 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { Err(err) => { if let Some(reason) = self.irreversible.take() { let report = err.into_fatal_report(reason); - let poison = self.session.engine.poisoner.poison(report); + let poison = self.session.engine().poisoner.poison(report); drop(self.publish_lease.take()); Err(poison.into()) } else { @@ -564,7 +661,7 @@ impl<'table, 'session> TableCheckpointer<'table, 'session> { } } -impl Drop for TableCheckpointer<'_, '_> { +impl Drop for TableCheckpointer<'_, '_, S> { fn drop(&mut self) { debug_assert!(self.irreversible.is_none() || self.publish_lease.is_some()); let Some(_) = self.publish_lease else { @@ -577,7 +674,7 @@ impl Drop for TableCheckpointer<'_, '_> { "event=engine_poison component=table action=poison result=error error={:?}", report ); - let _ = self.session.engine.poisoner.poison(report); + let _ = self.session.engine().poisoner.poison(report); } else { self.table.checkpoint_workflow.finish_publication(); } @@ -808,6 +905,59 @@ impl SecondaryCheckpointSidecar { } } +/// Prepare reversible table-freeze workflow and root-mutation authority. +pub(crate) fn prepare_freeze_table_operation( + scope: PreparedMaintenanceScope, + table: Arc
, + max_rows: usize, +) -> StdResult, FreezeOutcome> { + let attempt = Arc::clone(&table).begin_freeze()?; + let root_mutation = match TableCheckpointRootMutationScope::acquire(Arc::clone(&table)) { + Ok(root_mutation) => root_mutation, + Err(reason) => return Err(FreezeOutcome::Cancelled { reason }), + }; + let table_id = table.table_id(); + Ok(PreparedMaintenanceExecution::::table( + scope, + FreezeTableResources { + attempt: Some(attempt), + _root_mutation: root_mutation, + table, + max_rows, + }, + "accepted table freeze panicked", + table_id, + )) +} + +/// Prepare reversible table-checkpoint workflow and root-mutation authority. +pub(crate) fn prepare_checkpoint_table_operation( + scope: PreparedMaintenanceScope, + table: Arc
, +) -> StdResult, CheckpointOutcome> { + let attempt = match Arc::clone(&table).begin_checkpoint() { + Ok(attempt) => attempt, + Err(reason) => return Err(CheckpointOutcome::Cancelled { reason }), + }; + let root_mutation = match TableCheckpointRootMutationScope::acquire(Arc::clone(&table)) { + Ok(root_mutation) => root_mutation, + Err(reason) => return Err(CheckpointOutcome::Cancelled { reason }), + }; + let table_id = table.table_id(); + Ok( + PreparedMaintenanceExecution::::table( + scope, + CheckpointTableResources { + attempt: Some(attempt), + _root_mutation: root_mutation, + table, + }, + "accepted table checkpoint panicked", + table_id, + ), + ) +} + /// Builds the durable secondary DiskTree key encoder for one index spec. pub(crate) fn secondary_disk_tree_encoder( metadata: &TableMetadata, @@ -1226,6 +1376,7 @@ impl Table { layout: &TableRuntimeLayout, sidecar: &mut SecondaryCheckpointSidecar, checkpoint_ts: TrxID, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) -> RuntimeOrFatalResult<()> { let metadata = layout.metadata(); sidecar.assert_matches_metadata(metadata); @@ -1236,7 +1387,7 @@ impl Table { // file fork as LWC and delete metadata. A later error abandons the // fork, so no secondary root can be published on its own. #[cfg(test)] - test_hooks::maybe_force_secondary_sidecar_error()?; + test_hooks::maybe_force_secondary_sidecar_error(maintenance_test)?; if mutable_file.secondary_index_roots().len() != metadata.idx.index_slot_count() { return Err(Report::new(DataIntegrityError::InvalidRootInvariant) @@ -1540,7 +1691,10 @@ impl Table { engine.shutdown_listener(), ]; #[cfg(test)] - test_hooks::run_test_checkpoint_retry_after_listener_registration_hook().await; + test_hooks::run_test_checkpoint_retry_after_listener_registration_hook( + &session.engine().maintenance_test, + ) + .await; ensure_maintenance_wait_running(session, "observe active-root checkpoint retry")?; if self.active_root_retry_ready(effective_ts, trx_sys.published_gc_horizon()) { @@ -1624,7 +1778,10 @@ impl Table { listeners.push(engine.poisoner.listener()); listeners.push(engine.shutdown_listener()); #[cfg(test)] - test_hooks::run_test_checkpoint_retry_after_listener_registration_hook().await; + test_hooks::run_test_checkpoint_retry_after_listener_registration_hook( + &session.engine().maintenance_test, + ) + .await; ensure_maintenance_wait_running( session, @@ -1655,7 +1812,10 @@ impl Table { engine.shutdown_listener(), ]; #[cfg(test)] - test_hooks::run_test_checkpoint_retry_after_listener_registration_hook().await; + test_hooks::run_test_checkpoint_retry_after_listener_registration_hook( + &session.engine().maintenance_test, + ) + .await; ensure_maintenance_wait_running( session, "observe frozen-page checkpoint retry", @@ -1714,24 +1874,27 @@ impl Table { page_info.page_id ) }); - self.refresh_frozen_page_readiness(&page_guard, batch, page_idx, cutoff_ts); + self.refresh_frozen_page_readiness( + &page_guard, + batch, + page_idx, + cutoff_ts, + #[cfg(test)] + &session.engine().maintenance_test, + ); Ok(()) } - /// Claim and freeze a contiguous hot-page prefix up to the requested row budget. - pub(crate) async fn freeze( + /// Execute a freeze using caller-prepared workflow and root authority. + async fn freeze_prepared( &self, - session: &SessionOperationPin, + session: &S, max_rows: usize, - ) -> RuntimeResult { - let mut attempt = match self.checkpoint_workflow.begin_freeze(&self.lifecycle) { - Ok(attempt) => attempt, - Err(outcome) => return Ok(outcome), - }; - let _root_mutation_lease = match self.try_begin_checkpoint_root_mutation() { - Ok(lease) => lease, - Err(reason) => return Ok(FreezeOutcome::Cancelled { reason }), - }; + mut attempt: PreparedFreezeAttempt, + ) -> RuntimeResult + where + S: SessionRuntimeAccess + ?Sized, + { let guards = session.pool_guards(); let mut rows = 0usize; let mut pages = Vec::new(); @@ -1757,18 +1920,23 @@ impl Table { .load_frozen_pages_for_transition(&guards, &pages) .await?; #[cfg(test)] - test_hooks::run_test_freeze_after_loading_hook().await; - let publish = - self.validate_and_publish_loaded_pages_frozen(&page_guards, &pages, &mut attempt); + test_hooks::run_test_freeze_after_loading_hook(&session.engine().maintenance_test).await; + let publish = self.validate_and_publish_loaded_pages_frozen( + &page_guards, + &pages, + &mut attempt, + #[cfg(test)] + &session.engine().maintenance_test, + ); if !publish { - return Ok(attempt.cancelled(&self.lifecycle)); + return Ok(attempt.cancelled()); } - // The fence is allocated only after every selected page has published + // Allocate the fence only after every selected page has published // FROZEN under its state lock. - let frozen_ts = session.engine.trx_sys.allocate_snapshot_fence(); + let frozen_ts = session.engine().trx_sys.allocate_snapshot_fence(); let batch = FrozenPageBatch::new(self.table_id(), frozen_ts, heap_redo_start_ts, rows, pages); - Ok(attempt.finish(batch, &self.lifecycle)) + Ok(attempt.finish(batch)) } async fn heap_redo_start_from( @@ -1792,6 +1960,7 @@ impl Table { guards: &PoolGuards, prepared_pages: &[Option], mut collect_visible_row: Option, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) -> RuntimeResult> where C: FnMut(&RowPage, usize, RowID), @@ -1800,7 +1969,7 @@ impl Table { use super::test_hooks as table_test_hooks; #[cfg(test)] - table_test_hooks::maybe_force_lwc_build_error()?; + table_test_hooks::maybe_force_lwc_build_error(maintenance_test)?; let mut lwc_blocks = Vec::new(); if !prepared_pages.is_empty() { let mut builder = LwcBuilder::new(metadata.col.as_ref()); @@ -1927,21 +2096,20 @@ impl Table { Ok(lwc_blocks) } - /// Execute one user-table checkpoint attempt against table-owned workflow state. - pub(crate) async fn checkpoint( + /// Execute one checkpoint with caller-prepared workflow/root authority. + async fn checkpoint_prepared( &self, - session: &SessionOperationPin, - ) -> RuntimeOrFatalResult { + session: &S, + attempt: PreparedCheckpointAttempt, + ) -> RuntimeOrFatalResult + where + S: SessionRuntimeAccess + ?Sized, + { let table_id = self.table_id(); - let result = match self.checkpoint_workflow.begin_checkpoint(&self.lifecycle) { - Ok(attempt) => { - let mut checkpointer = TableCheckpointer::new(self, session, attempt); - let result = checkpointer.run().await; - checkpointer.resolve(result) - } - Err(reason) => Ok(CheckpointOutcome::Cancelled { reason }), - }; - result + let mut checkpointer = TableCheckpointer::new(self, session, attempt); + let result = checkpointer.run().await; + checkpointer + .resolve(result) .inspect(|outcome| match outcome { CheckpointOutcome::Published { checkpoint_ts, @@ -2030,208 +2198,171 @@ where #[cfg(test)] mod tests { pub(crate) mod test_hooks { + use crate::engine::Engine; use crate::error::{FatalError, FatalResult, RuntimeError, RuntimeResult}; + use crate::table::tests::MaintenanceTestController; use error_stack::Report; - use std::cell::{Cell, RefCell}; use std::future::Future; - use std::pin::Pin; - - type TableHook = Box Pin + 'static>> + 'static>; - type FallibleTableHook = Box< - dyn FnOnce() -> Pin> + 'static>> + 'static, - >; - - thread_local! { - static TEST_FORCE_SECONDARY_SIDECAR_ERROR: Cell = const { Cell::new(false) }; - static TEST_FORCE_POST_PUBLISH_CHECKPOINT_ERROR: Cell = const { Cell::new(false) }; - static TEST_FORCE_CHECKPOINT_COMMIT_ERROR: Cell = const { Cell::new(false) }; - static TEST_FREEZE_AFTER_LOADING_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_CHECKPOINT_AFTER_TRX_START_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_CHECKPOINT_AFTER_PUBLISH_ADMISSION_HOOK: RefCell> = - const { RefCell::new(None) }; - static TEST_CHECKPOINT_RETRY_AFTER_LISTENER_REGISTRATION_HOOK: - RefCell> = const { RefCell::new(None) }; - static TEST_SILENT_WATERMARK_MUTATION_HOOK: RefCell> = - const { RefCell::new(None) }; - } - pub(crate) fn set_test_force_secondary_sidecar_error(enabled: bool) { - TEST_FORCE_SECONDARY_SIDECAR_ERROR.with(|flag| flag.set(enabled)); + pub(crate) fn set_test_force_secondary_sidecar_error(engine: &Engine, enabled: bool) { + engine + .inner() + .maintenance_test + .set_force_secondary_sidecar_error(enabled); } - pub(crate) fn maybe_force_secondary_sidecar_error() -> RuntimeResult<()> { - if TEST_FORCE_SECONDARY_SIDECAR_ERROR.with(|flag| flag.get()) { + pub(crate) fn maybe_force_secondary_sidecar_error( + test: &MaintenanceTestController, + ) -> RuntimeResult<()> { + if test.force_secondary_sidecar_error() { return Err(Report::new(RuntimeError::CheckpointExecution) .attach("injected secondary-index sidecar failure")); } Ok(()) } - pub(crate) fn set_test_force_post_publish_checkpoint_error(enabled: bool) { - TEST_FORCE_POST_PUBLISH_CHECKPOINT_ERROR.with(|flag| flag.set(enabled)); + pub(crate) struct ForcePostPublishCheckpointErrorGuard { + test: MaintenanceTestController, } - pub(crate) struct ForcePostPublishCheckpointErrorGuard; - impl ForcePostPublishCheckpointErrorGuard { - pub(crate) fn new() -> Self { - set_test_force_post_publish_checkpoint_error(true); - Self + pub(crate) fn new(engine: &Engine) -> Self { + let test = engine.inner().maintenance_test.clone(); + test.set_force_post_publish_checkpoint_error(true); + Self { test } } } impl Drop for ForcePostPublishCheckpointErrorGuard { fn drop(&mut self) { - set_test_force_post_publish_checkpoint_error(false); + self.test.set_force_post_publish_checkpoint_error(false); } } - pub(crate) fn maybe_force_post_publish_checkpoint_error() -> FatalResult<()> { - if TEST_FORCE_POST_PUBLISH_CHECKPOINT_ERROR.with(Cell::get) { + pub(crate) fn maybe_force_post_publish_checkpoint_error( + test: &MaintenanceTestController, + ) -> FatalResult<()> { + if test.force_post_publish_checkpoint_error() { return Err(Report::new(FatalError::CheckpointWrite) .attach("forced post-publication table checkpoint failure")); } Ok(()) } - pub(crate) struct ForceCheckpointCommitErrorGuard; + pub(crate) struct ForceCheckpointCommitErrorGuard { + test: MaintenanceTestController, + } impl ForceCheckpointCommitErrorGuard { - pub(crate) fn new() -> Self { - TEST_FORCE_CHECKPOINT_COMMIT_ERROR.with(|flag| flag.set(true)); - Self + pub(crate) fn new(engine: &Engine) -> Self { + let test = engine.inner().maintenance_test.clone(); + test.set_force_checkpoint_commit_error(true); + Self { test } } } impl Drop for ForceCheckpointCommitErrorGuard { fn drop(&mut self) { - TEST_FORCE_CHECKPOINT_COMMIT_ERROR.with(|flag| flag.set(false)); + self.test.set_force_checkpoint_commit_error(false); } } - pub(crate) fn maybe_force_checkpoint_commit_error() -> FatalResult<()> { - if TEST_FORCE_CHECKPOINT_COMMIT_ERROR.with(Cell::get) { + pub(crate) fn maybe_force_checkpoint_commit_error( + test: &MaintenanceTestController, + ) -> FatalResult<()> { + if test.force_checkpoint_commit_error() { return Err(Report::new(FatalError::CheckpointWrite) .attach("forced table checkpoint commit failure")); } Ok(()) } - pub(crate) fn set_test_freeze_after_loading_hook(hook: F) + pub(crate) fn set_test_freeze_after_loading_hook(engine: &Engine, hook: F) where - F: FnOnce() -> Fut + 'static, - Fut: Future + 'static, + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, { - TEST_FREEZE_AFTER_LOADING_HOOK.with(|slot| { - let old = slot - .borrow_mut() - .replace(Box::new(move || Box::pin(hook()))); - assert!(old.is_none(), "freeze loading hook already installed"); - }); + engine + .inner() + .maintenance_test + .install_freeze_after_loading_hook(hook); } - pub(crate) fn set_test_checkpoint_after_trx_start_hook(hook: F) + pub(crate) fn set_test_checkpoint_after_trx_start_hook(engine: &Engine, hook: F) where - F: FnOnce() -> Fut + 'static, - Fut: Future + 'static, + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, { - TEST_CHECKPOINT_AFTER_TRX_START_HOOK.with(|slot| { - let old = slot - .borrow_mut() - .replace(Box::new(move || Box::pin(hook()))); - assert!( - old.is_none(), - "checkpoint transaction-start hook already installed" - ); - }); + engine + .inner() + .maintenance_test + .install_checkpoint_after_trx_start_hook(hook); } - pub(crate) fn set_test_checkpoint_after_publish_admission_hook(hook: F) - where - F: FnOnce() -> Fut + 'static, - Fut: Future + 'static, + pub(crate) fn set_test_checkpoint_after_publish_admission_hook( + engine: &Engine, + hook: F, + ) where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, { - TEST_CHECKPOINT_AFTER_PUBLISH_ADMISSION_HOOK.with(|slot| { - let old = slot - .borrow_mut() - .replace(Box::new(move || Box::pin(hook()))); - assert!( - old.is_none(), - "checkpoint publish-admission hook already installed" - ); - }); + engine + .inner() + .maintenance_test + .install_checkpoint_after_publish_admission_hook(hook); } - pub(crate) fn set_test_checkpoint_retry_after_listener_registration_hook(hook: F) - where - F: FnOnce() -> Fut + 'static, - Fut: Future + 'static, + pub(crate) fn set_test_checkpoint_retry_after_listener_registration_hook( + engine: &Engine, + hook: F, + ) where + F: FnOnce() -> Fut + Send + 'static, + Fut: Future + Send + 'static, { - TEST_CHECKPOINT_RETRY_AFTER_LISTENER_REGISTRATION_HOOK.with(|slot| { - let old = slot - .borrow_mut() - .replace(Box::new(move || Box::pin(hook()))); - assert!( - old.is_none(), - "checkpoint retry listener-registration hook already installed" - ); - }); + engine + .inner() + .maintenance_test + .install_checkpoint_retry_after_listener_registration_hook(hook); } - pub(crate) fn set_test_silent_watermark_mutation_hook(hook: F) + pub(crate) fn set_test_silent_watermark_mutation_hook(engine: &Engine, hook: F) where - F: FnOnce() -> Fut + 'static, - Fut: Future> + 'static, + F: FnOnce() -> Fut + Send + 'static, + Fut: Future> + Send + 'static, { - TEST_SILENT_WATERMARK_MUTATION_HOOK.with(|slot| { - let old = slot - .borrow_mut() - .replace(Box::new(move || Box::pin(hook()))); - assert!( - old.is_none(), - "silent watermark mutation hook already installed" - ); - }); + engine + .inner() + .maintenance_test + .install_silent_watermark_mutation_hook(hook); } - pub(crate) async fn run_test_checkpoint_after_trx_start_hook() { - let hook = TEST_CHECKPOINT_AFTER_TRX_START_HOOK.with(|slot| slot.borrow_mut().take()); - if let Some(hook) = hook { - hook().await; - } + pub(crate) async fn run_test_checkpoint_after_trx_start_hook( + test: &MaintenanceTestController, + ) { + test.run_checkpoint_after_trx_start_hook().await; } - pub(crate) async fn run_test_checkpoint_after_publish_admission_hook() { - let hook = - TEST_CHECKPOINT_AFTER_PUBLISH_ADMISSION_HOOK.with(|slot| slot.borrow_mut().take()); - if let Some(hook) = hook { - hook().await; - } + pub(crate) async fn run_test_checkpoint_after_publish_admission_hook( + test: &MaintenanceTestController, + ) { + test.run_checkpoint_after_publish_admission_hook().await; } - pub(crate) async fn run_test_checkpoint_retry_after_listener_registration_hook() { - let hook = TEST_CHECKPOINT_RETRY_AFTER_LISTENER_REGISTRATION_HOOK - .with(|slot| slot.borrow_mut().take()); - if let Some(hook) = hook { - hook().await; - } + pub(crate) async fn run_test_checkpoint_retry_after_listener_registration_hook( + test: &MaintenanceTestController, + ) { + test.run_checkpoint_retry_after_listener_registration_hook() + .await; } - pub(crate) async fn run_test_silent_watermark_mutation_hook() -> RuntimeResult<()> { - let hook = TEST_SILENT_WATERMARK_MUTATION_HOOK.with(|slot| slot.borrow_mut().take()); - match hook { - Some(hook) => hook().await, - None => Ok(()), - } + pub(crate) async fn run_test_silent_watermark_mutation_hook( + test: &MaintenanceTestController, + ) -> RuntimeResult<()> { + test.run_silent_watermark_mutation_hook().await } - pub(crate) async fn run_test_freeze_after_loading_hook() { - let hook = TEST_FREEZE_AFTER_LOADING_HOOK.with(|slot| slot.borrow_mut().take()); - if let Some(hook) = hook { - hook().await; - } + pub(crate) async fn run_test_freeze_after_loading_hook(test: &MaintenanceTestController) { + test.run_freeze_after_loading_hook().await; } } @@ -2266,28 +2397,24 @@ mod tests { set_test_silent_watermark_mutation_hook, }; use crate::table::test_hooks::{ - ForceLwcBuildErrorGuard, set_test_freeze_page_state_locked_hook, - set_test_frozen_page_row_scan_hook, set_test_frozen_page_scan_hook, - set_test_frozen_pages_ready_hook, set_test_hot_row_write_before_state_lock_hook, - set_test_locked_page_plan_rebuild_hook, set_test_optimistic_page_plan_comparison_hook, - set_test_stable_page_plans_refreshed_hook, set_test_transition_page_published_hook, + ForceLwcBuildErrorGuard, set_test_frozen_page_row_scan_hook, + set_test_frozen_page_scan_hook, set_test_frozen_pages_ready_hook, + set_test_hot_row_write_before_state_lock_hook, set_test_locked_page_plan_rebuild_hook, + set_test_optimistic_page_plan_comparison_hook, set_test_stable_page_plans_refreshed_hook, + set_test_transition_page_published_hook, }; use crate::table::tests::*; use crate::table::{DeleteMarker, TableTerminal}; use crate::trx::MIN_ACTIVE_TRX_ID; - use crate::trx::Transaction; use crate::trx::purge::PurgeTestEvent; use crate::trx::stmt::tests as stmt_tests; use crate::trx::tests::{discard_transaction_after_fatal_rollback, shared_trx_status}; use crate::trx::undo::{OwnedRowUndo, RowUndoHead, RowUndoKind}; use crate::trx::ver_map::RowPageState; use futures::FutureExt; - use futures::future::pending; - use std::cell::{Cell, RefCell}; use std::cmp::Ordering; - use std::panic::{AssertUnwindSafe, catch_unwind}; - use std::rc::Rc; use std::sync::Arc; + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}; use std::task::Poll; use std::thread; use tempfile::TempDir; @@ -2433,7 +2560,7 @@ mod tests { let table = table_for_internal_assertion(engine, table_id); let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); - set_test_checkpoint_after_publish_admission_hook(move || async move { + set_test_checkpoint_after_publish_admission_hook(engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -2774,6 +2901,7 @@ mod tests { &guards, &[Some(prepared)], None::, + &engine.inner().maintenance_test, ) .await { @@ -2815,21 +2943,27 @@ mod tests { .frozen_page_ids() .unwrap() .len(); - let analysis_count = Rc::new(Cell::new(0usize)); - let hook_analysis_count = Rc::clone(&analysis_count); - set_test_frozen_page_scan_hook(move |_| { - hook_analysis_count.set(hook_analysis_count.get() + 1); + let analysis_count = Arc::new(AtomicUsize::new(0)); + let hook_analysis_count = Arc::clone(&analysis_count); + set_test_frozen_page_scan_hook(&engine, move |_| { + hook_analysis_count.fetch_add(1, AtomicOrdering::Relaxed); }); - let analysis_count_at_build = Rc::new(Cell::new(usize::MAX)); - let hook_analysis_count = Rc::clone(&analysis_count); - let hook_analysis_count_at_build = Rc::clone(&analysis_count_at_build); - set_test_checkpoint_after_publish_admission_hook(move || async move { - hook_analysis_count_at_build.set(hook_analysis_count.get()); + let analysis_count_at_build = Arc::new(AtomicUsize::new(usize::MAX)); + let hook_analysis_count = Arc::clone(&analysis_count); + let hook_analysis_count_at_build = Arc::clone(&analysis_count_at_build); + set_test_checkpoint_after_publish_admission_hook(&engine, move || async move { + hook_analysis_count_at_build.store( + hook_analysis_count.load(AtomicOrdering::Relaxed), + AtomicOrdering::Relaxed, + ); }); let outcome = session.checkpoint_table(table_id).await.unwrap(); assert!(matches!(outcome, CheckpointOutcome::Published { .. })); - assert!(analysis_count.get() >= frozen_page_count); - assert_eq!(analysis_count.get(), analysis_count_at_build.get()); + assert!(analysis_count.load(AtomicOrdering::Relaxed) >= frozen_page_count); + assert_eq!( + analysis_count.load(AtomicOrdering::Relaxed), + analysis_count_at_build.load(AtomicOrdering::Relaxed) + ); let name_key = name_key(&name); let table = table_for_internal_assertion(&engine, table_id); @@ -2907,19 +3041,19 @@ mod tests { let deleted_row_id = page_guard .page() .row_id(page_guard.page().header.row_count() - 1); - let hook_page = Rc::new(RefCell::new(Some(page_guard))); - let refreshed_hook_page = Rc::clone(&hook_page); - let refreshed_hook_ran = Rc::new(Cell::new(false)); - let hook_refreshed_hook_ran = Rc::clone(&refreshed_hook_ran); - set_test_stable_page_plans_refreshed_hook(move || { - hook_refreshed_hook_ran.set(true); - let page_guard = refreshed_hook_page.borrow_mut().take().unwrap(); + let hook_page = Arc::new(parking_lot::Mutex::new(Some(page_guard))); + let refreshed_hook_page = Arc::clone(&hook_page); + let refreshed_hook_ran = Arc::new(AtomicBool::new(false)); + let hook_refreshed_hook_ran = Arc::clone(&refreshed_hook_ran); + set_test_stable_page_plans_refreshed_hook(&engine, move || { + hook_refreshed_hook_ran.store(true, AtomicOrdering::Relaxed); + let page_guard = refreshed_hook_page.lock().take().unwrap(); delete_last_frozen_row_image(page_guard); }); let outcome = session.checkpoint_table(table_id).await.unwrap(); assert!(matches!(outcome, CheckpointOutcome::Published { .. })); - assert!(refreshed_hook_ran.get()); + assert!(refreshed_hook_ran.load(AtomicOrdering::Relaxed)); assert!( table .file() @@ -3081,11 +3215,11 @@ mod tests { #[test] fn test_secondary_sidecar_failure_keeps_checkpoint_root_atomic() { - struct ResetSidecarHook; + struct ResetSidecarHook(MaintenanceTestController); impl Drop for ResetSidecarHook { fn drop(&mut self) { - set_test_force_secondary_sidecar_error(false); + self.0.set_force_secondary_sidecar_error(false); } } @@ -3121,8 +3255,8 @@ mod tests { .clone(); wait_for_checkpoint_root_ready(&mut session, table_id).await; - set_test_force_secondary_sidecar_error(true); - let _reset = ResetSidecarHook; + set_test_force_secondary_sidecar_error(&engine, true); + let _reset = ResetSidecarHook(engine.inner().maintenance_test.clone()); session.checkpoint_table(table_id).await.unwrap_err(); let root_after = table_for_internal_assertion(&engine, table_id) @@ -3762,6 +3896,36 @@ mod tests { }); } + #[test] + fn test_prepared_freeze_attempt_drop_restores_idle() { + smol::block_on(async { + let temp_dir = TempDir::new().unwrap(); + let engine = lightweight_test_engine(&temp_dir, "prepared-freeze-drop").await; + let table_id = create_table2_for_test(&engine).await; + let table = table_for_internal_assertion(&engine, table_id); + + let attempt = Arc::clone(&table).begin_freeze().unwrap(); + assert_eq!(table.checkpoint_workflow.state_name(), "Freezing"); + assert_eq!( + Arc::clone(&table).begin_freeze().err().unwrap(), + FreezeOutcome::Cancelled { + reason: CheckpointCancelReason::FreezeInProgress, + } + ); + assert_eq!( + table + .checkpoint_workflow + .begin_checkpoint(&table.lifecycle) + .err() + .unwrap(), + CheckpointCancelReason::FreezeInProgress + ); + + drop(attempt); + assert_eq!(table.checkpoint_workflow.state_name(), "Idle"); + }); + } + #[test] fn test_repeated_freeze_returns_original_table_owned_batch() { smol::block_on(async { @@ -3790,7 +3954,7 @@ mod tests { } #[test] - fn test_cancelled_freeze_loading_restores_idle_and_active_pages() { + fn test_dropped_freeze_observer_does_not_cancel_loading() { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let engine = @@ -3800,8 +3964,8 @@ mod tests { insert_rows(table_id, &mut first_session, 0, 200, "cancel-freeze").await; let (entered_tx, entered_rx) = flume::bounded(1); - let (_release_tx, release_rx) = flume::bounded::<()>(1); - set_test_freeze_after_loading_hook(move || async move { + let (release_tx, release_rx) = flume::bounded::<()>(1); + set_test_freeze_after_loading_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -3836,22 +4000,24 @@ mod tests { ); } + release_tx.send_async(()).await.unwrap(); + wait_session_idle(&engine, &mut first_session).await; let table = table_for_internal_assertion(&engine, table_id); - assert_eq!(table.checkpoint_workflow.state_name(), "Idle"); - assert!(table.checkpoint_workflow.frozen_page_ids().is_none()); + assert_eq!(table.checkpoint_workflow.state_name(), "Frozen"); + assert!(table.checkpoint_workflow.frozen_page_ids().is_some()); let page_states = row_page_states(&table, &first_session.pool_guards()).await; assert!(!page_states.is_empty()); assert!( page_states .iter() - .all(|state| *state == RowPageState::Active) + .all(|state| *state == RowPageState::Frozen) ); assert!(matches!( first_session .freeze_table(table_id, usize::MAX) .await .unwrap(), - FreezeOutcome::Frozen { .. } + FreezeOutcome::AlreadyFrozen { .. } )); }); } @@ -3897,43 +4063,52 @@ mod tests { #[test] fn test_freeze_panics_on_non_active_selected_page() { - for unexpected in [RowPageState::Frozen, RowPageState::Transition] { - let panic = catch_unwind(AssertUnwindSafe(|| { - smol::block_on(async { - let temp_dir = TempDir::new().unwrap(); - let engine = lightweight_test_engine(&temp_dir, "freeze-invariant").await; - let table_id = create_table2_for_test(&engine).await; - let mut session = engine.new_session().unwrap(); - insert_rows(table_id, &mut session, 0, 1, "invariant").await; - let table = table_for_internal_assertion(&engine, table_id); - let page_id = hot_page_ids(&table, &session.pool_guards()).await[0]; - let page_guard = table - .mem - .must_get_row_page_shared(&session.pool_guards(), page_id) - .await - .unwrap(); - *page_guard.unwrap_vmap().write_state() = unexpected; - drop(page_guard); + smol::block_on(async { + for unexpected in [RowPageState::Frozen, RowPageState::Transition] { + let temp_dir = TempDir::new().unwrap(); + let engine = lightweight_test_engine(&temp_dir, "freeze-invariant").await; + let table_id = create_table2_for_test(&engine).await; + let mut session = engine.new_session().unwrap(); + insert_rows(table_id, &mut session, 0, 1, "invariant").await; + let table = table_for_internal_assertion(&engine, table_id); + let page_id = hot_page_ids(&table, &session.pool_guards()).await[0]; + let page_guard = table + .mem + .must_get_row_page_shared(&session.pool_guards(), page_id) + .await + .unwrap(); + *page_guard.unwrap_vmap().write_state() = unexpected; + drop(page_guard); - // The selected-page invariant must panic before an outcome is returned. - let _ = session.freeze_table(table_id, usize::MAX).await; - }); - })) - .expect_err("freeze must reject an unexpected selected-page state"); - let message = panic - .downcast_ref::() - .map(String::as_str) - .or_else(|| panic.downcast_ref::<&str>().copied()) - .unwrap_or("unknown panic"); - assert!( - message.contains("admitted freeze requires ACTIVE page"), - "unexpected panic for {unexpected:?}: {message}" - ); - } + // The selected-page invariant must panic before an ordinary + // freeze outcome can be returned. + let err = session + .freeze_table(table_id, usize::MAX) + .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) + ); + remove_session_for_test(&engine.inner().session_registry, session.id()); + drop(table); + drop(session); + engine.shutdown(); + } + }); } #[test] - fn test_cancelled_checkpoint_before_publish_restores_source_state() { + fn test_dropped_checkpoint_observer_completes_source_publication() { smol::block_on(async { for frozen_source in [false, true] { let temp_dir = TempDir::new().unwrap(); @@ -3941,20 +4116,19 @@ mod tests { let table_id = create_table2_for_test(&engine).await; let mut session = engine.new_session().unwrap(); insert_rows(table_id, &mut session, 0, 8, "cancel").await; + let target_ts = session.last_cts(); let table = table_for_internal_assertion(&engine, table_id); - let frozen_page_ids = if frozen_source { + if frozen_source { assert_freeze_created( session.freeze_table(table_id, usize::MAX).await.unwrap(), ); - table.checkpoint_workflow.frozen_page_ids().unwrap() - } else { - Vec::new() - }; + } + session.wait_for_gc_horizon_after(target_ts).await.unwrap(); wait_for_checkpoint_root_ready(&mut session, table_id).await; let (entered_tx, entered_rx) = flume::bounded(1); - let (_release_tx, release_rx) = flume::bounded::<()>(1); - set_test_checkpoint_after_trx_start_hook(move || async move { + let (release_tx, release_rx) = flume::bounded::<()>(1); + set_test_checkpoint_after_trx_start_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -3971,20 +4145,10 @@ mod tests { } } - if frozen_source { - assert_eq!(table.checkpoint_workflow.state_name(), "Frozen"); - assert_eq!( - table.checkpoint_workflow.frozen_page_ids().unwrap(), - frozen_page_ids - ); - let states = row_page_states(&table, &session.pool_guards()).await; - assert!(states.iter().all(|state| *state == RowPageState::Frozen)); - } else { - assert_eq!(table.checkpoint_workflow.state_name(), "Idle"); - let states = row_page_states(&table, &session.pool_guards()).await; - assert!(states.iter().all(|state| *state == RowPageState::Active)); - } + release_tx.send_async(()).await.unwrap(); wait_session_idle(&engine, &mut session).await; + assert_eq!(table.checkpoint_workflow.state_name(), "Idle"); + assert!(table.checkpoint_workflow.frozen_page_ids().is_none()); drop(table); drop(session); engine.shutdown(); @@ -4030,8 +4194,9 @@ mod tests { let (locked_tx, locked_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); let mut freeze_session = engine.new_session().unwrap(); + let maintenance_test = engine.inner().maintenance_test.clone(); let freeze_handle = thread::spawn(move || { - set_test_freeze_page_state_locked_hook(move |page_id| { + maintenance_test.install_freeze_page_state_locked_hook(move |page_id| { locked_tx.send(page_id).unwrap(); release_rx.recv().unwrap(); }); @@ -4169,7 +4334,7 @@ mod tests { wait_for_checkpoint_root_ready(&mut session, table_id).await; let res = { - let _guard = ForcePostPublishCheckpointErrorGuard::new(); + let _guard = ForcePostPublishCheckpointErrorGuard::new(&engine); session.checkpoint_table(table_id).await }; @@ -4197,7 +4362,7 @@ mod tests { let root_before = table.file().active_root_unchecked().clone(); let result = { - let _failure = ForceCheckpointCommitErrorGuard::new(); + let _failure = ForceCheckpointCommitErrorGuard::new(&engine); session.checkpoint_table(table_id).await }; let err = result.unwrap_err(); @@ -4297,17 +4462,16 @@ mod tests { assert_freeze_created(session.freeze_table(table_id, usize::MAX).await.unwrap()); let target_ts = session.last_cts(); session.wait_for_gc_horizon_after(target_ts).await.unwrap(); - let reader_holder: Rc>> = - Rc::new(RefCell::new(None)); - let reader_sts = Rc::new(Cell::new(TrxID::new(0))); - let hook_reader_holder = Rc::clone(&reader_holder); - let hook_reader_sts = Rc::clone(&reader_sts); + let reader_holder = Arc::new(parking_lot::Mutex::new(None)); + let reader_sts = Arc::new(parking_lot::Mutex::new(TrxID::new(0))); + let hook_reader_holder = Arc::clone(&reader_holder); + let hook_reader_sts = Arc::clone(&reader_sts); let hook_engine = engine.new_ref().unwrap(); - set_test_checkpoint_after_trx_start_hook(move || async move { + set_test_checkpoint_after_trx_start_hook(&engine, move || async move { let mut reader_session = hook_engine.new_session().unwrap(); let reader = reader_session.begin_trx().unwrap(); - hook_reader_sts.set(reader.sts()); - *hook_reader_holder.borrow_mut() = Some((reader_session, reader)); + *hook_reader_sts.lock() = reader.sts(); + *hook_reader_holder.lock() = Some((reader_session, reader)); }); let outcome = session.checkpoint_table(table_id).await.unwrap(); @@ -4319,8 +4483,8 @@ mod tests { .active_root_unchecked() .clone(); let effective_ts = active_root.effective_ts(); - assert!(checkpoint_ts < reader_sts.get()); - assert!(effective_ts > reader_sts.get()); + assert!(checkpoint_ts < *reader_sts.lock()); + assert!(effective_ts > *reader_sts.lock()); let delayed = session.checkpoint_table(table_id).await.unwrap(); let CheckpointOutcome::Delayed { reason } = delayed else { @@ -4336,11 +4500,11 @@ mod tests { }; assert_eq!(delayed_table_id, table_id); assert_eq!(delayed_effective_ts, effective_ts); - assert!(min_active_sts <= reader_sts.get()); + assert!(min_active_sts <= *reader_sts.lock()); assert!(delayed_effective_ts >= min_active_sts); let (_, mut reader) = reader_holder - .borrow_mut() + .lock() .take() .expect("reader hook should install an active transaction"); reader @@ -4549,18 +4713,21 @@ mod tests { panic!("expected active-root delay: {reason:?}"); }; - let hook_ran = Rc::new(Cell::new(false)); - let hook_flag = Rc::clone(&hook_ran); + let hook_ran = Arc::new(AtomicBool::new(false)); + let hook_flag = Arc::clone(&hook_ran); let trx_sys = engine.inner().trx_sys.clone(); - set_test_checkpoint_retry_after_listener_registration_hook(move || async move { - hook_flag.set(true); - assert!(trx_sys.publish_gc_horizon(effective_ts.saturating_add(1))); - }); + set_test_checkpoint_retry_after_listener_registration_hook( + &engine, + move || async move { + hook_flag.store(true, AtomicOrdering::Relaxed); + assert!(trx_sys.publish_gc_horizon(effective_ts.saturating_add(1))); + }, + ); checkpoint_session .wait_for_checkpoint_retry(reason) .await .unwrap(); - assert!(hook_ran.get()); + assert!(hook_ran.load(AtomicOrdering::Relaxed)); reader.rollback().await.unwrap(); engine.shutdown(); @@ -4677,7 +4844,7 @@ mod tests { let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); - set_test_checkpoint_after_trx_start_hook(move || async move { + set_test_checkpoint_after_trx_start_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -4740,7 +4907,7 @@ mod tests { let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); - set_test_checkpoint_after_trx_start_hook(move || async move { + set_test_checkpoint_after_trx_start_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -4796,7 +4963,7 @@ mod tests { .checkpoint_workflow .begin_checkpoint(&table.lifecycle) .unwrap(); - let root_lease = table.try_begin_checkpoint_root_mutation().unwrap(); + let root_lease = TableCheckpointRootMutationScope::acquire(Arc::clone(&table)).unwrap(); let frozen_pages = attempt.batch().unwrap().pages.clone(); let transition_pages = table .load_frozen_pages_for_transition(&session.pool_guards(), &frozen_pages) @@ -4804,13 +4971,14 @@ mod tests { .unwrap(); let cutoff_ts = engine.inner().trx_sys.published_gc_horizon(); let drop_table = Arc::clone(&table); - set_test_stable_page_plans_refreshed_hook(move || { + set_test_stable_page_plans_refreshed_hook(&engine, move || { let _drain = drop_table.start_drop_lifecycle().unwrap(); }); let delay = table.prepare_page_transition( &transition_pages, attempt.batch_mut().unwrap(), cutoff_ts, + &engine.inner().maintenance_test, ); assert!(delay.is_none()); let result = table @@ -4840,7 +5008,7 @@ mod tests { let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); - set_test_freeze_after_loading_hook(move || async move { + set_test_freeze_after_loading_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -4944,8 +5112,9 @@ mod tests { engine.inner().trx_sys.request_dropped_table_purge(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); wait_path_exists(&table_file_path, false).await; @@ -5073,7 +5242,7 @@ mod tests { let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); - set_test_checkpoint_after_publish_admission_hook(move || async move { + set_test_checkpoint_after_publish_admission_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -5160,27 +5329,27 @@ mod tests { .must_get_row_page_shared(&frozen_foreground.pool_guards(), frozen_page_ids[1]) .await .unwrap(); - let analysis_count = Rc::new(Cell::new(0usize)); - let first_page_analysis_count = Rc::new(Cell::new(0usize)); - let mutation_started = Rc::new(Cell::new(false)); - let hook_analysis_count = Rc::clone(&analysis_count); - let hook_first_page_analysis_count = Rc::clone(&first_page_analysis_count); - set_test_frozen_page_scan_hook(move |page_id| { - hook_analysis_count.set(hook_analysis_count.get() + 1); + let analysis_count = Arc::new(AtomicUsize::new(0)); + let first_page_analysis_count = Arc::new(AtomicUsize::new(0)); + let mutation_started = Arc::new(AtomicBool::new(false)); + let hook_analysis_count = Arc::clone(&analysis_count); + let hook_first_page_analysis_count = Arc::clone(&first_page_analysis_count); + set_test_frozen_page_scan_hook(&engine, move |page_id| { + hook_analysis_count.fetch_add(1, AtomicOrdering::Relaxed); if page_id != first_frozen_page_id { return; } - hook_first_page_analysis_count.set(hook_first_page_analysis_count.get() + 1); + hook_first_page_analysis_count.fetch_add(1, AtomicOrdering::Relaxed); }); - let first_frozen_page = Rc::new(RefCell::new(Some(first_frozen_page))); - let row_hook_page = Rc::clone(&first_frozen_page); - let row_hook_mutation_started = Rc::clone(&mutation_started); - set_test_frozen_page_row_scan_hook(move |page_id, row_idx| { + let first_frozen_page = Arc::new(parking_lot::Mutex::new(Some(first_frozen_page))); + let row_hook_page = Arc::clone(&first_frozen_page); + let row_hook_mutation_started = Arc::clone(&mutation_started); + set_test_frozen_page_row_scan_hook(&engine, move |page_id, row_idx| { if page_id == first_frozen_page_id && row_idx == 0 - && !row_hook_mutation_started.replace(true) + && !row_hook_mutation_started.swap(true, AtomicOrdering::Relaxed) { - let page_guard = row_hook_page.borrow(); + let page_guard = row_hook_page.lock(); page_guard .as_ref() .unwrap() @@ -5188,15 +5357,16 @@ mod tests { .begin_frozen_mutation(); } }); - let comparison_hook_page = Rc::clone(&first_frozen_page); + let comparison_hook_page = Arc::clone(&first_frozen_page); set_test_optimistic_page_plan_comparison_hook( + &engine, move |page_id, version_before, version_after, retained| { if page_id != first_frozen_page_id || version_before == version_after { return; } assert_eq!(version_after, version_before + 1); assert!(!retained); - let page_guard = comparison_hook_page.borrow(); + let page_guard = comparison_hook_page.lock(); let guard_ref = page_guard.as_ref().unwrap(); let page = guard_ref.page(); let row_idx = page.header.row_count() - 1; @@ -5204,7 +5374,7 @@ mod tests { page.inc_approx_deleted(); guard_ref.unwrap_vmap().finish_frozen_mutation(); drop(page_guard); - comparison_hook_page.borrow_mut().take(); + comparison_hook_page.lock().take(); }, ); let suffix_key = { @@ -5285,7 +5455,7 @@ mod tests { }); let hook_prefix_update_done_rx = prefix_update_done_rx.clone(); let hook_prefix_delete_done_rx = prefix_delete_done_rx.clone(); - set_test_transition_page_published_hook(move |page_id| { + set_test_transition_page_published_hook(&engine, move |page_id| { assert_eq!(page_id, first_frozen_page_id); assert_eq!( second_frozen_page.unwrap_vmap().inspect_state(), @@ -5307,7 +5477,7 @@ mod tests { let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); - set_test_checkpoint_after_publish_admission_hook(move || async move { + set_test_checkpoint_after_publish_admission_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -5350,8 +5520,11 @@ mod tests { checkpoint.await.unwrap() }; assert!(matches!(outcome, CheckpointOutcome::Published { .. })); - assert_eq!(analysis_count.get(), frozen_page_count + 2); - assert_eq!(first_page_analysis_count.get(), 2); + assert_eq!( + analysis_count.load(AtomicOrdering::Relaxed), + frozen_page_count + 2 + ); + assert_eq!(first_page_analysis_count.load(AtomicOrdering::Relaxed), 2); prefix_update_thread.join().unwrap(); prefix_delete_thread.join().unwrap(); suffix_delete_thread.join().unwrap(); @@ -5408,49 +5581,48 @@ mod tests { .must_get_row_page_shared(&session.pool_guards(), second_page_id) .await .unwrap(); - let first_page = Rc::new(RefCell::new(Some(first_page))); - let second_page = Rc::new(RefCell::new(Some(second_page))); - let total_analysis_count = Rc::new(Cell::new(0usize)); - let first_analysis_count = Rc::new(Cell::new(0usize)); - let second_analysis_count = Rc::new(Cell::new(0usize)); - let cross_page_delete_completed = Rc::new(Cell::new(false)); - - let refreshed_first_page = Rc::clone(&first_page); - set_test_stable_page_plans_refreshed_hook(move || { - let page_guard = refreshed_first_page.borrow_mut().take().unwrap(); + let first_page = Arc::new(parking_lot::Mutex::new(Some(first_page))); + let second_page = Arc::new(parking_lot::Mutex::new(Some(second_page))); + let total_analysis_count = Arc::new(AtomicUsize::new(0)); + let first_analysis_count = Arc::new(AtomicUsize::new(0)); + let second_analysis_count = Arc::new(AtomicUsize::new(0)); + let cross_page_delete_completed = Arc::new(AtomicBool::new(false)); + + let refreshed_first_page = Arc::clone(&first_page); + set_test_stable_page_plans_refreshed_hook(&engine, move || { + let page_guard = refreshed_first_page.lock().take().unwrap(); delete_last_frozen_row_image(page_guard); }); - let hook_total_analysis_count = Rc::clone(&total_analysis_count); - let hook_first_analysis_count = Rc::clone(&first_analysis_count); - let hook_second_analysis_count = Rc::clone(&second_analysis_count); - let hook_second_page = Rc::clone(&second_page); - let hook_cross_page_delete_completed = Rc::clone(&cross_page_delete_completed); - set_test_frozen_page_scan_hook(move |page_id| { - hook_total_analysis_count.set(hook_total_analysis_count.get() + 1); + let hook_total_analysis_count = Arc::clone(&total_analysis_count); + let hook_first_analysis_count = Arc::clone(&first_analysis_count); + let hook_second_analysis_count = Arc::clone(&second_analysis_count); + let hook_second_page = Arc::clone(&second_page); + let hook_cross_page_delete_completed = Arc::clone(&cross_page_delete_completed); + set_test_frozen_page_scan_hook(&engine, move |page_id| { + hook_total_analysis_count.fetch_add(1, AtomicOrdering::Relaxed); if page_id == first_page_id { - let count = hook_first_analysis_count.get() + 1; - hook_first_analysis_count.set(count); + let count = hook_first_analysis_count.fetch_add(1, AtomicOrdering::Relaxed) + 1; if count == 2 { - let page_guard = hook_second_page.borrow_mut().take().unwrap(); + let page_guard = hook_second_page.lock().take().unwrap(); assert_eq!( page_guard.unwrap_vmap().inspect_state(), RowPageState::Frozen ); delete_last_frozen_row_image(page_guard); - hook_cross_page_delete_completed.set(true); + hook_cross_page_delete_completed.store(true, AtomicOrdering::Relaxed); } } else if page_id == second_page_id { - hook_second_analysis_count.set(hook_second_analysis_count.get() + 1); + hook_second_analysis_count.fetch_add(1, AtomicOrdering::Relaxed); } }); let outcome = session.checkpoint_table(table_id).await.unwrap(); assert!(matches!(outcome, CheckpointOutcome::Published { .. })); - assert!(cross_page_delete_completed.get()); - assert!(first_analysis_count.get() >= 2); - assert!(second_analysis_count.get() >= 2); - assert!(total_analysis_count.get() >= page_ids.len() + 2); + assert!(cross_page_delete_completed.load(AtomicOrdering::Relaxed)); + assert!(first_analysis_count.load(AtomicOrdering::Relaxed) >= 2); + assert!(second_analysis_count.load(AtomicOrdering::Relaxed) >= 2); + assert!(total_analysis_count.load(AtomicOrdering::Relaxed) >= page_ids.len() + 2); }); } @@ -5480,29 +5652,29 @@ mod tests { .unwrap(), )); } - let page_guards = Rc::new(RefCell::new(page_guards)); - let analysis_counts = Rc::new(RefCell::new(vec![0usize; page_ids.len()])); + let page_guards = Arc::new(parking_lot::Mutex::new(page_guards)); + let analysis_counts = Arc::new(parking_lot::Mutex::new(vec![0usize; page_ids.len()])); - let refreshed_page_guards = Rc::clone(&page_guards); - set_test_stable_page_plans_refreshed_hook(move || { - for page_guard in refreshed_page_guards.borrow_mut().iter_mut() { + let refreshed_page_guards = Arc::clone(&page_guards); + set_test_stable_page_plans_refreshed_hook(&engine, move || { + for page_guard in refreshed_page_guards.lock().iter_mut() { let page_guard = page_guard.take().unwrap(); delete_last_frozen_row_image(page_guard); } }); let hook_page_ids = page_ids.clone(); - let hook_analysis_counts = Rc::clone(&analysis_counts); - set_test_frozen_page_scan_hook(move |page_id| { + let hook_analysis_counts = Arc::clone(&analysis_counts); + set_test_frozen_page_scan_hook(&engine, move |page_id| { let page_idx = hook_page_ids .iter() .position(|candidate| *candidate == page_id) .unwrap(); - hook_analysis_counts.borrow_mut()[page_idx] += 1; + hook_analysis_counts.lock()[page_idx] += 1; }); let outcome = session.checkpoint_table(table_id).await.unwrap(); assert!(matches!(outcome, CheckpointOutcome::Published { .. })); - assert!(analysis_counts.borrow().iter().all(|count| *count >= 2)); + assert!(analysis_counts.lock().iter().all(|count| *count >= 2)); }); } @@ -5529,17 +5701,17 @@ mod tests { .await .unwrap(); let row_id = first_page.page().row_id(0); - let first_page = Rc::new(RefCell::new(Some(first_page))); - let undo_owner = Rc::new(RefCell::new(None)); - let hook_undo_owner = Rc::clone(&undo_owner); - let hook_first_page = Rc::clone(&first_page); + let first_page = Arc::new(parking_lot::Mutex::new(Some(first_page))); + let undo_owner = Arc::new(parking_lot::Mutex::new(None)); + let hook_undo_owner = Arc::clone(&undo_owner); + let hook_first_page = Arc::clone(&first_page); let pre_fence_sts = batch.frozen_ts().saturating_sub(2); let ownership_status = Arc::new(shared_trx_status( MIN_ACTIVE_TRX_ID + pre_fence_sts.as_u64(), )); let hook_ownership_status = Arc::clone(&ownership_status); - set_test_frozen_pages_ready_hook(move || { - let page_guard = hook_first_page.borrow_mut().take().unwrap(); + set_test_frozen_pages_ready_hook(&engine, move || { + let page_guard = hook_first_page.lock().take().unwrap(); let page = page_guard.page(); let map = page_guard.unwrap_vmap(); let undo = OwnedRowUndo::new(table_id, None, page.row_id(0), RowUndoKind::Lock); @@ -5549,24 +5721,24 @@ mod tests { undo.leak(), ))); map.finish_frozen_mutation(); - hook_undo_owner.borrow_mut().replace(undo); + hook_undo_owner.lock().replace(undo); drop(page_guard); }); - let publish_admitted = Rc::new(Cell::new(false)); - let hook_publish_admitted = Rc::clone(&publish_admitted); - set_test_checkpoint_after_publish_admission_hook(move || async move { - hook_publish_admitted.set(true); + let publish_admitted = Arc::new(AtomicBool::new(false)); + let hook_publish_admitted = Arc::clone(&publish_admitted); + set_test_checkpoint_after_publish_admission_hook(&engine, move || async move { + hook_publish_admitted.store(true, AtomicOrdering::Relaxed); }); let outcome = session.checkpoint_table(table_id).await.unwrap(); assert!(matches!(outcome, CheckpointOutcome::Published { .. })); - assert!(publish_admitted.get()); + assert!(publish_admitted.load(AtomicOrdering::Relaxed)); assert_eq!(table.checkpoint_workflow.state_name(), "Idle"); let Some(DeleteMarker::Ref(marker_status)) = table.deletion_buffer().get(row_id) else { panic!("stable-plan refresh must install the ownership marker"); }; assert!(Arc::ptr_eq(&marker_status, &ownership_status)); - undo_owner.borrow_mut().take(); + undo_owner.lock().take(); }); } @@ -5593,17 +5765,17 @@ mod tests { .await .unwrap(); let row_id = first_page.page().row_id(0); - let first_page = Rc::new(RefCell::new(Some(first_page))); - let undo_owner = Rc::new(RefCell::new(None)); - let hook_undo_owner = Rc::clone(&undo_owner); - let hook_first_page = Rc::clone(&first_page); + let first_page = Arc::new(parking_lot::Mutex::new(Some(first_page))); + let undo_owner = Arc::new(parking_lot::Mutex::new(None)); + let hook_undo_owner = Arc::clone(&undo_owner); + let hook_first_page = Arc::clone(&first_page); let pre_fence_sts = batch.frozen_ts().saturating_sub(2); let ownership_status = Arc::new(shared_trx_status( MIN_ACTIVE_TRX_ID + pre_fence_sts.as_u64(), )); let hook_ownership_status = Arc::clone(&ownership_status); - set_test_stable_page_plans_refreshed_hook(move || { - let page_guard = hook_first_page.borrow_mut().take().unwrap(); + set_test_stable_page_plans_refreshed_hook(&engine, move || { + let page_guard = hook_first_page.lock().take().unwrap(); let page = page_guard.page(); let map = page_guard.unwrap_vmap(); let undo = OwnedRowUndo::new(table_id, None, page.row_id(0), RowUndoKind::Lock); @@ -5613,13 +5785,13 @@ mod tests { undo.leak(), ))); map.finish_frozen_mutation(); - hook_undo_owner.borrow_mut().replace(undo); + hook_undo_owner.lock().replace(undo); drop(page_guard); }); - let locked_rebuild_observed = Rc::new(Cell::new(false)); - let hook_locked_rebuild_observed = Rc::clone(&locked_rebuild_observed); + let locked_rebuild_observed = Arc::new(AtomicBool::new(false)); + let hook_locked_rebuild_observed = Arc::clone(&locked_rebuild_observed); let hook_table = Arc::downgrade(&table); - set_test_locked_page_plan_rebuild_hook(move |page_id| { + set_test_locked_page_plan_rebuild_hook(&engine, move |page_id| { assert_eq!(page_id, first_page_id); assert_eq!( hook_table @@ -5629,24 +5801,24 @@ mod tests { .state_name(), "Transition" ); - hook_locked_rebuild_observed.set(true); + hook_locked_rebuild_observed.store(true, AtomicOrdering::Relaxed); }); - let publish_admitted = Rc::new(Cell::new(false)); - let hook_publish_admitted = Rc::clone(&publish_admitted); - set_test_checkpoint_after_publish_admission_hook(move || async move { - hook_publish_admitted.set(true); + let publish_admitted = Arc::new(AtomicBool::new(false)); + let hook_publish_admitted = Arc::clone(&publish_admitted); + set_test_checkpoint_after_publish_admission_hook(&engine, move || async move { + hook_publish_admitted.store(true, AtomicOrdering::Relaxed); }); let outcome = session.checkpoint_table(table_id).await.unwrap(); assert!(matches!(outcome, CheckpointOutcome::Published { .. })); - assert!(publish_admitted.get()); - assert!(locked_rebuild_observed.get()); + assert!(publish_admitted.load(AtomicOrdering::Relaxed)); + assert!(locked_rebuild_observed.load(AtomicOrdering::Relaxed)); assert_eq!(table.checkpoint_workflow.state_name(), "Idle"); let Some(DeleteMarker::Ref(marker_status)) = table.deletion_buffer().get(row_id) else { panic!("locked rebuild must install the ownership marker"); }; assert!(Arc::ptr_eq(&marker_status, &ownership_status)); - undo_owner.borrow_mut().take(); + undo_owner.lock().take(); }); } @@ -5673,7 +5845,7 @@ mod tests { let (entered_tx, entered_rx) = flume::bounded(1); let (release_tx, release_rx) = flume::bounded(1); - set_test_checkpoint_after_publish_admission_hook(move || async move { + set_test_checkpoint_after_publish_admission_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); release_rx.recv_async().await.unwrap(); }); @@ -5682,7 +5854,7 @@ mod tests { let mut delete_session = engine.new_session().unwrap(); let mut delete_trx = delete_session.begin_trx().unwrap(); let (update_err, delete_err) = { - let _failure = ForceLwcBuildErrorGuard::new(); + let _failure = ForceLwcBuildErrorGuard::new(&engine); let checkpoint = checkpoint_session.checkpoint_table(table_id).fuse(); futures::pin_mut!(checkpoint); let entered = entered_rx.recv_async().fuse(); @@ -5935,16 +6107,17 @@ mod tests { } #[test] - fn test_publication_admission_cancellation_remains_reversible() { + fn test_dropped_publication_observer_does_not_cancel_accepted_checkpoint() { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let engine = lightweight_test_engine(&temp_dir, "publish-admission-cancel").await; let mut session = engine.new_session().unwrap(); let (table_id, _) = prepare_silent_checkpoint_failure(&engine, &mut session).await; let (entered_tx, entered_rx) = flume::bounded(1); - set_test_checkpoint_after_publish_admission_hook(move || async move { + let (release_tx, release_rx) = flume::bounded(1); + set_test_checkpoint_after_publish_admission_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); - pending().await + release_rx.recv_async().await.unwrap(); }); let mut checkpoint = Box::pin(session.checkpoint_table(table_id).fuse()); let mut entered = Box::pin(entered_rx.recv_async().fuse()); @@ -5956,18 +6129,23 @@ mod tests { } drop(checkpoint); - assert!(engine.inner().poisoner.poison_error().is_none()); + release_tx.send_async(()).await.unwrap(); + wait_session_idle(&engine, &mut session).await; assert_eq!( table_for_internal_assertion(&engine, table_id) .checkpoint_workflow .state_name(), "Idle" ); - assert!(matches!( - session.checkpoint_table(table_id).await.unwrap(), - CheckpointOutcome::Published { silent: true, .. } - )); + let watermark = engine + .catalog() + .storage + .table_replay_silent_watermarks() + .find_uncommitted_by_table_id(&session.pool_guards(), table_id) + .await + .unwrap(); + assert!(watermark.is_some()); }); } @@ -5980,7 +6158,7 @@ mod tests { let (table_id, root_before) = prepare_silent_checkpoint_failure(&engine, &mut session).await; let guards = session.pool_guards(); - set_test_silent_watermark_mutation_hook(|| async { + set_test_silent_watermark_mutation_hook(&engine, || async { Err(Report::new(InternalError::SecondaryIndexBindingMismatch) .attach("test silent-watermark mutation failure") .change_context(RuntimeError::CatalogAccess)) @@ -6006,7 +6184,7 @@ mod tests { } #[test] - fn test_silent_watermark_mutation_cancellation_poisons_catalog_write() { + fn test_dropped_observer_preserves_silent_watermark_mutation_failure() { smol::block_on(async { let temp_dir = TempDir::new().unwrap(); let engine = lightweight_test_engine(&temp_dir, "silent-mutation-cancel").await; @@ -6015,9 +6193,13 @@ mod tests { prepare_silent_checkpoint_failure(&engine, &mut session).await; let guards = session.pool_guards(); let (entered_tx, entered_rx) = flume::bounded(1); - set_test_silent_watermark_mutation_hook(move || async move { + let (release_tx, release_rx) = flume::bounded(1); + set_test_silent_watermark_mutation_hook(&engine, move || async move { entered_tx.send_async(()).await.unwrap(); - pending().await + release_rx.recv_async().await.unwrap(); + Err(Report::new(InternalError::SecondaryIndexBindingMismatch) + .attach("test observer-dropped silent-watermark mutation failure") + .change_context(RuntimeError::CatalogAccess)) }); let mut checkpoint = Box::pin(session.checkpoint_table(table_id).fuse()); let mut entered = Box::pin(entered_rx.recv_async().fuse()); @@ -6029,12 +6211,13 @@ mod tests { } drop(checkpoint); - + release_tx.send_async(()).await.unwrap(); + wait_session_idle(&engine, &mut session).await; let poison = engine .inner() .poisoner .poison_error() - .expect("cancelled silent mutation should poison storage"); + .expect("observer-dropped silent mutation failure should poison storage"); assert_eq!(*poison.current_context(), FatalError::CatalogWrite); assert_silent_watermark_absent(&engine, &guards, table_id).await; assert_root_metadata_unchanged( @@ -6486,16 +6669,16 @@ mod tests { .await .unwrap(), ); - let analysis_count = Rc::new(Cell::new(0usize)); - let hook_analysis_count = Rc::clone(&analysis_count); - set_test_frozen_page_scan_hook(move |_| { - hook_analysis_count.set(hook_analysis_count.get() + 1); + let analysis_count = Arc::new(AtomicUsize::new(0)); + let hook_analysis_count = Arc::clone(&analysis_count); + set_test_frozen_page_scan_hook(&engine, move |_| { + hook_analysis_count.fetch_add(1, AtomicOrdering::Relaxed); }); let outcome = checkpoint_session.checkpoint_table(table_id).await.unwrap(); let CheckpointOutcome::Delayed { reason } = outcome else { panic!("two unresolved images should delay checkpoint: {outcome:?}"); }; - assert_eq!(analysis_count.get(), 1); + assert_eq!(analysis_count.load(AtomicOrdering::Relaxed), 1); assert!(matches!( reason, CheckpointDelayReason::FrozenPageCutoff { @@ -6508,7 +6691,7 @@ mod tests { let frozen_page_ids = table.checkpoint_workflow.frozen_page_ids().unwrap(); let mut cancelled_wait = Box::pin(checkpoint_session.wait_for_checkpoint_retry(reason)); assert!(futures::poll!(cancelled_wait.as_mut()).is_pending()); - assert_eq!(analysis_count.get(), 1); + assert_eq!(analysis_count.load(AtomicOrdering::Relaxed), 1); assert_eq!(table.checkpoint_workflow.state_name(), "Frozen"); drop(cancelled_wait); assert_eq!(table.checkpoint_workflow.state_name(), "Frozen"); @@ -6519,20 +6702,20 @@ mod tests { let mut wait = Box::pin(checkpoint_session.wait_for_checkpoint_retry(reason)); assert!(futures::poll!(wait.as_mut()).is_pending()); - assert_eq!(analysis_count.get(), 1); + assert_eq!(analysis_count.load(AtomicOrdering::Relaxed), 1); writer1.rollback().await.unwrap(); assert!(futures::poll!(wait.as_mut()).is_pending()); - assert_eq!(analysis_count.get(), 1); + assert_eq!(analysis_count.load(AtomicOrdering::Relaxed), 1); let mut unrelated_session = engine.new_session().unwrap(); let unrelated = unrelated_session.begin_trx().unwrap(); unrelated.rollback().await.unwrap(); assert!(futures::poll!(wait.as_mut()).is_pending()); - assert_eq!(analysis_count.get(), 1); + assert_eq!(analysis_count.load(AtomicOrdering::Relaxed), 1); writer2.rollback().await.unwrap(); wait.await.unwrap(); - assert_eq!(analysis_count.get(), 2); + assert_eq!(analysis_count.load(AtomicOrdering::Relaxed), 2); assert!(matches!( checkpoint_session .checkpoint_table_with_wait(table_id) @@ -6592,11 +6775,11 @@ mod tests { .unwrap(); let cutoff_ts = trx_sys.published_gc_horizon(); assert!(cutoff_ts <= writer_cts); - let analysis_count = Rc::new(Cell::new(0usize)); - let hook_analysis_count = Rc::clone(&analysis_count); - set_test_frozen_page_scan_hook(move |page_id| { + let analysis_count = Arc::new(AtomicUsize::new(0)); + let hook_analysis_count = Arc::clone(&analysis_count); + set_test_frozen_page_scan_hook(&engine, move |page_id| { assert_eq!(page_id, frozen_page_id); - hook_analysis_count.set(hook_analysis_count.get() + 1); + hook_analysis_count.fetch_add(1, AtomicOrdering::Relaxed); }); let outcome = checkpoint_session.checkpoint_table(table_id).await.unwrap(); @@ -6622,7 +6805,7 @@ mod tests { panic!("delayed checkpoint should retain its frozen batch: {repeated:?}"); }; assert_eq!(batch.stable_page_count(), 1); - assert_eq!(analysis_count.get(), 1); + assert_eq!(analysis_count.load(AtomicOrdering::Relaxed), 1); let same_cutoff_retry = checkpoint_session.checkpoint_table(table_id).await.unwrap(); let CheckpointOutcome::Delayed { reason } = same_cutoff_retry else { @@ -6636,7 +6819,7 @@ mod tests { .. } if retry_cutoff_ts == cutoff_ts )); - assert_eq!(analysis_count.get(), 1); + assert_eq!(analysis_count.load(AtomicOrdering::Relaxed), 1); reader.commit().await.unwrap(); checkpoint_session @@ -6648,7 +6831,7 @@ mod tests { retry, CheckpointOutcome::Published { silent: false, .. } )); - assert!(analysis_count.get() >= 2); + assert!(analysis_count.load(AtomicOrdering::Relaxed) >= 2); }); } @@ -6693,25 +6876,25 @@ mod tests { .unwrap(); wait_for_checkpoint_root_ready(&mut setup, table_id).await; - let analysis_counts = Rc::new(RefCell::new(vec![0usize; page_ids.len()])); + let analysis_counts = Arc::new(parking_lot::Mutex::new(vec![0usize; page_ids.len()])); let hook_page_ids = page_ids.clone(); - let hook_analysis_counts = Rc::clone(&analysis_counts); - set_test_frozen_page_scan_hook(move |page_id| { + let hook_analysis_counts = Arc::clone(&analysis_counts); + set_test_frozen_page_scan_hook(&engine, move |page_id| { let page_idx = hook_page_ids .iter() .position(|candidate| *candidate == page_id) .unwrap(); - hook_analysis_counts.borrow_mut()[page_idx] += 1; + hook_analysis_counts.lock()[page_idx] += 1; }); - let ready_hook_ran = Rc::new(Cell::new(false)); - let hook_ready_hook_ran = Rc::clone(&ready_hook_ran); - let counts_at_first_delay = Rc::new(RefCell::new(Vec::new())); - let hook_counts_at_first_delay = Rc::clone(&counts_at_first_delay); - let ready_hook_analysis_counts = Rc::clone(&analysis_counts); - set_test_frozen_pages_ready_hook(move || { - hook_ready_hook_ran.set(true); - let before_retry = hook_counts_at_first_delay.borrow(); - let after_readiness = ready_hook_analysis_counts.borrow(); + let ready_hook_ran = Arc::new(AtomicBool::new(false)); + let hook_ready_hook_ran = Arc::clone(&ready_hook_ran); + let counts_at_first_delay = Arc::new(parking_lot::Mutex::new(Vec::new())); + let hook_counts_at_first_delay = Arc::clone(&counts_at_first_delay); + let ready_hook_analysis_counts = Arc::clone(&analysis_counts); + set_test_frozen_pages_ready_hook(&engine, move || { + hook_ready_hook_ran.store(true, AtomicOrdering::Relaxed); + let before_retry = hook_counts_at_first_delay.lock(); + let after_readiness = ready_hook_analysis_counts.lock(); assert_eq!(before_retry.len(), after_readiness.len()); for (page_idx, (before, after)) in before_retry.iter().zip(after_readiness.iter()).enumerate() @@ -6741,18 +6924,18 @@ mod tests { assert_eq!(page_id, delayed_page_id); assert_eq!(stable_page_count, delayed_page_idx); assert!(unresolved_status); - assert!(!ready_hook_ran.get()); + assert!(!ready_hook_ran.load(AtomicOrdering::Relaxed)); assert!( - analysis_counts.borrow()[..=delayed_page_idx] + analysis_counts.lock()[..=delayed_page_idx] .iter() .all(|count| *count == 1) ); assert!( - analysis_counts.borrow()[delayed_page_idx + 1..] + analysis_counts.lock()[delayed_page_idx + 1..] .iter() .all(|count| *count == 0) ); - *counts_at_first_delay.borrow_mut() = analysis_counts.borrow().clone(); + *counts_at_first_delay.lock() = analysis_counts.lock().clone(); let validation = table.checkpoint_workflow.frozen_page_validation().unwrap(); assert_eq!(validation.len(), page_ids.len()); @@ -6783,10 +6966,10 @@ mod tests { setup.wait_for_checkpoint_retry(reason).await.unwrap(); let retry = setup.checkpoint_table(table_id).await.unwrap(); assert!(matches!(retry, CheckpointOutcome::Published { .. })); - assert!(ready_hook_ran.get()); - assert!(analysis_counts.borrow()[delayed_page_idx] >= 2); + assert!(ready_hook_ran.load(AtomicOrdering::Relaxed)); + assert!(analysis_counts.lock()[delayed_page_idx] >= 2); assert!( - analysis_counts.borrow()[delayed_page_idx + 1..] + analysis_counts.lock()[delayed_page_idx + 1..] .iter() .all(|count| *count >= 1) ); @@ -6898,7 +7081,7 @@ mod tests { wait_for_checkpoint_root_ready(&mut session, table_id).await; let res = { - let _guard = ForceLwcBuildErrorGuard::new(); + let _guard = ForceLwcBuildErrorGuard::new(&engine); session.checkpoint_table(table_id).await }; assert!(res.is_err()); diff --git a/doradb-storage/src/table/recover.rs b/doradb-storage/src/table/recover.rs index f417429a..40f40e94 100644 --- a/doradb-storage/src/table/recover.rs +++ b/doradb-storage/src/table/recover.rs @@ -260,6 +260,7 @@ mod tests { use crate::buffer::page::PAGE_SIZE; use crate::catalog::tests::{ assert_dropped_table_floor, assert_no_dropped_table_operational_state, + wait_for_no_dropped_table_operational_state, }; use crate::catalog::{TableMetadata, USER_TABLE_ID_START}; use crate::engine::Engine; @@ -660,11 +661,13 @@ mod tests { let (table_spec, index_specs) = drop_table_test_spec(); let _ = session.create_table(table_spec, index_specs).await.unwrap(); engine - .catalog() - .checkpoint_now(&engine.inner().trx_sys) + .new_session() + .unwrap() + .checkpoint_catalog() .await .unwrap(); - wait_path_exists(&table_file_path, false).await; + wait_for_no_dropped_table_operational_state(&engine, table_id).await; + assert!(!std::path::Path::new(&table_file_path).exists()); assert!(engine.catalog().retained_dropped_table_ids_now().is_empty()); assert_no_dropped_table_operational_state(engine.catalog(), table_id); assert_eq!( diff --git a/doradb-storage/src/trx/mod.rs b/doradb-storage/src/trx/mod.rs index 0fd16c7a..7acc95a9 100644 --- a/doradb-storage/src/trx/mod.rs +++ b/doradb-storage/src/trx/mod.rs @@ -27,6 +27,10 @@ mod sys_trx; pub(crate) mod undo; pub(crate) mod ver_map; +pub(crate) use retention::{ + prepare_catalog_redo_maintenance_operation, prepare_redo_truncation_operation, +}; +pub(crate) use sys::RedoRetentionScope; pub(crate) use sys_trx::{RetiredRowPageBatch, SysTrxPayload}; use crate::buffer::PoolGuards; diff --git a/doradb-storage/src/trx/retention.rs b/doradb-storage/src/trx/retention.rs index 0f9e6346..366dbb18 100644 --- a/doradb-storage/src/trx/retention.rs +++ b/doradb-storage/src/trx/retention.rs @@ -1,7 +1,7 @@ -use crate::catalog::CatalogCheckpointOutcome; +use crate::catalog::{CatalogCheckpointOutcome, CatalogCheckpointScope}; use crate::error::{ - DataIntegrityError, DataIntegrityResult, FatalError, IoError, RuntimeError, - RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, + CompletionErrorBridge, CompletionResult, DataIntegrityError, DataIntegrityResult, FatalError, + IoError, RuntimeError, RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, }; use crate::id::{TableID, TrxID}; use crate::log::{ @@ -11,11 +11,16 @@ use crate::obs; use crate::recovery::stream::{ RedoReplayPlanner, RedoRetentionSegment, RedoRetentionSegmentState, RedoSegmentCtsRange, }; +use crate::runtime::mandatory::PreparedExecution; use crate::session::{ - CatalogRedoMaintenanceOutcome, RedoTruncationBlockerInfo, RedoTruncationOutcome, + AcceptedMaintenanceScope, CatalogRedoMaintenanceOutcome, MaintenanceExecutionSpec, + PreparedMaintenanceExecution, PreparedMaintenanceScope, RedoTruncationBlockerInfo, + RedoTruncationOutcome, }; +#[cfg(test)] +use crate::table::tests::MaintenanceTestController; use crate::table::{LiveTableRedoReplayFloor, TableRedoReplayFloor}; -use crate::trx::sys::{CatalogRedoRetentionProgress, TransactionSystem}; +use crate::trx::sys::{CatalogRedoRetentionProgress, RedoRetentionScope, TransactionSystem}; use error_stack::{Report, ResultExt}; use std::collections::{BTreeMap, BTreeSet}; use std::fs; @@ -112,6 +117,113 @@ impl PendingDroppedTableRedoFloor { } } +struct RedoTruncationResources { + catalog_scope: CatalogCheckpointScope, + _redo_scope: RedoRetentionScope, +} + +struct RedoTruncationExecution; + +impl MaintenanceExecutionSpec for RedoTruncationExecution { + type Output = RedoTruncationOutcome; + type Resources = RedoTruncationResources; + type PanicLabel = &'static str; + + const LABEL: &'static str = "truncate_redo_log"; + + async fn execute( + scope: &mut AcceptedMaintenanceScope, + resources: &mut Self::Resources, + _panic_label: &mut Self::PanicLabel, + ) -> CompletionResult { + let engine = scope.engine().clone(); + let result = engine + .trx_sys + .truncate_redo_log_prepared( + || resources.catalog_scope.release(), + #[cfg(test)] + &engine.maintenance_test, + ) + .await + .map_err(CompletionErrorBridge::capture_runtime_or_fatal); + scope.mark_terminal_ready(); + result + } +} + +struct CatalogRedoMaintenanceResources { + catalog_scope: CatalogCheckpointScope, + _redo_scope: RedoRetentionScope, +} + +struct CatalogRedoMaintenanceExecution; + +impl MaintenanceExecutionSpec for CatalogRedoMaintenanceExecution { + type Output = CatalogRedoMaintenanceOutcome; + type Resources = CatalogRedoMaintenanceResources; + type PanicLabel = &'static str; + + const LABEL: &'static str = "checkpoint_catalog_and_truncate_redo_log"; + + async fn execute( + scope: &mut AcceptedMaintenanceScope, + resources: &mut Self::Resources, + _panic_label: &mut Self::PanicLabel, + ) -> CompletionResult { + let engine = scope.engine().clone(); + let result = engine + .trx_sys + .checkpoint_catalog_and_truncate_redo_log_prepared( + || resources.catalog_scope.release(), + #[cfg(test)] + &engine.maintenance_test, + ) + .await + .map_err(CompletionErrorBridge::capture_runtime_or_fatal); + scope.mark_terminal_ready(); + result + } +} + +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +struct RedoCleanupCounts { + removed_files: usize, + already_missing_files: usize, + failed_unlink_files: usize, +} + +/// Prepare one redo truncation with catalog and redo authority held. +pub(crate) fn prepare_redo_truncation_operation( + catalog_scope: CatalogCheckpointScope, + redo_scope: RedoRetentionScope, + scope: PreparedMaintenanceScope, +) -> impl PreparedExecution { + PreparedMaintenanceExecution::::global( + scope, + RedoTruncationResources { + catalog_scope, + _redo_scope: redo_scope, + }, + "accepted redo truncation panicked", + ) +} + +/// Prepare combined catalog checkpoint and redo truncation authority. +pub(crate) fn prepare_catalog_redo_maintenance_operation( + catalog_scope: CatalogCheckpointScope, + redo_scope: RedoRetentionScope, + scope: PreparedMaintenanceScope, +) -> impl PreparedExecution { + PreparedMaintenanceExecution::::global( + scope, + CatalogRedoMaintenanceResources { + catalog_scope, + _redo_scope: redo_scope, + }, + "accepted combined catalog/redo maintenance panicked", + ) +} + impl TransactionSystem { /// Compute a side-effect-free redo truncation plan. #[inline] @@ -167,16 +279,23 @@ impl TransactionSystem { /// Advance the durable redo retention marker and unlink obsolete redo files. /// - /// Lock ordering matches catalog checkpoint: acquire the catalog checkpoint - /// lease before the redo-retention lease. The catalog lease protects the - /// `catalog.mtb` root fork used to publish `first_redo_log_seq`, while the - /// redo-retention lease protects the retained redo suffix, catalog-safe - /// progress cache, and cleanup below the marker. They are separate because - /// the marker is catalog bootstrap metadata, but unlink races are about the - /// redo file family rather than catalog metadata shape. - pub(crate) async fn truncate_redo_log(&self) -> RuntimeOrFatalResult { - let catalog_checkpoint_lease = self.catalog.begin_checkpoint().await; - let _redo_retention_lease = self.begin_redo_retention().await; + /// The caller holds [`CatalogCheckpointScope`] acquired before + /// [`RedoRetentionScope`]. Catalog authority protects the `catalog.mtb` root + /// fork used to publish `first_redo_log_seq`, while redo-retention authority + /// protects the retained redo suffix, catalog-safe progress cache, and + /// cleanup below the marker. + /// + /// This method acquires neither scope. After marker publication, it invokes + /// `release_catalog` to release only catalog authority; the caller retains + /// redo-retention authority through obsolete-file cleanup. + async fn truncate_redo_log_prepared( + &self, + release_catalog: F, + #[cfg(test)] maintenance_test: &MaintenanceTestController, + ) -> RuntimeOrFatalResult + where + F: FnOnce(), + { // Session admission happens before the async gate waits above. Recheck // here so poison published while truncation was queued prevents marker // publication and physical redo cleanup. @@ -220,14 +339,19 @@ impl TransactionSystem { previous_first_retained_file_seq }; - drop(catalog_checkpoint_lease); + release_catalog(); let file_prefix = self .config .file_prefix() .change_context(RuntimeError::RedoLogAccess) .map_err(RuntimeOrFatalError::from)?; - let cleanup = cleanup_obsolete_redo_files(&file_prefix, new_first_retained_file_seq) - .map_err(RuntimeOrFatalError::from)?; + let cleanup = cleanup_obsolete_redo_files( + &file_prefix, + new_first_retained_file_seq, + #[cfg(test)] + maintenance_test, + ) + .map_err(RuntimeOrFatalError::from)?; Ok(RedoTruncationOutcome { previous_first_retained_file_seq, @@ -245,14 +369,14 @@ impl TransactionSystem { } /// Run catalog checkpoint and redo truncation from one gated retention observation. - pub(crate) async fn checkpoint_catalog_and_truncate_redo_log( + async fn checkpoint_catalog_and_truncate_redo_log_prepared( &self, - ) -> RuntimeOrFatalResult { - // 1. Acquire gates in the same order as standalone truncation. The - // catalog gate protects the `catalog.mtb` root writer, while the redo - // retention gate stays held until obsolete-file cleanup is finished. - let catalog_checkpoint_lease = self.catalog.begin_checkpoint().await; - let _redo_retention_lease = self.begin_redo_retention().await; + release_catalog: F, + #[cfg(test)] maintenance_test: &MaintenanceTestController, + ) -> RuntimeOrFatalResult + where + F: FnOnce(), + { // Session admission happened before these async waits. Recheck after // the gates so queued storage poison cannot publish metadata or unlink // redo files. @@ -419,14 +543,19 @@ impl TransactionSystem { // 7. Release only the catalog gate before filesystem cleanup. The redo // retention lease remains held through cleanup so checkpoint scans // cannot race disappearing retained redo files. - drop(catalog_checkpoint_lease); + release_catalog(); let file_prefix = self .config .file_prefix() .change_context(RuntimeError::RedoLogAccess) .map_err(RuntimeOrFatalError::from)?; - let cleanup = cleanup_obsolete_redo_files(&file_prefix, new_first_retained_file_seq) - .map_err(RuntimeOrFatalError::from)?; + let cleanup = cleanup_obsolete_redo_files( + &file_prefix, + new_first_retained_file_seq, + #[cfg(test)] + maintenance_test, + ) + .map_err(RuntimeOrFatalError::from)?; // 8. Return both halves of the maintenance result. The redo outcome // reports marker advancement, retryable cleanup counts, and the @@ -450,13 +579,6 @@ impl TransactionSystem { } } -#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] -struct RedoCleanupCounts { - removed_files: usize, - already_missing_files: usize, - failed_unlink_files: usize, -} - fn projected_catalog_redo_retention_progress( scanned: Option, cached: Option, @@ -527,12 +649,13 @@ fn catalog_progress_for_final_marker( fn cleanup_obsolete_redo_files( file_prefix: &str, first_retained_file_seq: u32, + #[cfg(test)] maintenance_test: &MaintenanceTestController, ) -> RuntimeResult { let mut counts = RedoCleanupCounts::default(); for descriptor in obsolete_redo_log_files_below_marker(file_prefix, first_retained_file_seq)? { debug_assert!(descriptor.seq < first_retained_file_seq); #[cfg(test)] - tests::run_redo_cleanup_before_unlink_hook(descriptor.seq, &descriptor.path); + maintenance_test.run_redo_cleanup_before_unlink_hook(descriptor.seq, &descriptor.path); match fs::remove_file(&descriptor.path) { Ok(()) => counts.removed_files = counts.removed_files.saturating_add(1), Err(err) if err.kind() == IoErrorKind::NotFound => { @@ -796,62 +919,32 @@ pub(crate) mod tests { CatalogSafeRedoSegment, RedoRetentionSegment, RedoRetentionSegmentState, RedoSegmentCtsRange, }; + use crate::table::tests::{MaintenanceTestController, RedoCleanupBeforeUnlinkHook}; use crate::table::{LiveTableRedoReplayFloor, TableRedoReplayFloor}; use crate::trx::sys::CatalogRedoRetentionProgress; - use parking_lot::{Mutex, MutexGuard}; - use std::path::Path; - use std::sync::{Arc, OnceLock, mpsc}; - use std::thread; - use std::time::Duration; + use std::panic::{AssertUnwindSafe, catch_unwind}; + use std::sync::Arc; - type BeforeUnlinkHook = Arc; - - fn before_unlink_hook_slot() -> &'static Mutex> { - static HOOK: OnceLock>> = OnceLock::new(); - HOOK.get_or_init(|| Mutex::new(None)) - } - - fn before_unlink_hook_install_lock() -> &'static Mutex<()> { - static INSTALL_LOCK: OnceLock> = OnceLock::new(); - INSTALL_LOCK.get_or_init(|| Mutex::new(())) - } - - /// Guard that restores the previous redo cleanup before-unlink hook on drop. - /// - /// The process-wide install lock is held for the guard lifetime so parallel - /// tests cannot overwrite each other's global hook state. + /// Guard that clears one engine's redo cleanup hook on drop. pub(crate) struct RedoCleanupBeforeUnlinkHookGuard { - previous: Option, - _install_guard: MutexGuard<'static, ()>, + test: MaintenanceTestController, } impl Drop for RedoCleanupBeforeUnlinkHookGuard { #[inline] fn drop(&mut self) { - *before_unlink_hook_slot().lock() = self.previous.take(); + self.test.clear_redo_cleanup_before_unlink_hook(); } } /// Install a hook invoked after obsolete redo discovery and before unlink. #[inline] pub(crate) fn install_redo_cleanup_before_unlink_hook( - hook: BeforeUnlinkHook, + test: &MaintenanceTestController, + hook: RedoCleanupBeforeUnlinkHook, ) -> RedoCleanupBeforeUnlinkHookGuard { - let install_guard = before_unlink_hook_install_lock().lock(); - let mut slot = before_unlink_hook_slot().lock(); - let previous = slot.replace(hook); - RedoCleanupBeforeUnlinkHookGuard { - previous, - _install_guard: install_guard, - } - } - - #[inline] - pub(crate) fn run_redo_cleanup_before_unlink_hook(file_seq: u32, path: &Path) { - let hook = before_unlink_hook_slot().lock().clone(); - if let Some(hook) = hook { - hook(file_seq, path); - } + test.install_redo_cleanup_before_unlink_hook(hook); + RedoCleanupBeforeUnlinkHookGuard { test: test.clone() } } #[inline] @@ -938,27 +1031,16 @@ pub(crate) mod tests { #[test] fn redo_cleanup_before_unlink_hook_installation_is_exclusive() { - let first = install_redo_cleanup_before_unlink_hook(Arc::new(|_, _| {})); - let (started_tx, started_rx) = mpsc::channel(); - let (installed_tx, installed_rx) = mpsc::channel(); - - let installer = thread::spawn(move || { - started_tx.send(()).unwrap(); - let _second = install_redo_cleanup_before_unlink_hook(Arc::new(|_, _| {})); - installed_tx.send(()).unwrap(); - }); - - started_rx.recv_timeout(Duration::from_secs(5)).unwrap(); - assert!( - installed_rx - .recv_timeout(Duration::from_millis(50)) - .is_err(), - "second hook installer should wait for the first guard" - ); - - drop(first); - installed_rx.recv_timeout(Duration::from_secs(5)).unwrap(); - installer.join().unwrap(); + let first_test = MaintenanceTestController::default(); + let second_test = MaintenanceTestController::default(); + let _first = install_redo_cleanup_before_unlink_hook(&first_test, Arc::new(|_, _| {})); + let _second = install_redo_cleanup_before_unlink_hook(&second_test, Arc::new(|_, _| {})); + + catch_unwind(AssertUnwindSafe(|| { + let _duplicate = + install_redo_cleanup_before_unlink_hook(&first_test, Arc::new(|_, _| {})); + })) + .expect_err("one engine must reject a duplicate redo cleanup hook"); } #[test] diff --git a/doradb-storage/src/trx/sys.rs b/doradb-storage/src/trx/sys.rs index 6d0dcdb8..6ca45197 100644 --- a/doradb-storage/src/trx/sys.rs +++ b/doradb-storage/src/trx/sys.rs @@ -15,6 +15,7 @@ use crate::error::{ use crate::file::fs::FileSystem; use crate::file::table_file::{MutableTableFile, OldRoot, TableFile}; use crate::id::{SessionID, SessionOperationKey, TrxID}; +use crate::latch::ExclusiveGate; use crate::log::redo::RedoLogs; use crate::log::{EnqueuePrecommitError, LogFileSealer, LogWriteDriver, RedoLog, RedoLogWriter}; use crate::notify::MonotonicU64; @@ -42,7 +43,7 @@ use crate::trx::{ use crossbeam_utils::CachePadded; use either::Either::{Left, Right}; use error_stack::{Report, ResultExt}; -use event_listener::{Event, EventListener, listener}; +use event_listener::EventListener; use flume::{Receiver, Sender}; use parking_lot::{Mutex, MutexGuard}; use std::collections::BTreeMap; @@ -86,81 +87,42 @@ impl CatalogRedoRetentionProgress { } } -#[derive(Debug, Default)] -struct RedoRetentionGateState { - active: bool, -} - -/// Transaction-system-wide gate for marker-based redo retention work. +/// Lifetime-free redo-retention exclusion scope. /// /// Catalog checkpoint and redo truncation both reason about the durable /// first-retained redo marker, the retained redo suffix on disk, and the -/// in-memory catalog-safe segment progress cache. The gate serializes those +/// in-memory catalog-safe segment progress cache. This scope serializes those /// sections so a checkpoint cannot scan one marker/suffix while truncation /// publishes another marker or unlinks files below it. /// -/// This intentionally remains separate from `CatalogCheckpointGate`. The -/// catalog gate excludes `catalog.mtb` root writers and metadata DDL, but it is -/// released before redo truncation performs filesystem cleanup; this gate stays -/// held through cleanup so retained-redo scans never race disappearing files. -struct RedoRetentionGate { - state: Mutex, - changed: Event, +/// This authority intentionally remains separate from catalog checkpoint +/// admission. Catalog admission excludes `catalog.mtb` root writers and +/// metadata DDL, but is released before redo truncation performs filesystem +/// cleanup; redo retention stays held through cleanup so retained-redo scans +/// never race disappearing files. +pub(crate) struct RedoRetentionScope { + trx_sys: QuiescentGuard, + active: bool, } -impl RedoRetentionGate { - #[inline] - fn new() -> Self { +impl RedoRetentionScope { + /// Acquire redo-retention authority during caller preparation. + pub(crate) async fn acquire(trx_sys: QuiescentGuard) -> Self { + trx_sys.redo_retention_gate.acquire().await; Self { - state: Mutex::new(RedoRetentionGateState::default()), - changed: Event::new(), - } - } - - /// Acquire exclusive access to redo-retention planning or publication. - async fn acquire(&self) -> RedoRetentionLease<'_> { - loop { - { - let mut state = self.state.lock(); - if !state.active { - state.active = true; - return RedoRetentionLease { gate: self }; - } - } - listener!(self.changed => listener); - { - let state = self.state.lock(); - if !state.active { - continue; - } - } - listener.await; + trx_sys, + active: true, } } - - #[inline] - fn release(&self) { - let mut state = self.state.lock(); - debug_assert!(state.active); - state.active = false; - drop(state); - self.changed.notify(usize::MAX); - } } -/// RAII lease for a redo-retention planning/publication/cleanup section. -/// -/// While held, catalog checkpoint retained-redo scans, catalog-safe progress -/// publication, redo truncation planning, marker publication, and obsolete-file -/// cleanup run as one serialized retention observation. -pub(crate) struct RedoRetentionLease<'a> { - gate: &'a RedoRetentionGate, -} - -impl Drop for RedoRetentionLease<'_> { +impl Drop for RedoRetentionScope { #[inline] fn drop(&mut self) { - self.gate.release(); + if self.active { + self.active = false; + self.trx_sys.redo_retention_gate.release(); + } } } @@ -630,7 +592,7 @@ pub(crate) struct TransactionSystem { /// /// This is separate from the catalog checkpoint gate: it protects redo /// marker/suffix/progress consistency, not catalog metadata DDL ordering. - redo_retention_gate: CachePadded, + redo_retention_gate: CachePadded, } impl TransactionSystem { @@ -741,7 +703,7 @@ impl TransactionSystem { dropped_table_files: CachePadded::new(Mutex::new(dropped_table_files)), fatal_rollback_retention: CachePadded::new(Mutex::new(Vec::new())), catalog_redo_retention: CachePadded::new(Mutex::new(None)), - redo_retention_gate: CachePadded::new(RedoRetentionGate::new()), + redo_retention_gate: CachePadded::new(ExclusiveGate::new()), } } @@ -767,16 +729,6 @@ impl TransactionSystem { self.catalog_redo_retention.lock().clone() } - /// Acquire the exclusive redo-retention gate. - /// - /// Use this around code that observes or changes the durable first-retained - /// redo marker together with retained redo files or catalog-safe segment - /// progress. - #[inline] - pub(crate) async fn begin_redo_retention(&self) -> RedoRetentionLease<'_> { - self.redo_retention_gate.acquire().await - } - /// Retain undo/effect ownership after rollback can no longer finish safely. /// /// This is only for fatal rollback access failures after storage has been @@ -2112,23 +2064,6 @@ pub(crate) mod tests { }); } - #[test] - fn test_redo_retention_gate_serializes_leases() { - smol::block_on(async { - let gate = RedoRetentionGate::new(); - let lease = gate.acquire().await; - let mut waiter = Box::pin(gate.acquire()); - - assert!(matches!( - futures::poll!(waiter.as_mut()), - std::task::Poll::Pending - )); - - drop(lease); - let _next_lease = waiter.await; - }); - } - #[test] fn test_transaction_system() { smol::block_on(async {