From 0d06ca18931c781ea069c624064ac942421a66c1 Mon Sep 17 00:00:00 2001 From: jiangzhe Date: Sat, 8 Aug 2026 08:32:32 +0800 Subject: [PATCH 1/2] introduce QuadError and narrow audited error convergence --- docs/error-spec.md | 65 +- docs/process/coding-guidance.md | 6 +- docs/public-error-audit.csv | 57 +- ...or-and-narrow-audited-error-convergence.md | 605 ++++++++++++++++ docs/tasks/next-id | 2 +- doradb-storage/src/catalog/index.rs | 33 +- doradb-storage/src/catalog/table.rs | 5 + doradb-storage/src/conf/mod.rs | 1 + doradb-storage/src/conf/trx.rs | 29 + doradb-storage/src/engine.rs | 402 ++++++----- doradb-storage/src/error.rs | 670 ++++++++++++++++-- doradb-storage/src/file/mod.rs | 35 +- doradb-storage/src/log/mod.rs | 6 +- doradb-storage/src/runtime/mandatory.rs | 69 +- doradb-storage/src/session.rs | 191 +++-- doradb-storage/src/table/access.rs | 188 +++-- doradb-storage/src/trx/mod.rs | 51 +- doradb-storage/src/trx/stmt.rs | 4 + doradb-storage/src/trx/stream_stmt.rs | 3 +- doradb-storage/src/trx/sys.rs | 91 ++- 20 files changed, 1958 insertions(+), 555 deletions(-) create mode 100644 docs/tasks/000263-introduce-quad-error-and-narrow-audited-error-convergence.md diff --git a/docs/error-spec.md b/docs/error-spec.md index 87d95d86..255d0fc4 100644 --- a/docs/error-spec.md +++ b/docs/error-spec.md @@ -144,31 +144,59 @@ Disclosure is approved only at one of these boundaries: - a public Doradb method returning the public `Result` alias; - an external trait whose signature is fixed to the public result; -- a genuine orchestration owner whose producer set spans multiple independent - domains and cannot be represented by an existing constrained carrier. +- a constrained carrier's disclosure implementation; or +- the three callback-mutation helpers that must forward an arbitrary public + `Error` returned by `Statement::table_mutate_mvcc`'s caller. Reusable private helpers do not return public `Result` merely to make `?` compile. Test helpers follow the same rule: test a typed producer as typed, and use a public adapter only when asserting public classification. +Configuration convergence is owned by public `Engine::bootstrap`. Startup +validates and normalizes transaction configuration there, including resolving +the redo-file prefix, before passing a `ValidatedTrxSysConfig` into the +Runtime-typed transaction-system component. + ## Constrained carriers -Three carriers encode closed multi-domain contracts without adding a synthetic -error-stack frame: +Select the narrowest stable contract in this order: + +1. one native typed domain; +2. an exact two-domain carrier; then +3. `QuadResult` when three or four common integration domains are reachable. + +Four pairwise carriers encode exact two-domain contracts without adding a +synthetic error-stack frame: - `OperationOrRuntimeError` contains either an Operation report or a Runtime report; - `OperationOrFatalError` contains either an Operation report or a Fatal report; -- `RuntimeOrFatalError` contains either a Runtime report or a Fatal report. +- `RuntimeOrFatalError` contains either a Runtime report or a Fatal report; and +- `LifecycleOrFatalError` contains either a Lifecycle report or a Fatal report. Structural `From` implementations into these carriers are allowed because the native report is preserved and the destination explicitly represents that domain. These are not public convergence conversions. +`QuadError` is the closed final-integration carrier for exactly Operation, +Runtime, Lifecycle, and Fatal. It flattens the pairwise carriers by moving their +native report directly into the matching arm. It deliberately has no Config, +Resource, IO, DataIntegrity, Internal, public `Error`, or completion-bridge +arm. A fifth arm changes the integration design and requires a new design +review rather than a routine extension. + +Resource, IO, and DataIntegrity can enter `QuadError` only after a semantic +owner stacks a specific Runtime context such as `TableAccess`, `IndexAccess`, +`CatalogAccess`, `Recovery`, `RedoLogAccess`, or `TransactionCommit`. There is +no generic physical-domain conversion into Quad. Config remains owned by +public bootstrap. + Carrier extensions add attachments to either arm and can replace only the non-Fatal Runtime context where that operation is owned. Fatal always bypasses -ordinary reinterpretation. +Runtime and Lifecycle reinterpretation. Poison-aware admission therefore +returns Fatal without a Lifecycle frame, while shutdown, closed-session, and +discarded-transaction rejection remain Lifecycle. Do not introduce a general sum-error framework. Add a carrier only when a small, stable producer set is repeatedly shared and no existing carrier fits. @@ -176,9 +204,10 @@ small, stable producer set is repeatedly shared and no existing carrier fits. ## Completion and Fatal transport `CompletionErrorBridge` transports one canonical typed report across an async -completion or multiple waiters. Its accepted roots are closed and audited: IO, -Resource, DataIntegrity, Lifecycle, Runtime, and Fatal. The bridge itself must -never appear as a frame in the reconstructed or public report. +completion or multiple waiters. Its accepted roots are closed and audited: +Operation, IO, Resource, DataIntegrity, Lifecycle, Runtime, and Fatal. The +bridge itself must never appear as a frame in the reconstructed or public +report. Cloning a bridge shares its immutable canonical state. Each consumer rebuilds an independent physical report, retains the registered source frames and @@ -186,6 +215,18 @@ attachments, and installs the consumer-owned outer context. A Runtime report may contain a private Internal frame beneath it; that frame is diagnostic only and does not become a completion root or public kind. +Mandatory completion observers return the typed bridge. Their semantic owner +uses a named replay policy: `into_runtime_or_fatal` for an exact pairwise +contract or `into_quad` for the common integration set. `into_quad` preserves +Operation, Runtime, Lifecycle, and Fatal roots; it stacks raw Resource, IO, or +DataIntegrity roots beneath the caller-supplied Runtime context. + +Immediately after replay, the semantic owner attaches one combined diagnostic +with the public operation, completion-wait phase, and available request +identifiers. The attachment is added to whichever native carrier arm was +reconstructed, including Operation, Lifecycle, and Fatal arms that do not use +the fallback Runtime context. `QuadError` remains a frame-less carrier. + `SharedFatalError` provides equivalent fan-out for a canonical Fatal report. Poison publication and every waiter retain the initiating source and Fatal reason. @@ -250,13 +291,13 @@ The principal convergence owners are: | Area | Boundary | | --- | --- | | value and rows | public decode/access adapters and fixed external traits | -| engine | build orchestration, new-session admission, and shutdown facades | +| engine | public bootstrap, new-session admission, and shutdown facades | | session | public table, checkpoint, retention, and transaction operations | | transaction | public lock, statement execution, commit, and rollback | | statement/stream | public DML and stream iteration methods | | log configuration | fixed `FromStr` adapter over typed validation | -| catalog/table | public semantic facades and genuine Runtime-or-Fatal policy owners | -| recovery/startup | transaction-system bootstrap over typed recovery helpers | +| catalog/table | public semantic facades plus callback mutation error transport | +| recovery/startup | typed recovery helpers beneath public Engine bootstrap | Lower buffer, file, log internals, index, table, purge, retention, recovery, and component suppliers stay typed or use one of the constrained carriers. A new diff --git a/docs/process/coding-guidance.md b/docs/process/coding-guidance.md index af5d48b1..c4915707 100644 --- a/docs/process/coding-guidance.md +++ b/docs/process/coding-guidance.md @@ -30,13 +30,17 @@ We rely on tooling to enforce style. ### Error Handling * **Typed Domain Reports**: Internal domain-specific functions should return the matching report alias, such as `ConfigResult`, `OperationResult`, `ResourceResult`, `DataIntegrityResult`, `LifecycleResult`, `FatalResult`, or `InternalResult`. Use the crate-wide `crate::error::Result` only at public API boundaries or in functions that intentionally combine several unrelated domains. +* **Multi-Domain Selection**: Prefer one native domain, then an exact pairwise carrier, then `QuadResult` only when three or four of Operation, Runtime, Lifecycle, and Fatal are reachable. `QuadError` has fixed arity and membership; adding a fifth arm requires a new design review. +* **Physical Integration Ownership**: Resource, IO, and DataIntegrity enter a common integration carrier only after the semantic owner stacks a specific Runtime context. Config convergence belongs to public Engine bootstrap. Do not add blanket lower-domain conversions merely to satisfy `?`. * **Crate-Owned Trait Errors**: When implementations of a crate-owned trait have different failure domains, use an associated error type and let each implementation expose its narrowest result. Keep generic dispatch typed and convert only in a caller that actually combines unrelated implementations. Use crate `Result` directly only for an externally fixed signature or an implementation that is itself mixed-domain. * **Fieldless Error Variants**: Define stable error classifications as fieldless `thiserror` variants. Put request-specific details, identifiers, values, and explanatory text in `error-stack` attachments instead of variant fields. * **Context at the Caller**: Attach operation names, table or block identifiers, configuration field names, and other caller-owned context with `attach` or `attach_with` where that context becomes known. Do not pass parameters down the call stack solely so a leaf function can format an error message. * **Poison Helper Depth**: Keep Fatal report construction, poison logging, and publication at the owning policy boundary. A shared production poison-publication helper should have at least three callers; otherwise inline it. Do not stack thin domain wrappers: a policy owner may call at most one shared domain helper before `EnginePoisoner`. Substantive state-machine or wait algorithms and the core `EnginePoisoner` API are not publication-wrapper layers. * **Attachment Granularity**: Combine printable diagnostic facts owned by one semantic boundary into one attachment. Keep typed attachments separate so callers can inspect them, and keep attachments on opposite sides of `change_context` separate because they describe different error frames. * **Cross-Domain Conversion**: Use `change_context` at the boundary where one domain consumes another domain's failure, then attach the consuming operation's context. Preserve the original report frames; do not convert to `crate::error::Error`, downcast it, and rebuild a new report. -* **Completion Transport**: Finish the owned typed report and capture it once with `CompletionErrorBridge::capture` at the failed handoff. Intermediate completion forwarders clone the bridge unchanged; only the typed or public policy owner materializes it and adds caller-owned context. Never capture a public, Runtime, or already materialized bridge report. +* **Fatal Bypass**: Never replace a Fatal report with Runtime or Lifecycle. Poison-aware admission returns Fatal directly; ordinary shutdown and unavailable session or transaction state remain Lifecycle. +* **Completion Transport**: Finish the owned typed report and capture it once with `CompletionErrorBridge::capture` at the failed handoff. Intermediate completion forwarders clone the bridge unchanged; only the typed or public policy owner materializes it. Immediately after named replay into a constrained carrier, that owner attaches one combined operation/phase diagnostic so every reconstructed native arm receives the boundary context. Never capture a public, Runtime, or already materialized bridge report. +* **Public Error Ownership**: Public `Error` is limited to public Doradb APIs, externally fixed trait adapters, constrained-carrier disclosure, and the three full-table mutation helpers that transport an arbitrary caller callback error. Reusable internal orchestration remains typed. * **Validation Pattern**: Use `crate::error::Validation` for optimistic logic checks (Valid/Invalid) where failure is a normal control flow, distinct from `Result` (exceptional failures). * **Runtime Failures vs. Contracts**: Incidental `unwrap()` / `expect()` in runtime paths remains prohibited, and external or otherwise valid runtime failures must remain typed results. A proven internal contract may use a release assertion at the narrowest owning site when its constructor, exact allocation, ownership boundary, or fixed lifecycle establishes the precondition. Document the invariant locally and include the component, edge, type/length, column, or other identifying detail in the assertion diagnostic. Do not use `debug_assert!` as the only guard for a correctness contract or treat this rule as general permission to panic. diff --git a/docs/public-error-audit.csv b/docs/public-error-audit.csv index e1d4e6b4..f5f8f4b3 100644 --- a/docs/public-error-audit.csv +++ b/docs/public-error-audit.csv @@ -1,26 +1,26 @@ file,function_or_method,disclose_calls -doradb-storage/src/catalog/index.rs,CreateIndexPlan::new,2 -doradb-storage/src/catalog/index.rs,DropIndexPlan::new,2 +doradb-storage/src/engine.rs,Engine::bootstrap,25 doradb-storage/src/engine.rs,Engine::new_session,1 doradb-storage/src/engine.rs,Engine::try_shutdown,1 -doradb-storage/src/engine.rs,bootstrap_inner,23 +doradb-storage/src/error.rs,LifecycleOrFatalError::disclose,2 doradb-storage/src/error.rs,OperationOrFatalError::disclose,2 doradb-storage/src/error.rs,OperationOrRuntimeError::disclose,2 +doradb-storage/src/error.rs,QuadError::disclose,4 doradb-storage/src/error.rs,RuntimeOrFatalError::disclose,2 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,5 doradb-storage/src/session.rs,Session::buffer_pool_stats,1 -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::checkpoint_catalog,4 +doradb-storage/src/session.rs,Session::checkpoint_catalog_and_truncate_redo_log,4 +doradb-storage/src/session.rs,Session::checkpoint_table,6 +doradb-storage/src/session.rs,Session::cleanup_secondary_mem_indexes,6 doradb-storage/src/session.rs,Session::close,4 -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,5 +doradb-storage/src/session.rs,Session::create_index,11 +doradb-storage/src/session.rs,Session::create_table,5 +doradb-storage/src/session.rs,Session::drop_index,10 +doradb-storage/src/session.rs,Session::drop_table,5 +doradb-storage/src/session.rs,Session::freeze_table,6 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::logical_lock_stats,1 @@ -28,41 +28,28 @@ doradb-storage/src/session.rs,Session::mandatory_runtime_stats,1 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,3 +doradb-storage/src/session.rs,Session::truncate_redo_log,4 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_maintenance_boundary,4 +doradb-storage/src/session.rs,Session::wait_for_gc_horizon_after,2 +doradb-storage/src/session.rs,Session::wait_for_purge_completion_after,2 doradb-storage/src/table/access.rs,LazyRow::val,2 -doradb-storage/src/table/access.rs,UserTableAccessor::delete_known_cold_row,2 -doradb-storage/src/table/access.rs,UserTableAccessor::delete_known_hot_row,3 -doradb-storage/src/table/access.rs,UserTableAccessor::delete_unique_mvcc,12 -doradb-storage/src/table/access.rs,UserTableAccessor::insert_mvcc,2 -doradb-storage/src/table/access.rs,UserTableAccessor::mutate_cold_rows_mvcc,9 -doradb-storage/src/table/access.rs,UserTableAccessor::mutate_hot_rows_mvcc,4 +doradb-storage/src/table/access.rs,UserTableAccessor::mutate_cold_rows_mvcc,12 +doradb-storage/src/table/access.rs,UserTableAccessor::mutate_hot_rows_mvcc,7 doradb-storage/src/table/access.rs,UserTableAccessor::table_mutate_mvcc,1 -doradb-storage/src/table/access.rs,UserTableAccessor::update_known_cold_row,4 -doradb-storage/src/table/access.rs,UserTableAccessor::update_known_hot_row,5 -doradb-storage/src/table/access.rs,UserTableAccessor::update_unique_mvcc_input,18 -doradb-storage/src/table/access.rs,UserTableAccessor::validate_table_mutation_update,1 -doradb-storage/src/trx/mod.rs,Transaction::commit,1 +doradb-storage/src/trx/mod.rs,Transaction::commit,2 doradb-storage/src/trx/mod.rs,Transaction::exec,2 doradb-storage/src/trx/mod.rs,Transaction::lock_table,2 doradb-storage/src/trx/mod.rs,Transaction::rollback,2 -doradb-storage/src/trx/stmt.rs,Statement::table_delete_unique_mvcc,3 +doradb-storage/src/trx/stmt.rs,Statement::table_delete_unique_mvcc,4 doradb-storage/src/trx/stmt.rs,Statement::table_index_lookup_mvcc,2 doradb-storage/src/trx/stmt.rs,Statement::table_index_scan_mvcc,3 -doradb-storage/src/trx/stmt.rs,Statement::table_insert_mvcc,3 +doradb-storage/src/trx/stmt.rs,Statement::table_insert_mvcc,4 doradb-storage/src/trx/stmt.rs,Statement::table_lookup_unique_mvcc,2 doradb-storage/src/trx/stmt.rs,Statement::table_mutate_mvcc,2 doradb-storage/src/trx/stmt.rs,Statement::table_scan_mvcc,2 -doradb-storage/src/trx/stmt.rs,Statement::table_update_unique_mvcc,4 -doradb-storage/src/trx/stmt.rs,Statement::table_upsert_unique_mvcc,4 +doradb-storage/src/trx/stmt.rs,Statement::table_update_unique_mvcc,5 +doradb-storage/src/trx/stmt.rs,Statement::table_upsert_unique_mvcc,5 doradb-storage/src/trx/stream_stmt.rs,IndexScanMvccStream::next,2 doradb-storage/src/trx/stream_stmt.rs,StreamStmt::table_index_scan_mvcc,5 -doradb-storage/src/trx/sys.rs,TransactionSystem::bootstrap,5 -doradb-storage/src/trx/sys.rs,TransactionSystem::build,1 -doradb-storage/src/trx/sys.rs,TransactionSystem::commit_prepared,1 -doradb-storage/src/trx/sys.rs,TransactionSystem::commit_transaction,2 doradb-storage/src/value.rs,ValKind::try_from,1 diff --git a/docs/tasks/000263-introduce-quad-error-and-narrow-audited-error-convergence.md b/docs/tasks/000263-introduce-quad-error-and-narrow-audited-error-convergence.md new file mode 100644 index 00000000..b01c9656 --- /dev/null +++ b/docs/tasks/000263-introduce-quad-error-and-narrow-audited-error-convergence.md @@ -0,0 +1,605 @@ +--- +id: 000263 +title: Introduce QuadError and Narrow Audited Error Convergence +status: proposal # proposal | implemented | superseded +created: 2026-08-07 +github_issue: 960 +--- + +# Task: Introduce QuadError and Narrow Audited Error Convergence + +## Summary + +Introduce a crate-private, closed `QuadError` carrier for the final internal +integration layer that can preserve native Operation, Runtime, Lifecycle, and +Fatal reports without first converting them to the public `Error`. Add the +exact `LifecycleOrFatalError` pair for health-aware admission paths, while +retaining the existing single-domain results and pairwise carriers as the +preferred contracts. + +Use the existing direct `.disclose()` audit as a bottom-up migration inventory. +For every audited internal convergence owner, trace its real leaf producers, +stack Resource, IO, or DataIntegrity beneath an operation-owned Runtime context +where required, and select the narrowest result contract: one native domain, +an exact two-domain carrier, or `QuadResult` for three or four of the four +integration domains. Public `Result` should remain only at public API methods, +fixed external-trait adapters, constrained-carrier disclosure implementations, +and the documented callback transport that must accept a caller-produced +public `Error`. + +This migration also corrects poisoned admission. A poison report remains Fatal +instead of being replaced by `LifecycleError::RuntimeUnavailable`; ordinary +shutdown, closed-session, and discarded-transaction rejection remains +Lifecycle. + +## Context + +Issue Labels: + +- type:task +- priority:medium +- codex + +Source Backlogs: + +- docs/backlogs/000178-common-multi-domain-error-carrier.md + +The storage error model has eight publicly classifiable private domains plus +the non-public Internal domain. Reusable producers are expected to retain a +typed `Report` until a public or external-trait boundary, but a few +higher-level owners currently return the top-level public `Result` because +their producer sets do not fit one domain or one of the three existing +carriers: + +- `OperationOrRuntimeError`; +- `OperationOrFatalError`; and +- `RuntimeOrFatalError`. + +The canonical audit currently records 67 production callables containing 228 +direct `.disclose()` method calls. Public Engine, Session, Transaction, +Statement, stream, `LazyRow`, and external-trait adapters are valid convergence +owners. The audit also exposes avoidable internal owners: + +- `bootstrap_inner`; +- `CreateIndexPlan::new` and `DropIndexPlan::new`; +- `wait_for_maintenance_boundary`; +- transaction-system bootstrap, component build, and user commit; +- non-callback `UserTableAccessor` DML integration helpers; and +- callback mutation helpers whose internal sources are disclosed early because + the callback itself returns the public result type. + +`CompletionObserver::wait` also converts a `CompletionErrorBridge` with +function-form `DiscloseError::disclose`. It is not present in the direct-method +audit, but it is a known internal public-error owner and is included in this +migration. The audit tool remains intentionally simple; this task does not +expand it into a visibility, return-type, or function-form analyzer. + +Several poison health checks currently use: + +```rust,ignore +poisoner + .ensure_healthy() + .change_context(LifecycleError::RuntimeUnavailable) +``` + +This replaces a current Fatal report with a Lifecycle context. It causes +poisoned admission to classify publicly as Lifecycle even though the engine +has entered its fatal one-way state. Shutdown is a legitimate Lifecycle +outcome, but poison must retain its Fatal identity and source chain. + +Configuration is limited to public bootstrap. Resource exhaustion, IO, and +data-integrity reports remain narrow at their native producers but can be +stacked below the Runtime operation that owns their integration. Therefore the +common final carrier needs exactly Operation, Runtime, Lifecycle, and Fatal; +it must not grow Config, Resource, IO, DataIntegrity, Internal, or public +`Error` arms. + +RFC 0023 is implemented historical context for the typed-domain and disclosure +model, not an active parent program for this task. The present change passes +the RFC complexity gate as one bounded internal refactor: it changes no public +signature, persisted representation, transaction protocol, recovery +algorithm, or staged rollout contract. + +## Goals + +1. Add a fixed-cardinality `QuadError`/`QuadResult` integration carrier for + Operation, Runtime, Lifecycle, and Fatal reports. +2. Add `LifecycleOrFatalError`/`LifecycleOrFatalResult` for paths whose exact + reachable producer set is Lifecycle or Fatal. +3. Keep single-domain results and exact pairwise carriers preferred over + `QuadResult`. +4. Revisit every existing direct `.disclose()` audit row bottom-up and narrow + internal return types as far as their real producer sets permit. +5. Remove public `Error` from known internal bootstrap, completion, + transaction, catalog-plan, maintenance-wait, and non-callback table-DML + owners. +6. Keep Config at public Engine bootstrap and require explicit Runtime + ownership before Resource, IO, or DataIntegrity enters `QuadError`. +7. Preserve native reports, source frames, attachments, and Fatal bypass + semantics through carrier and completion-bridge conversion. +8. Classify poisoned admission as Fatal and ordinary lifecycle rejection as + Lifecycle. +9. Document and mechanically refresh the remaining approved public-error + convergence inventory. + +## Non-Goals + +1. No public `Error`, `ErrorKind`, or `Result` API signature changes. +2. No Config, Resource, IO, DataIntegrity, Internal, or public `Error` arm in + `QuadError`. +3. No automatic conversion from a lower physical domain into Runtime without a + caller-owned semantic Runtime context. +4. No generic type-level error-set framework, variadic carrier, or arbitrary + carrier-generation system. +5. No removal of existing single-domain aliases or pairwise carriers. +6. No exact three-domain carrier family; integration paths with three of the + four common domains use `QuadResult`. +7. No parameterization of mandatory completion storage by task-specific error + types; `CompletionErrorBridge` remains the move/clone-safe transport. +8. No redesign of public transaction or row-mutation callback error contracts. +9. No poison-aware logical-lock or hot-row wait cancellation from backlogs + 000177 or 000179. +10. No persistent catalog, table, redo, checkpoint, or recovery format change. +11. No transaction ordering, rollback, MVCC, DDL publication, or recovery + semantic change. +12. No expansion of `tools/error_audit.rs` beyond its existing direct + `.disclose()` method-call inventory. + +## Plan + +### Bottom-up narrowing rules + +Treat each current audit row as a review obligation rather than an allowlist. +Start from the callable's actual leaf results and work upward through its call +graph: + +1. Preserve an infallible or neutral outcome as `Infallible`, `Option`, a + status enum, or the existing neutral result when no error is owned. +2. Use the native `ConfigResult`, `OperationResult`, `ResourceResult`, + `IoResult`, `DataIntegrityResult`, `LifecycleResult`, `RuntimeResult`, or + `FatalResult` when one domain is reachable. +3. Use an exact pairwise carrier when exactly two stable integration domains + are reachable. Add `LifecycleOrFatalResult`; retain the existing three + pairwise results. +4. Use `QuadResult` only when three or four of Operation, Runtime, Lifecycle, + and Fatal remain reachable at one higher-level owner. +5. Keep public `Result` only at a public Doradb API, an externally fixed trait, + or the explicit callback transport described below. + +Resource, IO, and DataIntegrity are counted only after the caller that owns the +larger operation chooses a specific Runtime context. Examples include +`CatalogAccess`, `TableAccess`, `IndexAccess`, `RedoLogAccess`, `Recovery`, +`CheckpointExecution`, and `TransactionCommit`. Do not add a generic +"integration failed" context merely to make conversion compile. Fatal reports +always bypass Runtime and Lifecycle replacement. + +Apply the following disposition to the current audit inventory: + +| Current owner group | Required disposition | Narrow target | +| --- | --- | --- | +| Public Engine, Session, Transaction, Statement, stream, and `LazyRow` methods | Retain disclosure at the public facade | Public `Result` | +| `LogSync::from_str` and `ValKind::try_from` | Retain fixed external-trait convergence | Public trait error | +| `DiscloseError` implementations for constrained carriers | Retain conversion infrastructure | Native report to public `Error` | +| `bootstrap_inner` | Move convergence into public `Engine::bootstrap` | Public bootstrap only | +| `CreateIndexPlan::new`, `DropIndexPlan::new` | Preserve Operation; stack root-shape integrity under `CatalogAccess` | `OperationOrRuntimeResult` | +| `wait_for_maintenance_boundary` | Preserve shutdown and poison independently | `LifecycleOrFatalResult` | +| `TransactionSystem::{bootstrap, build}` | Validate Config before entry and own recovery integration as Runtime | `RuntimeResult` | +| `TransactionSystem::{commit_prepared, commit_transaction}` | Stack Resource under commit Runtime; preserve Lifecycle and Fatal | `QuadResult` | +| Non-callback `UserTableAccessor` DML helpers | Narrow leaves and combine only at real DML owners | Native, pairwise, or `QuadResult` | +| Callback mutation transport | Retain only where arbitrary callback `Error` must be forwarded | Documented public-result exception | + +Use compiler fallout to catch forwarding methods that do not themselves call +`.disclose()`. Refresh `docs/public-error-audit.csv` after migration. The final +CSV need not minimize carrier-disclosure implementation rows, but it must have +no internal convergence rows outside the constrained-carrier infrastructure +and documented callback transport. + +Do not change `tools/error_audit.rs` or its CSV schema. + +### Closed integration carriers + +Add these crate-private types in `doradb-storage/src/error.rs`: + +```rust,ignore +pub(crate) enum QuadError { + Operation(Report), + Runtime(Report), + Lifecycle(Report), + Fatal(Report), +} + +pub(crate) type QuadResult = result::Result; + +pub(crate) enum LifecycleOrFatalError { + Lifecycle(Report), + Fatal(Report), +} + +pub(crate) type LifecycleOrFatalResult = + result::Result; +``` + +Both carriers: + +- delegate `Debug` and `Display` to the contained report; +- implement `DiscloseError` without adding a carrier frame; +- implement `MultiDomainResultExt::{attach, attach_with}` by modifying the + contained report; +- accept structural `From>` conversions only for their declared + domains; and +- never become an `error_stack` context themselves. + +`QuadError` also flattens the existing `OperationOrRuntimeError`, +`OperationOrFatalError`, `RuntimeOrFatalError`, and the new +`LifecycleOrFatalError`. Conversion moves the native report directly into the +matching arm; it must not wrap one carrier inside another report. + +Do not implement `From` for Config, Resource, IO, DataIntegrity, Internal, +public `Error`, or `CompletionErrorBridge`. Lower domains require an explicit +Runtime context at their semantic owner. Completion bridges use a named replay +method so their policy is visible. + +`QuadError` is deliberately cardinality-named. Adding a fifth arm is a new +design decision, not a routine extension of this task. + +### Preserve Fatal admission + +Change health-aware admission paths from Lifecycle-only results to +`LifecycleOrFatalResult`: + +- `EngineInner::acquire_admission`; +- `EngineInner::with_admitted_operation`; +- `Engine::new_session_inner`; +- `Session::pin_observer`; +- `Session::pin_operation`; +- the health-aware portion of `Session::begin_trx`; +- `Transaction::checkout`; and +- `MandatoryRuntime::submit`. + +Keep pure lifecycle operations Lifecycle-typed: + +- `EngineLifecycle::admit`; +- weak session upgrade and registry/lifecycle checks; +- session close/discard checks; +- `Transaction::checkout_terminal` and terminal claiming, which must remain + available for cleanup after poison; +- lifecycle state transitions; and +- poison-tolerant inspection through `Session::pin_inspection`. + +Remove every +`change_context(LifecycleError::RuntimeUnavailable)` health conversion. +Forward the original Fatal report into the pairwise carrier and add only +caller-owned attachments. Remove `LifecycleError::RuntimeUnavailable` after a +producer audit confirms that no semantic producer remains. + +Public behavior is intentionally corrected: + +- engine poison before or during admission produces `ErrorKind::Fatal` with no + Lifecycle frame above the Fatal report; +- engine shutdown and unavailable session/transaction state remain + `ErrorKind::Lifecycle`; and +- already accepted mandatory work and poison-observable diagnostics retain + their existing ownership and availability rules. + +### Typed mandatory completion observation + +Keep `CompletionResult` and `CompletionErrorBridge` as the closed transport +used by completion cells and accepted mandatory execution. Change +`CompletionObserver::wait` to return `CompletionResult` directly instead of +calling `DiscloseError::disclose`. + +Add: + +```rust,ignore +impl CompletionErrorBridge { + pub(crate) fn into_quad( + self, + runtime_context: RuntimeError, + ) -> QuadError; +} +``` + +Replay the bridge without a public-Error round trip: + +| Reconstructed outer source | `into_quad` result | +| --- | --- | +| Operation | `QuadError::Operation` | +| Runtime | `QuadError::Runtime` | +| Lifecycle | `QuadError::Lifecycle` | +| Fatal | `QuadError::Fatal` | +| Resource | Source report changed to supplied Runtime context | +| IO | Source report changed to supplied Runtime context | +| DataIntegrity | Source report changed to supplied Runtime context | + +`CompletionSourceReport` has no Config or Internal arm, so neither can enter +this conversion. Preserve all replayed source frames and attachments and do +not leave a `CompletionErrorBridge` frame in the reconstructed report. + +At each observer, select the smallest result supported by the accepted task's +real producer set: + +- Runtime/Fatal maintenance completion continues through + `into_runtime_or_fatal`; +- Operation/Runtime/Fatal or + Operation/Runtime/Lifecycle/Fatal DDL completion uses `into_quad`; and +- a public Session method performs the final disclosure. + +The supplied Runtime context belongs to the public operation: + +- catalog DDL and catalog checkpoint use `CatalogAccess`; +- table checkpoint uses `CheckpointExecution`; +- table freeze and table cleanup use `TableAccess`; +- index work uses `IndexAccess`; and +- redo retention/truncation uses `RedoLogAccess`. + +An already reconstructed Runtime report retains its existing, more specific +context. The fallback context is used only for a raw Resource, IO, or +DataIntegrity root. + +### Catalog and DDL plan narrowing + +Change `CreateIndexPlan::new` and `DropIndexPlan::new` to +`OperationOrRuntimeResult`. + +- Metadata absence and invalid requested index state remain Operation. +- `validate_create_index_root_shape` and + `validate_drop_index_root_shape` remain DataIntegrity producers. +- Each plan constructor owns catalog integration and changes those + DataIntegrity reports to `RuntimeError::CatalogAccess`, retaining the + integrity frame and root/table/index attachments. + +Public `Session::{create_index, drop_index}` discloses the pairwise plan result. +Accepted index DDL keeps Operation, Runtime, Lifecycle, and Fatal sources typed +through completion replay and discloses only at the public Session method. + +Apply the same bottom-up rule while reviewing create/drop table completion: +retain a narrower pairwise completion when its actual producer set permits it; +use `QuadResult` only when at least three common domains are reachable. + +### Table and statement narrowing + +Audit the public-result region in `UserTableAccessor` from its existing narrow +leaf helpers upward. + +Target contracts include: + +- `validate_table_mutation_update` becomes `OperationResult`; +- known cold/hot delete and update integration that combines Operation, + Runtime, and Fatal becomes `QuadResult`; +- `insert_mvcc`, `upsert_unique_mvcc`, `update_unique_mvcc`, + `update_unique_mvcc_input`, and `delete_unique_mvcc` become `QuadResult` + where their current Operation/Runtime/Fatal producer set remains reachable; +- existing Runtime-only, Operation/Runtime, Operation/Fatal, and Runtime/Fatal + leaf helpers keep their narrower contracts; and +- IO, Resource, and DataIntegrity encountered by table/index integration + receive `TableAccess` or `IndexAccess` before entering a common carrier. + +Do not manufacture a three-domain carrier for these paths. Flatten pairwise +leaf errors into `QuadError` at the first owner that genuinely needs three +domains. Public `Statement` DML methods disclose the final carrier. + +`Statement::table_mutate_mvcc` accepts: + +```rust,ignore +F: for<'row> FnMut(&mut LazyRow<'row>) -> Result +``` + +The callback may return any public error previously obtained by its caller. +Changing that public contract is out of scope. Therefore +`UserTableAccessor::{table_mutate_mvcc, mutate_cold_rows_mvcc, +mutate_hot_rows_mvcc}` may retain public `Result` solely as callback-error +transport. Narrow every helper below them first, then disclose a typed helper +only where it must merge with the arbitrary callback error. Document these +three functions as the remaining genuine mixed-owner exception; do not allow +the exception to spread to point DML or non-callback helpers. + +### Transaction commit narrowing + +Rename the private `RuntimeError::SystemTransactionCommit` context to +`RuntimeError::TransactionCommit` so it describes both user and system +transaction integration. + +Keep `commit_prepared_no_wait`, catalog commit, and system commit on +`RuntimeOrFatalResult` where that remains their exact producer set. Change +user-facing transaction-system integration: + +- `TransactionSystem::commit_prepared` returns `QuadResult`; +- `TransactionSystem::commit_transaction` returns `QuadResult`; +- `FailedPrecommitReason::Resource` changes its Resource report to + `RuntimeError::TransactionCommit`, retaining the Resource source; +- `FailedPrecommitReason::Shutdown` remains Lifecycle; +- poison, rollback-cleanup failure, redo failure, and mandatory panic remain + Fatal; and +- fatal rollback cleanup bypasses Runtime and Lifecycle wrapping. + +Public `Transaction::commit` performs the sole final disclosure. Preserve +ordered commit, CTS publication, failed-precommit cleanup, session-state +release, lock release, and retry behavior. + +This intentionally changes public classification for user precommit resource +rejection from Resource to Runtime. The lower Resource frame and diagnostic +attachments must remain inspectable. System commit remains Runtime/Fatal as +before. + +### Bootstrap ownership + +Make public `Engine::bootstrap` the only startup-wide public-error convergence +owner. Fold the current private `bootstrap_inner` body into the public method, +or split it into typed substeps that do not return public `Result`; do not keep +a private public-result coordinator under another name. + +Introduce a crate-private validated transaction configuration prepared at the +public bootstrap boundary. It owns the normalized `TrxSysConfig` and resolved +redo file prefix: + +```rust,ignore +pub(crate) struct ValidatedTrxSysConfig { + config: TrxSysConfig, + file_prefix: String, +} +``` + +Construction performs `TrxSysConfig::validate` and `file_prefix` while Config +can still be disclosed by public `Engine::bootstrap`. The +`TransactionSystem` component accepts the validated type, stores the inner +configuration, and uses the prepared prefix without another Config result. + +Change: + +- `TransactionSystem::bootstrap` to `RuntimeResult`; +- `Component for TransactionSystem::Error` to `Report`; and +- its component `build` method to `RuntimeResult`. + +Retain recovery IO and DataIntegrity sources beneath `RuntimeError::Recovery` +or the existing more specific Runtime contexts. Startup worker/resource +failures retain their existing component-owned Runtime contexts. Invalid +configuration still discloses as `ErrorKind::Config` from public bootstrap, +and storage-root contention remains Lifecycle. + +Preserve component registration order, reverse rollback/shutdown order, +storage-layout marker sequencing, failure atomicity, and worker reclamation. + +### Documentation and audit closure + +Update `docs/error-spec.md` and `docs/process/coding-guidance.md` with: + +- the single-domain, exact-pairwise, then Quad selection order; +- the fixed membership and arity contract of `QuadError`; +- the rule that lower physical domains require an explicit Runtime owner; +- Fatal bypass semantics; +- Config ownership at public bootstrap; +- public `Error` ownership limited to public/external boundaries and the + callback exception; and +- the rule that a fifth Quad arm requires a new design review. + +Run the unchanged audit generator: + +```bash +tools/error_audit.rs --write docs/public-error-audit.csv +``` + +The refreshed inventory must contain no rows for: + +- `bootstrap_inner`; +- `CreateIndexPlan::new`; +- `DropIndexPlan::new`; +- `wait_for_maintenance_boundary`; +- `TransactionSystem::bootstrap`; +- `TransactionSystem::build`; +- `TransactionSystem::commit_prepared`; +- `TransactionSystem::commit_transaction`; or +- non-callback `UserTableAccessor` DML helpers. + +Expected remaining internal rows are constrained-carrier disclosure +implementations and the three documented callback mutation transport +functions. Public facade and external-trait adapter rows remain valid. Review +the diff row by row rather than accepting a lower aggregate count alone. + +## Implementation Notes + +## Impacts + +| Area | Planned effect | +| --- | --- | +| Public API | No signature or `ErrorKind` enum change | +| Public classification | Poisoned admission becomes Fatal; owned lower-domain integration becomes Runtime | +| Error carriers | Add fixed `QuadError` and exact `LifecycleOrFatalError` | +| Engine | Public bootstrap owns Config and all startup convergence | +| Mandatory runtime | Submission is Lifecycle/Fatal; observer returns a typed bridge | +| Session | Public methods remain final disclosure owners | +| Catalog DDL | Plan construction narrows to Operation/Runtime | +| Table DML | Non-callback helpers become native, pairwise, or Quad | +| Transaction commit | User integration becomes Quad; system paths stay pairwise | +| Documentation | Error model and coding guidance define arity and ownership rules | +| Audit | Existing direct-method tool is unchanged; generated inventory shrinks internally | +| Persisted data | No representation or compatibility change | +| Unsafe code | No new unsafe contract or expected unsafe-code change | +| Performance | Enum matching and report moves only; no intended I/O or scheduling change | + +Primary risks are: + +- using `QuadResult` where a native or pairwise type is sufficient; +- accidentally wrapping Fatal beneath Runtime or Lifecycle; +- losing replayed completion frames or attachments; +- changing startup validation or rollback ordering while moving Config + ownership; and +- broadening the callback exception beyond its caller-supplied public error. + +The bottom-up audit disposition, absence of lower-domain `From` +implementations, focused report-frame tests, and startup failure-atomicity +tests are the required mitigations. + +## Test Cases + +1. Construct each `QuadError` arm from its native report. Verify delegated + `Debug`/`Display`, static and lazy attachments, final `ErrorKind`, and the + retained native report frame after disclosure. +2. Flatten every existing pairwise carrier and `LifecycleOrFatalError` into + `QuadError`. Verify the carrier types do not appear as report contexts and + the original source/attachments remain present. +3. Verify Resource, IO, DataIntegrity, Config, Internal, public `Error`, and + `CompletionErrorBridge` have no structural `From` path into `QuadError`. + Exercise explicit lower-domain-to-Runtime conversions at representative + catalog, table, recovery, and commit owners. +4. Replay completion Operation, Runtime, Lifecycle, and Fatal roots through + `into_quad` and verify their outer domains are unchanged. Replay Resource, + IO, and DataIntegrity roots and verify the supplied Runtime context is outer + while the lower frame and attachments remain. +5. Verify completion replay leaves no `CompletionErrorBridge` frame and an + existing Runtime context is not replaced by the fallback Runtime context. +6. Poison before admission and while an admission waiter is waking for Engine, + Session, Transaction checkout, and mandatory submission. Verify Fatal + public classification, the initiating Fatal frame, and no Lifecycle frame + above it. +7. Verify shutdown, closed session, discarded transaction, busy shutdown, and + mandatory admission closure remain Lifecycle. Verify poison-tolerant + diagnostics and terminal cleanup remain available according to their + existing contracts. +8. Exercise user commit resource rejection and verify public Runtime + classification with `RuntimeError::TransactionCommit` above the retained + Resource report. Verify shutdown remains Lifecycle and redo/rollback/poison + failure remains Fatal. +9. Exercise system and catalog commits and verify their existing + Runtime/Fatal typed behavior, cleanup, CTS ordering, lock release, and + session-state transitions are unchanged. +10. Exercise CREATE/DROP INDEX invalid request and invalid root shape. + Operation failures remain Operation; root-shape DataIntegrity appears + beneath `RuntimeError::CatalogAccess` and classifies publicly as Runtime. +11. Exercise point insert/upsert/update/delete and full-table mutation across + hot and cold rows. Verify existing Operation outcomes, Runtime contexts, + Fatal poison propagation, undo/redo effects, and retry behavior. +12. Return a caller-produced public error from `table_mutate_mvcc`. Verify the + callback error is forwarded unchanged while non-callback DML paths contain + no internal public-error convergence. +13. Bootstrap with invalid transaction configuration and verify + `ErrorKind::Config`. Inject recovery IO/DataIntegrity failures and verify + their retained lower frames under the existing Runtime recovery context. +14. Re-run startup worker-spawn, layout-marker, storage-root lease, rollback + join-panic, and partial-component failure tests to prove registration and + cleanup ordering is unchanged. +15. Regenerate `docs/public-error-audit.csv` and review every removed, retained, + moved, and newly added carrier-disclosure row against the required + inventory disposition. +16. Run focused error, poison/admission, mandatory runtime, transaction, + catalog-index, table-access, session, completion, and bootstrap tests before + the full validation matrix. +17. Run: + + ```bash + rtk cargo fmt --check + rtk cargo build --workspace + rtk cargo clippy --workspace --all-targets -- -D warnings + rtk cargo nextest run --workspace + rtk cargo clippy -p doradb-storage --no-default-features --features libaio --all-targets -- -D warnings + rtk cargo nextest run -p doradb-storage --no-default-features --features libaio + tools/style_audit.rs + rtk git diff --check + ``` + +## Open Questions + +None. The four Quad domains, single/pairwise preference, lower-domain Runtime +ownership, Config bootstrap ownership, direct-method audit scope, and callback +exception are resolved decisions. A newly discovered fifth integration domain +or a need to redesign public callback errors must be recorded as separate +design work rather than widening this carrier. diff --git a/docs/tasks/next-id b/docs/tasks/next-id index 96c23a2f..01457e8a 100644 --- a/docs/tasks/next-id +++ b/docs/tasks/next-id @@ -1 +1 @@ -000263 +000264 diff --git a/doradb-storage/src/catalog/index.rs b/doradb-storage/src/catalog/index.rs index 44243648..2e7fd732 100644 --- a/doradb-storage/src/catalog/index.rs +++ b/doradb-storage/src/catalog/index.rs @@ -2,9 +2,9 @@ use crate::buffer::{EvictableBufferPool, PoolGuard, PoolGuards}; use crate::catalog::{Catalog, IndexNo, IndexSpec, TableMetadata, catalog_table_id_from_slot}; use crate::engine::EngineCore; use crate::error::{ - CompletionErrorBridge, CompletionResult, DataIntegrityError, DataIntegrityResult, - DiscloseResultExt, FatalError, OperationError, OperationOrRuntimeResult, OperationResult, - Result, RuntimeError, RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, + CompletionErrorBridge, CompletionResult, DataIntegrityError, DataIntegrityResult, FatalError, + OperationError, OperationOrRuntimeResult, OperationResult, RuntimeError, RuntimeOrFatalError, + RuntimeOrFatalResult, RuntimeResult, }; use crate::file::cow_file::SUPER_BLOCK_ID; use crate::file::table_file::{ActiveRoot, MutableTableFile}; @@ -118,13 +118,18 @@ pub(crate) struct CreateIndexPlan { impl CreateIndexPlan { /// Captures the stable layout, root, and allocated metadata shape. - pub(crate) fn new(table_id: TableID, table: Arc, index_spec: IndexSpec) -> Result { + pub(crate) fn new( + table_id: TableID, + table: Arc
, + index_spec: IndexSpec, + ) -> OperationOrRuntimeResult { let old_layout = table.layout_snapshot(); let old_metadata = old_layout.metadata(); let active_root = table.file().active_root_unchecked().clone(); - validate_create_index_root_shape(table_id, &active_root, old_metadata).disclose()?; - let (index_no, new_metadata_value) = - old_metadata.try_with_created_index(index_spec).disclose()?; + validate_create_index_root_shape(table_id, &active_root, old_metadata) + .change_context(RuntimeError::CatalogAccess) + .attach("operation=create_index, phase=validate_root_shape")?; + let (index_no, new_metadata_value) = old_metadata.try_with_created_index(index_spec)?; let new_metadata = Arc::new(new_metadata_value); let index_no_usize = usize::from(index_no); let new_index_spec = new_metadata @@ -159,7 +164,11 @@ pub(crate) struct DropIndexPlan { impl DropIndexPlan { /// Captures the stable active slot, layout, and replacement root shape. - pub(crate) fn new(table_id: TableID, table: Arc
, index_no: IndexNo) -> Result { + pub(crate) fn new( + table_id: TableID, + table: Arc
, + index_no: IndexNo, + ) -> OperationOrRuntimeResult { let old_layout = table.layout_snapshot(); let old_metadata = old_layout.metadata(); let index_no_usize = usize::from(index_no); @@ -170,14 +179,14 @@ impl DropIndexPlan { Report::new(OperationError::IndexNotFound).attach(format!( "drop index target not found: table_id={table_id}, index_no={index_no}, reason=inactive_metadata_slot" )) - }) - .disclose()?; + })?; old_layout .secondary_index(index_no_usize) .expect("active index metadata must have a matching runtime index"); let active_root = table.file().active_root_unchecked().clone(); validate_drop_index_root_shape(table_id, index_no_usize, &active_root, old_metadata) - .disclose()?; + .change_context(RuntimeError::CatalogAccess) + .attach("operation=drop_index, phase=validate_root_shape")?; let new_metadata = Arc::new(old_metadata.without_index(index_no)); let mut secondary_index_roots = active_root.secondary_index_roots.clone(); secondary_index_roots[index_no_usize] = SUPER_BLOCK_ID; @@ -1995,7 +2004,7 @@ pub(crate) mod tests { TrxSysConfig, }; use crate::engine::Engine; - use crate::error::LifecycleError; + use crate::error::{LifecycleError, Result}; use crate::file::cow_file::tests::old_root_drop_count; use crate::file::table_file::ActiveRoot; use crate::index::IndexBatchStream; diff --git a/doradb-storage/src/catalog/table.rs b/doradb-storage/src/catalog/table.rs index b027c644..8b87571e 100644 --- a/doradb-storage/src/catalog/table.rs +++ b/doradb-storage/src/catalog/table.rs @@ -3000,6 +3000,11 @@ pub(crate) mod tests { err.report().downcast_ref::().copied(), Some(RuntimeError::CatalogAccess) ); + let report = format!("{err:?}"); + assert!( + report.contains("operation=create_table, phase=wait_mandatory_completion"), + "{report}" + ); assert_no_user_table_publication(&engine, table_id); assert!(engine.inner().poisoner.poison_error().is_none()); assert!(!has_ddl_lock_resource( diff --git a/doradb-storage/src/conf/mod.rs b/doradb-storage/src/conf/mod.rs index 2effc6cb..d741c244 100644 --- a/doradb-storage/src/conf/mod.rs +++ b/doradb-storage/src/conf/mod.rs @@ -15,4 +15,5 @@ pub use self::engine::{EngineConfig, MandatoryRuntimeConfig}; pub use self::fs::FileSystemConfig; pub(crate) use self::fs::ValidatedFileSystemConfig; pub use self::trx::TrxSysConfig; +pub(crate) use self::trx::ValidatedTrxSysConfig; pub use crate::log::LogSync; diff --git a/doradb-storage/src/conf/trx.rs b/doradb-storage/src/conf/trx.rs index 04af38ae..79c5a9df 100644 --- a/doradb-storage/src/conf/trx.rs +++ b/doradb-storage/src/conf/trx.rs @@ -218,6 +218,35 @@ impl TrxSysConfig { } } +/// Validated transaction configuration and its resolved redo-file prefix. +/// +/// Public engine bootstrap constructs this wrapper while Config disclosure is +/// still owned at the public boundary. Transaction-system components consume +/// it without reopening configuration-domain failure paths. +pub(crate) struct ValidatedTrxSysConfig { + config: TrxSysConfig, + file_prefix: String, +} + +impl ValidatedTrxSysConfig { + /// Validate, normalize, and resolve one transaction-system configuration. + #[inline] + pub(crate) fn try_new(mut config: TrxSysConfig) -> ConfigResult { + config.validate()?; + let file_prefix = config.file_prefix()?; + Ok(Self { + config, + file_prefix, + }) + } + + /// Consume the wrapper into normalized configuration and resolved prefix. + #[inline] + pub(crate) fn into_parts(self) -> (TrxSysConfig, String) { + (self.config, self.file_prefix) + } +} + #[inline] fn normalize_redo_file_max_size( requested_file_max_size: usize, diff --git a/doradb-storage/src/engine.rs b/doradb-storage/src/engine.rs index 87d71ce0..1ffc8712 100644 --- a/doradb-storage/src/engine.rs +++ b/doradb-storage/src/engine.rs @@ -15,9 +15,10 @@ use crate::component::{ ComponentRegistry, ComponentShutdownOutcome, DiskPoolConfig, EnginePools, IndexPoolConfig, MetaPoolConfig, RegistryBuilder, }; -use crate::conf::EngineConfig; +use crate::conf::{EngineConfig, ValidatedTrxSysConfig}; use crate::error::{ - ConfigError, DiscloseError, DiscloseResultExt, LifecycleError, LifecycleResult, Result, + ConfigError, DiscloseError, DiscloseResultExt, LifecycleError, LifecycleOrFatalError, + LifecycleOrFatalResult, LifecycleResult, Result, }; use crate::file::fs::{FileSystem, FileSystemWorkers}; use crate::id::SessionID; @@ -279,8 +280,183 @@ impl Engine { #[inline] pub async fn bootstrap(config: EngineConfig) -> Result { obs::info!("event=engine_lifecycle component=engine action=build_start result=ok"); - bootstrap_inner(config) - .await + let result = async { + let resolved = config + .resolve_storage_paths() + .disclose()? + .prepare_storage_root() + .disclose()?; + let lock_path = resolved.lock_path(); + let lease = match StorageRootLease::try_acquire(&resolved).disclose()? { + StorageRootLeaseAttempt::Acquired(lease) => lease, + StorageRootLeaseAttempt::Contended { + diagnostic, + diagnostic_status, + } => { + let report = Report::new(LifecycleError::StorageRootInUse).attach(format!( + "operation=acquire_storage_root, storage_root={}, lock_path={}, owner_diagnostic={diagnostic_status}", + resolved.storage_root_path().display(), + lock_path.display() + )); + let report = if let Some(diagnostic) = diagnostic { + report.attach(format!( + "owner_pid={}, owner_acquired_unix_ms={}", + diagnostic.pid, diagnostic.acquired_unix_ms + )) + } else { + report + }; + return Err(report.disclose()); + } + }; + let mut builder = RegistryBuilder::new(); + // Root ownership is registered first so every failure and reverse + // shutdown path releases it only after all subordinate components stop. + builder + .build::(lease) + .await + .unwrap_or_else(|never| match never {}); + resolved.cleanup_stale_marker_temps().disclose()?; + let marker_was_present = resolved.validate_marker_if_present().disclose()?; + // Startup prefers a small, durable-safety-focused preflight over trying + // to exhaust every possible path conflict up front. It is acceptable for + // later setup steps to fail, but those failures must not clobber durable + // files or persist `storage-layout.toml` before the engine is fully built. + resolved.ensure_directories().disclose()?; + + let file = config.file.data_dir(resolved.data_dir_path()); + let readonly_buffer_size = file.readonly_buffer_size; + let file = file.validate().disclose()?; + let trx_cfg = config.trx.log_dir(resolved.log_dir_path()); + let catalog_cfg = CatalogConfig::new(trx_cfg.recovery_disable_dml_validation); + let trx_cfg = ValidatedTrxSysConfig::try_new(trx_cfg).disclose()?; + // Components are registered in one fixed dependency order. Reverse + // registration order then defines both explicit shutdown order and the + // final owner drop order. + builder + .build::(()) + .await + .unwrap_or_else(|never| match never {}); + builder + .build::(config.mandatory_runtime.clone()) + .await + .disclose()?; + builder.build::(file).await.disclose()?; + builder + .build::(DiskPoolConfig::new(readonly_buffer_size)) + .await + .disclose()?; + builder + .build::(MetaPoolConfig::new(config.meta_buffer.as_u64() as usize)) + .await + .disclose()?; + builder + .build::(IndexPoolConfig::new( + config.index_buffer.as_u64() as usize, + resolved.index_swap_file_path(), + config.index_max_file_size.as_u64() as usize, + )) + .await + .disclose()?; + builder + .build::( + config + .data_buffer + .role(PoolRole::Mem) + .data_swap_file(resolved.data_swap_file_path()), + ) + .await + .disclose()?; + builder.build::(()).await.disclose()?; + builder + .build::(()) + .await + .disclose()?; + builder + .build::(()) + .await + .unwrap_or_else(|never| match never {}); + // Catalog owns user-table runtimes, and those runtimes retain buffer-pool + // guards for row/index/readonly access. Register catalog after the pools it + // can pin so reverse shutdown/drop order releases table guards before pool + // owners are torn down. + builder.build::(catalog_cfg).await.disclose()?; + builder + .build::(trx_cfg) + .await + .disclose()?; + builder + .build::(()) + .await + .disclose()?; + builder + .build::(()) + .await + .disclose()?; + builder + .build::(()) + .await + .disclose()?; + + if marker_was_present { + if !resolved.validate_marker_if_present().disclose()? { + return Err(Report::new(ConfigError::StorageLayoutMismatch) + .attach(format!( + "operation=revalidate_storage_layout_marker, phase=post_component_build, marker_path={}, reason=initially_present_marker_disappeared", + resolved.marker_path().display() + )) + .disclose()); + } + } else { + resolved.persist_marker().disclose()?; + } + let registry = builder.finish(); + let poisoner = registry.dependency::(); + let mandatory_runtime = registry.dependency::(); + let catalog = registry.dependency::(); + let trx_sys = registry.dependency::(); + let meta_pool = registry.dependency::(); + let index_pool = registry.dependency::(); + let mem_pool = registry.dependency::(); + let table_fs = registry.dependency::(); + let disk_pool = registry.dependency::(); + let lock_manager = registry.dependency::(); + let session_registry = Arc::new(SessionRegistry::new()); + let lifecycle = Arc::new(EngineLifecycle::new()); + let core = Arc::new(EngineCore { + poisoner, + mandatory_runtime, + catalog, + trx_sys, + pools: EnginePools::new( + meta_pool.clone_inner(), + index_pool.clone_inner(), + mem_pool.clone_inner(), + disk_pool.clone_inner(), + ), + table_fs, + lock_manager, + session_registry: Arc::downgrade(&session_registry), + #[cfg(test)] + table_ddl_test: TableDdlTestController::default(), + #[cfg(test)] + index_ddl_test: IndexDdlTestController::default(), + #[cfg(test)] + maintenance_test: MaintenanceTestController::default(), + }); + let engine_inner = EngineInner { + core, + session_registry, + lifecycle, + next_session_id: AtomicU64::new(FIRST_SESSION_ID.as_u64()), + }; + Ok(Engine { + inner: Arc::new(engine_inner), + components: Some(registry), + }) + } + .await; + result .inspect(|_| { obs::info!("event=engine_lifecycle component=engine action=build_finish result=ok"); }) @@ -312,7 +488,7 @@ impl Engine { } #[inline] - fn new_session_inner(&self) -> LifecycleResult { + fn new_session_inner(&self) -> LifecycleOrFatalResult { let inner = self.inner(); inner.with_admitted_operation(|| { let id = inner.next_session_id(); @@ -593,15 +769,14 @@ impl EngineInner { /// user callback, statement execution, blocking I/O, registry guard /// retention, or `.await` point. #[inline] - pub(crate) fn acquire_admission(&self) -> LifecycleResult> { + pub(crate) fn acquire_admission(&self) -> LifecycleOrFatalResult> { let admission = self .lifecycle .admit() .attach_with(|| "phase=acquire_engine_lifecycle_admission")?; - self.poisoner - .ensure_healthy() - .change_context(LifecycleError::RuntimeUnavailable) - .attach_with(|| "phase=check_engine_health")?; + self.poisoner.ensure_healthy().map_err(|error| { + LifecycleOrFatalError::from(error.attach("phase=check_engine_health")) + })?; Ok(admission) } @@ -611,7 +786,10 @@ impl EngineInner { /// strong pinning. The closure must not perform user callbacks, statement /// execution, blocking I/O, or async waits. #[inline] - pub(crate) fn with_admitted_operation(&self, f: impl FnOnce() -> T) -> LifecycleResult { + pub(crate) fn with_admitted_operation( + &self, + f: impl FnOnce() -> T, + ) -> LifecycleOrFatalResult { let _admission = self.acquire_admission()?; Ok(f()) } @@ -626,179 +804,6 @@ impl Deref for EngineInner { } } -#[inline] -async fn bootstrap_inner(config: EngineConfig) -> Result { - let resolved = config - .resolve_storage_paths() - .disclose()? - .prepare_storage_root() - .disclose()?; - let lock_path = resolved.lock_path(); - let lease = match StorageRootLease::try_acquire(&resolved).disclose()? { - StorageRootLeaseAttempt::Acquired(lease) => lease, - StorageRootLeaseAttempt::Contended { - diagnostic, - diagnostic_status, - } => { - let report = Report::new(LifecycleError::StorageRootInUse).attach(format!( - "operation=acquire_storage_root, storage_root={}, lock_path={}, owner_diagnostic={diagnostic_status}", - resolved.storage_root_path().display(), - lock_path.display() - )); - let report = if let Some(diagnostic) = diagnostic { - report.attach(format!( - "owner_pid={}, owner_acquired_unix_ms={}", - diagnostic.pid, diagnostic.acquired_unix_ms - )) - } else { - report - }; - return Err(report.disclose()); - } - }; - let mut builder = RegistryBuilder::new(); - // Root ownership is registered first so every failure and reverse - // shutdown path releases it only after all subordinate components stop. - builder - .build::(lease) - .await - .unwrap_or_else(|never| match never {}); - resolved.cleanup_stale_marker_temps().disclose()?; - let marker_was_present = resolved.validate_marker_if_present().disclose()?; - // Startup prefers a small, durable-safety-focused preflight over trying - // to exhaust every possible path conflict up front. It is acceptable for - // later setup steps to fail, but those failures must not clobber durable - // files or persist `storage-layout.toml` before the engine is fully built. - resolved.ensure_directories().disclose()?; - - let file = config.file.data_dir(resolved.data_dir_path()); - let readonly_buffer_size = file.readonly_buffer_size; - let file = file.validate().disclose()?; - let trx_cfg = config.trx.log_dir(resolved.log_dir_path()); - let catalog_cfg = CatalogConfig::new(trx_cfg.recovery_disable_dml_validation); - // Components are registered in one fixed dependency order. Reverse - // registration order then defines both explicit shutdown order and the - // final owner drop order. - builder - .build::(()) - .await - .unwrap_or_else(|never| match never {}); - builder - .build::(config.mandatory_runtime.clone()) - .await - .disclose()?; - builder.build::(file).await.disclose()?; - builder - .build::(DiskPoolConfig::new(readonly_buffer_size)) - .await - .disclose()?; - builder - .build::(MetaPoolConfig::new(config.meta_buffer.as_u64() as usize)) - .await - .disclose()?; - builder - .build::(IndexPoolConfig::new( - config.index_buffer.as_u64() as usize, - resolved.index_swap_file_path(), - config.index_max_file_size.as_u64() as usize, - )) - .await - .disclose()?; - builder - .build::( - config - .data_buffer - .role(PoolRole::Mem) - .data_swap_file(resolved.data_swap_file_path()), - ) - .await - .disclose()?; - builder.build::(()).await.disclose()?; - builder - .build::(()) - .await - .disclose()?; - builder - .build::(()) - .await - .unwrap_or_else(|never| match never {}); - // Catalog owns user-table runtimes, and those runtimes retain buffer-pool - // guards for row/index/readonly access. Register catalog after the pools it - // can pin so reverse shutdown/drop order releases table guards before pool - // owners are torn down. - builder.build::(catalog_cfg).await.disclose()?; - builder.build::(trx_cfg).await?; - builder - .build::(()) - .await - .disclose()?; - builder - .build::(()) - .await - .disclose()?; - builder - .build::(()) - .await - .disclose()?; - - if marker_was_present { - if !resolved.validate_marker_if_present().disclose()? { - return Err(Report::new(ConfigError::StorageLayoutMismatch) - .attach(format!( - "operation=revalidate_storage_layout_marker, phase=post_component_build, marker_path={}, reason=initially_present_marker_disappeared", - resolved.marker_path().display() - )) - .disclose()); - } - } else { - resolved.persist_marker().disclose()?; - } - let registry = builder.finish(); - let poisoner = registry.dependency::(); - let mandatory_runtime = registry.dependency::(); - let catalog = registry.dependency::(); - let trx_sys = registry.dependency::(); - let meta_pool = registry.dependency::(); - let index_pool = registry.dependency::(); - let mem_pool = registry.dependency::(); - let table_fs = registry.dependency::(); - let disk_pool = registry.dependency::(); - let lock_manager = registry.dependency::(); - let session_registry = Arc::new(SessionRegistry::new()); - let lifecycle = Arc::new(EngineLifecycle::new()); - let core = Arc::new(EngineCore { - poisoner, - mandatory_runtime, - catalog, - trx_sys, - pools: EnginePools::new( - meta_pool.clone_inner(), - index_pool.clone_inner(), - mem_pool.clone_inner(), - disk_pool.clone_inner(), - ), - table_fs, - lock_manager, - session_registry: Arc::downgrade(&session_registry), - #[cfg(test)] - table_ddl_test: TableDdlTestController::default(), - #[cfg(test)] - index_ddl_test: IndexDdlTestController::default(), - #[cfg(test)] - maintenance_test: MaintenanceTestController::default(), - }); - let engine_inner = EngineInner { - core, - session_registry, - lifecycle, - next_session_id: AtomicU64::new(FIRST_SESSION_ID.as_u64()), - }; - Ok(Engine { - inner: Arc::new(engine_inner), - components: Some(registry), - }) -} - #[cfg(test)] mod tests { use super::*; @@ -850,6 +855,31 @@ mod tests { assert!(output.contains("state=ShuttingDown"), "{output}"); } + #[test] + fn test_poisoned_engine_new_session_admission_remains_fatal() { + smol::block_on(async { + let root = TempDir::new().unwrap(); + let engine = Engine::bootstrap(test_engine_config_for(root.path())) + .await + .unwrap(); + let _ = engine + .inner() + .poisoner + .poison(Report::new(FatalError::RedoWrite).attach("test admission poison")); + + let error = match engine.new_session() { + Ok(_) => panic!("poisoned engine must reject new sessions"), + Err(error) => error, + }; + assert_eq!(error.kind(), ErrorKind::Fatal); + assert_eq!( + error.report().downcast_ref::().copied(), + Some(FatalError::RedoWrite) + ); + assert!(error.report().downcast_ref::().is_none()); + }); + } + fn test_engine_config_for(root: &Path) -> EngineConfig { EngineConfig::default() .storage_root(root) @@ -2505,10 +2535,10 @@ mod tests { .await .unwrap(); - let mut config = TrxSysConfig::default() + let config = TrxSysConfig::default() .log_dir(&log_dir) .log_file_stem("pending-startup-cleanup"); - config.validate().unwrap(); + let config = ValidatedTrxSysConfig::try_new(config).unwrap(); let (trx_sys, startup) = TransactionSystem::bootstrap( config, engine.inner().poisoner.clone(), diff --git a/doradb-storage/src/error.rs b/doradb-storage/src/error.rs index d9cd203f..11d23ff6 100644 --- a/doradb-storage/src/error.rs +++ b/doradb-storage/src/error.rs @@ -198,8 +198,6 @@ pub(crate) enum DataIntegrityError { pub(crate) enum LifecycleError { #[error("storage root is already in use")] StorageRootInUse, - #[error("runtime is unavailable")] - RuntimeUnavailable, #[error("storage engine is shut down")] Shutdown, #[error("storage engine shutdown is busy")] @@ -250,9 +248,9 @@ pub(crate) enum RuntimeError { /// Reversible checkpoint orchestration failed. #[error("checkpoint execution failed")] CheckpointExecution, - /// System-transaction preparation or commit admission failed. - #[error("system transaction commit failed")] - SystemTransactionCommit, + /// User- or system-transaction preparation or commit integration failed. + #[error("transaction commit failed")] + TransactionCommit, } /// Fieldless resource-domain errors carried underneath `ErrorKind::Resource`. @@ -588,6 +586,60 @@ impl CompletionErrorBridge { } } + /// Replays a completion into the common four-domain integration carrier. + /// + /// Operation, Runtime, Lifecycle, and Fatal roots retain their native + /// outer domain. Lower physical roots are stacked beneath the + /// caller-supplied Runtime context. + #[inline] + pub(crate) fn into_quad(self, runtime_context: RuntimeError) -> QuadError { + enum RootDomain { + Operation, + Runtime, + Lifecycle, + Fatal, + Physical, + } + + let root_domain = match &self.0.canonical { + CompletionSourceReport::Operation(_) => RootDomain::Operation, + CompletionSourceReport::Runtime(_) => RootDomain::Runtime, + CompletionSourceReport::Lifecycle(_) => RootDomain::Lifecycle, + CompletionSourceReport::Fatal(_) => RootDomain::Fatal, + CompletionSourceReport::Io(_) + | CompletionSourceReport::Resource(_) + | CompletionSourceReport::DataIntegrity(_) => RootDomain::Physical, + }; + + #[cfg(test)] + self.0.reconstructions.fetch_add(1, Ordering::Relaxed); + + let builder = self.replay_builder(); + match root_domain { + RootDomain::Operation => QuadError::Operation( + builder + .into_operation() + .expect("Operation completion source must reconstruct as Operation"), + ), + RootDomain::Runtime => QuadError::Runtime( + builder + .into_runtime() + .expect("Runtime completion source must reconstruct as Runtime"), + ), + RootDomain::Lifecycle => QuadError::Lifecycle( + builder + .into_lifecycle() + .expect("Lifecycle completion source must reconstruct as Lifecycle"), + ), + RootDomain::Fatal => QuadError::Fatal( + builder + .into_fatal() + .expect("Fatal completion source must reconstruct as Fatal"), + ), + RootDomain::Physical => QuadError::Runtime(builder.finish(runtime_context)), + } + } + /// Reconstructs a completion whose producer contract guarantees Fatal. /// /// # Panics @@ -626,18 +678,6 @@ impl CompletionErrorBridge { builder } - fn public_error_kind(&self) -> ErrorKind { - self.0 - .replay - .iter() - .rev() - .find_map(|frame| match frame { - ReplayFrame::Context(context) => context.error_kind(), - ReplayFrame::Attachment(_) => None, - }) - .expect("validated completion bridge must contain a real context") - } - #[inline] fn fatal_context(&self) -> Option { self.0.canonical.fatal_context() @@ -765,14 +805,6 @@ impl Debug for CompletionErrorBridge { } } -impl DiscloseError for CompletionErrorBridge { - #[inline] - fn disclose(self) -> Error { - let kind = self.public_error_kind(); - Error(self.replace_context(kind)) - } -} - /// Cloneable source-bearing failure whose current context is always Fatal. /// /// This wrapper carries fatal policy state through redo, transaction cleanup, @@ -1147,6 +1179,277 @@ impl RuntimeOrFatalResultExt for RuntimeOrFatalResult { } } +/// Constrained carrier for lifecycle rejection and fatal engine health exits. +/// +/// The reports remain in their native domains until an outward public +/// boundary. This carrier is deliberately not an `error-stack` context. +pub(crate) enum LifecycleOrFatalError { + /// A request rejected by ordinary lifecycle state. + Lifecycle(Report), + /// A request rejected by the engine's one-way fatal state. + Fatal(Report), +} + +impl Debug for LifecycleOrFatalError { + #[inline] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Lifecycle(report) => Debug::fmt(report, f), + Self::Fatal(report) => Debug::fmt(report, f), + } + } +} + +impl Display for LifecycleOrFatalError { + #[inline] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Lifecycle(report) => Display::fmt(report, f), + Self::Fatal(report) => Display::fmt(report, f), + } + } +} + +impl LifecycleOrFatalError { + #[inline] + fn attach(self, attachment: &'static str) -> Self { + match self { + Self::Lifecycle(report) => Self::Lifecycle(report.attach(attachment)), + Self::Fatal(report) => Self::Fatal(report.attach(attachment)), + } + } + + #[inline] + fn attach_with(self, attachment: F) -> Self + where + F: FnOnce() -> String, + { + match self { + Self::Lifecycle(report) => Self::Lifecycle(report.attach(attachment())), + Self::Fatal(report) => Self::Fatal(report.attach(attachment())), + } + } +} + +impl From> for LifecycleOrFatalError { + #[inline] + fn from(report: Report) -> Self { + Self::Lifecycle(report) + } +} + +impl From> for LifecycleOrFatalError { + #[inline] + fn from(report: Report) -> Self { + Self::Fatal(report) + } +} + +impl From for LifecycleOrFatalError { + #[inline] + fn from(error: SharedFatalError) -> Self { + Self::Fatal(error.into_report()) + } +} + +impl DiscloseError for LifecycleOrFatalError { + #[inline] + fn disclose(self) -> Error { + match self { + Self::Lifecycle(report) => report.disclose(), + Self::Fatal(report) => report.disclose(), + } + } +} + +/// Result carrying either ordinary lifecycle rejection or a Fatal report. +pub(crate) type LifecycleOrFatalResult = result::Result; + +impl MultiDomainResultExt for LifecycleOrFatalResult { + #[inline] + fn attach(self, attachment: &'static str) -> Self { + self.map_err(|error| error.attach(attachment)) + } + + #[inline] + fn attach_with(self, attachment: F) -> Self + where + F: FnOnce() -> String, + { + self.map_err(|error| error.attach_with(attachment)) + } +} + +/// Closed four-domain carrier for final internal integration owners. +/// +/// The fixed membership is Operation, Runtime, Lifecycle, and Fatal. Lower +/// physical domains require an explicit Runtime owner before entering this +/// carrier. This carrier is deliberately not an `error-stack` context. +pub(crate) enum QuadError { + /// A terminal semantic operation failure. + Operation(Report), + /// A recoverable runtime-integration failure. + Runtime(Report), + /// An ordinary lifecycle rejection. + Lifecycle(Report), + /// A failure that already crossed a Fatal policy boundary. + Fatal(Report), +} + +impl Debug for QuadError { + #[inline] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Operation(report) => Debug::fmt(report, f), + Self::Runtime(report) => Debug::fmt(report, f), + Self::Lifecycle(report) => Debug::fmt(report, f), + Self::Fatal(report) => Debug::fmt(report, f), + } + } +} + +impl Display for QuadError { + #[inline] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Operation(report) => Display::fmt(report, f), + Self::Runtime(report) => Display::fmt(report, f), + Self::Lifecycle(report) => Display::fmt(report, f), + Self::Fatal(report) => Display::fmt(report, f), + } + } +} + +impl QuadError { + #[inline] + fn attach(self, attachment: &'static str) -> Self { + match self { + Self::Operation(report) => Self::Operation(report.attach(attachment)), + Self::Runtime(report) => Self::Runtime(report.attach(attachment)), + Self::Lifecycle(report) => Self::Lifecycle(report.attach(attachment)), + Self::Fatal(report) => Self::Fatal(report.attach(attachment)), + } + } + + #[inline] + fn attach_with(self, attachment: F) -> Self + where + F: FnOnce() -> String, + { + match self { + Self::Operation(report) => Self::Operation(report.attach(attachment())), + Self::Runtime(report) => Self::Runtime(report.attach(attachment())), + Self::Lifecycle(report) => Self::Lifecycle(report.attach(attachment())), + Self::Fatal(report) => Self::Fatal(report.attach(attachment())), + } + } +} + +impl From> for QuadError { + #[inline] + fn from(report: Report) -> Self { + Self::Operation(report) + } +} + +impl From> for QuadError { + #[inline] + fn from(report: Report) -> Self { + Self::Runtime(report) + } +} + +impl From> for QuadError { + #[inline] + fn from(report: Report) -> Self { + Self::Lifecycle(report) + } +} + +impl From> for QuadError { + #[inline] + fn from(report: Report) -> Self { + Self::Fatal(report) + } +} + +impl From for QuadError { + #[inline] + fn from(error: SharedFatalError) -> Self { + Self::Fatal(error.into_report()) + } +} + +impl From for QuadError { + #[inline] + fn from(error: OperationOrRuntimeError) -> Self { + match error { + OperationOrRuntimeError::Operation(report) => Self::Operation(report), + OperationOrRuntimeError::Runtime(report) => Self::Runtime(report), + } + } +} + +impl From for QuadError { + #[inline] + fn from(error: OperationOrFatalError) -> Self { + match error { + OperationOrFatalError::Operation(report) => Self::Operation(report), + OperationOrFatalError::Fatal(report) => Self::Fatal(report), + } + } +} + +impl From for QuadError { + #[inline] + fn from(error: RuntimeOrFatalError) -> Self { + match error { + RuntimeOrFatalError::Runtime(report) => Self::Runtime(report), + RuntimeOrFatalError::Fatal(report) => Self::Fatal(report), + } + } +} + +impl From for QuadError { + #[inline] + fn from(error: LifecycleOrFatalError) -> Self { + match error { + LifecycleOrFatalError::Lifecycle(report) => Self::Lifecycle(report), + LifecycleOrFatalError::Fatal(report) => Self::Fatal(report), + } + } +} + +impl DiscloseError for QuadError { + #[inline] + fn disclose(self) -> Error { + match self { + Self::Operation(report) => report.disclose(), + Self::Runtime(report) => report.disclose(), + Self::Lifecycle(report) => report.disclose(), + Self::Fatal(report) => report.disclose(), + } + } +} + +/// Result carrying the fixed Operation/Runtime/Lifecycle/Fatal integration set. +pub(crate) type QuadResult = result::Result; + +impl MultiDomainResultExt for QuadResult { + #[inline] + fn attach(self, attachment: &'static str) -> Self { + self.map_err(|error| error.attach(attachment)) + } + + #[inline] + fn attach_with(self, attachment: F) -> Self + where + F: FnOnce() -> String, + { + self.map_err(|error| error.attach_with(attachment)) + } +} + #[derive(Clone, Copy)] enum ReplayContext { Config(ConfigError), @@ -1234,21 +1537,6 @@ impl ReplayContext { } } } - - #[inline] - const fn error_kind(self) -> Option { - match self { - ReplayContext::Config(_) => Some(ErrorKind::Config), - ReplayContext::Operation(_) => Some(ErrorKind::Operation), - ReplayContext::Resource(_) => Some(ErrorKind::Resource), - ReplayContext::Io(_) => Some(ErrorKind::Io), - ReplayContext::DataIntegrity(_) => Some(ErrorKind::DataIntegrity), - ReplayContext::Lifecycle(_) => Some(ErrorKind::Lifecycle), - ReplayContext::Runtime(_) => Some(ErrorKind::Runtime), - ReplayContext::Fatal(_) => Some(ErrorKind::Fatal), - ReplayContext::Internal(_) => None, - } - } } enum ReplayAttachment { @@ -1359,6 +1647,51 @@ impl ReplayReportBuilder { | Self::Internal(_) => None, } } + + #[inline] + fn into_operation(self) -> Option> { + match self { + Self::Operation(report) => Some(report), + Self::Config(_) + | Self::Resource(_) + | Self::Io(_) + | Self::DataIntegrity(_) + | Self::Lifecycle(_) + | Self::Runtime(_) + | Self::Fatal(_) + | Self::Internal(_) => None, + } + } + + #[inline] + fn into_runtime(self) -> Option> { + match self { + Self::Runtime(report) => Some(report), + Self::Config(_) + | Self::Operation(_) + | Self::Resource(_) + | Self::Io(_) + | Self::DataIntegrity(_) + | Self::Lifecycle(_) + | Self::Fatal(_) + | Self::Internal(_) => None, + } + } + + #[inline] + fn into_lifecycle(self) -> Option> { + match self { + Self::Lifecycle(report) => Some(report), + Self::Config(_) + | Self::Operation(_) + | Self::Resource(_) + | Self::Io(_) + | Self::DataIntegrity(_) + | Self::Runtime(_) + | Self::Fatal(_) + | Self::Internal(_) => None, + } + } } /// Printable secondary-index binding mismatch context. @@ -1427,15 +1760,6 @@ impl Error { self.0 } - /// Lazily adds boundary context without changing the existing error classification. - #[inline] - pub(crate) fn attach_with(self, attachment: F) -> Self - where - F: FnOnce() -> String, - { - Error(self.0.attach(attachment())) - } - #[inline] fn fmt_report_line(report: &Report, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut first = true; @@ -1962,6 +2286,95 @@ mod tests { assert!(format!("{err:?}").contains("checkpoint write failed")); } + #[test] + fn lifecycle_or_fatal_preserves_domain_and_attachments() { + let lifecycle: LifecycleOrFatalResult<()> = + Err(Report::new(LifecycleError::Shutdown).into()); + let error = lifecycle + .attach("operation=admit") + .expect_err("shutdown must reject admission") + .disclose(); + assert_eq!(error.kind(), ErrorKind::Lifecycle); + assert_eq!( + error.report().downcast_ref::().copied(), + Some(LifecycleError::Shutdown) + ); + assert!(format!("{error:?}").contains("operation=admit")); + + let fatal: LifecycleOrFatalResult<()> = Err(Report::new(FatalError::Poisoned).into()); + let error = fatal + .attach_with(|| "phase=health_check".to_owned()) + .expect_err("poison must reject admission") + .disclose(); + assert_eq!(error.kind(), ErrorKind::Fatal); + assert_eq!( + error.report().downcast_ref::().copied(), + Some(FatalError::Poisoned) + ); + assert!(error.report().downcast_ref::().is_none()); + assert!(format!("{error:?}").contains("phase=health_check")); + } + + #[test] + fn quad_native_arms_disclose_without_carrier_context() { + let cases = [ + ( + QuadError::from(Report::new(OperationError::DuplicateKey)), + ErrorKind::Operation, + ), + ( + QuadError::from(Report::new(RuntimeError::TableAccess)), + ErrorKind::Runtime, + ), + ( + QuadError::from(Report::new(LifecycleError::Shutdown)), + ErrorKind::Lifecycle, + ), + ( + QuadError::from(Report::new(FatalError::Poisoned)), + ErrorKind::Fatal, + ), + ]; + + for (carrier, expected_kind) in cases { + let error = carrier.attach("operation=quad_test").disclose(); + assert_eq!(error.kind(), expected_kind); + assert!(error.report().downcast_ref::().is_none()); + assert!(format!("{error:?}").contains("operation=quad_test")); + } + } + + #[test] + fn quad_flattens_pairwise_carriers_without_losing_reports() { + let operation = QuadError::from(OperationOrRuntimeError::Operation( + Report::new(OperationError::IndexNotFound).attach("pair=operation_runtime"), + )); + let runtime = QuadError::from(RuntimeOrFatalError::Runtime( + Report::new(RuntimeError::IndexAccess).attach("pair=runtime_fatal"), + )); + let fatal = QuadError::from(OperationOrFatalError::Fatal( + Report::new(FatalError::RedoWrite).attach("pair=operation_fatal"), + )); + let lifecycle = QuadError::from(LifecycleOrFatalError::Lifecycle( + Report::new(LifecycleError::Shutdown).attach("pair=lifecycle_fatal"), + )); + + assert!(matches!(&operation, QuadError::Operation(_))); + assert!(matches!(&runtime, QuadError::Runtime(_))); + assert!(matches!(&fatal, QuadError::Fatal(_))); + assert!(matches!(&lifecycle, QuadError::Lifecycle(_))); + for (carrier, attachment) in [ + (operation, "pair=operation_runtime"), + (runtime, "pair=runtime_fatal"), + (fatal, "pair=operation_fatal"), + (lifecycle, "pair=lifecycle_fatal"), + ] { + let error = carrier.disclose(); + assert!(format!("{error:?}").contains(attachment)); + assert!(error.report().downcast_ref::().is_none()); + } + } + #[test] fn test_buffer_pool_init_report_converts_losslessly_to_public_runtime() { let report = Report::new(ResourceError::BufferPoolSizeTooSmall) @@ -2234,9 +2647,10 @@ mod tests { assert!(second_output.contains("complete test backend read")); let err = bridge - .disclose() - .attach_with(|| "public completion boundary".to_owned()); - assert_eq!(err.kind(), ErrorKind::Io); + .into_quad(RuntimeError::FileRootAccess) + .attach("public completion boundary") + .disclose(); + assert_eq!(err.kind(), ErrorKind::Runtime); assert!(err.report().downcast_ref::().is_some()); assert!( err.report() @@ -2367,6 +2781,141 @@ mod tests { assert!(format!("{fatal:?}").contains("fatal carrier")); } + #[test] + fn completion_bridge_into_quad_preserves_common_outer_domains() { + let operation = CompletionErrorBridge::capture( + Report::new(OperationError::IndexNotFound).attach("operation source"), + ) + .into_quad(RuntimeError::CatalogAccess) + .attach("operation=create_index, phase=wait_mandatory_completion"); + let QuadError::Operation(operation) = operation else { + panic!("Operation completion must remain Operation") + }; + assert_eq!( + operation.downcast_ref::().copied(), + Some(OperationError::IndexNotFound) + ); + assert!(format!("{operation:?}").contains("operation source")); + assert!( + format!("{operation:?}") + .contains("operation=create_index, phase=wait_mandatory_completion") + ); + assert!(operation.downcast_ref::().is_none()); + assert!(operation.downcast_ref::().is_none()); + + let runtime = CompletionErrorBridge::capture( + Report::new(RuntimeError::IndexAccess).attach("runtime source"), + ) + .into_quad(RuntimeError::CatalogAccess) + .attach("operation=create_index, phase=wait_mandatory_completion"); + let QuadError::Runtime(runtime) = runtime else { + panic!("Runtime completion must remain Runtime") + }; + assert_eq!(runtime.current_context(), &RuntimeError::IndexAccess); + assert!(format!("{runtime:?}").contains("runtime source")); + assert!( + format!("{runtime:?}") + .contains("operation=create_index, phase=wait_mandatory_completion") + ); + assert!(runtime.downcast_ref::().is_none()); + assert!(runtime.downcast_ref::().is_none()); + + let lifecycle = CompletionErrorBridge::capture( + Report::new(LifecycleError::Shutdown).attach("lifecycle source"), + ) + .into_quad(RuntimeError::CatalogAccess) + .attach("operation=create_index, phase=wait_mandatory_completion"); + let QuadError::Lifecycle(lifecycle) = lifecycle else { + panic!("Lifecycle completion must remain Lifecycle") + }; + assert!( + format!("{lifecycle:?}") + .contains("operation=create_index, phase=wait_mandatory_completion") + ); + assert!(lifecycle.downcast_ref::().is_none()); + assert!(lifecycle.downcast_ref::().is_none()); + + let fatal = CompletionErrorBridge::capture( + Report::new(FatalError::RedoWrite).attach("fatal source"), + ) + .into_quad(RuntimeError::CatalogAccess) + .attach("operation=create_index, phase=wait_mandatory_completion"); + let QuadError::Fatal(fatal) = fatal else { + panic!("Fatal completion must remain Fatal") + }; + assert!(fatal.downcast_ref::().is_none()); + assert!(format!("{fatal:?}").contains("fatal source")); + assert!( + format!("{fatal:?}") + .contains("operation=create_index, phase=wait_mandatory_completion") + ); + assert!(fatal.downcast_ref::().is_none()); + assert!(fatal.downcast_ref::().is_none()); + } + + #[test] + fn completion_bridge_into_quad_stacks_physical_roots_under_runtime() { + let resource = CompletionErrorBridge::capture( + Report::new(ResourceError::BufferPoolFull).attach("resource source"), + ) + .into_quad(RuntimeError::TransactionCommit) + .attach("operation=commit_transaction, phase=wait_redo_group_commit"); + let QuadError::Runtime(resource) = resource else { + panic!("Resource completion must enter Quad through Runtime") + }; + assert_eq!(resource.current_context(), &RuntimeError::TransactionCommit); + assert_eq!( + resource.downcast_ref::().copied(), + Some(ResourceError::BufferPoolFull) + ); + assert!( + format!("{resource:?}") + .contains("operation=commit_transaction, phase=wait_redo_group_commit") + ); + assert!(resource.downcast_ref::().is_none()); + assert!(resource.downcast_ref::().is_none()); + + let io = CompletionErrorBridge::capture( + Report::new(IoError::from(IoErrorKind::BrokenPipe)).attach("io source"), + ) + .into_quad(RuntimeError::RedoLogAccess) + .attach("operation=truncate_redo_log, phase=wait_mandatory_completion"); + let QuadError::Runtime(io) = io else { + panic!("IO completion must enter Quad through Runtime") + }; + assert_eq!(io.current_context(), &RuntimeError::RedoLogAccess); + assert_eq!( + io.downcast_ref::().copied().map(IoError::kind), + Some(IoErrorKind::BrokenPipe) + ); + assert!( + format!("{io:?}") + .contains("operation=truncate_redo_log, phase=wait_mandatory_completion") + ); + assert!(io.downcast_ref::().is_none()); + assert!(io.downcast_ref::().is_none()); + + let integrity = CompletionErrorBridge::capture( + Report::new(DataIntegrityError::ChecksumMismatch).attach("integrity source"), + ) + .into_quad(RuntimeError::Recovery) + .attach("operation=recover_transaction_system, phase=wait_completion"); + let QuadError::Runtime(integrity) = integrity else { + panic!("Data-integrity completion must enter Quad through Runtime") + }; + assert_eq!(integrity.current_context(), &RuntimeError::Recovery); + assert_eq!( + integrity.downcast_ref::().copied(), + Some(DataIntegrityError::ChecksumMismatch) + ); + assert!( + format!("{integrity:?}") + .contains("operation=recover_transaction_system, phase=wait_completion") + ); + assert!(integrity.downcast_ref::().is_none()); + assert!(integrity.downcast_ref::().is_none()); + } + #[test] fn test_completion_bridge_runtime_conversion_composes_static_attachment() { let bridge = CompletionErrorBridge::capture( @@ -2375,7 +2924,7 @@ mod tests { .change_context(RuntimeError::IndexAccess), ); let result: RuntimeOrFatalResult<()> = - Err(bridge.into_runtime_or_fatal(RuntimeError::SystemTransactionCommit)); + Err(bridge.into_runtime_or_fatal(RuntimeError::TransactionCommit)); let error = result .attach("operation=commit_system_transaction") @@ -2384,10 +2933,7 @@ mod tests { panic!("non-Fatal completion must reconstruct as Runtime") }; - assert_eq!( - report.current_context(), - &RuntimeError::SystemTransactionCommit - ); + assert_eq!(report.current_context(), &RuntimeError::TransactionCommit); assert_eq!( report.downcast_ref::().copied(), Some(InternalError::SecondaryIndexOutOfBounds) @@ -2408,7 +2954,7 @@ mod tests { .change_context(FatalError::RedoWrite), ); let result: RuntimeOrFatalResult<()> = - Err(bridge.into_runtime_or_fatal(RuntimeError::SystemTransactionCommit)); + Err(bridge.into_runtime_or_fatal(RuntimeError::TransactionCommit)); let error = result .attach("operation=commit_system_transaction") @@ -2483,7 +3029,7 @@ mod tests { ); assert!(reconstructed.downcast_ref::().is_none()); - let public = bridge.disclose(); + let public = bridge.into_quad(RuntimeError::TableAccess).disclose(); assert_eq!(public.kind(), ErrorKind::Fatal); assert_eq!( public @@ -2549,9 +3095,9 @@ mod tests { assert!(output.contains("redo write policy"), "{output}"); let public = shared - .into_completion_bridge() - .disclose() - .attach_with(|| "wait for shared fatal completion".to_owned()); + .into_report() + .attach("wait for shared fatal completion") + .disclose(); assert_eq!(public.kind(), ErrorKind::Fatal); assert_eq!( public.report().downcast_ref::().copied(), diff --git a/doradb-storage/src/file/mod.rs b/doradb-storage/src/file/mod.rs index 3b1359a7..c4a71632 100644 --- a/doradb-storage/src/file/mod.rs +++ b/doradb-storage/src/file/mod.rs @@ -837,7 +837,7 @@ mod tests { ColumnAttributes, ColumnSpec, IndexAttributes, IndexKey, IndexSpec, USER_TABLE_ID_START, }; use crate::compression::BitPackable; - use crate::error::{DiscloseError, RuntimeError}; + use crate::error::{DiscloseResultExt, MultiDomainResultExt, RuntimeError}; use crate::file::fs::tests::{TestFileSystem, build_test_fs}; use crate::file::table_file::TableFile; use crate::id::TrxID; @@ -1187,11 +1187,12 @@ mod tests { .on_complete(TableFsSubmission::Write(submission), Ok(expected_len - 1)); assert_eq!(kind, IOKind::Write); - let wait_result = waiter.wait_result().await.map_err(|report| { - report - .disclose() - .attach_with(|| "wait for table file background write".to_owned()) - }); + let wait_result = waiter + .wait_result() + .await + .map_err(|report| report.into_quad(RuntimeError::FileRootAccess)) + .attach("wait for table file background write") + .disclose(); assert!(wait_result.as_ref().is_err_and(|err| { err.report() .downcast_ref::() @@ -1262,11 +1263,12 @@ mod tests { let kind = state_machine.on_complete(TableFsSubmission::Sync(submission), Ok(1)); assert_eq!(kind, IOKind::Fsync); - let wait_result = waiter.wait_result().await.map_err(|report| { - report - .disclose() - .attach_with(|| "wait for table file background fsync".to_owned()) - }); + let wait_result = waiter + .wait_result() + .await + .map_err(|report| report.into_quad(RuntimeError::FileRootAccess)) + .attach("wait for table file background fsync") + .disclose(); let err = wait_result.expect_err("nonzero fsync completion should fail"); assert_eq!( err.report() @@ -1306,11 +1308,12 @@ mod tests { ); assert_eq!(kind, IOKind::Fsync); - let wait_result = waiter.wait_result().await.map_err(|report| { - report - .disclose() - .attach_with(|| "wait for table file background fsync".to_owned()) - }); + let wait_result = waiter + .wait_result() + .await + .map_err(|report| report.into_quad(RuntimeError::FileRootAccess)) + .attach("wait for table file background fsync") + .disclose(); let err = wait_result.expect_err("backend fsync completion should fail"); assert!( err.report().downcast_ref::().is_some(), diff --git a/doradb-storage/src/log/mod.rs b/doradb-storage/src/log/mod.rs index 74270288..c8c9e85d 100644 --- a/doradb-storage/src/log/mod.rs +++ b/doradb-storage/src/log/mod.rs @@ -2442,8 +2442,8 @@ mod tests { use crate::conf::{EngineConfig, EvictableBufferPoolConfig, TrxSysConfig}; use crate::engine::Engine; use crate::error::{ - DataIntegrityError, ErrorKind, FatalError, IoError, IoResult, LifecycleError, Result, - RuntimeError, SharedFatalError, + DataIntegrityError, DiscloseResultExt, ErrorKind, FatalError, IoError, IoResult, + LifecycleError, Result, RuntimeError, SharedFatalError, }; use crate::id::{PageID, RowID, TableID}; use crate::io::{ @@ -2870,7 +2870,7 @@ mod tests { RowID::new(1), ); let prepared = sys_trx.prepare(); - trx_sys.commit_prepared(prepared).await + trx_sys.commit_prepared(prepared).await.disclose() }) }) } diff --git a/doradb-storage/src/runtime/mandatory.rs b/doradb-storage/src/runtime/mandatory.rs index ba6e9968..01de62cb 100644 --- a/doradb-storage/src/runtime/mandatory.rs +++ b/doradb-storage/src/runtime/mandatory.rs @@ -4,8 +4,9 @@ use crate::component::{ }; use crate::conf::MandatoryRuntimeConfig; use crate::error::{ - CompletionErrorBridge, CompletionResult, ConfigError, ConfigResult, DiscloseError, FatalError, - LifecycleError, LifecycleResult, Result, RuntimeError, RuntimeResult, SharedFatalError, + CompletionErrorBridge, CompletionResult, ConfigError, ConfigResult, FatalError, LifecycleError, + LifecycleOrFatalError, LifecycleOrFatalResult, LifecycleResult, RuntimeError, RuntimeResult, + SharedFatalError, }; use crate::id::{SessionOperationKey, TableID}; use crate::obs; @@ -599,9 +600,9 @@ pub(crate) struct CompletionObserver { } impl CompletionObserver { - /// Wait for the mandatory task and disclose its terminal result. + /// Wait for the mandatory task and return its typed completion transport. #[inline] - pub(crate) async fn wait(mut self) -> Result { + pub(crate) async fn wait(mut self) -> CompletionResult { let result = self.inner.completion.wait_take_result().await; let mut observation = self.inner.observation.lock(); assert!( @@ -611,7 +612,7 @@ impl CompletionObserver { *observation = ObservationState::Consumed; self.armed = false; drop(observation); - result.map_err(DiscloseError::disclose) + result } } @@ -981,16 +982,16 @@ impl QuiescentGuard { pub(crate) async fn submit( &self, prepared: E, - ) -> LifecycleResult> + ) -> LifecycleOrFatalResult> where E: PreparedExecution, { let poison_listener = self.poisoner.listener(); if let Err(error) = self.poisoner.ensure_healthy() { self.admission.close(); - return Err(error - .change_context(LifecycleError::RuntimeUnavailable) - .attach("phase=mandatory_admission_health_check")); + return Err(LifecycleOrFatalError::from( + error.attach("phase=mandatory_admission_health_check"), + )); } let admission_started_at = Instant::now(); let acquire = self.admission.acquire(self.clone()); @@ -1000,12 +1001,12 @@ impl QuiescentGuard { Either::Left((result, _)) => result?, Either::Right((_, _)) => { self.admission.close(); - return Err(self - .poisoner - .ensure_healthy() - .expect_err("poison event requires a published fatal reason") - .change_context(LifecycleError::RuntimeUnavailable) - .attach("phase=mandatory_admission_poison_wake")); + return Err(LifecycleOrFatalError::from( + self.poisoner + .ensure_healthy() + .expect_err("poison event requires a published fatal reason") + .attach("phase=mandatory_admission_poison_wake"), + )); } }; // Winning admission is the poison-race linearization point. A later @@ -1189,7 +1190,7 @@ mod tests { use super::*; use crate::component::RegistryBuilder; use crate::conf::MandatoryRuntimeConfig; - use crate::error::{ErrorKind, OperationError}; + use crate::error::{FatalError, OperationError}; use crate::thread::{SpawnTestEvent, fail_spawn_named, observe_spawn_named}; use std::panic::{self, AssertUnwindSafe}; use std::sync::Arc; @@ -1211,7 +1212,7 @@ mod tests { } #[test] - fn mandatory_observer_discloses_operation_error() { + fn mandatory_observer_retains_operation_error() { runtime::block_on(async { let metadata = MandatoryTaskMetadata::operation("test", None); let (producer, observer) = MandatoryCompletion::<()>::endpoints( @@ -1222,7 +1223,10 @@ mod tests { Report::new(OperationError::TableNotFound).attach("operation=test"), ))); let error = observer.wait().await.unwrap_err(); - assert_eq!(error.kind(), ErrorKind::Operation); + assert_eq!( + error.downcast_ref::().copied(), + Some(OperationError::TableNotFound) + ); }); } @@ -1787,7 +1791,10 @@ mod tests { .wait() .await .unwrap_err(); - assert_eq!(error.kind(), ErrorKind::Operation); + assert_eq!( + error.downcast_ref::().copied(), + Some(OperationError::TableNotFound) + ); let stats = mandatory.stats().operation; assert_eq!(stats.submitted_count, 1); assert_eq!(stats.started_count, 1); @@ -2044,13 +2051,35 @@ mod tests { .await .unwrap(); let error = observer.wait().await.unwrap_err(); - assert_eq!(error.kind(), ErrorKind::Fatal); + assert_eq!( + error.downcast_ref::().copied(), + Some(FatalError::MandatoryTaskPanic) + ); assert_eq!(finishes.load(Ordering::Relaxed), 0); assert_eq!(handled.load(Ordering::Relaxed), 1); let poison = mandatory.poisoner.poison_error().unwrap(); let poison = format!("{poison:?}"); assert!(poison.contains("task_class=operation"), "{poison}"); assert!(poison.contains("task_label=execute_panic"), "{poison}"); + let rejected = mandatory + .submit(SyntheticPrepared { + moves: Arc::new(AtomicUsize::new(0)), + finishes: Arc::new(AtomicUsize::new(0)), + fail: false, + }) + .await; + let error = match rejected { + Ok(_) => panic!("poisoned mandatory runtime must reject new work"), + Err(error) => error, + }; + let LifecycleOrFatalError::Fatal(error) = error else { + panic!("poisoned mandatory admission must remain Fatal") + }; + assert_eq!( + error.downcast_ref::().copied(), + Some(FatalError::MandatoryTaskPanic) + ); + assert!(error.downcast_ref::().is_none()); mandatory.drain_callers().await; let stats = mandatory.stats().operation; assert_eq!(stats.submitted_count, 1); diff --git a/doradb-storage/src/session.rs b/doradb-storage/src/session.rs index 13935d57..31e19b57 100644 --- a/doradb-storage/src/session.rs +++ b/doradb-storage/src/session.rs @@ -12,7 +12,8 @@ use crate::catalog::{ use crate::engine::{EngineAdmission, EngineCore, EngineLifecycle}; use crate::error::{ CompletionErrorBridge, CompletionResult, DiscloseError, DiscloseResultExt, FatalError, - LifecycleError, LifecycleResult, MultiDomainResultExt, OperationError, OperationResult, Result, + LifecycleError, LifecycleOrFatalError, LifecycleOrFatalResult, LifecycleResult, + MultiDomainResultExt, OperationError, OperationResult, Result, RuntimeError, }; use crate::id::{OperationID, SessionID, SessionOperationKey, TableID, TrxID}; use crate::lock::{ @@ -768,10 +769,11 @@ impl Session { /// This path rejects storage poison. Poison-observable read-only diagnostics /// use [`Self::pin_inspection`] instead. #[inline] - pub(crate) fn pin_observer(&self) -> LifecycleResult { + pub(crate) fn pin_observer(&self) -> LifecycleOrFatalResult { if self.closed.get() { return Err(Report::new(LifecycleError::SessionUnavailable) - .attach(format!("session_id={}", self.id))); + .attach(format!("session_id={}", self.id)) + .into()); } let admitted = self .session @@ -785,8 +787,11 @@ impl Session { .runtime() .poisoner .ensure_healthy() - .change_context(LifecycleError::RuntimeUnavailable) - .attach_with(|| format!("session_id={}, phase=check_engine_health", self.id))?; + .map_err(|error| { + LifecycleOrFatalError::from( + error.attach(format!("session_id={}, phase=check_engine_health", self.id)), + ) + })?; admitted .runtime() .state() @@ -798,10 +803,14 @@ impl Session { /// Reserves one stable entry for an effectful public session operation. #[inline] - fn pin_operation(&self, kind: SessionOperationKind) -> LifecycleResult { + fn pin_operation( + &self, + kind: SessionOperationKind, + ) -> LifecycleOrFatalResult { if self.closed.get() { return Err(Report::new(LifecycleError::SessionUnavailable) - .attach(format!("session_id={}", self.id))); + .attach(format!("session_id={}", self.id)) + .into()); } let admitted = self .session @@ -815,13 +824,12 @@ impl Session { .runtime() .poisoner .ensure_healthy() - .change_context(LifecycleError::RuntimeUnavailable) - .attach_with(|| { - format!( + .map_err(|error| { + LifecycleOrFatalError::from(error.attach(format!( "session_id={}, kind={}, phase=check_engine_health", self.id, kind.label() - ) + ))) })?; let (entry, authority) = admitted .runtime() @@ -909,7 +917,6 @@ impl Session { .runtime() .poisoner .ensure_healthy() - .change_context(LifecycleError::RuntimeUnavailable) .attach_with(|| format!("session_id={}, phase=check_engine_health", self.id)) .disclose()?; let trx = admitted @@ -943,7 +950,6 @@ impl Session { .runtime() .poisoner .ensure_healthy() - .change_context(LifecycleError::RuntimeUnavailable) .attach_with(|| { format!( "operation=close_session, session_id={}, phase=check_engine_health", @@ -992,7 +998,12 @@ impl Session { .attach("operation=create_table") .disclose()?; drop(mandatory_runtime); - observer.wait().await + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::CatalogAccess)) + .attach("operation=create_table, phase=wait_mandatory_completion") + .disclose() } /// Build and publish a new secondary index for an existing user table. @@ -1030,14 +1041,23 @@ impl Session { .await .attach("operation=create_index") .disclose()?; - let plan = CreateIndexPlan::new(table_id, table, index_spec)?; + let plan = CreateIndexPlan::new(table_id, table, index_spec).disclose()?; let observer = mandatory_runtime .submit(PreparedCreateIndex::new(gates, scope, plan)) .await .attach("operation=create_index") .disclose()?; drop(mandatory_runtime); - observer.wait().await + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::IndexAccess)) + .attach_with(|| { + format!( + "operation=create_index, phase=wait_mandatory_completion, table_id={table_id}" + ) + }) + .disclose() } /// Logically drop an active secondary index from an existing user table. @@ -1067,14 +1087,23 @@ impl Session { .await .attach("operation=drop_index") .disclose()?; - let plan = DropIndexPlan::new(table_id, table, index_no)?; + let plan = DropIndexPlan::new(table_id, table, index_no).disclose()?; let observer = mandatory_runtime .submit(PreparedDropIndex::new(gates, scope, plan)) .await .attach("operation=drop_index") .disclose()?; drop(mandatory_runtime); - observer.wait().await + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::IndexAccess)) + .attach_with(|| { + format!( + "operation=drop_index, phase=wait_mandatory_completion, table_id={table_id}, index_no={index_no}" + ) + }) + .disclose() } /// Logically drop an existing user table. @@ -1097,7 +1126,16 @@ impl Session { .attach("operation=drop_table") .disclose()?; drop(mandatory_runtime); - observer.wait().await + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::CatalogAccess)) + .attach_with(|| { + format!( + "operation=drop_table, phase=wait_mandatory_completion, table_id={table_id}" + ) + }) + .disclose() } /// Run one online catalog checkpoint. @@ -1125,7 +1163,13 @@ impl Session { .attach("operation=checkpoint_catalog") .disclose()?; drop(mandatory_runtime); - observer.wait().await.map(|_| ()) + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::CatalogAccess)) + .attach("operation=checkpoint_catalog, phase=wait_mandatory_completion") + .map(|_| ()) + .disclose() } /// Run catalog checkpoint and redo-log truncation as one maintenance operation. @@ -1154,7 +1198,14 @@ impl Session { .attach("operation=checkpoint_catalog_and_truncate_redo_log") .disclose()?; drop(mandatory_runtime); - observer.wait().await + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::CatalogAccess)) + .attach( + "operation=checkpoint_catalog_and_truncate_redo_log, phase=wait_mandatory_completion", + ) + .disclose() } /// Physically remove recovery-obsolete sealed redo prefix files. @@ -1182,7 +1233,12 @@ impl Session { .attach("operation=truncate_redo_log") .disclose()?; drop(mandatory_runtime); - observer.wait().await + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::RedoLogAccess)) + .attach("operation=truncate_redo_log, phase=wait_mandatory_completion") + .disclose() } /// Return a monotonic transaction-system statistics snapshot. @@ -1320,7 +1376,16 @@ impl Session { .attach_with(|| format!("operation=freeze_table, table_id={table_id}")) .disclose()?; drop(mandatory_runtime); - observer.wait().await + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::TableAccess)) + .attach_with(|| { + format!( + "operation=freeze_table, phase=wait_mandatory_completion, table_id={table_id}, max_rows={max_rows}" + ) + }) + .disclose() } /// Persist eligible state using the table-owned canonical frozen batch. @@ -1351,7 +1416,16 @@ impl Session { .attach_with(|| format!("operation=checkpoint_table, table_id={table_id}")) .disclose()?; drop(mandatory_runtime); - observer.wait().await + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::CheckpointExecution)) + .attach_with(|| { + format!( + "operation=checkpoint_table, phase=wait_mandatory_completion, table_id={table_id}" + ) + }) + .disclose() } /// Wait until retry may be useful for one self-identifying checkpoint delay. @@ -1417,7 +1491,9 @@ impl Session { .pin_observer() .attach("operation=wait_for_gc_horizon") .disclose()?; - wait_for_maintenance_boundary(&session, ts, MaintenanceBoundary::GcHorizon).await + wait_for_maintenance_boundary(&session, ts, MaintenanceBoundary::GcHorizon) + .await + .disclose() } /// Wait for completed purge-horizon-cycle progress to become strictly newer. @@ -1429,7 +1505,9 @@ impl Session { .pin_observer() .attach("operation=wait_for_purge_completion") .disclose()?; - wait_for_maintenance_boundary(&session, ts, MaintenanceBoundary::PurgeCompletion).await + wait_for_maintenance_boundary(&session, ts, MaintenanceBoundary::PurgeCompletion) + .await + .disclose() } /// Returns total number of hot row pages for an existing user table. @@ -1490,7 +1568,16 @@ impl Session { .attach_with(|| format!("operation=cleanup_secondary_mem_indexes, table_id={table_id}")) .disclose()?; drop(mandatory_runtime); - observer.wait().await + observer + .wait() + .await + .map_err(|error| error.into_quad(RuntimeError::IndexAccess)) + .attach_with(|| { + format!( + "operation=cleanup_secondary_mem_indexes, phase=wait_mandatory_completion, table_id={table_id}, clean_live_entries={clean_live_entries}" + ) + }) + .disclose() } /// Acquires an explicit session-lifetime table lock. @@ -3197,17 +3284,21 @@ async fn wait_for_maintenance_boundary( session: &SessionObserverPin, ts: TrxID, boundary: MaintenanceBoundary, -) -> Result { +) -> LifecycleOrFatalResult { let trx_sys = &session.runtime.trx_sys; loop { - session.runtime.poisoner.ensure_healthy().disclose()?; + session + .runtime + .poisoner + .ensure_healthy() + .map_err(LifecycleOrFatalError::from)?; if session.runtime.state().admission.shutdown_started() { return Err(Report::new(LifecycleError::Shutdown) .attach(format!( "maintenance progress wait observed engine shutdown: boundary={}, target_ts={ts}", boundary.name() )) - .disclose()); + .into()); } let observed = boundary.observed(session); if observed > ts { @@ -3219,14 +3310,18 @@ async fn wait_for_maintenance_boundary( let poison_listener = session.runtime.poisoner.listener(); let shutdown_listener = session.runtime.state().admission.shutdown_listener(); - session.runtime.poisoner.ensure_healthy().disclose()?; + session + .runtime + .poisoner + .ensure_healthy() + .map_err(LifecycleOrFatalError::from)?; if session.runtime.state().admission.shutdown_started() { return Err(Report::new(LifecycleError::Shutdown) .attach(format!( "maintenance progress wait observed engine shutdown: boundary={}, target_ts={ts}", boundary.name() )) - .disclose()); + .into()); } let observed = boundary.observed(session); if observed > ts { @@ -3485,12 +3580,9 @@ pub(crate) mod tests { } #[inline] - fn assert_runtime_unavailable_after_fatal(err: Error, fatal: FatalError) { - assert_eq!(err.kind(), ErrorKind::Lifecycle); - assert_eq!( - err.report().downcast_ref::().copied(), - Some(LifecycleError::RuntimeUnavailable) - ); + fn assert_fatal_admission_error(err: Error, fatal: FatalError) { + assert_eq!(err.kind(), ErrorKind::Fatal); + assert!(err.report().downcast_ref::().is_none()); assert_eq!( err.report().downcast_ref::().copied(), Some(fatal) @@ -4109,6 +4201,9 @@ pub(crate) mod tests { Ok(_) => panic!("closed session must reject new observers"), Err(err) => err, }; + let LifecycleOrFatalError::Lifecycle(err) = err else { + panic!("closed session must remain a Lifecycle rejection") + }; assert_eq!(err.current_context(), &LifecycleError::SessionUnavailable); drop(observer); @@ -4253,7 +4348,9 @@ pub(crate) mod tests { let observer = scope.spawn(move || { observer_barrier.wait(); if inspection { - session.pin_inspection() + session + .pin_inspection() + .map_err(LifecycleOrFatalError::from) } else { session.pin_observer() } @@ -4262,6 +4359,9 @@ pub(crate) mod tests { match (shutdown.join().unwrap(), observer.join().unwrap()) { (Ok(()), Err(err)) => { + let LifecycleOrFatalError::Lifecycle(err) = err else { + panic!("shutdown admission must remain Lifecycle") + }; assert_eq!(err.current_context(), &LifecycleError::Shutdown); } (Err(err), Ok(observer)) => { @@ -5713,6 +5813,13 @@ pub(crate) mod tests { err.report().downcast_ref::().copied(), Some(FatalError::CheckpointWrite) ); + let report = format!("{err:?}"); + assert!( + report.contains( + "operation=checkpoint_catalog_and_truncate_redo_log, phase=wait_mandatory_completion" + ), + "{report}" + ); assert!(publish_hook.call_count() > 0); let after = engine.inner().core.catalog().storage.checkpoint_snapshot(); assert_eq!( @@ -6602,7 +6709,7 @@ pub(crate) mod tests { Ok(_) => panic!("normal observer admission must reject storage poison"), Err(err) => err, }; - assert_runtime_unavailable_after_fatal(err, FatalError::RedoWrite); + assert_fatal_admission_error(err, FatalError::RedoWrite); assert_eq!(session.list_table_ids().unwrap(), vec![table_id]); assert!(session.transaction_system_stats().is_ok()); @@ -6612,10 +6719,10 @@ pub(crate) mod tests { assert!(session.logical_lock_stats().is_ok()); let err = session.truncate_redo_log().await.unwrap_err(); - assert_runtime_unavailable_after_fatal(err, FatalError::RedoWrite); + assert_fatal_admission_error(err, FatalError::RedoWrite); let err = session.checkpoint_catalog().await.unwrap_err(); - assert_runtime_unavailable_after_fatal(err, FatalError::RedoWrite); + assert_fatal_admission_error(err, FatalError::RedoWrite); }); } } diff --git a/doradb-storage/src/table/access.rs b/doradb-storage/src/table/access.rs index c43aa3b6..c1ef7f7d 100644 --- a/doradb-storage/src/table/access.rs +++ b/doradb-storage/src/table/access.rs @@ -8,8 +8,8 @@ use crate::catalog::{TableColumnLayout, TableMetadata}; use crate::error::{ DataIntegrityError, DataIntegrityResult, DiscloseError, DiscloseResultExt, FatalResult, InternalError, MultiDomainResultExt, OperationError, OperationOrFatalResult, - OperationOrRuntimeError, OperationOrRuntimeResult, Result, RuntimeError, RuntimeOrFatalResult, - RuntimeResult, + OperationOrRuntimeError, OperationOrRuntimeResult, OperationResult, QuadResult, Result, + RuntimeError, RuntimeOrFatalResult, RuntimeResult, }; use crate::file::FileKind; use crate::file::cow_file::SUPER_BLOCK_ID; @@ -3345,6 +3345,9 @@ impl<'op> UserTableAccessor<'op> { } /// Mutate callback-selected rows from one original latest-read worklist. + /// + /// This public-result contract is retained solely to transport an arbitrary + /// public error returned by the caller-owned mutation callback. pub(crate) async fn table_mutate_mvcc( &self, rt: TrxRuntime<'_>, @@ -3394,6 +3397,9 @@ impl<'op> UserTableAccessor<'op> { } /// Mutate callback-selected rows from the statement's persisted region. + /// + /// This helper retains public `Result` only while merging typed storage + /// failures with an arbitrary public error from the mutation callback. async fn mutate_cold_rows_mvcc( &self, rt: TrxRuntime<'_>, @@ -3510,7 +3516,8 @@ impl<'op> UserTableAccessor<'op> { } RowMutation::Update(update) => { state.outcome.update_count += 1; - self.validate_table_mutation_update(validator, &update)?; + self.validate_table_mutation_update(validator, &update) + .disclose()?; if update.is_empty() { state.value_buffer = lazy_row.into_reusable_buffer(); } else { @@ -3541,7 +3548,8 @@ impl<'op> UserTableAccessor<'op> { index_keys, root_snapshot, ) - .await?; + .await + .disclose()?; } PendingColdMutation::Update { row_id, @@ -3557,7 +3565,8 @@ impl<'op> UserTableAccessor<'op> { update, root_snapshot, ) - .await?; + .await + .disclose()?; state.tracker.observe_insert(inserted); } } @@ -3574,6 +3583,9 @@ impl<'op> UserTableAccessor<'op> { } /// Mutate callback-selected rows from the statement's original hot pages. + /// + /// This helper retains public `Result` only while merging typed storage + /// failures with an arbitrary public error from the mutation callback. async fn mutate_hot_rows_mvcc( &self, rt: TrxRuntime<'_>, @@ -3660,11 +3672,13 @@ impl<'op> UserTableAccessor<'op> { index_keys, root_snapshot, ) - .await?; + .await + .disclose()?; } RowMutation::Update(update) => { state.outcome.update_count += 1; - self.validate_table_mutation_update(validator, &update)?; + self.validate_table_mutation_update(validator, &update) + .disclose()?; state.value_buffer = lazy_row.into_reusable_buffer(); if update.is_empty() { continue; @@ -3680,7 +3694,8 @@ impl<'op> UserTableAccessor<'op> { update, root_snapshot, ) - .await?; + .await + .disclose()?; if let Some(inserted) = inserted { state.tracker.observe_insert(inserted); } @@ -3696,15 +3711,14 @@ impl<'op> UserTableAccessor<'op> { &self, validator: Option<&DmlValidator<'_>>, update: &[UpdateCol], - ) -> Result<()> { + ) -> OperationResult<()> { if let Some(validator) = validator { validator .validate_sparse_update(update) .change_context(OperationError::InvalidDmlInput) .attach_with(|| { format!("operation=table_mutate_mvcc, table_id={}", self.table_id()) - }) - .disclose()?; + })?; } Ok(()) } @@ -3772,15 +3786,14 @@ impl<'op> UserTableAccessor<'op> { row_id: RowID, index_keys: WriteIndexKeySet<'op>, root_snapshot: &TableRootSnapshot<'_>, - ) -> Result<()> { + ) -> QuadResult<()> { self.claim_known_cold_row(rt, row_id) .await - .attach("full-table mutation cold delete marker ownership") - .disclose()?; + .attach("full-table mutation cold delete marker ownership")?; self.install_cold_delete_effects(rt, effects, row_id, index_keys, root_snapshot) .await - .attach("full-table mutation cold delete index masking") - .disclose() + .attach("full-table mutation cold delete index masking")?; + Ok(()) } #[inline] @@ -3792,22 +3805,19 @@ impl<'op> UserTableAccessor<'op> { old_row: Vec, update: Vec, root_snapshot: &TableRootSnapshot<'_>, - ) -> Result { + ) -> QuadResult { self.claim_known_cold_row(rt, row_id) .await - .attach("full-table mutation cold update marker ownership") - .disclose()?; + .attach("full-table mutation cold update marker ownership")?; let old_index_keys = WriteIndexKeySet::from_full_row(self, &old_row); self.install_cold_delete_effects(rt, effects, row_id, old_index_keys, root_snapshot) .await - .attach("full-table mutation cold update delete effects") - .disclose()?; + .attach("full-table mutation cold update delete effects")?; let new_row = self.build_cold_update_row(old_row, RowUpdateInput::Sparse(update)); let new_index_keys = WriteIndexKeySet::from_full_row(self, &new_row); let (new_row_id, new_guard) = self .insert_row_internal(rt, effects, new_row, RowUndoKind::Insert, Vec::new()) - .await - .disclose()?; + .await?; self.insert_index_set( rt, effects, @@ -3817,8 +3827,7 @@ impl<'op> UserTableAccessor<'op> { root_snapshot, ) .await - .attach("full-table mutation cold replacement index claim") - .disclose()?; + .attach("full-table mutation cold replacement index claim")?; let inserted = InsertedRow::new(new_guard.page_id(), new_row_id); Ok(inserted) } @@ -3832,11 +3841,10 @@ impl<'op> UserTableAccessor<'op> { row_id: RowID, index_keys: WriteIndexKeySet<'op>, root_snapshot: &TableRootSnapshot<'_>, - ) -> Result<()> { + ) -> QuadResult<()> { let result = HotRowMutator::new(self.table_id(), self.metadata(), rt, &page_guard, row_id) .delete_known_row(effects) - .await - .disclose()?; + .await?; match result { DeleteInternal::Ok => { let proof = self.owned_row_page_index_set_proof(row_id, index_keys, root_snapshot); @@ -3845,12 +3853,12 @@ impl<'op> UserTableAccessor<'op> { drop(page_guard); self.defer_delete_owned_row_index_set(rt, effects, proof) .await - .attach("full-table mutation hot delete index masking") - .disclose() + .attach("full-table mutation hot delete index masking")?; + Ok(()) } DeleteInternal::NotFound => Err(Report::new(OperationError::WriteConflict) .attach("full-table mutation hot row changed after visibility") - .disclose()), + .into()), DeleteInternal::RetryInTransition => { unreachable!( "full-table mutation observed TRANSITION while holding TableData(X): table_id={}, row_id={row_id}", @@ -3869,11 +3877,10 @@ impl<'op> UserTableAccessor<'op> { row_id: RowID, update: Vec, root_snapshot: &TableRootSnapshot<'_>, - ) -> Result> { + ) -> QuadResult> { let result = HotRowMutator::new(self.table_id(), self.metadata(), rt, &page_guard, row_id) .update_known_row(effects, RowUpdateInput::Sparse(update)) - .await - .disclose()?; + .await?; match result { UpdateRowInplace::Ok(new_row_id, index_change_cols) => { debug_assert_eq!(row_id, new_row_id); @@ -3887,15 +3894,14 @@ impl<'op> UserTableAccessor<'op> { root_snapshot, ) .await - .attach("full-table mutation hot key change") - .disclose()?; + .attach("full-table mutation hot key change")?; } Ok(None) } UpdateRowInplace::RowDeleted(_) | UpdateRowInplace::RowNotFound(_) => { Err(Report::new(OperationError::WriteConflict) .attach("full-table mutation hot row changed after visibility") - .disclose()) + .into()) } UpdateRowInplace::RetryInTransition(_) => { // Checkpoint transition holds TableData(IS), which is @@ -3912,8 +3918,7 @@ impl<'op> UserTableAccessor<'op> { let old_index_keys = WriteIndexKeySet::from_full_row(self, &old_row); let (new_row_id, index_change_cols, new_guard) = self .move_update_for_space(rt, effects, old_row, update, old_row_id, page_guard) - .await - .disclose()?; + .await?; let proof = self.owned_row_page_index_set_proof(old_row_id, old_index_keys, root_snapshot); let result = if index_change_cols.is_empty() { @@ -3934,9 +3939,7 @@ impl<'op> UserTableAccessor<'op> { .await }; let inserted = InsertedRow::new(new_guard.page_id(), new_row_id); - result - .attach("full-table mutation hot move index update") - .disclose()?; + result.attach("full-table mutation hot move index update")?; Ok(Some(inserted)) } } @@ -4110,7 +4113,7 @@ impl<'op> UserTableAccessor<'op> { rt: TrxRuntime<'_>, effects: &mut StmtEffects, cols: Vec, - ) -> Result { + ) -> QuadResult { let metadata = self.metadata(); debug_assert!(cols.len() == metadata.col.col_count()); debug_assert!({ @@ -4125,8 +4128,7 @@ impl<'op> UserTableAccessor<'op> { // handle if any following index insert fails. let (row_id, page_guard) = self .insert_row_internal(rt, effects, cols, RowUndoKind::Insert, Vec::new()) - .await - .disclose()?; + .await?; // This foreground method is a genuine mixed seam: row allocation above // can already contribute Runtime-or-Fatal, while index claims contribute // Operation-or-Runtime. Convert each native carrier only here. @@ -4135,8 +4137,7 @@ impl<'op> UserTableAccessor<'op> { // needed for MVCC visibility. self.insert_index_set(rt, effects, keys, row_id, &page_guard, &root_snapshot) .await - .attach("insert MVCC secondary index claim") - .disclose()?; + .attach("insert MVCC secondary index claim")?; Ok(row_id) } @@ -4148,7 +4149,7 @@ impl<'op> UserTableAccessor<'op> { unique_index_no: usize, cols: Vec, log_by_key: bool, - ) -> Result { + ) -> QuadResult { let key = unique_key_from_full_row( self.metadata(), unique_index_no, @@ -4181,7 +4182,7 @@ impl<'op> UserTableAccessor<'op> { key_vals: &[Val], update: Vec, log_by_key: bool, - ) -> Result { + ) -> QuadResult { let input = RowUpdateInput::Sparse(update); match self .update_unique_mvcc_input(rt, effects, index_no, key_vals, input, log_by_key) @@ -4201,7 +4202,7 @@ impl<'op> UserTableAccessor<'op> { key_vals: &[Val], mut input: RowUpdateInput, log_by_key: bool, - ) -> Result { + ) -> QuadResult { debug_assert!(index_no < self.sec_idx_len()); debug_assert!( self.metadata() @@ -4222,11 +4223,10 @@ impl<'op> UserTableAccessor<'op> { 'retry: loop { let attempt = 'attempt: { let root_snapshot = self.root_snapshot(rt.ctx()); - let handle = self - .snapshot_index_read_handle(rt.pool_guards(), &root_snapshot, index_no) - .disclose()?; - let index = handle.bind_unique().disclose()?; - match index.lookup(key_vals, rt.sts()).await.disclose()? { + let handle = + self.snapshot_index_read_handle(rt.pool_guards(), &root_snapshot, index_no)?; + let index = handle.bind_unique()?; + match index.lookup(key_vals, rt.sts()).await? { None => return Ok(UpdateUniqueMvcc::NotFound(input)), Some((row_id, _)) => match self .find_row_location(rt.pool_guards(), row_id) @@ -4255,8 +4255,7 @@ impl<'op> UserTableAccessor<'op> { row_shape_fingerprint, |vals| metadata.idx.match_key(index_no, key_vals, vals), ) - .await - .disclose()? + .await? { ColdRowUpdateRead::Ok(vals) => vals, ColdRowUpdateRead::NotFound => { @@ -4265,7 +4264,7 @@ impl<'op> UserTableAccessor<'op> { ColdRowUpdateRead::WriteConflict => { return Err(Report::new(OperationError::WriteConflict) .attach("update MVCC cold row read") - .disclose()); + .into()); } ColdRowUpdateRead::Preparing(listener) => { break 'attempt PointMutationAttempt::Preparing(listener); @@ -4289,7 +4288,7 @@ impl<'op> UserTableAccessor<'op> { Err(DeletionError::WriteConflict) => { return Err(Report::new(OperationError::WriteConflict) .attach("update MVCC cold delete marker ownership") - .disclose()); + .into()); } Err(DeletionError::AlreadyDeleted) => { return Ok(UpdateUniqueMvcc::NotFound(input)); @@ -4303,8 +4302,7 @@ impl<'op> UserTableAccessor<'op> { old_index_keys, &root_snapshot, ) - .await - .disclose()?; + .await?; let new_row = self.build_cold_update_row(old_vals, input); let new_index_keys = WriteIndexKeySet::from_full_row(self, &new_row); @@ -4316,8 +4314,7 @@ impl<'op> UserTableAccessor<'op> { RowUndoKind::Insert, Vec::new(), ) - .await - .disclose()?; + .await?; // Row allocation can already contribute Runtime-or-Fatal; // keep index mutation typed until this mixed seam. self.insert_index_set( @@ -4329,8 +4326,7 @@ impl<'op> UserTableAccessor<'op> { &root_snapshot, ) .await - .attach("update MVCC cold replacement index claim") - .disclose()?; + .attach("update MVCC cold replacement index claim")?; return Ok(UpdateUniqueMvcc::Updated(new_row_id)); } Ok(RowLocation::RowPage(page_id)) => { @@ -4344,8 +4340,7 @@ impl<'op> UserTableAccessor<'op> { page_id, row_id, ) - .await - .disclose()? + .await? else { continue 'retry; }; @@ -4355,7 +4350,7 @@ impl<'op> UserTableAccessor<'op> { row_id, } } - Err(err) => return Err(err.disclose()), + Err(err) => return Err(err.into()), }, } }; @@ -4366,14 +4361,13 @@ impl<'op> UserTableAccessor<'op> { row_id, } => (root_snapshot, page_guard, row_id), PointMutationAttempt::Preparing(listener) => { - self.wait_prepare_retry(rt, listener).await.disclose()?; + self.wait_prepare_retry(rt, listener).await?; continue; } }; let res = HotRowMutator::new(self.table_id(), self.metadata(), rt, &page_guard, row_id) .update_inplace(effects, index_no, key_vals, input, log_by_key) - .await - .disclose()?; + .await?; match res { UpdateRowInplace::Ok(new_row_id, index_change_cols) => { debug_assert!(row_id == new_row_id); @@ -4390,8 +4384,7 @@ impl<'op> UserTableAccessor<'op> { &root_snapshot, ) .await - .attach("update MVCC key-change index update") - .disclose()?; + .attach("update MVCC key-change index update")?; return Ok(UpdateUniqueMvcc::Updated(new_row_id)); } // otherwise, do nothing return Ok(UpdateUniqueMvcc::Updated(row_id)); @@ -4403,9 +4396,7 @@ impl<'op> UserTableAccessor<'op> { input = returned_input; // Release the row page so the checkpoint transition can complete. drop(page_guard); - self.wait_transition_route_or_poison(rt, row_id) - .await - .disclose()?; + self.wait_transition_route_or_poison(rt, row_id).await?; } UpdateRowInplace::NoFreeSpaceOrFrozen(old_row_id, old_row, returned_input) => { // In-place update failed after the old row was locked and @@ -4424,8 +4415,7 @@ impl<'op> UserTableAccessor<'op> { old_row_id, page_guard, ) - .await - .disclose()?; + .await?; let proof = self.owned_row_page_index_set_proof( old_row_id, old_index_keys, @@ -4442,16 +4432,14 @@ impl<'op> UserTableAccessor<'op> { proof, ) .await - .attach("update MVCC moved-row index update") - .disclose()?; + .attach("update MVCC moved-row index update")?; return Ok(UpdateUniqueMvcc::Updated(new_row_id)); } else { self.update_indexes_only_row_id_change( rt, effects, old_row_id, new_row_id, proof, ) .await - .attach("update MVCC moved-row index update") - .disclose()?; + .attach("update MVCC moved-row index update")?; return Ok(UpdateUniqueMvcc::Updated(new_row_id)); } } @@ -4466,7 +4454,7 @@ impl<'op> UserTableAccessor<'op> { effects: &mut StmtEffects, index_no: usize, key_vals: &[Val], - ) -> Result { + ) -> QuadResult { debug_assert!(index_no < self.sec_idx_len()); debug_assert!( self.metadata() @@ -4483,11 +4471,10 @@ impl<'op> UserTableAccessor<'op> { 'retry: loop { let attempt = 'attempt: { let root_snapshot = self.root_snapshot(rt.ctx()); - let handle = self - .snapshot_index_read_handle(rt.pool_guards(), &root_snapshot, index_no) - .disclose()?; - let index = handle.bind_unique().disclose()?; - match index.lookup(key_vals, rt.sts()).await.disclose()? { + let handle = + self.snapshot_index_read_handle(rt.pool_guards(), &root_snapshot, index_no)?; + let index = handle.bind_unique()?; + match index.lookup(key_vals, rt.sts()).await? { None => return Ok(DeleteMvcc::NotFound), Some((row_id, _)) => { match self.find_row_location(rt.pool_guards(), row_id).await { @@ -4509,8 +4496,7 @@ impl<'op> UserTableAccessor<'op> { row_idx, row_shape_fingerprint, ) - .await - .disclose()?; + .await?; if !index_key_matches(index_keys.as_slice(), index_no, key_vals) { return Ok(DeleteMvcc::NotFound); } @@ -4535,8 +4521,7 @@ impl<'op> UserTableAccessor<'op> { index_keys, &root_snapshot, ) - .await - .disclose()?; + .await?; return Ok(DeleteMvcc::Deleted); } Ok(DeletionClaim::Preparing(listener)) => { @@ -4545,7 +4530,7 @@ impl<'op> UserTableAccessor<'op> { Err(DeletionError::WriteConflict) => { return Err(Report::new(OperationError::WriteConflict) .attach("delete MVCC cold delete marker ownership") - .disclose()); + .into()); } Err(DeletionError::AlreadyDeleted) => { return Ok(DeleteMvcc::NotFound); @@ -4563,8 +4548,7 @@ impl<'op> UserTableAccessor<'op> { page_id, row_id, ) - .await - .disclose()? + .await? else { continue 'retry; }; @@ -4574,7 +4558,7 @@ impl<'op> UserTableAccessor<'op> { row_id, } } - Err(err) => return Err(err.disclose()), + Err(err) => return Err(err.into()), } } } @@ -4586,22 +4570,19 @@ impl<'op> UserTableAccessor<'op> { row_id, } => (root_snapshot, page_guard, row_id), PointMutationAttempt::Preparing(listener) => { - self.wait_prepare_retry(rt, listener).await.disclose()?; + self.wait_prepare_retry(rt, listener).await?; continue; } }; let res = HotRowMutator::new(self.table_id(), self.metadata(), rt, &page_guard, row_id) .delete(effects, index_no, key_vals, false) - .await - .disclose()?; + .await?; match res { DeleteInternal::NotFound => return Ok(DeleteMvcc::NotFound), DeleteInternal::RetryInTransition => { // Release the row page so the checkpoint transition can complete. drop(page_guard); - self.wait_transition_route_or_poison(rt, row_id) - .await - .disclose()?; + self.wait_transition_route_or_poison(rt, row_id).await?; } DeleteInternal::Ok => { // Successful row undo ownership excludes another writer, @@ -4615,8 +4596,7 @@ impl<'op> UserTableAccessor<'op> { // Physical index entries remain until rollback unmasks // them or index GC removes them after they are invisible. self.defer_delete_owned_row_index_set(rt, effects, proof) - .await - .disclose()?; + .await?; return Ok(DeleteMvcc::Deleted); } } diff --git a/doradb-storage/src/trx/mod.rs b/doradb-storage/src/trx/mod.rs index b9afa74a..42166083 100644 --- a/doradb-storage/src/trx/mod.rs +++ b/doradb-storage/src/trx/mod.rs @@ -42,7 +42,8 @@ use crate::completion::Completion; use crate::engine::EngineCore; use crate::error::{ CompletionErrorBridge, DiscloseError, DiscloseResultExt, Error, FatalError, FatalResult, - LifecycleError, LifecycleResult, OperationResult, ResourceError, Result, RuntimeError, + LifecycleError, LifecycleOrFatalError, LifecycleOrFatalResult, LifecycleResult, + MultiDomainResultExt, OperationResult, ResourceError, Result, RuntimeError, RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, SharedFatalError, }; use crate::id::{SessionID, SessionOperationKey, TableID, TrxID}; @@ -141,7 +142,7 @@ impl Transaction { /// Check out the mutable core for one crate-internal operation under admission. #[inline] - pub(crate) fn checkout(&mut self) -> LifecycleResult { + pub(crate) fn checkout(&mut self) -> LifecycleOrFatalResult { let admitted = self.session.upgrade().attach_with(|| { format!( "operation_key={}, trx_id={}", @@ -158,12 +159,11 @@ impl Transaction { .runtime() .poisoner .ensure_healthy() - .change_context(LifecycleError::RuntimeUnavailable) - .attach_with(|| { - format!( + .map_err(|error| { + LifecycleOrFatalError::from(error.attach(format!( "operation_key={}, trx_id={}, phase=check_engine_health", self.operation_key, self.trx_id - ) + ))) })?; let entry = admitted .runtime() @@ -302,7 +302,7 @@ impl Transaction { .attach("operation=commit_active_transaction") .disclose()?; let trx_sys = claim.engine().trx_sys.clone(); - trx_sys.commit_transaction(claim).await + trx_sys.commit_transaction(claim).await.disclose() } /// Rollback the transaction. @@ -2173,11 +2173,11 @@ impl FailedPrecommitReason { match self { FailedPrecommitReason::Fatal(error) => RuntimeOrFatalError::from(error), FailedPrecommitReason::Resource(reason) => RuntimeOrFatalError::from( - Report::new(reason).change_context(RuntimeError::SystemTransactionCommit), + Report::new(reason).change_context(RuntimeError::TransactionCommit), ), FailedPrecommitReason::Shutdown => RuntimeOrFatalError::from( Report::new(LifecycleError::Shutdown) - .change_context(RuntimeError::SystemTransactionCommit), + .change_context(RuntimeError::TransactionCommit), ), } } @@ -3537,7 +3537,7 @@ pub(crate) mod tests { pub(crate) fn install_transaction_ddl_redo( trx: &mut Transaction, ddl: DDLRedo, - ) -> LifecycleResult<()> { + ) -> LifecycleOrFatalResult<()> { let mut checkout = trx.checkout()?; checkout.inner_mut().effects_mut().install_ddl_redo(ddl); Ok(()) @@ -4191,6 +4191,9 @@ pub(crate) mod tests { Ok(_) => panic!("wrong transaction id must not claim the exact operation entry"), Err(err) => err, }; + let LifecycleOrFatalError::Lifecycle(err) = err else { + panic!("discarded transaction must remain a Lifecycle rejection") + }; assert_eq!( err.downcast_ref::().copied(), Some(LifecycleError::TransactionDiscarded) @@ -6506,6 +6509,34 @@ pub(crate) mod tests { }); } + #[test] + fn test_transaction_checkout_preserves_fatal_poison() { + smol::block_on(async { + let (_temp_dir, engine) = test_engine("transaction_checkout_fatal_poison").await; + let mut session = engine.new_session().unwrap(); + let mut trx = session.begin_trx().unwrap(); + let _ = engine + .inner() + .poisoner + .poison(Report::new(FatalError::RedoWrite).attach("test checkout poison")); + + let error = match trx.checkout() { + Ok(_) => panic!("poisoned transaction checkout must be rejected"), + Err(error) => error, + }; + let LifecycleOrFatalError::Fatal(error) = error else { + panic!("poisoned transaction checkout must remain Fatal") + }; + assert_eq!( + error.downcast_ref::().copied(), + Some(FatalError::RedoWrite) + ); + assert!(error.downcast_ref::().is_none()); + + trx.rollback().await.unwrap(); + }); + } + #[test] fn test_transaction_effect_predicates_split_durability_from_ordering() { smol::block_on(async { diff --git a/doradb-storage/src/trx/stmt.rs b/doradb-storage/src/trx/stmt.rs index 1809ac96..ca90228c 100644 --- a/doradb-storage/src/trx/stmt.rs +++ b/doradb-storage/src/trx/stmt.rs @@ -654,6 +654,7 @@ impl<'stmt> Statement<'stmt> { .accessor_with_layout(&layout) .insert_mvcc(rt, effects, cols) .await + .disclose() } /// Inserts or replaces one catalog-owned user-table row by table id and unique key. @@ -699,6 +700,7 @@ impl<'stmt> Statement<'stmt> { .accessor_with_layout(&layout) .upsert_unique_mvcc(rt, effects, unique_index_no, cols, false) .await + .disclose() } /// Updates one catalog-owned user-table row by table id and unique key. @@ -743,6 +745,7 @@ impl<'stmt> Statement<'stmt> { .accessor_with_layout(&layout) .update_unique_mvcc(rt, effects, index_no, key_vals, update, false) .await + .disclose() } /// Deletes one catalog-owned user-table row by table id and unique key. @@ -780,6 +783,7 @@ impl<'stmt> Statement<'stmt> { .accessor_with_layout(&layout) .delete_unique_mvcc(rt, effects, index_no, key_vals) .await + .disclose() } /// Inserts one catalog-table row through the foreground lock-aware path. diff --git a/doradb-storage/src/trx/stream_stmt.rs b/doradb-storage/src/trx/stream_stmt.rs index f5b69540..a666651a 100644 --- a/doradb-storage/src/trx/stream_stmt.rs +++ b/doradb-storage/src/trx/stream_stmt.rs @@ -1,6 +1,7 @@ use crate::buffer::EvictableBufferPool; use crate::error::{ - DiscloseResultExt, OperationError, OperationOrFatalResult, Result, RuntimeResult, + DiscloseResultExt, MultiDomainResultExt, OperationError, OperationOrFatalResult, Result, + RuntimeResult, }; use crate::id::TableID; use crate::index::{ diff --git a/doradb-storage/src/trx/sys.rs b/doradb-storage/src/trx/sys.rs index 9037dfe3..0d9f9c36 100644 --- a/doradb-storage/src/trx/sys.rs +++ b/doradb-storage/src/trx/sys.rs @@ -6,11 +6,11 @@ use crate::component::{ Component, ComponentRegistry, EnginePools, FirstPanic, IndexPool, MemPool, MetaPool, ShelfScope, Supplier, panic_payload_description, }; -use crate::conf::TrxSysConfig; +use crate::conf::{TrxSysConfig, ValidatedTrxSysConfig}; use crate::error::{ - CompletionErrorBridge, DataIntegrityError, DataIntegrityResult, DiscloseError, - DiscloseResultExt, Error, FatalError, FatalResult, LifecycleResult, MultiDomainResultExt, - Result, RuntimeError, RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, + CompletionErrorBridge, DataIntegrityError, DataIntegrityResult, FatalError, FatalResult, + LifecycleResult, MultiDomainResultExt, QuadError, QuadResult, RuntimeError, + RuntimeOrFatalError, RuntimeOrFatalResult, RuntimeResult, }; use crate::file::fs::FileSystem; use crate::file::table_file::{MutableTableFile, OldRoot, TableFile}; @@ -580,6 +580,8 @@ pub(crate) struct TransactionSystem { pub(crate) redo_log: CachePadded, /// Transaction system configuration. pub(crate) config: CachePadded, + /// Bootstrap-validated redo file prefix reused by runtime maintenance. + file_prefix: CachePadded, /// Catalog of the database. pub(crate) catalog: QuiescentGuard, /// Table file facade used by background dropped-table cleanup. @@ -618,18 +620,15 @@ pub(crate) struct TransactionSystem { impl TransactionSystem { /// Recover durable state and bootstrap transaction-system startup resources. - /// - /// This is a genuine mixed startup owner: configuration, recovery IO, - /// persisted-data validation, runtime setup, and resource failures meet - /// here before engine construction exposes the public result. pub(crate) async fn bootstrap( - config: TrxSysConfig, + validated: ValidatedTrxSysConfig, poisoner: QuiescentGuard, mandatory_runtime: QuiescentGuard, pools: EnginePools, table_fs: QuiescentGuard, catalog: QuiescentGuard, - ) -> Result<(Self, PendingTransactionWorkerStartups)> { + ) -> RuntimeResult<(Self, PendingTransactionWorkerStartups)> { + let (config, file_prefix) = validated.into_parts(); debug_assert!(config.purge_threads != 0); debug_assert!( (1..=256).contains(&config.gc_buckets) && config.gc_buckets.is_power_of_two() @@ -640,20 +639,17 @@ impl TransactionSystem { let pool_guards = pools.pool_guards().clone(); let (purge_tx, purge_rx) = flume::unbounded(); - let file_prefix = config.file_prefix().disclose()?; let recovery_resources = RecoveryResources::new(pools, table_fs.clone(), &catalog); - let coordinator = recovery_resources - .prepare(&config, file_prefix) - .disclose()?; - let (max_recovered_cts, finalizer) = coordinator.recover_all().await.disclose()?; - let initial_trx_ts = recovery_initial_trx_ts(max_recovered_cts) - .change_context(RuntimeError::Recovery) - .disclose()?; - let (redo_log, initial_redo_header) = finalizer.finalize(purge_tx.clone()).disclose()?; + let coordinator = recovery_resources.prepare(&config, file_prefix.clone())?; + let (max_recovered_cts, finalizer) = coordinator.recover_all().await?; + let initial_trx_ts = + recovery_initial_trx_ts(max_recovered_cts).change_context(RuntimeError::Recovery)?; + let (redo_log, initial_redo_header) = finalizer.finalize(purge_tx.clone())?; let redo_log = CachePadded::new(redo_log); let trx_sys = Self::new( config, + file_prefix, poisoner, mandatory_runtime, catalog, @@ -687,6 +683,7 @@ impl TransactionSystem { #[inline] fn new( config: TrxSysConfig, + file_prefix: String, poisoner: QuiescentGuard, mandatory_runtime: QuiescentGuard, catalog: QuiescentGuard, @@ -713,6 +710,7 @@ impl TransactionSystem { gc_buckets: gc_buckets.into_boxed_slice(), redo_log, config: CachePadded::new(config), + file_prefix: CachePadded::new(file_prefix), catalog, table_fs, poisoner, @@ -961,20 +959,15 @@ impl TransactionSystem { } } - /// Enqueue a prepared transaction and wait for ordered commit completion. - /// - /// The shared user-commit completion has a closed but genuinely mixed - /// producer set: intrinsic Resource rejection, Lifecycle shutdown, or - /// Fatal redo/rollback failure. This helper intentionally retains the - /// public result rather than introducing a one-use sum carrier. + /// Enqueue a prepared user transaction and wait for ordered commit completion. #[inline] - pub(crate) async fn commit_prepared(&self, trx: PreparedTrx) -> Result { + pub(crate) async fn commit_prepared(&self, trx: PreparedTrx) -> QuadResult { let (cts, waiter) = self.enqueue_prepared_waiter(trx); - waiter.wait_result().await.map_err(|report| { - report - .disclose() - .attach_with(|| format!("wait for redo group commit: commit_ts={cts}")) - })?; + waiter + .wait_result() + .await + .map_err(|bridge| bridge.into_quad(RuntimeError::TransactionCommit)) + .attach_with(|| format!("wait for redo group commit: commit_ts={cts}"))?; assert!(TrxID::new(self.redo_log.persisted_cts.load(Ordering::Relaxed)) >= cts); Ok(cts) } @@ -1161,10 +1154,7 @@ impl TransactionSystem { /// Commit an active transaction. /// - /// This is the internal owner of the public user-commit boundary. Its - /// failures are limited to Resource, Lifecycle, and Fatal reports from - /// ordered completion or mandatory rollback, but remain on `Result` so the - /// completion source and commit-timestamp attachment are disclosed once. + /// This is the typed internal owner of the public user-commit boundary. /// The commit process is implemented as group commit. /// If multiple transactions are being committed at the same time, one of them /// will become leader of the commit group. Others become followers waiting for @@ -1174,13 +1164,13 @@ impl TransactionSystem { pub(crate) async fn commit_transaction( &self, claim: SessionOperationCompletionClaim, - ) -> Result { + ) -> QuadResult { if let Err(err) = self.poisoner.ensure_healthy() { let completion = self.enqueue_terminal_rollback(claim, "rollback poisoned commit"); Self::wait_terminal_rollback(completion, "wait for poisoned commit rollback cleanup") .await - .disclose()?; - return Err(err.disclose()); + .map_err(QuadError::from)?; + return Err(err.into()); } // Prepare redo log first, this may take some time, // so keep it out of lock scope, and we can fill cts after the lock is held. @@ -1671,11 +1661,7 @@ impl TransactionSystem { &self, ) -> RuntimeResult { Ok(CatalogCheckpointScanConfig { - file_prefix: self - .config - .file_prefix() - .change_context(RuntimeError::CatalogAccess) - .attach("operation=build_catalog_checkpoint_scan_config")?, + file_prefix: self.file_prefix.as_str().to_owned(), read_ahead_depth: self.config.catalog_checkpoint_scan_io_depth, }) } @@ -1690,22 +1676,19 @@ impl Supplier for TransactionSystem { } impl Component for TransactionSystem { - type Config = TrxSysConfig; + type Config = ValidatedTrxSysConfig; type Owned = Self; type Access = QuiescentGuard; - // Component construction combines config validation with genuinely mixed - // recovery/bootstrap sources, so this is the remaining startup-wide owner. - type Error = Error; + type Error = Report; const NAME: &'static str = "trx_sys"; #[inline] async fn build( - mut config: Self::Config, + config: Self::Config, registry: &mut ComponentRegistry, mut shelf: ShelfScope<'_, Self>, - ) -> Result<()> { - config.validate().disclose()?; + ) -> RuntimeResult<()> { let meta_pool = registry.dependency::(); let index_pool = registry.dependency::(); let mem_pool = registry.dependency::(); @@ -1885,8 +1868,10 @@ pub(crate) mod tests { redo_log: RedoLog, ) -> (TransactionSystem, Receiver) { let (purge_tx, purge_rx) = flume::unbounded(); + let file_prefix = config.file_prefix().unwrap(); let trx_sys = TransactionSystem::new( config, + file_prefix, engine.inner().poisoner.clone(), engine.inner().mandatory_runtime.clone(), engine.inner().catalog.clone(), @@ -2473,6 +2458,12 @@ pub(crate) mod tests { let err = trx.commit().await.unwrap_err(); + assert_eq!(err.kind(), crate::error::ErrorKind::Runtime); + assert_eq!( + err.report().downcast_ref::().copied(), + Some(RuntimeError::TransactionCommit), + "{err:?}" + ); assert_eq!( err.report().downcast_ref::().copied(), Some(ResourceError::StorageFileCapacityExceeded), From a794d8806622360cd0755bf99e2d292cf757c44b Mon Sep 17 00:00:00 2001 From: jiangzhe Date: Sat, 8 Aug 2026 12:57:21 +0800 Subject: [PATCH 2/2] resolve task --- docs/backlogs/000000-template.md | 17 +- ...00178-common-multi-domain-error-carrier.md | 17 +- ...or-and-narrow-audited-error-convergence.md | 788 ++++++------------ doradb-storage/src/session.rs | 56 +- doradb-storage/src/trx/stmt.rs | 4 + 5 files changed, 299 insertions(+), 583 deletions(-) rename docs/backlogs/{ => closed}/000178-common-multi-domain-error-carrier.md (86%) diff --git a/docs/backlogs/000000-template.md b/docs/backlogs/000000-template.md index bb28d3c1..a2b0dfa8 100644 --- a/docs/backlogs/000000-template.md +++ b/docs/backlogs/000000-template.md @@ -41,16 +41,7 @@ Briefly describe what outcome would indicate this item is done. Extra context that helps future task creation. -## Close Reason (Added When Closed) - -When a backlog item is moved to `docs/backlogs/closed/`, append: - -```md -## Close Reason - -- Type: -- Detail: -- Closed By: -- Reference: -- Closed At: -``` +Closure metadata is added only by `tools/backlog.rs close-doc`. It records the +resolution type, explanatory detail, closing actor, task/issue/PR reference, +and closure date. Do not add a Close Reason section to an open backlog; the +close command appends the populated section when archiving it. diff --git a/docs/backlogs/000178-common-multi-domain-error-carrier.md b/docs/backlogs/closed/000178-common-multi-domain-error-carrier.md similarity index 86% rename from docs/backlogs/000178-common-multi-domain-error-carrier.md rename to docs/backlogs/closed/000178-common-multi-domain-error-carrier.md index e8e52938..a86ee926 100644 --- a/docs/backlogs/000178-common-multi-domain-error-carrier.md +++ b/docs/backlogs/closed/000178-common-multi-domain-error-carrier.md @@ -29,17 +29,10 @@ A future task selects and implements a consistent carrier strategy; poison remai ## Notes (Optional) - -## Close Reason (Added When Closed) - -When a backlog item is moved to `docs/backlogs/closed/`, append: - -```md ## Close Reason -- Type: -- Detail: -- Closed By: -- Reference: -- Closed At: -``` +- Type: implemented +- Detail: Implemented via docs/tasks/000263-introduce-quad-error-and-narrow-audited-error-convergence.md +- Closed By: backlog close +- Reference: User decision +- Closed At: 2026-08-08 diff --git a/docs/tasks/000263-introduce-quad-error-and-narrow-audited-error-convergence.md b/docs/tasks/000263-introduce-quad-error-and-narrow-audited-error-convergence.md index b01c9656..67b175d7 100644 --- a/docs/tasks/000263-introduce-quad-error-and-narrow-audited-error-convergence.md +++ b/docs/tasks/000263-introduce-quad-error-and-narrow-audited-error-convergence.md @@ -1,7 +1,7 @@ --- id: 000263 title: Introduce QuadError and Narrow Audited Error Convergence -status: proposal # proposal | implemented | superseded +status: implemented # proposal | implemented | superseded created: 2026-08-07 github_issue: 960 --- @@ -10,27 +10,21 @@ github_issue: 960 ## Summary -Introduce a crate-private, closed `QuadError` carrier for the final internal -integration layer that can preserve native Operation, Runtime, Lifecycle, and -Fatal reports without first converting them to the public `Error`. Add the -exact `LifecycleOrFatalError` pair for health-aware admission paths, while -retaining the existing single-domain results and pairwise carriers as the -preferred contracts. - -Use the existing direct `.disclose()` audit as a bottom-up migration inventory. -For every audited internal convergence owner, trace its real leaf producers, -stack Resource, IO, or DataIntegrity beneath an operation-owned Runtime context -where required, and select the narrowest result contract: one native domain, -an exact two-domain carrier, or `QuadResult` for three or four of the four -integration domains. Public `Result` should remain only at public API methods, -fixed external-trait adapters, constrained-carrier disclosure implementations, -and the documented callback transport that must accept a caller-produced -public `Error`. - -This migration also corrects poisoned admission. A poison report remains Fatal -instead of being replaced by `LifecycleError::RuntimeUnavailable`; ordinary -shutdown, closed-session, and discarded-transaction rejection remains -Lifecycle. +The storage integration layer previously disclosed several typed error domains +into public `Error` too early, while poisoned admission sometimes replaced a +Fatal report with Lifecycle context. That weakened internal contracts and +misclassified poison at public boundaries. + +The shipped change adds crate-private `QuadError`/`QuadResult` for the fixed +Operation, Runtime, Lifecycle, and Fatal integration set, plus the exact +`LifecycleOrFatalError` pair for health-aware admission. Internal bootstrap, +completion, catalog, table, maintenance, and transaction paths now preserve +typed reports until an actual public or external boundary. + +Poisoned admission now discloses as Fatal without a Lifecycle frame. Ordinary +shutdown and unavailable session or transaction state remain Lifecycle. Lower +Resource, IO, and DataIntegrity reports enter the common carrier only beneath +a caller-owned Runtime context. ## Context @@ -42,564 +36,246 @@ Issue Labels: Source Backlogs: -- docs/backlogs/000178-common-multi-domain-error-carrier.md - -The storage error model has eight publicly classifiable private domains plus -the non-public Internal domain. Reusable producers are expected to retain a -typed `Report` until a public or external-trait boundary, but a few -higher-level owners currently return the top-level public `Result` because -their producer sets do not fit one domain or one of the three existing -carriers: - -- `OperationOrRuntimeError`; -- `OperationOrFatalError`; and -- `RuntimeOrFatalError`. - -The canonical audit currently records 67 production callables containing 228 -direct `.disclose()` method calls. Public Engine, Session, Transaction, -Statement, stream, `LazyRow`, and external-trait adapters are valid convergence -owners. The audit also exposes avoidable internal owners: - -- `bootstrap_inner`; -- `CreateIndexPlan::new` and `DropIndexPlan::new`; -- `wait_for_maintenance_boundary`; -- transaction-system bootstrap, component build, and user commit; -- non-callback `UserTableAccessor` DML integration helpers; and -- callback mutation helpers whose internal sources are disclosed early because - the callback itself returns the public result type. - -`CompletionObserver::wait` also converts a `CompletionErrorBridge` with -function-form `DiscloseError::disclose`. It is not present in the direct-method -audit, but it is a known internal public-error owner and is included in this -migration. The audit tool remains intentionally simple; this task does not -expand it into a visibility, return-type, or function-form analyzer. - -Several poison health checks currently use: - -```rust,ignore -poisoner - .ensure_healthy() - .change_context(LifecycleError::RuntimeUnavailable) -``` - -This replaces a current Fatal report with a Lifecycle context. It causes -poisoned admission to classify publicly as Lifecycle even though the engine -has entered its fatal one-way state. Shutdown is a legitimate Lifecycle -outcome, but poison must retain its Fatal identity and source chain. - -Configuration is limited to public bootstrap. Resource exhaustion, IO, and -data-integrity reports remain narrow at their native producers but can be -stacked below the Runtime operation that owns their integration. Therefore the -common final carrier needs exactly Operation, Runtime, Lifecycle, and Fatal; -it must not grow Config, Resource, IO, DataIntegrity, Internal, or public -`Error` arms. +- docs/backlogs/closed/000178-common-multi-domain-error-carrier.md + +The original direct-method audit contained 67 production callables and 228 +`.disclose()` calls. It identified valid public convergence owners alongside +avoidable internal owners in engine bootstrap, mandatory completion replay, +catalog plans, maintenance waits, transaction integration, and non-callback +table DML. + +The existing pairwise carriers were appropriate for exact two-domain paths, +but integration owners spanning three or four common domains either returned +public `Result` or risked replacing a native report with another context. In +particular, converting `EnginePoisoner::ensure_healthy()` from Fatal to +`LifecycleError::RuntimeUnavailable` lost the fatal classification. + +Configuration remains owned by public bootstrap. Resource, IO, and +DataIntegrity remain narrow at their native producers and gain a semantic +Runtime context only where a higher-level operation owns their integration. +The row-mutation callback remains the sole documented internal transport for +an arbitrary caller-produced public error. RFC 0023 is implemented historical context for the typed-domain and disclosure -model, not an active parent program for this task. The present change passes -the RFC complexity gate as one bounded internal refactor: it changes no public -signature, persisted representation, transaction protocol, recovery -algorithm, or staged rollout contract. +model, not a parent program for this task. No active parent RFC was linked. ## Goals -1. Add a fixed-cardinality `QuadError`/`QuadResult` integration carrier for - Operation, Runtime, Lifecycle, and Fatal reports. -2. Add `LifecycleOrFatalError`/`LifecycleOrFatalResult` for paths whose exact - reachable producer set is Lifecycle or Fatal. -3. Keep single-domain results and exact pairwise carriers preferred over - `QuadResult`. -4. Revisit every existing direct `.disclose()` audit row bottom-up and narrow - internal return types as far as their real producer sets permit. -5. Remove public `Error` from known internal bootstrap, completion, - transaction, catalog-plan, maintenance-wait, and non-callback table-DML - owners. -6. Keep Config at public Engine bootstrap and require explicit Runtime - ownership before Resource, IO, or DataIntegrity enters `QuadError`. -7. Preserve native reports, source frames, attachments, and Fatal bypass - semantics through carrier and completion-bridge conversion. -8. Classify poisoned admission as Fatal and ordinary lifecycle rejection as - Lifecycle. -9. Document and mechanically refresh the remaining approved public-error - convergence inventory. +1. Provide a closed four-domain carrier for final internal integration. +2. Provide an exact Lifecycle/Fatal carrier for health-aware admission. +3. Prefer native results, then exact pairwise carriers, then `QuadResult`. +4. Preserve native report frames, attachments, and Fatal bypass semantics. +5. Remove avoidable public-error convergence from the audited internal paths. +6. Keep Config disclosure at public bootstrap and require explicit Runtime + ownership for lower physical domains. +7. Correct poisoned admission to classify publicly as Fatal while preserving + Lifecycle classification for ordinary shutdown and invalid lifecycle state. +8. Retain public API signatures and refresh the documented disclosure audit. ## Non-Goals -1. No public `Error`, `ErrorKind`, or `Result` API signature changes. +1. No public `Error`, `ErrorKind`, or `Result` signature change. 2. No Config, Resource, IO, DataIntegrity, Internal, or public `Error` arm in `QuadError`. -3. No automatic conversion from a lower physical domain into Runtime without a - caller-owned semantic Runtime context. -4. No generic type-level error-set framework, variadic carrier, or arbitrary - carrier-generation system. -5. No removal of existing single-domain aliases or pairwise carriers. -6. No exact three-domain carrier family; integration paths with three of the - four common domains use `QuadResult`. -7. No parameterization of mandatory completion storage by task-specific error - types; `CompletionErrorBridge` remains the move/clone-safe transport. -8. No redesign of public transaction or row-mutation callback error contracts. -9. No poison-aware logical-lock or hot-row wait cancellation from backlogs - 000177 or 000179. -10. No persistent catalog, table, redo, checkpoint, or recovery format change. -11. No transaction ordering, rollback, MVCC, DDL publication, or recovery - semantic change. -12. No expansion of `tools/error_audit.rs` beyond its existing direct - `.disclose()` method-call inventory. +3. No generic error-set framework, variadic carrier, or three-domain carrier + family. +4. No removal of useful single-domain aliases or exact pairwise carriers. +5. No parameterization of mandatory completion storage by task-specific error + types. +6. No redesign of the public row-mutation callback contract. +7. No change to transaction ordering, MVCC, DDL publication, rollback, + checkpoint, or recovery semantics. +8. No persisted catalog, table, redo, checkpoint, or recovery format change. +9. No expansion of `tools/error_audit.rs` beyond direct `.disclose()` method + calls. +10. No poison-aware logical-lock or hot-row wait cancellation work. ## Plan -### Bottom-up narrowing rules - -Treat each current audit row as a review obligation rather than an allowlist. -Start from the callable's actual leaf results and work upward through its call -graph: - -1. Preserve an infallible or neutral outcome as `Infallible`, `Option`, a - status enum, or the existing neutral result when no error is owned. -2. Use the native `ConfigResult`, `OperationResult`, `ResourceResult`, - `IoResult`, `DataIntegrityResult`, `LifecycleResult`, `RuntimeResult`, or - `FatalResult` when one domain is reachable. -3. Use an exact pairwise carrier when exactly two stable integration domains - are reachable. Add `LifecycleOrFatalResult`; retain the existing three - pairwise results. -4. Use `QuadResult` only when three or four of Operation, Runtime, Lifecycle, - and Fatal remain reachable at one higher-level owner. -5. Keep public `Result` only at a public Doradb API, an externally fixed trait, - or the explicit callback transport described below. - -Resource, IO, and DataIntegrity are counted only after the caller that owns the -larger operation chooses a specific Runtime context. Examples include -`CatalogAccess`, `TableAccess`, `IndexAccess`, `RedoLogAccess`, `Recovery`, -`CheckpointExecution`, and `TransactionCommit`. Do not add a generic -"integration failed" context merely to make conversion compile. Fatal reports -always bypass Runtime and Lifecycle replacement. - -Apply the following disposition to the current audit inventory: - -| Current owner group | Required disposition | Narrow target | -| --- | --- | --- | -| Public Engine, Session, Transaction, Statement, stream, and `LazyRow` methods | Retain disclosure at the public facade | Public `Result` | -| `LogSync::from_str` and `ValKind::try_from` | Retain fixed external-trait convergence | Public trait error | -| `DiscloseError` implementations for constrained carriers | Retain conversion infrastructure | Native report to public `Error` | -| `bootstrap_inner` | Move convergence into public `Engine::bootstrap` | Public bootstrap only | -| `CreateIndexPlan::new`, `DropIndexPlan::new` | Preserve Operation; stack root-shape integrity under `CatalogAccess` | `OperationOrRuntimeResult` | -| `wait_for_maintenance_boundary` | Preserve shutdown and poison independently | `LifecycleOrFatalResult` | -| `TransactionSystem::{bootstrap, build}` | Validate Config before entry and own recovery integration as Runtime | `RuntimeResult` | -| `TransactionSystem::{commit_prepared, commit_transaction}` | Stack Resource under commit Runtime; preserve Lifecycle and Fatal | `QuadResult` | -| Non-callback `UserTableAccessor` DML helpers | Narrow leaves and combine only at real DML owners | Native, pairwise, or `QuadResult` | -| Callback mutation transport | Retain only where arbitrary callback `Error` must be forwarded | Documented public-result exception | - -Use compiler fallout to catch forwarding methods that do not themselves call -`.disclose()`. Refresh `docs/public-error-audit.csv` after migration. The final -CSV need not minimize carrier-disclosure implementation rows, but it must have -no internal convergence rows outside the constrained-carrier infrastructure -and documented callback transport. - -Do not change `tools/error_audit.rs` or its CSV schema. - -### Closed integration carriers - -Add these crate-private types in `doradb-storage/src/error.rs`: - -```rust,ignore -pub(crate) enum QuadError { - Operation(Report), - Runtime(Report), - Lifecycle(Report), - Fatal(Report), -} - -pub(crate) type QuadResult = result::Result; - -pub(crate) enum LifecycleOrFatalError { - Lifecycle(Report), - Fatal(Report), -} - -pub(crate) type LifecycleOrFatalResult = - result::Result; -``` - -Both carriers: - -- delegate `Debug` and `Display` to the contained report; -- implement `DiscloseError` without adding a carrier frame; -- implement `MultiDomainResultExt::{attach, attach_with}` by modifying the - contained report; -- accept structural `From>` conversions only for their declared - domains; and -- never become an `error_stack` context themselves. - -`QuadError` also flattens the existing `OperationOrRuntimeError`, -`OperationOrFatalError`, `RuntimeOrFatalError`, and the new -`LifecycleOrFatalError`. Conversion moves the native report directly into the -matching arm; it must not wrap one carrier inside another report. - -Do not implement `From` for Config, Resource, IO, DataIntegrity, Internal, -public `Error`, or `CompletionErrorBridge`. Lower domains require an explicit -Runtime context at their semantic owner. Completion bridges use a named replay -method so their policy is visible. - -`QuadError` is deliberately cardinality-named. Adding a fifth arm is a new -design decision, not a routine extension of this task. - -### Preserve Fatal admission - -Change health-aware admission paths from Lifecycle-only results to -`LifecycleOrFatalResult`: - -- `EngineInner::acquire_admission`; -- `EngineInner::with_admitted_operation`; -- `Engine::new_session_inner`; -- `Session::pin_observer`; -- `Session::pin_operation`; -- the health-aware portion of `Session::begin_trx`; -- `Transaction::checkout`; and -- `MandatoryRuntime::submit`. - -Keep pure lifecycle operations Lifecycle-typed: - -- `EngineLifecycle::admit`; -- weak session upgrade and registry/lifecycle checks; -- session close/discard checks; -- `Transaction::checkout_terminal` and terminal claiming, which must remain - available for cleanup after poison; -- lifecycle state transitions; and -- poison-tolerant inspection through `Session::pin_inspection`. - -Remove every -`change_context(LifecycleError::RuntimeUnavailable)` health conversion. -Forward the original Fatal report into the pairwise carrier and add only -caller-owned attachments. Remove `LifecycleError::RuntimeUnavailable` after a -producer audit confirms that no semantic producer remains. - -Public behavior is intentionally corrected: - -- engine poison before or during admission produces `ErrorKind::Fatal` with no - Lifecycle frame above the Fatal report; -- engine shutdown and unavailable session/transaction state remain - `ErrorKind::Lifecycle`; and -- already accepted mandatory work and poison-observable diagnostics retain - their existing ownership and availability rules. - -### Typed mandatory completion observation - -Keep `CompletionResult` and `CompletionErrorBridge` as the closed transport -used by completion cells and accepted mandatory execution. Change -`CompletionObserver::wait` to return `CompletionResult` directly instead of -calling `DiscloseError::disclose`. - -Add: - -```rust,ignore -impl CompletionErrorBridge { - pub(crate) fn into_quad( - self, - runtime_context: RuntimeError, - ) -> QuadError; -} -``` - -Replay the bridge without a public-Error round trip: - -| Reconstructed outer source | `into_quad` result | -| --- | --- | -| Operation | `QuadError::Operation` | -| Runtime | `QuadError::Runtime` | -| Lifecycle | `QuadError::Lifecycle` | -| Fatal | `QuadError::Fatal` | -| Resource | Source report changed to supplied Runtime context | -| IO | Source report changed to supplied Runtime context | -| DataIntegrity | Source report changed to supplied Runtime context | - -`CompletionSourceReport` has no Config or Internal arm, so neither can enter -this conversion. Preserve all replayed source frames and attachments and do -not leave a `CompletionErrorBridge` frame in the reconstructed report. - -At each observer, select the smallest result supported by the accepted task's -real producer set: - -- Runtime/Fatal maintenance completion continues through - `into_runtime_or_fatal`; -- Operation/Runtime/Fatal or - Operation/Runtime/Lifecycle/Fatal DDL completion uses `into_quad`; and -- a public Session method performs the final disclosure. - -The supplied Runtime context belongs to the public operation: - -- catalog DDL and catalog checkpoint use `CatalogAccess`; -- table checkpoint uses `CheckpointExecution`; -- table freeze and table cleanup use `TableAccess`; -- index work uses `IndexAccess`; and -- redo retention/truncation uses `RedoLogAccess`. - -An already reconstructed Runtime report retains its existing, more specific -context. The fallback context is used only for a raw Resource, IO, or -DataIntegrity root. - -### Catalog and DDL plan narrowing - -Change `CreateIndexPlan::new` and `DropIndexPlan::new` to -`OperationOrRuntimeResult`. - -- Metadata absence and invalid requested index state remain Operation. -- `validate_create_index_root_shape` and - `validate_drop_index_root_shape` remain DataIntegrity producers. -- Each plan constructor owns catalog integration and changes those - DataIntegrity reports to `RuntimeError::CatalogAccess`, retaining the - integrity frame and root/table/index attachments. - -Public `Session::{create_index, drop_index}` discloses the pairwise plan result. -Accepted index DDL keeps Operation, Runtime, Lifecycle, and Fatal sources typed -through completion replay and discloses only at the public Session method. - -Apply the same bottom-up rule while reviewing create/drop table completion: -retain a narrower pairwise completion when its actual producer set permits it; -use `QuadResult` only when at least three common domains are reachable. - -### Table and statement narrowing - -Audit the public-result region in `UserTableAccessor` from its existing narrow -leaf helpers upward. - -Target contracts include: - -- `validate_table_mutation_update` becomes `OperationResult`; -- known cold/hot delete and update integration that combines Operation, - Runtime, and Fatal becomes `QuadResult`; -- `insert_mvcc`, `upsert_unique_mvcc`, `update_unique_mvcc`, - `update_unique_mvcc_input`, and `delete_unique_mvcc` become `QuadResult` - where their current Operation/Runtime/Fatal producer set remains reachable; -- existing Runtime-only, Operation/Runtime, Operation/Fatal, and Runtime/Fatal - leaf helpers keep their narrower contracts; and -- IO, Resource, and DataIntegrity encountered by table/index integration - receive `TableAccess` or `IndexAccess` before entering a common carrier. - -Do not manufacture a three-domain carrier for these paths. Flatten pairwise -leaf errors into `QuadError` at the first owner that genuinely needs three -domains. Public `Statement` DML methods disclose the final carrier. - -`Statement::table_mutate_mvcc` accepts: - -```rust,ignore -F: for<'row> FnMut(&mut LazyRow<'row>) -> Result -``` - -The callback may return any public error previously obtained by its caller. -Changing that public contract is out of scope. Therefore -`UserTableAccessor::{table_mutate_mvcc, mutate_cold_rows_mvcc, -mutate_hot_rows_mvcc}` may retain public `Result` solely as callback-error -transport. Narrow every helper below them first, then disclose a typed helper -only where it must merge with the arbitrary callback error. Document these -three functions as the remaining genuine mixed-owner exception; do not allow -the exception to spread to point DML or non-callback helpers. - -### Transaction commit narrowing - -Rename the private `RuntimeError::SystemTransactionCommit` context to -`RuntimeError::TransactionCommit` so it describes both user and system -transaction integration. - -Keep `commit_prepared_no_wait`, catalog commit, and system commit on -`RuntimeOrFatalResult` where that remains their exact producer set. Change -user-facing transaction-system integration: - -- `TransactionSystem::commit_prepared` returns `QuadResult`; -- `TransactionSystem::commit_transaction` returns `QuadResult`; -- `FailedPrecommitReason::Resource` changes its Resource report to - `RuntimeError::TransactionCommit`, retaining the Resource source; -- `FailedPrecommitReason::Shutdown` remains Lifecycle; -- poison, rollback-cleanup failure, redo failure, and mandatory panic remain - Fatal; and -- fatal rollback cleanup bypasses Runtime and Lifecycle wrapping. - -Public `Transaction::commit` performs the sole final disclosure. Preserve -ordered commit, CTS publication, failed-precommit cleanup, session-state -release, lock release, and retry behavior. - -This intentionally changes public classification for user precommit resource -rejection from Resource to Runtime. The lower Resource frame and diagnostic -attachments must remain inspectable. System commit remains Runtime/Fatal as -before. - -### Bootstrap ownership - -Make public `Engine::bootstrap` the only startup-wide public-error convergence -owner. Fold the current private `bootstrap_inner` body into the public method, -or split it into typed substeps that do not return public `Result`; do not keep -a private public-result coordinator under another name. - -Introduce a crate-private validated transaction configuration prepared at the -public bootstrap boundary. It owns the normalized `TrxSysConfig` and resolved -redo file prefix: - -```rust,ignore -pub(crate) struct ValidatedTrxSysConfig { - config: TrxSysConfig, - file_prefix: String, -} -``` - -Construction performs `TrxSysConfig::validate` and `file_prefix` while Config -can still be disclosed by public `Engine::bootstrap`. The -`TransactionSystem` component accepts the validated type, stores the inner -configuration, and uses the prepared prefix without another Config result. - -Change: - -- `TransactionSystem::bootstrap` to `RuntimeResult`; -- `Component for TransactionSystem::Error` to `Report`; and -- its component `build` method to `RuntimeResult`. - -Retain recovery IO and DataIntegrity sources beneath `RuntimeError::Recovery` -or the existing more specific Runtime contexts. Startup worker/resource -failures retain their existing component-owned Runtime contexts. Invalid -configuration still discloses as `ErrorKind::Config` from public bootstrap, -and storage-root contention remains Lifecycle. - -Preserve component registration order, reverse rollback/shutdown order, -storage-layout marker sequencing, failure atomicity, and worker reclamation. - -### Documentation and audit closure - -Update `docs/error-spec.md` and `docs/process/coding-guidance.md` with: - -- the single-domain, exact-pairwise, then Quad selection order; -- the fixed membership and arity contract of `QuadError`; -- the rule that lower physical domains require an explicit Runtime owner; -- Fatal bypass semantics; -- Config ownership at public bootstrap; -- public `Error` ownership limited to public/external boundaries and the - callback exception; and -- the rule that a fifth Quad arm requires a new design review. - -Run the unchanged audit generator: - -```bash -tools/error_audit.rs --write docs/public-error-audit.csv -``` - -The refreshed inventory must contain no rows for: - -- `bootstrap_inner`; -- `CreateIndexPlan::new`; -- `DropIndexPlan::new`; -- `wait_for_maintenance_boundary`; -- `TransactionSystem::bootstrap`; -- `TransactionSystem::build`; -- `TransactionSystem::commit_prepared`; -- `TransactionSystem::commit_transaction`; or -- non-callback `UserTableAccessor` DML helpers. - -Expected remaining internal rows are constrained-carrier disclosure -implementations and the three documented callback mutation transport -functions. Public facade and external-trait adapter rows remain valid. Review -the diff row by row rather than accepting a lower aggregate count alone. +### Carrier architecture + +`QuadError` is a crate-private enum with exactly four native report arms: +Operation, Runtime, Lifecycle, and Fatal. It delegates formatting and public +disclosure to the contained report, supports eager and lazy attachments, and +does not appear as an `error-stack` context. + +The carrier accepts structural conversions only from its four native report +types, shared Fatal reports, and the existing exact pairwise carriers. +Pairwise conversion moves the native report directly into the matching arm; +it never nests one carrier inside another report. + +`LifecycleOrFatalError` provides the exact two-domain contract for admission +and health checks. It follows the same frame-less formatting, attachment, and +disclosure behavior. + +Result selection follows this order: + +1. one native domain; +2. an exact pairwise carrier; +3. `QuadResult` when three or four common integration domains remain. + +Resource, IO, and DataIntegrity have no structural conversion into +`QuadError`. Their semantic owner first changes context to a specific Runtime +operation such as CatalogAccess, TableAccess, Recovery, RedoLogAccess, +CheckpointExecution, FileRootAccess, or TransactionCommit. Fatal always +bypasses Runtime and Lifecycle wrapping. + +### Boundary ownership and data flow + +Public Engine, Session, Transaction, Statement, stream, and `LazyRow` methods +remain final disclosure owners. External trait adapters and carrier disclosure +implementations also remain valid convergence points. + +Public `Engine::bootstrap` owns startup-wide Config disclosure. A validated +transaction configuration wrapper resolves and stores the normalized +configuration and redo prefix before typed transaction-system construction. +Transaction bootstrap and component build then return Runtime-only results. + +Health-aware Engine, Session, Transaction, and mandatory-runtime admission use +Lifecycle/Fatal results. Poison retains its initiating Fatal report; lifecycle +admission closure, session closure, transaction discard, and shutdown retain +Lifecycle results. Cleanup and inspection paths that must remain available +after poison keep their poison-tolerant contracts. + +Mandatory completion cells continue storing `CompletionErrorBridge` so +accepted tasks can be observed safely. `CompletionObserver::wait` returns the +typed completion result rather than disclosing. `into_quad` reconstructs +Operation, Runtime, Lifecycle, or Fatal roots unchanged and places physical +roots beneath a supplied Runtime context. It leaves no bridge or carrier frame. + +Completion consumers attach operation, phase, and available request identity +after replay, including table, index, catalog, checkpoint, and redo operations. +Transaction commit can propagate the reconstructed Quad result through typed +internal layers before its public boundary discloses it. + +Catalog index plan construction returns Operation/Runtime: invalid requests +remain Operation, while invalid root shape retains its DataIntegrity source +beneath CatalogAccess. Non-callback point DML narrows to native, pairwise, or +Quad results. The three callback mutation helpers retain public `Result` only +to forward arbitrary callback errors. + +User transaction commit returns Quad internally. Precommit resource rejection +is retained beneath `RuntimeError::TransactionCommit`; shutdown remains +Lifecycle and poison, redo, rollback-cleanup, or mandatory panic remains Fatal. +System and catalog commit paths retain their narrower Runtime/Fatal contracts. + +### Correctness invariants + +- A public-error round trip never occurs during typed internal propagation. +- A carrier is never installed as a report context. +- Existing Runtime roots survive completion replay without replacement. +- Physical completion roots retain their source frames under the caller-owned + fallback Runtime context. +- Poison reports never gain an outer Runtime or Lifecycle frame. +- Every final write-path disclosure carries the same operation and table + identity context as its sibling read path. +- Maintenance poison and shutdown reports identify the observed boundary and + target timestamp. +- A fifth Quad arm requires new design work rather than routine extension. ## Implementation Notes +Implemented the complete bottom-up migration and retained the fixed four-arm +design. `QuadError`, `LifecycleOrFatalError`, their result aliases, flattening +conversions, attachment support, and frame-less disclosure are now the common +typed integration infrastructure. + +Engine bootstrap now performs Config-owned validation at the public boundary +and passes `ValidatedTrxSysConfig` into Runtime-typed transaction bootstrap. +The private public-result bootstrap coordinator was removed without changing +component registration, rollback, storage-marker, or shutdown order. + +Poisoned admission was corrected across Engine, Session, Transaction, and +mandatory submission. `LifecycleError::RuntimeUnavailable` was removed after +its producer audit found no remaining semantic use. Maintenance waits preserve +the Fatal report and attach boundary name plus target timestamp at both health +checks around listener registration. + +Completion observation remains layered deliberately: storage uses +`CompletionErrorBridge`, observers return `CompletionResult`, and integration +owners replay with `into_quad` or the narrower Runtime/Fatal conversion. A +review proposal to remove `into_quad` was rejected because transaction commit +must propagate a typed reconstructed result before public disclosure, and +physical completion roots still need an explicit Runtime owner. + +All ten public Session completion-bridge consumers attach operation and phase +context after replay. Review also added matching operation and `table_id` +attachments to insert, upsert, update, and delete write-path disclosure, and +added boundary context to both maintenance poison race checks. Mandatory +admission intentionally ignores the poison listener's value because that +listener has no meaningful result; the published poison report remains the +authoritative error. + +Catalog plans, non-callback table DML, transaction commit, file-root access, +and recovery integration now use the narrowest verified result type. The +public transaction precommit resource classification intentionally changes +from Resource to Runtime while retaining the Resource source report. + +The refreshed direct-method audit contains 54 callables and 197 disclose calls. +Removed rows include the private bootstrap coordinator, catalog plan builders, +maintenance wait helper, transaction-system bootstrap/build/commit helpers, +and non-callback table-access helpers. Remaining internal rows are constrained +carrier infrastructure or the documented callback transport exception. + +Documentation now records carrier selection order, fixed Quad membership, +physical-domain Runtime ownership, Fatal bypass, bootstrap Config ownership, +and public-error boundary rules. No deferred follow-up or plan deviation +requires a new backlog. + +Final validation passed after all review fixes: + +- formatting, diff checks, and the 15-file branch style audit; +- workspace build and clippy with warnings denied; +- 1,706 workspace nextest tests; +- alternate `libaio` clippy with warnings denied and 1,596 nextest tests; +- focused error, completion, admission, session, statement, transaction, + catalog, table, and bootstrap coverage. + ## Impacts -| Area | Planned effect | +| Area | Shipped effect | | --- | --- | | Public API | No signature or `ErrorKind` enum change | -| Public classification | Poisoned admission becomes Fatal; owned lower-domain integration becomes Runtime | -| Error carriers | Add fixed `QuadError` and exact `LifecycleOrFatalError` | -| Engine | Public bootstrap owns Config and all startup convergence | -| Mandatory runtime | Submission is Lifecycle/Fatal; observer returns a typed bridge | -| Session | Public methods remain final disclosure owners | -| Catalog DDL | Plan construction narrows to Operation/Runtime | -| Table DML | Non-callback helpers become native, pairwise, or Quad | -| Transaction commit | User integration becomes Quad; system paths stay pairwise | -| Documentation | Error model and coding guidance define arity and ownership rules | -| Audit | Existing direct-method tool is unchanged; generated inventory shrinks internally | -| Persisted data | No representation or compatibility change | -| Unsafe code | No new unsafe contract or expected unsafe-code change | +| Classification | Poisoned admission is Fatal; owned lower-domain integration is Runtime | +| Error carriers | Added fixed Quad and exact Lifecycle/Fatal carriers | +| Engine | Public bootstrap owns Config and startup convergence | +| Mandatory runtime | Submission is Lifecycle/Fatal; observation remains typed | +| Session and DDL | Public methods disclose typed completion and plan results | +| Table DML | Non-callback helpers use native, pairwise, or Quad results | +| Transaction commit | User integration is Quad; system paths remain Runtime/Fatal | +| Audit | Direct-method inventory reduced and callback exception documented | +| Persisted data | No representation, schema, or compatibility change | | Performance | Enum matching and report moves only; no intended I/O or scheduling change | -Primary risks are: - -- using `QuadResult` where a native or pairwise type is sufficient; -- accidentally wrapping Fatal beneath Runtime or Lifecycle; -- losing replayed completion frames or attachments; -- changing startup validation or rollback ordering while moving Config - ownership; and -- broadening the callback exception beyond its caller-supplied public error. - -The bottom-up audit disposition, absence of lower-domain `From` -implementations, focused report-frame tests, and startup failure-atomicity -tests are the required mitigations. - ## Test Cases -1. Construct each `QuadError` arm from its native report. Verify delegated - `Debug`/`Display`, static and lazy attachments, final `ErrorKind`, and the - retained native report frame after disclosure. -2. Flatten every existing pairwise carrier and `LifecycleOrFatalError` into - `QuadError`. Verify the carrier types do not appear as report contexts and - the original source/attachments remain present. -3. Verify Resource, IO, DataIntegrity, Config, Internal, public `Error`, and - `CompletionErrorBridge` have no structural `From` path into `QuadError`. - Exercise explicit lower-domain-to-Runtime conversions at representative - catalog, table, recovery, and commit owners. -4. Replay completion Operation, Runtime, Lifecycle, and Fatal roots through - `into_quad` and verify their outer domains are unchanged. Replay Resource, - IO, and DataIntegrity roots and verify the supplied Runtime context is outer - while the lower frame and attachments remain. -5. Verify completion replay leaves no `CompletionErrorBridge` frame and an - existing Runtime context is not replaced by the fallback Runtime context. -6. Poison before admission and while an admission waiter is waking for Engine, - Session, Transaction checkout, and mandatory submission. Verify Fatal - public classification, the initiating Fatal frame, and no Lifecycle frame - above it. -7. Verify shutdown, closed session, discarded transaction, busy shutdown, and - mandatory admission closure remain Lifecycle. Verify poison-tolerant - diagnostics and terminal cleanup remain available according to their - existing contracts. -8. Exercise user commit resource rejection and verify public Runtime - classification with `RuntimeError::TransactionCommit` above the retained - Resource report. Verify shutdown remains Lifecycle and redo/rollback/poison - failure remains Fatal. -9. Exercise system and catalog commits and verify their existing - Runtime/Fatal typed behavior, cleanup, CTS ordering, lock release, and - session-state transitions are unchanged. -10. Exercise CREATE/DROP INDEX invalid request and invalid root shape. - Operation failures remain Operation; root-shape DataIntegrity appears - beneath `RuntimeError::CatalogAccess` and classifies publicly as Runtime. -11. Exercise point insert/upsert/update/delete and full-table mutation across - hot and cold rows. Verify existing Operation outcomes, Runtime contexts, - Fatal poison propagation, undo/redo effects, and retry behavior. -12. Return a caller-produced public error from `table_mutate_mvcc`. Verify the - callback error is forwarded unchanged while non-callback DML paths contain - no internal public-error convergence. -13. Bootstrap with invalid transaction configuration and verify - `ErrorKind::Config`. Inject recovery IO/DataIntegrity failures and verify - their retained lower frames under the existing Runtime recovery context. -14. Re-run startup worker-spawn, layout-marker, storage-root lease, rollback - join-panic, and partial-component failure tests to prove registration and - cleanup ordering is unchanged. -15. Regenerate `docs/public-error-audit.csv` and review every removed, retained, - moved, and newly added carrier-disclosure row against the required - inventory disposition. -16. Run focused error, poison/admission, mandatory runtime, transaction, - catalog-index, table-access, session, completion, and bootstrap tests before - the full validation matrix. -17. Run: - - ```bash - rtk cargo fmt --check - rtk cargo build --workspace - rtk cargo clippy --workspace --all-targets -- -D warnings - rtk cargo nextest run --workspace - rtk cargo clippy -p doradb-storage --no-default-features --features libaio --all-targets -- -D warnings - rtk cargo nextest run -p doradb-storage --no-default-features --features libaio - tools/style_audit.rs - rtk git diff --check - ``` +1. Native Quad arms disclose to the matching public kind and retain their + native report, formatting, and attachments without a Quad frame. +2. Pairwise carriers flatten into Quad without nested carrier contexts. +3. Completion replay preserves common outer domains, stacks physical roots + beneath the supplied Runtime context, and leaves no bridge frame. +4. Post-replay operation and request attachments survive every Quad arm and + the physical fallback path. +5. Poison before or during admission remains Fatal with no Lifecycle frame; + shutdown and invalid lifecycle state remain Lifecycle. +6. Maintenance waits report both supported boundary names and target + timestamps while retaining Fatal identity. +7. User commit resource rejection is Runtime/TransactionCommit with the + Resource source retained; shutdown and fatal commit paths keep their domains. +8. CREATE/DROP INDEX invalid requests remain Operation, while invalid root + shapes are Runtime/CatalogAccess with DataIntegrity retained. +9. Point insert, upsert, update, and delete preserve existing MVCC behavior and + attach operation plus table identity before public disclosure. +10. Callback mutation forwards caller-produced public errors unchanged while + typed helpers below it avoid public convergence. +11. Invalid transaction configuration remains Config, and recovery physical + failures retain their source beneath Runtime recovery context. +12. Startup failure atomicity, component ordering, completion behavior, + transaction cleanup, lock release, and retry coverage remain passing. +13. The generated public-error audit matches the checked-in inventory and has + no disallowed internal convergence owners. ## Open Questions -None. The four Quad domains, single/pairwise preference, lower-domain Runtime -ownership, Config bootstrap ownership, direct-method audit scope, and callback -exception are resolved decisions. A newly discovered fifth integration domain -or a need to redesign public callback errors must be recorded as separate -design work rather than widening this carrier. +None. A fifth common integration domain or a redesign of public callback error +transport requires separate design work rather than widening this carrier. diff --git a/doradb-storage/src/session.rs b/doradb-storage/src/session.rs index 31e19b57..4da44675 100644 --- a/doradb-storage/src/session.rs +++ b/doradb-storage/src/session.rs @@ -3291,7 +3291,13 @@ async fn wait_for_maintenance_boundary( .runtime .poisoner .ensure_healthy() - .map_err(LifecycleOrFatalError::from)?; + .map_err(LifecycleOrFatalError::from) + .attach_with(|| { + format!( + "maintenance progress wait observed engine poison: boundary={}, target_ts={ts}", + boundary.name() + ) + })?; if session.runtime.state().admission.shutdown_started() { return Err(Report::new(LifecycleError::Shutdown) .attach(format!( @@ -3314,7 +3320,13 @@ async fn wait_for_maintenance_boundary( .runtime .poisoner .ensure_healthy() - .map_err(LifecycleOrFatalError::from)?; + .map_err(LifecycleOrFatalError::from) + .attach_with(|| { + format!( + "maintenance progress wait observed engine poison: boundary={}, target_ts={ts}", + boundary.name() + ) + })?; if session.runtime.state().admission.shutdown_started() { return Err(Report::new(LifecycleError::Shutdown) .attach(format!( @@ -5439,6 +5451,46 @@ pub(crate) mod tests { }); } + #[test] + fn test_maintenance_progress_wait_poison_reports_boundary_context() { + smol::block_on(async { + let root = TempDir::new().unwrap(); + let engine = Engine::bootstrap(EngineConfig::default().storage_root(root.path())) + .await + .unwrap(); + let session = engine.new_session().unwrap(); + let observer = session.pin_observer().unwrap(); + let target = engine.inner().trx_sys.purge_handoff_cts(); + let _ = engine + .inner() + .poisoner + .poison(Report::new(FatalError::RedoWrite).attach("maintenance wait poison")); + + for boundary in [ + MaintenanceBoundary::GcHorizon, + MaintenanceBoundary::PurgeCompletion, + ] { + let error = wait_for_maintenance_boundary(&observer, target, boundary) + .await + .unwrap_err(); + let LifecycleOrFatalError::Fatal(error) = error else { + panic!("poisoned maintenance wait must remain Fatal") + }; + assert_eq!( + error.downcast_ref::().copied(), + Some(FatalError::RedoWrite) + ); + assert!(error.downcast_ref::().is_none()); + let report = format!("{error:?}"); + let expected = format!( + "maintenance progress wait observed engine poison: boundary={}, target_ts={target}", + boundary.name() + ); + assert!(report.contains(&expected), "{report}"); + } + }); + } + #[test] fn test_session_checkpoint_catalog_persists_catalog_state() { smol::block_on(async { diff --git a/doradb-storage/src/trx/stmt.rs b/doradb-storage/src/trx/stmt.rs index ca90228c..3ab3e42c 100644 --- a/doradb-storage/src/trx/stmt.rs +++ b/doradb-storage/src/trx/stmt.rs @@ -654,6 +654,7 @@ impl<'stmt> Statement<'stmt> { .accessor_with_layout(&layout) .insert_mvcc(rt, effects, cols) .await + .attach_with(|| format!("operation={OPERATION}, table_id={table_id}")) .disclose() } @@ -700,6 +701,7 @@ impl<'stmt> Statement<'stmt> { .accessor_with_layout(&layout) .upsert_unique_mvcc(rt, effects, unique_index_no, cols, false) .await + .attach_with(|| format!("operation={OPERATION}, table_id={table_id}")) .disclose() } @@ -745,6 +747,7 @@ impl<'stmt> Statement<'stmt> { .accessor_with_layout(&layout) .update_unique_mvcc(rt, effects, index_no, key_vals, update, false) .await + .attach_with(|| format!("operation={OPERATION}, table_id={table_id}")) .disclose() } @@ -783,6 +786,7 @@ impl<'stmt> Statement<'stmt> { .accessor_with_layout(&layout) .delete_unique_mvcc(rt, effects, index_no, key_vals) .await + .attach_with(|| format!("operation={OPERATION}, table_id={table_id}")) .disclose() }