diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dfdf66b0..91260cca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -283,7 +283,7 @@ jobs: - name: Coverage summary working-directory: walshadow env: - MAX_MISSED_LINES: 3515 + MAX_MISSED_LINES: 3130 run: | awk -F'[:,]' -v max="$MAX_MISSED_LINES" ' /^DA:/ { f++; if ($3 > 0) h++ } diff --git a/Cargo.lock b/Cargo.lock index 76aae819..21dd88dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -565,6 +565,16 @@ dependencies = [ "memchr", ] +[[package]] +name = "ctor" +version = "1.0.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "914a755b7c2d4af2bdcff7ce1739e2db9a1b81a9b07123d8015786ae03c0980d" +dependencies = [ + "link-section", + "linktime-proc-macro", +] + [[package]] name = "ctutils" version = "0.4.2" @@ -1374,6 +1384,18 @@ dependencies = [ "libc", ] +[[package]] +name = "link-section" +version = "0.19.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "39c29a617ce3df32c08497bdc1ab6e2376e0b17948ac166a2fbe5977c5954cd9" + +[[package]] +name = "linktime-proc-macro" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e57c38c1e860fd37c604281cdfb1dd2216977fd76a50f85ba2f388ef3219616" + [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -3039,6 +3061,7 @@ dependencies = [ "clap", "clickhouse-c-rs", "crc32c", + "ctor", "fallible-iterator", "futures", "globset", diff --git a/Cargo.toml b/Cargo.toml index ed0443aa..ca6ffeb6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -101,6 +101,8 @@ name = "spool_read" harness = false [dev-dependencies] +# Pre-main tracing subscriber for tests +ctor = "1.0.13" tempfile = "3" # Enable paused time for batch deadline tests tokio = { version = "1", features = ["test-util"] } diff --git a/plans/INDEX.md b/plans/INDEX.md index daef34d1..a970f7a3 100644 --- a/plans/INDEX.md +++ b/plans/INDEX.md @@ -21,11 +21,13 @@ writes. A documented limitation does not imply code rejects it | Plan | Next step | |---|---| -| [Schema changes](schema.md) | Reject unsupported transitions before destination effects | +| [Value coercion](value_coercion.md) | Reject Decimal precision loss and signedness corruption; fix String NaN defaults; prove rejection recovery | +| [Runtime configuration](runtime_config.md) | Locate owning UI and reproduce stale/dead state; expose existing controls there | +| [Schema changes](schema.md) | Fix rename routing and changed-key tombstones; reject keyless/unlogged gaps before effects | | [Tablespaces](tablespaces.md) | Reject unsafe layouts before bootstrap, then add complete support | | [Catalog completeness](catalog.md) | Stop when a surviving relation has lost buffered payload | | [Bootstrap visibility](bootstrap.md) | Prove pending-row recovery across load modes and restart | -| [Verification](verification.md) | Prove outage, restart, and WAL-version behavior | +| [Verification](verification.md) | Prove value fidelity and restart without skipping WAL | | [100% line coverage](coverage100.md) | Close fixture, live-system, CLI, and fault-path gaps, then enforce 100% | ## Further work @@ -34,13 +36,11 @@ writes. A documented limitation does not imply code rejects it |---|---| | [Multi-database loads and metrics](multi_database.md) | Extend heap-page bootstrap and backup loads beyond primary database, attribute metrics | | [Fuzzing](fuzzing.md) | Find parser and schema-transition interactions beyond fixed regressions | -| [Performance](performance.md) | Locate bottlenecks before changing concurrency or allocation | -| [Runtime configuration](runtime_config.md) | Configure destination DDL, codecs, and settings; add commands and explain effective config | -| [Failover](failover.md) | Continue after unplanned promotion or across archived timeline changes | +| [Performance](performance.md) | Establish healthy sustained WAL baseline with exact reconciliation before tuning | +| [Failover](failover.md) | Fence unplanned promotion and prove remaining archive/restart fault cases | | [Shadow TOAST reclamation](shadow_toast.md) | Keep historical values readable under lag and restart | | [Replay callback](custom_rmgr.md) | Reduce measured command-boundary capture stalls | | [Dependencies](dependencies.md) | Replace generic protocol code when an adapter preserves behavior | -| [Value coercion](value_coercion.md) | Validate destination domains and align fast defaults with row substitution | | [Tier 2 containers](tier2.md) | Remove shadow round trips for array, map, and vector columns | | [Optional capabilities](extensions.md) | Meet a concrete routing, export, vector, or durability requirement | diff --git a/plans/catalog.md b/plans/catalog.md index b46eb875..e3299d6e 100644 --- a/plans/catalog.md +++ b/plans/catalog.md @@ -11,6 +11,12 @@ A markerless record can be tracked without its payload while its relation is unknown. If that relation resolves to an ordinary surviving table at commit, resolution currently warns that rows were not mirrored and continues +Reproduced at `1267db7` without PostgreSQL: seed `DescriptorLog` with an ordinary +relation, call `track_unresolvable(77, 150, rfn)`, then +`resolve_stash(..., 77, &[], 1000, ...)`. Resolution returns `Ok(())` despite +missing payload. This establishes missing rejection, not an end-to-end durable +cursor advance; assert that boundary when adding regression + Return an error for that outcome. Preserve legitimate discard behavior when relation was born and dropped within transaction. Test both branches and prove failure cannot advance durable progress past lost rows diff --git a/plans/coverage100.md b/plans/coverage100.md index 8709b447..a287f30c 100644 --- a/plans/coverage100.md +++ b/plans/coverage100.md @@ -3,79 +3,222 @@ Drive workspace line coverage to 100%, including product binaries and defensive paths. Use meaningful behavioral assertions, testable boundaries, and removal of confirmed dead code. Keep lints and existing zero-coverage-file gate enabled -Do not exclude files or disable coverage without human approval +Never exclude files, modules, or test code from coverage ## Measurement and gate Regenerate coverage with [development recipe](../docs/development.md#wal-fixtures-and-coverage) -and current [CI matrix](../.github/workflows/ci.yml). Use merged line data from every -matrix major for project target; retain per-major reports to find version -branches. Require every expected major's artifact before accepting merged result -An early-returned integration test does not establish coverage of its scenario - -Generate work list from `merged.lcov` and HTML artifact. Record covered and total -executable lines for revision under test, no historical percentage serves as -current baseline. Use function coverage only to find untouched routines; LCOV -function entries may repeat generic instantiations and do not count missed lines +and current [CI matrix](../.github/workflows/ci.yml). Merged job unions DA hits +from every major, so a merged miss is missed on all of 16/17/18/19. Require every +expected major's artifact before accepting merged result. An early-returned +integration test does not establish coverage of its scenario Merged job caps missed lines, unique `(source file, DA line)` entries with zero -hits, at a measured ceiling. Lower it after each completed tier and finish at -zero, which is integer covered/total equality. Unlike a percentage or covered -floor, a missed count neither rounds nor drops when covered code is deleted -A per-major `--fail-under-lines` check is not equivalent to merged coverage -across version-specific branches - -Per-major LCOV `LF`/`LH` and native llvm-cov summaries count llvm-cov's source -lines, a larger set than DA entries. Compare DA counts only against the ceiling - -Measure [PG module coverage](../pgext/README.md#coverage) separately from Rust -line target. Per-major 100% C gate implies merged 100%, so no merged C gate is -needed. Retain live/fault tests and C sanitizer campaigns to verify module -behavior beyond line execution - -## Work list - -Recheck each candidate against fresh report before adding tests. Anchor on files, -functions, and behavior rather than source line numbers - -Prioritize staged backfill restart around prepare/publish/swap/copy-back, then -direct/object-store handoff and gap replay. Exercise real state transitions -through live harnesses and assert recovery outcomes - -| Tier | Candidate gaps | Evidence | -|---|---|---| -| Pure units | Config coercion/errors, namespace/table/column validation, attnum overflow, mapping mutation, cursor corruption | Assert accepted values or precise rejection, round-trip persisted state | -| Fixtures and in-process pipeline | Deadline/idle/close flush, retry/reconnect, acknowledgement gaps and trailing events, worker failure | Assert emitted batches, contiguous durable frontier, and retained work after error | -| WAL and tuple matrix | Bad page magic/body, cross-page records, LZ4/ZSTD FPI corruption, short/4-byte varlena, invalid UTF-8, truncation, partial tuples, dropped columns, defaults, arrays | Assert decoded values or error without partial publication | -| Transaction and TOAST | Truncated chunks, detoast misses, spill failures, observer errors, subtransactions | Assert transaction disposition, cleanup, and restart floor | -| Live control plane | Catalog seed/refresh, shadow lifecycle, DDL, source unix/TCP/TLS, shadow transport, keepalive timeout | Exercise actual client state and verify failure/recovery | -| Bootstrap and archives | Direct/object-store modes, backup start/finish, tablespaces, archive fetch, concurrent writes | Compare exact final rows and durable handoff, reject missing evidence | -| CLI | `stream`, `filter`, `classify` setup, argument errors, startup failure, shutdown | Run process or extract cohesive testable setup, assert exit and externally visible state | -| Defensive tail | Failed reads/writes, disk full, fsync/rename, listener failure, socket handoff, cancellation | Inject deterministic faults and prove no premature acknowledgement or lost retry state | - -Reuse [in-process harness](../tests/common/inproc_harness.rs), WAL fixtures, and -existing live tests. For stopped-worker coverage, make inner sink fail, then -exercise subsequent queue send/flush and assert propagated failure. Use paused -time for deadlines and controlled readers/writers for transport faults - -Keep CLI orchestration in scope: bootstrap off/direct/object-store, emitter and -DDL setup, metrics/oracle options, retention, archive fetch, shadow startup, and -shutdown hooks. Extract pure helpers only where they express useful behavior -Do not split files merely to make exclusions easier - -Review unreachable public states before injecting impossible ones. For example, -a consuming close API may make an already-closed branch redundant. Remove only -after proving no caller or recovery path needs it; otherwise expose a narrow -fault boundary and assert contract - -## Sequencing and completion - -1. Refresh baseline and list missed branches -2. Extend live harnesses for staged backfill restart, handoff, and gap replay -3. Close pure-unit, fixture, schema, bootstrap, and transport gaps -4. Exercise CLI orchestration and deterministic OS/transport failures -5. Regenerate reports after each tier, lower ceiling, and retain regression inputs -6. Enforce literal 100% merged line coverage once no missed executable lines remain +hits, at 3115. Lower ceiling after each measured batch and finish at zero. +Per-major LCOV `LF`/`LH` and native llvm-cov summaries count a larger set than DA +entries, compare DA counts only against ceiling + +[PG module coverage](../pgext/README.md#coverage) is gated per major at 100% and +stays out of Rust target + +LCOV covers `src/**`, including in-file `#[cfg(test)]` modules; integration +tests under `tests/` are not counted. Generate fresh merged PG16/17/18/19 +coverage to establish hit/total counts and rank remaining misses + +## Coverage conventions + +- Test code counts. In-file `#[cfg(test)]` modules are measured and never + excluded. Failure paths of an assertion must not own a line: compare whole + values with `assert_eq!`, use `unwrap`/`expect`/`unwrap_err`, compute message + args before the assert, drop unused mock methods and helper branches +- Stop spawned daemons with `tools::stop_gracefully` (SIGINT, SIGKILL after + timeout); `ChildGuard` drop does so. Instrumented binaries write their profile + only on exit, so SIGKILL is reserved for deliberate crash points (`kill_restart` + crash step, `bootstrap_object_store_crash_ch` first run, + `control_plane_e2e::kill_daemon`), whose code is covered by the restarted run +- Lib, `walshadow-stream` and integration test binaries install a no-op + `tracing_subscriber::registry()` before main, so every callsite is enabled and + field expressions execute. Spawned daemons keep their own filter +- Remove dead code rather than test it. Guards on internal invariants become + unrepresentable state where a small type change allows, else stay and get a + unit test through the narrowest boundary + +## Invariant arms requiring type changes + +Each needs a type change beyond a local edit: +- bin bootstrap `BootstrapMode::Off` arm, needs start mode carried in `ShadowStart` +- `copy_backfill::note_opt_in` None handling and `backup_backfill::run_pass` + None/Copy arm, need a backup-only mode type through queue, ledger and trait +- batcher `HeapOp::Truncate` arm, needs a batcher-only op type +- `XactBuffer::stash_raw`/`fold_raw` missing rfn, needs rfn-carrying record type +- `descriptor_at_spanned` Present arms, need a narrower `LookupResult` +- `ops::control` save fragment path check, needs a typed fragment location +- `ch.rs` codec arms reachable only without default features + +`WalSegmentRemoved::from_start_replication` is live only against a mock, since +PG raises 58P01 after CopyBoth; cover via mock walsender + +## Seams + +Existing: +- `mock_feed()` pgwire backend in `source_feed` tests. Lift to a crate-level + test helper that scripts IDENTIFY_SYSTEM, TIMELINE_HISTORY, CopyData, CopyDone, + keepalive with reply, ErrorResponse. Unlocks `transition` probe/cross/verify + and `source_feed` event arms +- `ch::test_support::retry_server` fake CH native server. Extend with scripted + ServerException codes for `ch_ddl` refusal and passthrough arms, toast truncate + and backfill publish/swap failures +- `ChunkStore` trait with `MemChunkStore`, `ReadOnly`, `FailPrefetch` +- `RecordSink`, `TupleObserver`, `SegmentSink` failing doubles (`Fail`, `ErrAt`, + `ErrSegmentSink`) +- `BackupSource` trait, reachable only inside `backup_backfill` via in-file tests +- `ShadowConfig.pg_bin_dir`: empty dir gives MissingBinary, fake `pg_ctl`/`psql` + scripts give start failure and parse errors +- bridge `fake_worker` for protocol violation arms +- `metrics` `FailAfter` writer + +Filesystem faults without a new trait: directory in place of a file, file in +place of a directory, read-only directory. CI runs as non-root, so permission +faults hold + +Missing: +- Unix-socket walreceiver client for `shadow_stream` Unix listener +- Storage injection into object-store backfill pass, today built from settings +- Retention trim interval, hard-coded, so the housekeeping loop never iterates in + a test. Take interval from config or a hidden flag +- Walk checkpoint period knob for `backup_backfill` periodic checkpoint branches +- Hook between COPY chunks to run a rewrite mid-COPY + +## Work list by area + +Check candidate gaps against fresh merged report before adding tests + +### CLI and ops + +- `runtime_cfg::seed_runtime_config`: no spawned daemon sets + `[runtime_config] schema`, and inproc harness only sees rows over WAL. Graceful + `control_plane_e2e` boot with config rows pre-inserted (global, namespace, + table with glob match, column) and TOML `initial_load` mappings including a + bogus mode. Also covers `apply_toml_initial_loads`, bin `source_db` seed and + opt-in paths. SIGHUP with broken TOML, double SIGINT for forced exit +- `housekeeping` retention trim: needs interval seam, then graceful run with + `--retention-bytes 1` and a shadow connection drop for reconnect +- bin `bootstrap`: resume past extraction, metrics-only bootstrap, deferred + referrer handback, archive WAL hydrate, `--bootstrap-wal-from-archive` in direct + mode. Check coverage collected by graceful teardown before extending tests. + `resolve_shadow_start` overlap and `paths_overlap` are bin unit tests +- `session`: manifest corrupt/foreign with and without `--ignore-cursor`, sibling + branch and non-descendant boots, multi-db preflight, crossing retry/park and + walsender reconnect via `control_plane_e2e` promotion drills. `task_stopped`, + `SessionTasks::exited` outcomes and `adopt`/`shutdown` panic arms are units +- `ops::control` introspection verbs (`tables`, `schemas`, `columns`, + `--database`): add missing `ctl` calls to `control_plane_e2e`. Covers + `ctl` render, `introspect`. Pure `ok_with`, malformed dispatch, `pg_connect` + empty host are units +- `ops::init`: `remedy` per `PreflightError`, `select_tables`/`resolve_url` under + non-tty stdin, non-superuser source role in `init_e2e`. Spawn `init` subcommand + once to cover `InitArgs::into_opts` and dispatch. TTY picker needs a pty +- `ops::oracle` Native block decode validation and cell-count check as units. + `text_value` via ADD COLUMN with oracle-rendered default +- `tracing_setup` OTLP branch: spawn with an unreachable `--otlp-endpoint`, + invalid endpoint for parse error +- `shadow_proc` supervise restart/backoff: immediate-stop shadow postmaster under + graceful run, `probe_blocking` Err/panic as units +- `preflight::bootstrap` window-leg sender check and `wal_from_archive` against CI PG + +### Backfill + +- Object-store opt-in pass: every `backfill_staging_e2e` opt-in + lacks a `[backup]` section and falls back to COPY. Give fixture an FsStorage + backup root, push a backup and WAL past it, opt in above redo LSN with a + TOAST-external row. Covers `run_object_store_pass`, `walk_and_ship` gap leg, + `prescan_gap`, `replay_gap`, `drain_deferred`. Then crash after walk and rerun + for `reopen_walk_spools` and ready-checkpoint resume, or hand-seed checkpoints + as `backup_checkpoint_e2e` does +- `copy_backfill` pending tables: hold an open xact on target table across a + base-backup opt-in, commit or abort after walk, for `record_pending` and + `settle_ended_pending`. `copy_chunk_blocks = 1` on a multi-block table for chunk + loop and progress ledger. Empty table fast path. Opt-out and CH schema change + mid-pass for publish/swap edges. Two opt-ins inside coalesce window +- `copy_backfill` units: `wire_kind` per OID, `decode_field` per type and invalid + UTF-8, ledger version rejection. Ledger persist failures via fs faults +- `wal_replay`: window leg with toasted update, >64 subxacts, VACUUM FULL of + toasted table, DELETE under append-only mapping, DDL inside window. Extend + `bootstrap_window_ch` +- `visibility_pending` ship with no mapping, over slab size, disjoint columns, + settle after pending table dropped. `note_view`, empty `fresh_side` as units +- `bootstrap_marker` `ExtractedCheckpoint` write/read and `resumable_extraction` + with no pin, mismatched pin, missing spool, valid checkpoint as units +- `backup_page_walk` short page, bad line pointer, bad `t_hoff`, offnum bounds as + units. `backup_source` tar skip entries and kept symlinks as units +- `opt_in` alternates via `config_table` rows: `initial_load` none and bogus, + NULL `replicate`, forward-declared row, refused mode for unfollowed database +- `bootstrap_oracle` `present_sockets`, `should_drop_entry` as units + +### Pipeline, decode, xact, toast + +- `heap_decoder` truncation guards: table-driven units looping `buf[..n]` + through `decode_one_value`, `decode_varlena` per header form, and hand-built + records into `decode_heap_record`. Prefix/suffix partial tuples via + `decode_tuple_payload` directly. `missing_value_from_text` per fixed type and + `ADD COLUMN "char" DEFAULT` in `add_column_default` +- `xact_buffer`: drive `on_record` against crafted `DescriptorLog` for Retired, + ForeignDb, NotCovered, Ambiguous with and without marker, no-block heap record. + `handle_truncate` malformed and foreign/toast relids. `resolve_stash` ambiguity + and superseded generation. `decode_image_insert` needs a real 8 KiB page + fixture or VACUUM FULL of a toasted table with a mid-rewrite checkpoint. + `body_mem_max = 0` for file-backed bodies, `MemChunkStore` Missing/Generation +- `ch_emitter`: config loader errors in tempdirs, TOML table parser rejections, + `parse_decimal_text` and `timestamp_ticks` bounds, `oracle_cell` mismatches, + oversized oracle row, `"char"` in native type sweep. `ColumnBuf` Debug in one + format assert +- `ch_ddl`: DROP COLUMN through applicator without resolver, tier-3 fast default + with oracle, multi-db target ownership conflict, drop strategy Warn, + `render_create_table` edge columns. CH refusal codes via `retry_server` +- `reorder`: config reload removing an opted-in table in `runtime_config_e2e`, + `plan_mem_max = 0` for file-backed plans, plan dir faults, fatal before barrier +- `plan_spool` and `spill` replay corruption: write, patch bytes, replay, in the + style of existing corruption tests +- pipeline `bootstrap` resumable-walk checkpoint: barrier with a ticker firing + during walk, extend object-store crash or chunk resume test +- `batcher` oracle frame full and empty-table flush under global byte budget +- `shadow_landing::serves_toast`: runtime opt-in of toasted table under + `[toast] mode = "shadow"` +- small decoders (`visibility`, `wal_xact`, `fpi`, `jsonb`, `inet`) as crafted units + +### Source, catalog, filter + +- `transition` probe and cross guards via lifted mock walsender, `CrossingState` + and `ForkWait` Display as units. Double promotion (TL1 to TL3) for multi-hop + `branch_history` and missing source history +- `desc_log` decoder rejections: write log, patch magic, tag, CRC, header, + identity fields, unknown kinds, reopen. Round-trip rare variants +- `shadow_stream` state edges as units on `ShadowStreamState`, slow-client cap + with small threshold, Unix listener needs Unix walreceiver client +- `catalog::shadow` via fake `pg_bin_dir`, `wait_for_replay` timeout, + `validate_running` mismatches in `shadow_lifecycle` +- `streaming_walker` ShortRecord, ZeroRecord, zero tail, oversize continuation, + `truncate_to` below page start with existing page builders +- `queueing_record_sink` worker failure via failing sink doubles, span sampling + with a span registry +- `catalog_capture` resume below `covered_through` with in-flight command + boundaries, lost coverage by deleting desc log spill dir +- `source_feed` `check_slot` with logical slot and slot without restart LSN +- `type_bridge` literal and type mapping as one table-driven unit +- `engine` smgr marker eviction, mixed-scope catalog touch across target dbs +- `shadow_catalog` duplicate oid, odd hex, tablespace and no-toast tables + +## Sequencing + +1. Measure merged coverage, prune covered candidates and rank + remaining misses; lower 3115 ceiling if measured result permits +2. Pure units: decoders, corruption replays, parsers, Display, CLI args +3. Seam lifts: mock walsender, `retry_server` codes, retention interval, storage + injection +4. Live scenarios: object-store backfill pass and resume, runtime config seed, + control verbs, window-leg shapes, pending tables, promotion drills +5. Fault arms left after above, via fs layout faults and new seams +6. Enforce literal zero missed once no DA line remains uncovered Use [semantic fuzzing](fuzzing.md) to discover interactions, then convert useful findings into deterministic coverage tests. Coverage proves execution, recovery diff --git a/plans/dependencies.md b/plans/dependencies.md index 89c2e07e..b81265c4 100644 --- a/plans/dependencies.md +++ b/plans/dependencies.md @@ -49,7 +49,7 @@ and [staged publication](shadow_toast.md), including atomic visibility assumptio Use a shared governor limiter when introducing aggregate read budgets. Current `ObjectStoreSource::run` clones Settings for concurrent parts; each -`Settings::throttle_network` call in wal-rus 0.3.2 constructs an independent +`Settings::throttle_network` call in locked wal-rus 0.3.5 constructs an independent `RateLimited` reader. N active parts can approach N times configured network rate Current pacing tracks bytes since reader creation, allows initial read through, and accumulates idle credit without an explicit burst bound diff --git a/plans/failover.md b/plans/failover.md index 93f2d34d..b2770ce7 100644 --- a/plans/failover.md +++ b/plans/failover.md @@ -2,8 +2,7 @@ [Planned switchover](../docs/failover.md) already handles controlled descendant timeline crossing, including restart and stable endpoints. Remaining work covers -unplanned promotion, archive fallback across timelines, and backups taken on -ancestor timelines. Current crossing design is in +unplanned promotion and remaining archive/restart fault cases. See [recovery architecture](../architecture/recovery.md) ## Unplanned promotion @@ -29,22 +28,22 @@ remain errors ## Archives and backups -Resolve history before fetching segments. Use branch owning each segment and -each record range, including descendant filename for a fork segment. Validate -prefix shared with ancestor before accepting it. Do not substitute a partial -archive file for a complete required segment +Do not reimplement archive lineage or ancestor-backup replay. PostgreSQL 18.6 / +ClickHouse 26.8.1.951 tests at `1267db7` pass for ancestor-backup bootstrap, +[backup gap replay across promotion](../tests/backfill_gap_across_promotion.rs), +and [control-plane recovery](../tests/control_plane_e2e.rs): descendant copies +when ancestor segments are partial, two forks within one segment, source slot +ahead of replay, archive history discovery after source death, and restart inside +fork barrier -Make verified fork prefix available to archive-only shadow recovery or report -explicit wait for segment durability. Test restart before segment fills +Archive discovery after source death proves recovery within already accepted +branch; it does not authorize crossing onto an unproved live source. Retain that +distinction when extending recovery -Object-store bootstrap WAL hydration, its window leg, landed-WAL filtering, and -direct backup from a standby resolve branches through -[archive history](../src/source/archive_history.rs), on the same rules as the -table load's gap. A backup's timeline is lineage input, not proof it belongs to -current source. Reject backups outside ancestry - -Once archive fallback covers these cases, prove slotless pause can resume after -source recycles WAL. Missing history or segments must still stop replication +Prove slotless pause resumes after source recycles WAL. Add restart with an +unsealed descendant segment and archive-only shadow recovery, verifying either +durable fork prefix or explicit wait. Test missing history separately from +missing segment, including direct backups taken from a standby ## Transition implementation @@ -65,21 +64,13 @@ aborted continuation evidence expected by PostgreSQL reader, accepting page flag alone cannot make shadow cross safely. Test failure without shadow PANIC/FATAL, then enable only after source and shadow agree on overwritten LSN -Live archive fallback resolves each segment through the same archive-history -resolver the table load's gap uses, rather than the branch it happens to stream. -Resolve filename per segment and owning branch per record range -Handle multiple forks inside one segment and suppress repeated prefixes only -after validation, never send prefix twice through filter or decoder. Place -history files before shadow requests descendant WAL - Keep fork-prefix durability explicit when descendant segment is unsealed Coordinate atomic archive publication with [TOAST gate](shadow_toast.md) and -custom-record [provenance](custom_rmgr.md). Exercise archive-only reconnect at -fork and missing history separately from missing segment +custom-record [provenance](custom_rmgr.md) -Sequence ordinary-state fence, overwritten-continuation support, archive lineage -and fork-prefix durability, then ancestor-backup replay. Keep refusal until each -enabled case advances source, shadow, and durable floor consistently +Sequence ordinary-state fence, overwritten-continuation support, and remaining +fork-prefix durability proofs. Keep refusal until each enabled case advances +source, shadow, and durable floor consistently ## Completion diff --git a/plans/performance.md b/plans/performance.md index 7e011dd6..24500ed7 100644 --- a/plans/performance.md +++ b/plans/performance.md @@ -9,6 +9,28 @@ Correctness tests should assert outcomes without comparing wall-clock speed Keep performance runs on dedicated hardware, separate from shared CI runners Publish repeatable baselines and their variance before choosing regression bands +## Establish sustained WAL baseline + +Resolve source/destination mapping from status and destination DDL; use matching +identities for setup, queries, and cleanup. Prove a sentinel transaction arrives +before starting load. Keep failed recovery drills and table recreation separate +from performance runs + +Extend existing `sustained` workload toward 200 MB/s of source WAL, recording +achieved rate and byte units explicitly. Measure source LSN deltas over elapsed +time, not row count times nominal payload size. Record batch/commit cadence and +compression; distinguish sustained generation with bounded backlog from draining +a finite backlog after generator stops + +Track receive, shadow replay, contiguous destination acknowledgement, retained +WAL, queues, RSS, spill, and process restarts throughout run. Record final committed +marker's boundary and wait for destination acknowledgement to cover it, then reconcile +keys and payloads against source using `FINAL WHERE _is_deleted = 0`. +Do not use raw `count()` or newest `_commit_ts` alone to prove completion + +Report failures and recovery separately from rate; require healthy baseline and +exact reconciliation before attributing bottlenecks or changing concurrency + ## Initial load Initial-load and greenfield-bootstrap workloads already exist, including fresh diff --git a/plans/runtime_config.md b/plans/runtime_config.md index 67f73639..636896a2 100644 --- a/plans/runtime_config.md +++ b/plans/runtime_config.md @@ -5,6 +5,46 @@ column rules, and destination sorting keys already exist. Current syntax and precedence live in [configuration](../docs/configuration.md). Do not duplicate that surface here +## Operator health and recovery + +Locate UI/integration owning reported sync-time and restart behavior before +treating it as a reproduced daemon defect. Repository's Grafana dashboard is +metrics-only; local tests cannot verify external UI freshness. Define status +contract for consumers before adding another control service + +Distinguish process liveness, observation freshness, source receipt, shadow +replay, and contiguous destination acknowledgement. Derive last successful +delivery from acknowledged work, never poll time or source activity. Expire +cached observations to stale/unknown; preserve last known progress and failure +reason. Keep idle, paused, blocked, recovering, and unavailable states distinct + +Expose supervisor-backed restart through owning integration, since dead daemon +cannot serve its control socket. Preserve persistent state and resume floor; +show blocked value/config diagnostics and required correction before restart. +Treat repeated deterministic rejection separately from transient reconnects + +Test daemon kill, lost status endpoint, stale cached response, ClickHouse outage, +intentional pause, and idle source. Assert UI cannot claim fresh destination +delivery while unavailable or blocked. After correction and restart, require +acknowledged catch-up before reporting recovery complete + +## Expose existing table and schema selection + +Reuse `ctl tables`, `ctl add`, namespace config, and glob rules through owning +integration. Show resolved source/destination identities, exclusions, initial +load policy, pending load state, and errors + +Define schema selection as current plus future eligible tables, with explicit +exclusions and initial-load choice. Broad scope alone does not backfill existing +rows. Preview unsupported tables before applying scope; keep commit-ordered +config and existing load mechanisms + +Test populated-table opt-in during writes, schema selection with existing and +future tables, exclusions, repeated submission, reload, and restart. Compare +historical plus concurrent rows after loads complete. Extend +[config tests](../tests/runtime_config_e2e.rs) only where coverage is missing; +carry UI acceptance into owning integration + ## Source-side commands Add WAL-carried commands only for operations that do not fit stored config, diff --git a/plans/schema.md b/plans/schema.md index 566c1236..d28060d7 100644 --- a/plans/schema.md +++ b/plans/schema.md @@ -10,6 +10,27 @@ Start in [schema comparison](../src/schema.rs), [DDL application](../src/emit/ch_ddl.rs). Current operator contract lives in [schema changes](../docs/schema-changes.md) +## Reproduced gaps + +Checked revision `1267db7` on PostgreSQL 18.6 and ClickHouse 26.8.1.951 +Use [schema harness](../tests/schema_evolution_cdc.rs) with namespace auto-create, +fresh `probe` schema per case, and `pg_switch_wal()` before and after transition +Drain pipeline, then compare source with `FINAL WHERE _is_deleted = 0` +All cases below drain successfully despite different final rows + +| Setup and transition | Observed destination | Next step | +|---|---|---| +| Create `t(id int PRIMARY KEY, v text)`, insert `(1,'before')`, rename to `renamed`, insert `(2,'after')` | Old destination contains only `(1,'before')` | Emit relation-identity event and rekey route before following rows | +| Same keyed table, update `id = 2, v = 'after'` | Both `(1,'before')` and `(2,'after')` survive | Emit old-key tombstone and new row under same commit | +| Create `t(id int, v text)` with `REPLICA IDENTITY FULL`, insert `(1,'before')`, update `v = 'after'` | Both versions survive | Reject missing stable row key until duplicate semantics are defined | +| Create unlogged keyed table, insert two rows | Destination exists but contains no rows | Reject unlogged scope before destination creation | + +Drop/recreate under default retain also leaves both generations' rows. Treat +that as selected retention policy, not unexplained loss; test explicit warn/drop +policies separately. Column rename/drop without target-name overrides, `CLUSTER`, +and automatic nullable widening converged in these probes. Keep mapped-column, pinned-type, +restart, and combined-transaction cases below + ## Relation renames Table renames, schema moves, and schema renames change only relation name @@ -61,7 +82,7 @@ warning followed by incomplete routing is not rejection | Drop NOT NULL, then insert NULL | Pinned destination type only warns, and emitter writes type default for NULL into non-Nullable columns. Reject instead of substituting | | Create an unlogged table or switch to unlogged | Persistence never reaches schema diff. Reject replication scope that cannot receive its row changes | | Attach a populated partition | Define leaf routing and initial load before accepting parent scope | -| Drop and recreate a name | Test retained destination rows under retain and warn policies | +| Drop and recreate a name | Verify warn/drop policies and restart preserve chosen generation policy | | Drop with CASCADE or a refused RESTRICT | Apply chosen dependent-object policy and preserve unaffected mappings | Column DDL must address destination columns by mapped name. `DROP COLUMN` @@ -69,8 +90,8 @@ names old source column, so a column with a configured destination name stays in ClickHouse. `RENAME COLUMN` renames to new source name while mapping takes column rule's target name -Cover column rename and drop landing in ClickHouse, and `CLUSTER`. Compare -schema-change tests against canonical source rows, not literals +Cover configured target names through column rename/drop, including restart +Compare schema-change tests against canonical source rows, not literals ## Column retype @@ -94,9 +115,10 @@ Test or document remaining cases: - `REFRESH MATERIALIZED VIEW` also fills transient heap; verify handling of refreshed rows and removal of rows absent from new contents -Cover `text` to `int` with values ClickHouse cannot parse, key column `int` to -`bigint` including ClickHouse refusal, volatile `ADD COLUMN` defaults, retype -plus rename in one statement, DML after rewriting `ALTER` in same transaction, +Existing `alter_column_type_converges_through_rewrite` passes with source +`'abc'` rewritten through `USING length(s)` and integer widening. Extend it with +key column `int` to `bigint` including ClickHouse refusal, volatile `ADD COLUMN` +defaults, retype plus rename in one statement, DML after rewriting `ALTER` in same transaction, restart during rewrite transaction, and rewrite beyond spill threshold. Compare canonical source rows against destination `FINAL` diff --git a/plans/tablespaces.md b/plans/tablespaces.md index 86fb55e0..20a61787 100644 --- a/plans/tablespaces.md +++ b/plans/tablespaces.md @@ -5,6 +5,12 @@ tablespaces can lose initial rows, and shadow recovery can encounter source paths that do not exist locally. See [page walk](../src/backfill/backup_page_walk.rs) and [shadow backup sink](../src/backfill/backup_sink.rs) +Reproduced at `1267db7`: insert two descriptors into `CatalogMap` with distinct +tablespaces and relation OIDs but identical `(db_node, rel_node)`. `len()` is 1 +and lookup returns second descriptor. Key map by full `RelFileNode` before +accepting those layouts. This is an in-process collision proof; live backup loss +and shadow path failures still need isolated tablespace fixtures + First reject unsupported layouts before bootstrap changes state. Check database default tablespace and backup tablespace metadata, including tablespaces needed by managed shadow even when their user tables are not selected. Apply equivalent @@ -18,9 +24,10 @@ identity includes tablespace, database, and filenode. Descriptor history already uses all three; audit catalog tracker and backup maps that still omit tablespace Two tablespaces may contain equal database and filenode numbers -Verify direct backup forwards every tablespace archive to its sink. Then teach -page walk to recognize those files and equivalent object-store paths. A path -parser alone cannot recover files discarded by backup transport +`DirectSource` already forwards every `BackupEvent::Archive` to its sink, but +uses `meta.oid` only for logging. Carry archive identity into sink metadata, then +teach page walk to recognize those files and equivalent object-store paths +Prove identity survives transport rather than adding another archive loop Choose a shadow-local directory mapping and use it consistently during backup restore, CREATE TABLESPACE replay, and restart. Any WAL rewrite must preserve diff --git a/plans/value_coercion.md b/plans/value_coercion.md index ffa7fc1c..a3b13349 100644 --- a/plans/value_coercion.md +++ b/plans/value_coercion.md @@ -5,8 +5,60 @@ representable values and apply configured substitutes consistently across row encoding and fast defaults. Keep correctness checks independent of new policy options +## Recover from rejected values + +At `1267db7`, PostgreSQL 18.6 / ClickHouse 26.8.1.951 WAL probes preserve +`repeat('x', n)` TEXT at 8 KiB, 32 MiB, and 128 MiB under defaults, with exact +lengths and MD5s. An explicit 64 MiB cap substitutes NULL for 128 MiB under +default overflow policy; `error` returns decoded size `134217728` and cap +`67108864`. `numeric(10,2)` NaN returns an unsupported-value error naming column +and available policies. These are pipeline outcomes, not daemon crash evidence + +Test daemon exit and restart for those rejection cases with recorded config, +exit diagnostics, and durable positions. Keep incompressible values, other load +modes, and [recovery matrix](verification.md) separate from successful TEXT case + +Keep rejection as default for non-finite Decimal values. PostgreSQL constrained +numeric accepts NaN; ClickHouse Decimal cannot represent it. Existing String +mapping and explicit `nan` substitutes provide policy choices. Do not silently +skip a row or acknowledge failed work to keep pipeline running + +Report relation, column, reason, effective policy, and blocked WAL position +without raw payload. For size rejection include decoded bytes and configured +cap. Expose a recovery path that works while daemon is down: persist corrected +local config, then restart from retained WAL. Prove repeated restart without +correction blocks again, and correction delivers failed transaction plus later +work. A later source UPDATE or WAL-carried config edit cannot unblock earlier +rejected WAL by itself + +Review default oversize substitution explicitly: NULL/type-default output is +lossy even if process stays alive. Surface effective policy and substitution +counts to operators; distinguish intentional substitution from full-value +success. Keep [large-value tests](verification.md#verify-value-fidelity-and-recovery) +independent of [TOAST reclamation](shadow_toast.md) + ## Remaining work +Native insert-tail probes using +[existing harness](../tests/emitter_native_types.rs) reproduce these gaps: + +| Input and destination | Observed result | Implementation idea | +|---|---|---| +| Numeric `1.234` into `Decimal(10,2)` | `1.23`, acknowledged | Reject nonzero division remainder during rescaling | +| Numeric `100000000.00` into `Decimal(10,2)` | `100000000`, acknowledged | Carry declared precision in `DecimalWire`; enforce scaled magnitude below `10^p` | +| `Int4(-1)` into `UInt32` | `4294967295`, acknowledged | Validate signedness and domain before copying fixed bytes | + +These probes submit decoded cells directly; add source configuration/WAL tests +before claiming every override reaches this path. Wire-width checks already +reject larger physical overflows and do not replace declared-domain checks + +Live fast-default probe also diverges: create keyed table, insert one row, then +`ADD COLUMN v numeric DEFAULT 'NaN'` and insert another row with `v = 1.25`. +Automatic `Nullable(String)` stores old row as `nan`, unlike source text `NaN`; +new finite row remains `1.25`. Render non-finite String defaults as quoted source +text before applying Decimal-specific rejection or substitutes. Current renderer +returns bare `nan` regardless of target + 1. Validate finite date/timestamp values against supported destination calendar ranges, accounting for precision and timezone. Choose and document supported server ranges before implementation; integer storage bounds are insufficient @@ -20,8 +72,9 @@ options [schema plan](schema.md) 4. Distinguish real source NULL from absent delete payload. Reject source NULL into non-nullable columns while preserving required tombstone defaults -5. Count substitutions by relation and reason. Include column and effective - policy in errors without logging raw values. Keep default substitutions +5. Extend existing aggregate `toast_values_filled_oversize_total` with relation + and reason attribution and non-finite substitution counts. Include column + and effective policy in errors without logging raw values. Keep default substitutions separate from row counts; document possible recounting on retries Apply checks across streaming, COPY, heap-page, and backup loads. Document @@ -50,6 +103,16 @@ containers for unsupported elements ## Acceptance +- Replay Decimal NaN rejection after configuring a valid substitute; verify + finite neighbors, failed row, and later transactions converge without skipping + WAL. Treat remapping an existing Decimal column to String as a schema migration, + not a config-only retry +- Preserve Float32/Float64 NaN, infinities, and both zero signs by comparing bits + with source `float4send`/`float8send`; printed zero cannot establish sign fidelity +- Verify automatic numeric mappings at precision 76/77/1000, negative scale, + scale greater than precision, and unconstrained numeric against source text + Include trailing fractional zeros, SQL NULL, and String fast default `'NaN'`; + compare decoded default semantics, not SQL spelling such as `unhex('4e614e')` - Preserve finite calendar boundaries; reject adjacent out-of-range values - Reject Decimal values exceeding declared precision even when wire width fits - Accept `1.230` at scale 2; reject `1.234`, including negative equivalents diff --git a/plans/verification.md b/plans/verification.md index d436f279..d09abcf7 100644 --- a/plans/verification.md +++ b/plans/verification.md @@ -5,6 +5,68 @@ coverage reports to find gaps instead of keeping historical line counts or lists of already covered functions. Build and test commands live in [development guide](../docs/development.md) +## Verify value fidelity and recovery + +Preserve minimized SQL, effective config, +generated destination DDL, server/build versions, process exit reason, and +durable positions in regression fixtures. Run failure cases independently; +later stale queries after daemon exit do not establish additional failures + +WAL probes at `1267db7`, PostgreSQL 18.6 / ClickHouse 26.8.1.951, establish +compressible TEXT fidelity through 128 MiB and configured 64 MiB cap behavior +See [verified value gaps](value_coercion.md). Promote those probes into persistent +regressions; remaining investigation concerns daemon recovery and wider matrix +Distinguish policy rejection, panic, OOM kill, and query timeout. For rejection, +prove durable progress cannot skip failed work, unchanged restart fails again, +and corrected policy plus restart converges. Coordinate diagnostics and policy +with [value coercion](value_coercion.md), UI checks with +[runtime configuration](runtime_config.md#operator-health-and-recovery) + +Extend existing TOAST and type suites with: + +- An 8 KiB, 32 MiB, and 128 MiB ladder for TEXT, JSON/JSONB, INTEGER[], and TEXT[], + plus values just below, at, and above configured decoded-size cap. Account for + PostgreSQL representation overhead. Exercise compressible and incompressible + payloads, NULL/default overflow and error policy, spill, and restart +- Exact source/destination lengths and content digests for strings; element + counts, order, NULL elements, and content for arrays. Test streaming and + supported initial-load/value-mode combinations. Check resident memory and + progress when one value exceeds normal batch or reserved-memory budgets +- Extend 32 MiB JSONB verification beyond successful compressible String case: + `to_jsonb(repeat('x', 33554432))` and + `jsonb_build_object('v', repeat('x', 33554432))` match canonical source lengths + and MD5s after WAL replay. Test incompressible bodies, cap boundaries, explicit + native JSON mapping, and restart. Do not infer native JSON semantics from + automatic String mapping or row count + +For a fixture containing 8 KiB, 32 MiB, and 128 MiB TEXT rows, assert total +length of `167780352` bytes after all inserts succeed. Compare every expected +key and value separately rather than combining nullable, differently typed +lengths with `greatest` + +Reproduce with `public.doc(id int PRIMARY KEY, meta text, body text)`, default +EXTENDED storage, and `REPLICA IDENTITY FULL`, using +[TOAST harness](../tests/toast_e2e.rs). Insert separate transactions containing +`repeat('x', 8192)`, `repeat('x', 33554432)`, and `repeat('x', 134217728)`, then +switch WAL and drain. Compare source `length(body), md5(body)` against destination +`length(body), lower(hex(MD5(body)))` under `FINAL WHERE _is_deleted = 0` +Repeat with `inline_value_max = 67108864` and each overflow policy. Harness +`expect` panics on returned errors do not establish daemon panics + +Cover transaction and trigger scenarios where regression coverage is missing: +TRUNCATE between writes to two tables in one spilling transaction, including +abort and restart; BEFORE-trigger rewrites and suppressed UPDATE/DELETE; and +AFTER-trigger audit rows. Derive expected audit rows from fixture operations +and assert final heap effects rather than SQL command tags. Keep numeric boundary, +signed-zero, and default checks in [value coercion](value_coercion.md#acceptance) + +Isolate repeated runs by source and destination identities. If reusing tables, +wait for cleanup to replicate and verify source emptiness: DELETE can be vetoed +by a BEFORE trigger and can itself generate audit rows. Source DROP/recreate +under retain policy does not clear destination; test that behavior separately +with [schema lifecycle](schema.md). Avoid destination-only resets while old WAL +or backfill remains active + ## Pin WAL layouts Extend generated fixtures with commit records combining subtransactions, diff --git a/src/backfill/backup_page_walk.rs b/src/backfill/backup_page_walk.rs index 88bc7ccb..94b6065d 100644 --- a/src/backfill/backup_page_walk.rs +++ b/src/backfill/backup_page_walk.rs @@ -696,15 +696,7 @@ impl BackupSink for PageWalkSink { .fetch_add(1, Ordering::Relaxed); return Ok(FileAction::Skip); } - let desc = self.catalog.get(f.db, f.filenode); - let is_toast = self.catalog.is_toast(f.db, f.filenode); - if is_toast { - self.stats - .toast_files_observed - .fetch_add(1, Ordering::Relaxed); - } else if desc.is_some() { - self.stats.files_walked.fetch_add(1, Ordering::Relaxed); - } else { + let Some(desc) = self.catalog.get(f.db, f.filenode) else { // Filenode absent from map: seed race (greenfield) or non-opted // rel (filtered backfill pass, where this is most files). Skip // drains body without page buffering; mux honours the decline @@ -712,14 +704,15 @@ impl BackupSink for PageWalkSink { .files_skipped_unknown_filenode .fetch_add(1, Ordering::Relaxed); return Ok(FileAction::Skip); - } - let Some(desc) = desc else { - // TOAST heap whose descriptor the map lacks; count pages, no walk - return Ok(FileAction::Tap(Box::new(PageWalkEntry::counting( - f.segno.saturating_mul(RELSEG_BLOCKS), - self.stats.clone(), - )))); }; + let is_toast = &*desc.rel_name.namespace == PG_TOAST_NS; + if is_toast { + self.stats + .toast_files_observed + .fetch_add(1, Ordering::Relaxed); + } else { + self.stats.files_walked.fetch_add(1, Ordering::Relaxed); + } let lsn = self .lsn_overrides .get(&(desc.rfn.db_node, desc.rfn.rel_node)) @@ -789,20 +782,6 @@ pub struct PageWalkEntry { } impl PageWalkEntry { - /// Counts pages without decoding them - fn counting(block_no: u32, stats: Arc) -> Self { - Self { - block_no, - slab: Vec::with_capacity(SLAB_BYTES + PAGE_BYTES), - spare: None, - walk: None, - out: Out::Captured(Arc::default()), - stats, - pending: None, - finished: None, - } - } - /// Collect the in-flight walk: recover its slab and ship its tuples. /// Shipping here is what carries emitter backpressure back to the read async fn join_pending(&mut self) -> io::Result<()> { @@ -1133,6 +1112,7 @@ mod tests { use super::*; use crate::backfill::backup_source::EndInfo; + use crate::backfill::backup_source::testing::expect_tap; fn ld(a: &AtomicU64) -> u64 { a.load(Ordering::Relaxed) @@ -1140,10 +1120,7 @@ mod tests { /// `begin` must tap; hands back the owned entry sink async fn tap(sink: &PageWalkSink, meta: &FileMeta) -> Box { - match sink.begin(meta).await.unwrap() { - FileAction::Tap(e) => e, - other => panic!("expected Tap for {}, got {other:?}", meta.path.display()), - } + expect_tap(sink.begin(meta).await.unwrap()) } fn heap_meta(path: &str) -> FileMeta { @@ -1601,9 +1578,7 @@ mod tests { sink.begin(&part("base/5/16400.1")).await.unwrap(), FileAction::Skip )); - let FileAction::Tap(entry) = sink.begin(&part("base/5/16400")).await.unwrap() else { - panic!("an unrecorded heap file must tap"); - }; + let entry = expect_tap(sink.begin(&part("base/5/16400")).await.unwrap()); assert!( barrier.pop_finished().await.is_none(), "nothing reports before its body ends" diff --git a/src/backfill/backup_sink.rs b/src/backfill/backup_sink.rs index 31f30412..8a877410 100644 --- a/src/backfill/backup_sink.rs +++ b/src/backfill/backup_sink.rs @@ -213,6 +213,7 @@ impl BackupSink for MultiplexSink { #[cfg(test)] mod tests { use super::*; + use crate::backfill::backup_source::testing::expect_tap; use crate::backfill::pg_path::{BaseRelFile, RelFork, is_system_dir, parse_base_path}; use std::path::{Path, PathBuf}; @@ -379,12 +380,8 @@ mod tests { ), ]; for (meta, expected) in cases { - assert_eq!( - lander.classify(&meta), - expected, - "classify({}) wrong", - meta.path.display() - ); + let path = meta.path.display(); + assert_eq!(lander.classify(&meta), expected, "classify({path}) wrong"); } } @@ -446,9 +443,7 @@ mod tests { FileAction::Keep )); - let FileAction::Tap(mut entry) = mux.begin(&file("base/5/16400")).await.unwrap() else { - panic!("user heap must tap"); - }; + let mut entry = expect_tap(mux.begin(&file("base/5/16400")).await.unwrap()); entry.chunk(&[0u8; 1024]).await.unwrap(); entry.chunk(&[1u8; 512]).await.unwrap(); entry.end().await.unwrap(); diff --git a/src/backfill/backup_source.rs b/src/backfill/backup_source.rs index 5a613f24..14dd3716 100644 --- a/src/backfill/backup_source.rs +++ b/src/backfill/backup_source.rs @@ -288,7 +288,7 @@ where } /// One tar entry through the sink. Factored so callers can drive -/// non-tar-shaped FileMeta sequences (e.g. inline symlink emission). +/// non-tar-shaped FileMeta sequences pub async fn pump_entry(body: &mut R, meta: &FileMeta, target: &PumpTarget) -> io::Result<()> where R: AsyncRead + Unpin + ?Sized, @@ -296,7 +296,7 @@ where match target.sink.begin(meta).await? { FileAction::Keep => write_kept(body, meta, &target.data_dir).await, FileAction::Skip => drain_to_void(body).await, - FileAction::Tap(entry) => stream_to_entry(body, meta, entry, &target.stats).await, + FileAction::Tap(entry) => stream_to_entry(body, entry, &target.stats).await, } } @@ -316,17 +316,12 @@ where async fn stream_to_entry( body: &mut R, - meta: &FileMeta, mut entry: Box, stats: &PumpStats, ) -> io::Result<()> where R: AsyncRead + Unpin + ?Sized, { - if !matches!(meta.kind, FileKind::File) { - drain_to_void(body).await?; - return entry.end().await; - } let mut buf = [0u8; 64 * 1024]; loop { let n = body.read(&mut buf).await?; @@ -398,36 +393,21 @@ where Ok(()) } -/// Materialize a non-default tablespace symlink and pump it through the -/// sink. Both production impls get symlinks from inside the data-dir -/// archive, so this is unused today, exposed for future LocalDir shapes. -#[allow(dead_code)] -pub(crate) async fn emit_tablespace_symlink( - tablespace: &Tablespace, - target: &PumpTarget, -) -> io::Result<()> { - if tablespace.is_default() { - return Ok(()); - } - let meta = FileMeta { - path: PathBuf::from(format!("pg_tblspc/{}", tablespace.oid)), - size: 0, - mode: 0o755, - kind: FileKind::Symlink { - target: PathBuf::from(&tablespace.location), - }, - part: None, - }; - let mut body = tokio::io::empty(); - pump_entry(&mut body, &meta, target).await -} - #[cfg(test)] pub(crate) mod testing { //! Helpers reused across crate tests. use tokio::io::AsyncWriteExt; + use super::{EntrySink, FileAction}; + + #[track_caller] + pub fn expect_tap(action: FileAction) -> Box { + assert_eq!(format!("{action:?}"), "Tap"); + let FileAction::Tap(e) = action else { panic!() }; + e + } + /// In-memory tar roughly mirroring PG's BASE_BACKUP layout: empty /// pg_replslot dir + denylist file inside, global/ catalog, /// base//, base//, pg_control last. @@ -490,11 +470,9 @@ mod tests { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum Event { - Start { start_lsn: u64, timeline: u32 }, Begin { path: PathBuf, action: &'static str }, Chunk { len: usize }, End { path: PathBuf }, - Finish { end_lsn: u64 }, } /// Owned per-entry recorder, so the sink itself stays lock-free on the @@ -526,13 +504,6 @@ mod tests { #[async_trait] impl BackupSink for Arc { - async fn start(&self, info: &StartInfo) -> io::Result<()> { - self.events.lock().unwrap().push(Event::Start { - start_lsn: info.start_lsn, - timeline: info.timeline, - }); - Ok(()) - } async fn begin(&self, meta: &FileMeta) -> io::Result { let s = meta.path.to_string_lossy(); let action = if s.starts_with("pg_replslot/") { @@ -565,12 +536,6 @@ mod tests { } Ok(action) } - async fn finish(&self, info: &EndInfo) -> io::Result<()> { - self.events.lock().unwrap().push(Event::Finish { - end_lsn: info.end_lsn, - }); - Ok(()) - } } #[tokio::test] @@ -674,16 +639,13 @@ mod tests { assert!(!data_dir.join("base/5/16400").exists()); let events = recording.events.lock().unwrap(); - // Last file event must be pg_control end (contract 3) - let last_end = events - .iter() - .rev() - .find_map(|e| match e { - Event::End { path } => Some(path.clone()), - _ => None, + // Last event must be pg_control end (contract 3) + assert_eq!( + events.last(), + Some(&Event::End { + path: PathBuf::from("pg_control") }) - .unwrap(); - assert_eq!(last_end, PathBuf::from("pg_control")); + ); // Tapped chunks sum to the file body length let tapped_bytes: usize = events .iter() diff --git a/src/backfill/bootstrap_marker.rs b/src/backfill/bootstrap_marker.rs index 29199aff..20936233 100644 --- a/src/backfill/bootstrap_marker.rs +++ b/src/backfill/bootstrap_marker.rs @@ -133,10 +133,6 @@ impl BootstrapMarker { .context("bootstrap incomplete without a resolved backup pin; use operator recovery") } - pub fn pinned_backup_name(&self) -> Result<&str> { - self.pinned_backup() - } - fn check_retry(&self) -> Result<()> { self.pinned_backup()?; anyhow::ensure!( diff --git a/src/backfill/bootstrap_window.rs b/src/backfill/bootstrap_window.rs index 40cbe682..7a9310af 100644 --- a/src/backfill/bootstrap_window.rs +++ b/src/backfill/bootstrap_window.rs @@ -444,11 +444,9 @@ mod tests { log.descriptor_at(d.rfn, from), LookupResult::Present(got) if got.rel_name == d.rel_name )); + let prefix = log.descriptor_at(d.rfn, read_start(from)); assert!( - matches!( - log.descriptor_at(d.rfn, read_start(from)), - LookupResult::Present(_) - ), + matches!(prefix, LookupResult::Present(_)), "records in the alignment prefix decode too; their commits drop on `from_lsn`", ); assert!(matches!( diff --git a/src/backfill/opt_in.rs b/src/backfill/opt_in.rs index 094a17bf..aa4620ce 100644 --- a/src/backfill/opt_in.rs +++ b/src/backfill/opt_in.rs @@ -46,9 +46,7 @@ pub trait Backfiller: Send + Sync { /// Why this backfiller cannot serve `mode`, so callers refuse the request /// before creating a destination it would leave empty - fn refuses(&self, _mode: InitialLoadMode) -> Option<&'static str> { - None - } + fn refuses(&self, mode: InitialLoadMode) -> Option<&'static str>; } /// Dispatch one `config_table` row's inclusion intent. `opt_in_lsn` is the diff --git a/src/backfill/visibility_gate.rs b/src/backfill/visibility_gate.rs index ddff49b2..133e5626 100644 --- a/src/backfill/visibility_gate.rs +++ b/src/backfill/visibility_gate.rs @@ -521,7 +521,8 @@ mod tests { use crate::backfill::backup_page_walk::{ PAGE_BYTES, PageWalkSink, make_rel, make_rel_named, synth_single_tuple_page, }; - use crate::backfill::backup_source::{BackupSink, FileAction, FileMeta, StartInfo}; + use crate::backfill::backup_source::testing::expect_tap; + use crate::backfill::backup_source::{BackupSink, FileMeta, StartInfo}; use crate::backfill::spool::DEFERRED_SPOOL_MEM_MAX; use crate::backfill::spool::SpoolMark; use crate::decode::visibility::{ @@ -593,9 +594,7 @@ mod tests { mode: 0o600, ..Default::default() }; - let FileAction::Tap(mut entry) = sink.begin(&meta).await.unwrap() else { - panic!("{path} must tap"); - }; + let mut entry = expect_tap(sink.begin(&meta).await.unwrap()); for page in 0..pages { entry.chunk(&visible_page(page as i32)).await.unwrap(); } @@ -817,13 +816,8 @@ mod tests { for (want_seq, (ack, ack_task, mut msg_rx)) in [3u64, 7].into_iter().zip(tails) { let mut seqs = Vec::new(); while let Some(msg) = msg_rx.recv().await { - match msg { - BatcherMsg::Rows(chunk) => seqs.extend(chunk.rows.iter().map(|r| r.seq)), - BatcherMsg::Row(r) => seqs.push(r.seq), - BatcherMsg::FlushAll(reply) => { - let _ = reply.send(()); - } - } + let BatcherMsg::Rows(c) = msg else { panic!() }; + seqs.extend(c.rows.iter().map(|r| r.seq)); } assert_eq!(seqs, vec![want_seq; 2], "rows rode their own lane's tail"); drop(ack); diff --git a/src/backfill/wal_replay.rs b/src/backfill/wal_replay.rs index ff5d847d..335a6512 100644 --- a/src/backfill/wal_replay.rs +++ b/src/backfill/wal_replay.rs @@ -393,9 +393,11 @@ impl WalReplaySink { rows_cursor = upto; } } - // Live stream owns DDL/config apply + // Live stream owns DDL/config apply. xl_heap_truncate carries + // no block ref, so never passes the rfn filter WalkStep::Event(DrainEntry::Catalog(_)) - | WalkStep::Event(DrainEntry::Config(_)) => {} + | WalkStep::Event(DrainEntry::Config(_)) + | WalkStep::Truncate(_) => {} WalkStep::Event(DrainEntry::ToastBarrier { toast_relid, marker_lsn, @@ -407,11 +409,6 @@ impl WalReplaySink { .await .map_err(|e| SinkError::Other(format!("toast rewrite barrier: {e}")))?; } - WalkStep::Truncate(_) => { - // xl_heap_truncate carries no block ref, never passes the - // rfn filter - debug_assert!(false, "TRUNCATE heap in gap replay"); - } WalkStep::Heap(mut heap) => { let rfn = heap.decoded.rfn; // Decode TOAST chunks, route through parent row diff --git a/src/bin/stream/main.rs b/src/bin/stream/main.rs index dbf35ad3..6a5d9dc7 100644 --- a/src/bin/stream/main.rs +++ b/src/bin/stream/main.rs @@ -31,6 +31,13 @@ compile_error!( #[global_allocator] static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc; +/// Enable every callsite so coverage runs evaluate tracing field expressions +#[cfg(test)] +#[ctor::ctor(unsafe)] +fn enable_tracing() { + let _ = tracing::subscriber::set_global_default(tracing_subscriber::registry()); +} + mod archive; mod args; mod bootstrap; diff --git a/src/bin/stream/metrics_publish.rs b/src/bin/stream/metrics_publish.rs index cd718ec7..abff654a 100644 --- a/src/bin/stream/metrics_publish.rs +++ b/src/bin/stream/metrics_publish.rs @@ -608,6 +608,10 @@ pub(crate) fn bootstrap_gauges( mod tests { use super::*; + fn db_label(oid: u32) -> String { + format!("db{oid}") + } + /// A partial publish from inside the leg must not blank the fields the /// status loop owns, or the leg would look like a dead pipeline #[tokio::test] @@ -671,7 +675,7 @@ mod tests { boundary_hold: &boundary, by_database: Vec::new(), counters: StageCounters { - db_name: &|_| String::new(), + db_name: &db_label, emitter: Some(&emitter), oracle: [None, None], bootstrap: None, @@ -715,7 +719,7 @@ mod tests { let snap = stage_gauges(&StageCounters { db_name: &|oid| match oid { 5 => "app".to_owned(), - _ => format!("db{oid}"), + _ => db_label(oid), }, emitter: Some(&emitter), oracle: [None, None], @@ -739,7 +743,7 @@ mod tests { use walshadow::record::WAL_SEG_SIZE; let emitter = EmitterStats::default(); let counters = StageCounters { - db_name: &|_| String::new(), + db_name: &db_label, emitter: Some(&emitter), oracle: [None, None], bootstrap: None, @@ -794,7 +798,7 @@ mod tests { boot_oracle.rows.fetch_add(7, Ordering::Relaxed); let snap = stage_gauges(&StageCounters { - db_name: &|_| String::new(), + db_name: &db_label, emitter: None, oracle: [Some(&live_oracle), Some(&boot_oracle)], bootstrap: None, @@ -806,7 +810,7 @@ mod tests { assert_eq!(snap.bootstrap_attempt, 2); let boot_only = stage_gauges(&StageCounters { - db_name: &|_| String::new(), + db_name: &db_label, emitter: None, oracle: [None, Some(&boot_oracle)], bootstrap: None, diff --git a/src/bin/stream/session.rs b/src/bin/stream/session.rs index 4df06958..76da235e 100644 --- a/src/bin/stream/session.rs +++ b/src/bin/stream/session.rs @@ -1364,23 +1364,25 @@ pub(crate) async fn run_session( // already moved onto the target: replay, receive, and recovery state // beside the frozen frontier they have to reach // (architecture/recovery.md) - if !paused { + if let Some((_, pause_received)) = pause_frontier { + if promotion_polled_at.is_none_or(|t| t.elapsed() >= PROMOTION_POLL) { + promotion_polled_at = Some(Instant::now()); + promotion = match tokio::time::timeout( + PROMOTION_POLL, + promotion_gate(&mut feed, pause_received), + ) + .await + { + Ok(gate) => gate, + Err(_) => { + feed.drop_sql_client(); + PromotionGate::unreachable() + } + }; + } + } else { promotion = PromotionGate::blocked("not_paused"); promotion_polled_at = None; - } else if promotion_polled_at.is_none_or(|t| t.elapsed() >= PROMOTION_POLL) { - promotion_polled_at = Some(Instant::now()); - promotion = match tokio::time::timeout( - PROMOTION_POLL, - promotion_gate(&mut feed, pause_frontier), - ) - .await - { - Ok(gate) => gate, - Err(_) => { - feed.drop_sql_client(); - PromotionGate::unreachable() - } - }; } let (shadow_agg, shadow_served_tli) = { let state = shadow_state.lock().await; @@ -1449,7 +1451,7 @@ pub(crate) async fn run_session( let chunk = tokio::select! { biased; () = shutdown.cancelled() => break "signal", - err = tasks.exited() => return Err(err), + Some(err) = tasks.exited() => return Err(err), res = &mut fsync_task => return Err(task_stopped("segment fsync", res, &fsync_fatal)), res = &mut gc_task => return Err(task_stopped("descriptor log gc", res, &gc_fatal)), // Surfaced by the check after the crossing step @@ -1461,7 +1463,7 @@ pub(crate) async fn run_session( // arm and the pump continues from the same LSN. A pending crossing // also parks it — that connection is out of COPY until the // descendant is requested. - result = async { path.archive().expect("guarded by arm").next().await }, + result = async { path.archive()?.next().await }, if matches!(path, SourcePath::Archive(_)) && !paused && !crossing.pending() => { match result { Some(Ok((start_lsn, bytes))) => { @@ -2092,13 +2094,13 @@ impl SessionTasks { self.names.get(&id).copied().unwrap_or("session") } - /// First task to stop, as the error naming it. Pending while none has - async fn exited(&mut self) -> anyhow::Error { - match self.set.join_next_with_id().await { - Some(Ok((id, ()))) => anyhow::anyhow!("{} task exited", self.name(id)), - Some(Err(e)) => anyhow::anyhow!("{} task failed: {e}", self.name(e.id())), - None => std::future::pending().await, - } + /// First task to stop, as the error naming it. `None` once the set is empty + async fn exited(&mut self) -> Option { + let res = self.set.join_next_with_id().await?; + Some(match res { + Ok((id, ())) => anyhow::anyhow!("{} task exited", self.name(id)), + Err(e) => anyhow::anyhow!("{} task failed: {e}", self.name(e.id())), + }) } /// Abort what still runs, surfacing a task that panicked @@ -2138,10 +2140,10 @@ mod tests { let mut tasks = SessionTasks::default(); tasks.spawn("idle", std::future::pending()); tasks.spawn("quits", async {}); - let err = tasks.exited().await.to_string(); + let err = tasks.exited().await.unwrap().to_string(); assert!(err.contains("quits task exited"), "{err}"); tasks.spawn("panics", async { panic!("boom") }); - let err = tasks.exited().await.to_string(); + let err = tasks.exited().await.unwrap().to_string(); assert!( err.contains("panics task failed") && err.contains("boom"), "{err}" diff --git a/src/bin/stream/source_recovery.rs b/src/bin/stream/source_recovery.rs index 25aec0b9..8980211f 100644 --- a/src/bin/stream/source_recovery.rs +++ b/src/bin/stream/source_recovery.rs @@ -73,13 +73,7 @@ pub(crate) const PROMOTION_POLL: Duration = Duration::from_secs(1); /// Read the gate off `feed`'s sidecar SQL connection. Only meaningful while /// paused: `pause_received` is the frozen head the target has to reach, and an /// unfrozen one moves under the decision. -pub(crate) async fn promotion_gate( - feed: &mut SourceFeed, - pause_frontier: Option<(u64, u64)>, -) -> PromotionGate { - let Some((_, pause_received)) = pause_frontier else { - return PromotionGate::blocked("not_paused"); - }; +pub(crate) async fn promotion_gate(feed: &mut SourceFeed, pause_received: u64) -> PromotionGate { let client = match feed.sql_client().await { Ok(c) => c, Err(e) => { @@ -418,10 +412,8 @@ pub(crate) enum SourcePath { impl SourcePath { pub(crate) fn archive(&mut self) -> Option<&mut ArchiveFeed> { - match self { - Self::Archive(a) => Some(a), - _ => None, - } + let Self::Archive(a) = self else { return None }; + Some(a) } } @@ -674,12 +666,11 @@ mod tests { let floor = Monotone::new(Pos::new(0x6200_0000)); let mut recovery = recovery(None, &floor); let history = TimelineHistory::root(1); - let Err(error) = recovery + let error = recovery .fall_back(removed_wal(), &history, 1, floor.get()) .await - else { - panic!("removed WAL without archive must fail"); - }; + .err() + .unwrap(); assert!( error .to_string() diff --git a/src/catalog/desc_log.rs b/src/catalog/desc_log.rs index 1c976e63..88cca961 100644 --- a/src/catalog/desc_log.rs +++ b/src/catalog/desc_log.rs @@ -1638,20 +1638,6 @@ impl DescriptorLogs { } found } - - /// `[source] dbname`'s log, for paths that are single-database by - /// construction (bootstrap gap replay) - pub fn primary(&self) -> &Arc { - &self.by_db.first().expect("at least one log").1 - } - - pub fn all(&self) -> impl Iterator> { - self.by_db.iter().map(|(_, log)| log) - } - - pub fn is_empty(&self) -> bool { - self.by_db.is_empty() - } } #[cfg(test)] @@ -1808,18 +1794,18 @@ mod tests { assert!(!log.is_empty()); assert_eq!(log.covered_through(), 100); assert_eq!(log.head(), 300); - match log.descriptor_at(rfn(6001), 179) { - LookupResult::Present(d) => assert_eq!(d, d1), - other => panic!("expected d1, got {other:?}"), - } - match log.descriptor_at(rfn(6001), 180) { - LookupResult::Present(d) => assert_eq!(d, d2), - other => panic!("expected d2, got {other:?}"), - } - match log.descriptor_by_oid_at(101, u64::MAX) { - LookupResult::Present(d) => assert_eq!(d, d2), - other => panic!("expected d2 by oid, got {other:?}"), - } + assert_eq!( + log.descriptor_at(rfn(6001), 179), + LookupResult::Present(d1.clone()) + ); + assert_eq!( + log.descriptor_at(rfn(6001), 180), + LookupResult::Present(d2.clone()) + ); + assert_eq!( + log.descriptor_by_oid_at(101, u64::MAX), + LookupResult::Present(d2.clone()) + ); assert_eq!(log.descriptor_at(rfn(6001), 89), LookupResult::NotCovered); assert!(log.batch_at(300).unwrap().entries.is_empty()); assert!(log.batch_at(150).is_none()); @@ -1947,14 +1933,14 @@ mod tests { log.append_batch(batch(100, vec![present(90, &a), present(90, &b)])) .await .unwrap(); - match log.descriptor_at(a.rfn, 150) { - LookupResult::Present(d) => assert_eq!(d, a), - other => panic!("expected a, got {other:?}"), - } - match log.descriptor_at(b.rfn, 150) { - LookupResult::Present(d) => assert_eq!(d, b), - other => panic!("expected b, got {other:?}"), - } + assert_eq!( + log.descriptor_at(a.rfn, 150), + LookupResult::Present(a.clone()) + ); + assert_eq!( + log.descriptor_at(b.rfn, 150), + LookupResult::Present(b.clone()) + ); // Tombstoning one chain leaves the sibling untouched log.append_batch(batch( 200, @@ -1968,10 +1954,10 @@ mod tests { .await .unwrap(); assert_eq!(log.descriptor_at(b.rfn, 200), LookupResult::Dropped); - match log.descriptor_at(a.rfn, 200) { - LookupResult::Present(d) => assert_eq!(d, a), - other => panic!("expected a to survive b's drop, got {other:?}"), - } + assert_eq!( + log.descriptor_at(a.rfn, 200), + LookupResult::Present(a.clone()) + ); } #[tokio::test(flavor = "current_thread")] @@ -2130,10 +2116,10 @@ mod tests { log.force_gc(Pos::new(100)).await.unwrap(); assert_eq!(log.floor_at_write(), 100); // Active-at-floor survives, superseded predecessor dropped - match log.descriptor_at(rfn(8400), 150) { - LookupResult::Present(d) => assert_eq!(d, d2), - other => panic!("expected d2, got {other:?}"), - } + assert_eq!( + log.descriptor_at(rfn(8400), 150), + LookupResult::Present(d2.clone()) + ); assert_eq!(log.descriptor_at(rfn(8400), 20), LookupResult::NotCovered); // Batches at/below floor exist only as entry carriers assert!(log.batch_at(20).is_none()); @@ -2141,10 +2127,10 @@ mod tests { // Survives reopen from ckpt drop(log); let log = open(tmp.path()).await; - match log.descriptor_at(rfn(8400), 150) { - LookupResult::Present(d) => assert_eq!(d, d2), - other => panic!("expected d2 post-reopen, got {other:?}"), - } + assert_eq!( + log.descriptor_at(rfn(8400), 150), + LookupResult::Present(d2.clone()) + ); } /// `maybe_gc`'s threshold counts entries compaction would actually drop, @@ -2272,10 +2258,10 @@ mod tests { assert_eq!(log.batch_at(150).unwrap().entries.len(), 1); assert!(log.batch_at(200).unwrap().entries.is_empty()); // At-floor active entry retained below - match log.descriptor_at(rfn(8700), 100) { - LookupResult::Present(d) => assert_eq!(d, d1), - other => panic!("expected d1 at floor, got {other:?}"), - } + assert_eq!( + log.descriptor_at(rfn(8700), 100), + LookupResult::Present(d1.clone()) + ); } #[tokio::test(flavor = "current_thread")] @@ -2419,10 +2405,10 @@ mod tests { b.ambiguities .push(amb(AmbiguityScope::Rfn(rfn(9200)), 200, 300)); log.append_batch(b).await.unwrap(); - match log.descriptor_at(rfn(9200), 199) { - LookupResult::Present(d) => assert_eq!(d, d1), - other => panic!("expected d1 before interval, got {other:?}"), - } + assert_eq!( + log.descriptor_at(rfn(9200), 199), + LookupResult::Present(d1.clone()) + ); // [from, through): from covered, through not assert!(matches!( log.descriptor_at(rfn(9200), 200), @@ -2432,10 +2418,10 @@ mod tests { log.descriptor_at(rfn(9200), 299), LookupResult::Ambiguous(_) )); - match log.descriptor_at(rfn(9200), 300) { - LookupResult::Present(d) => assert_eq!(d, d2), - other => panic!("expected d2 at through_lsn, got {other:?}"), - } + assert_eq!( + log.descriptor_at(rfn(9200), 300), + LookupResult::Present(d2.clone()) + ); // Chain entry inside the interval stays shadowed even though it // exists: ambiguity wins over Present assert!(matches!( @@ -2549,10 +2535,10 @@ mod tests { log.descriptor_at(rfn(9500), 120), LookupResult::Ambiguous(_) )); - match log.descriptor_at(rfn(9500), 160) { - LookupResult::Present(d2) => assert_eq!(d2, d), - other => panic!("expected retained Present past interval, got {other:?}"), - } + assert_eq!( + log.descriptor_at(rfn(9500), 160), + LookupResult::Present(d.clone()) + ); } #[tokio::test(flavor = "current_thread")] diff --git a/src/catalog/shadow.rs b/src/catalog/shadow.rs index 00330041..c663142e 100644 --- a/src/catalog/shadow.rs +++ b/src/catalog/shadow.rs @@ -352,6 +352,7 @@ impl Shadow { "--encoding=UTF8", "--locale=C", "--no-instructions", + "--no-sync", ], )?; Ok(()) @@ -431,7 +432,8 @@ impl Shadow { max_prepared_transactions = {max_prepared_transactions}\n\ max_locks_per_transaction = {max_locks_per_transaction}\n\ restore_command = 'cp {filter_dir}/%f %p'\n\ - recovery_target_timeline = 'latest'\n", + recovery_target_timeline = 'latest'\n\ + wal_retrieve_retry_interval = '100ms'\n", sock = self.config.socket_str(), port = self.config.port, max_connections = floor.max_connections, diff --git a/src/catalog/shadow_catalog.rs b/src/catalog/shadow_catalog.rs index b8b0c119..459a31ab 100644 --- a/src/catalog/shadow_catalog.rs +++ b/src/catalog/shadow_catalog.rs @@ -276,12 +276,6 @@ impl ShadowCatalog { query_with_reconnect!(self, query, statement, params) } - /// Last observed `pg_last_wal_replay_lsn()` (None until shadow replays - /// anything, e.g. fresh standby start). - pub fn last_observed_replay(&self) -> Option { - self.last_replay_lsn - } - /// Wait until shadow's replay LSN ≥ `target`, returning the deciding poll's /// LSN. `target = 0` returns on the first non-zero LSN. /// diff --git a/src/decode/codecs/numeric.rs b/src/decode/codecs/numeric.rs index 5cc9731d..e716d2c3 100644 --- a/src/decode/codecs/numeric.rs +++ b/src/decode/codecs/numeric.rs @@ -350,10 +350,14 @@ mod tests { #[test] fn numeric_long_form_truncated_body() { let body = NUMERIC_POS.to_le_bytes().to_vec(); - match decode_numeric(&body) { - Err(CodecError::Truncated { offset: 2, .. }) => (), - other => panic!("expected Truncated at offset 2, got {other:?}"), - } + assert_eq!( + decode_numeric(&body), + Err(CodecError::Truncated { + offset: 2, + need: 4, + have: 2, + }) + ); } #[test] diff --git a/src/decode/decoder_sink.rs b/src/decode/decoder_sink.rs index ccd1930c..130099df 100644 --- a/src/decode/decoder_sink.rs +++ b/src/decode/decoder_sink.rs @@ -294,9 +294,6 @@ mod tests { #[test] fn observer_error_wraps_to_sink_other() { let e: SinkError = DecoderSinkError::Observer("boom".into()).into(); - match e { - SinkError::Other(msg) => assert!(msg.contains("boom"), "{msg}"), - other => panic!("expected Other, got {other:?}"), - } + assert_eq!(format!("{e:?}"), r#"Other("observer: boom")"#); } } diff --git a/src/decode/heap_decoder.rs b/src/decode/heap_decoder.rs index d619ec66..99460400 100644 --- a/src/decode/heap_decoder.rs +++ b/src/decode/heap_decoder.rs @@ -416,9 +416,6 @@ pub fn decode_heap_record( rel: &RelDescriptor, ) -> Result { let rm = record.header.resource_manager_id; - if rm != RmId::Heap as u8 && rm != RmId::Heap2 as u8 { - return Ok(SmallVec::new()); - } let info_op = record.header.info & XLOG_HEAP_OPMASK; let rfn = record .blocks @@ -427,25 +424,24 @@ pub fn decode_heap_record( .unwrap_or_default(); let xid = record.header.xact_id; - if rm == RmId::Heap as u8 { - match info_op { - XLOG_HEAP_INSERT => Ok(smallvec![decode_insert(record, source_lsn, rfn, xid, rel)?]), - XLOG_HEAP_UPDATE => Ok(smallvec![decode_update( - record, source_lsn, rfn, xid, rel, false, - )?]), - XLOG_HEAP_HOT_UPDATE => Ok(smallvec![decode_update( - record, source_lsn, rfn, xid, rel, true, - )?]), - XLOG_HEAP_DELETE => Ok(smallvec![ - decode_delete(record, source_lsn, rfn, xid, rel,)? - ]), - _ => Ok(SmallVec::new()), - } - } else { - match info_op { - XLOG_HEAP2_MULTI_INSERT => decode_multi_insert(record, source_lsn, rfn, xid, rel), - _ => Ok(SmallVec::new()), - } + if rm == RmId::Heap2 as u8 && info_op == XLOG_HEAP2_MULTI_INSERT { + return decode_multi_insert(record, source_lsn, rfn, xid, rel); + } + if rm != RmId::Heap as u8 { + return Ok(SmallVec::new()); + } + match info_op { + XLOG_HEAP_INSERT => Ok(smallvec![decode_insert(record, source_lsn, rfn, xid, rel)?]), + XLOG_HEAP_UPDATE => Ok(smallvec![decode_update( + record, source_lsn, rfn, xid, rel, false, + )?]), + XLOG_HEAP_HOT_UPDATE => Ok(smallvec![decode_update( + record, source_lsn, rfn, xid, rel, true, + )?]), + XLOG_HEAP_DELETE => Ok(smallvec![ + decode_delete(record, source_lsn, rfn, xid, rel,)? + ]), + _ => Ok(SmallVec::new()), } } @@ -990,10 +986,10 @@ fn decoded_size_or_skip(att: &RelAttr) -> Option { /// we can't peek to detect a short varlena header (`att_align_pointer`). fn align_for(cur: usize, attalign: char) -> usize { match attalign { - 'c' => cur, 's' => align_up(cur, 2), 'i' => align_up(cur, 4), 'd' => align_up(cur, 8), + // TYPALIGN_CHAR, unknown: no-align rather than panic _ => cur, } } @@ -1022,11 +1018,10 @@ fn att_align_nominal( return cur_offset; } match attalign { - 'c' => cur_offset, // TYPALIGN_CHAR 's' => align_up(cur_offset, 2), // TYPALIGN_SHORT 'i' => align_up(cur_offset, 4), // TYPALIGN_INT 'd' => align_up(cur_offset, 8), // TYPALIGN_DOUBLE - _ => cur_offset, // unknown: no-align rather than panic + _ => cur_offset, // TYPALIGN_CHAR, unknown: no-align } } @@ -1425,11 +1420,8 @@ pub(crate) fn take_toast_chunk_columns( let &ColumnValue::Int4(chunk_seq) = cols[1].as_ref()? else { return None; }; - let chunk_data = match cols[2].take()? { - ColumnValue::Bytea(b) => b, - // Text-typed toast chunk: re-encode to bytes (not a normal flow) - ColumnValue::Text(s) => s.into_bytes(), - _ => return None, + let ColumnValue::Bytea(chunk_data) = cols[2].take()? else { + return None; }; Some((chunk_id, chunk_seq as u32, chunk_data)) } @@ -1626,12 +1618,11 @@ mod tests { assert_eq!(v, ColumnValue::Text("cd".into())); assert_eq!(n, 3); // no terminator before buffer end - match decode_cstring(b"hello", 0) { - Err(DecodeError::Truncated { offset, need, have }) => { - assert_eq!((offset, need, have), (0, 1, 0)); - } - other => panic!("expected Truncated, got {other:?}"), - } + let err = decode_cstring(b"hello", 0).unwrap_err(); + assert_eq!( + format!("{err:?}"), + "Truncated { offset: 0, need: 1, have: 0 }" + ); } #[test] @@ -1944,15 +1935,15 @@ mod tests { let rec = record_with(RmId::Heap, XLOG_HEAP_INSERT, main_data, payload); let out = decode_heap_record(&rec, 0, &rel).unwrap().remove(0); let new = out.new.unwrap(); - match &new.columns[0] { - Some(ColumnValue::ExternalToast(p)) => { - assert_eq!(p.va_rawsize, 12345); - assert_eq!(p.va_extinfo, 678); - assert_eq!(p.va_valueid, 12); - assert_eq!(p.va_toastrelid, 99); - } - other => panic!("expected ExternalToast, got {other:?}"), - } + assert_eq!( + new.columns[0], + Some(ColumnValue::ExternalToast(ToastPointer { + va_rawsize: 12345, + va_extinfo: 678, + va_valueid: 12, + va_toastrelid: 99, + })) + ); } #[test] @@ -1971,13 +1962,13 @@ mod tests { let rec = record_with(RmId::Heap, XLOG_HEAP_INSERT, main_data, payload); let out = decode_heap_record(&rec, 0, &rel).unwrap().remove(0); let new = out.new.unwrap(); - match &new.columns[0] { - Some(ColumnValue::PgPending { type_oid, raw }) => { - assert_eq!(*type_oid, TSVECTOROID); - assert_eq!(raw.as_slice(), body); - } - other => panic!("expected PgPending, got {other:?}"), - } + assert_eq!( + new.columns[0], + Some(ColumnValue::PgPending { + type_oid: TSVECTOROID, + raw: body.to_vec(), + }) + ); } #[test] @@ -2180,13 +2171,14 @@ mod tests { TSVECTOROID, &short_varlena(&body), ))); - match missing_value_for(&a) { - ColumnValue::PgPending { type_oid, raw } => { - assert_eq!(type_oid, TSVECTOROID); - assert_eq!(raw, body); + // Tier-3 fast default stays pending + assert_eq!( + missing_value_for(&a), + ColumnValue::PgPending { + type_oid: TSVECTOROID, + raw: body.to_vec(), } - other => panic!("tier-3 fast default must stay pending, got {other:?}"), - } + ); } #[test] diff --git a/src/emit/ch_ddl.rs b/src/emit/ch_ddl.rs index f949c3fe..d74e8bcb 100644 --- a/src/emit/ch_ddl.rs +++ b/src/emit/ch_ddl.rs @@ -490,23 +490,18 @@ impl DdlApplicator { self.execute(&sql).await?; self.stats.alters_applied += 1; } - for attnum in &diff.dropped_columns { - // diff lists attnums only; resolve CH column name from old descriptor - let name = old - .attributes - .iter() - .find(|a| a.attnum == *attnum) - .map(|a| a.name.clone()); - let Some(name) = name else { - self.stats.skipped += 1; - continue; - }; + // diff lists attnums only; resolve CH column name from old descriptor + let dropped = old + .attributes + .iter() + .filter(|a| diff.dropped_columns.contains(&a.attnum)); + for att in dropped { // Surface the drop on CH even if TOML still references the // column; emitter then encodes NULL for the vanished attnum let sql = format!( "ALTER TABLE {} DROP COLUMN IF EXISTS {}", target, - quote_ident(&name) + quote_ident(&att.name) ); self.execute(&sql).await?; self.stats.alters_applied += 1; diff --git a/src/emit/ch_emitter.rs b/src/emit/ch_emitter.rs index 325bd4b4..8a495ccc 100644 --- a/src/emit/ch_emitter.rs +++ b/src/emit/ch_emitter.rs @@ -1455,16 +1455,6 @@ pub(crate) enum DecimalWidth { } impl DecimalWidth { - fn from_elem_size(size: usize) -> Option { - Some(match size { - 4 => Self::D32, - 8 => Self::D64, - 16 => Self::D128, - 32 => Self::D256, - _ => return None, - }) - } - fn bytes(self) -> usize { self as usize } @@ -1738,8 +1728,18 @@ impl ColumnBuf { } } - fn append_null(&mut self) -> Result<(), EmitterError> { + /// Use CH type defaults for non-nullable columns, NULL otherwise + fn append_default(&mut self) { match self { + Self::Fixed { width, bytes } => bytes.extend(std::iter::repeat_n(0u8, *width)), + Self::String { + offsets, + data, + absent, + } => { + data.extend_from_slice(absent); + offsets.push(data.len() as u64); + } Self::NullableFixed { width, null_map, @@ -1747,7 +1747,6 @@ impl ColumnBuf { } => { null_map.push(1); inner.extend(std::iter::repeat_n(0u8, *width)); - Ok(()) } Self::NullableString { offsets, @@ -1758,29 +1757,8 @@ impl ColumnBuf { null_map.push(1); data.extend_from_slice(absent); offsets.push(data.len() as u64); - Ok(()) } - Self::Oracle(o) => { - o.push(OracleCell::Default); - Ok(()) - } - _ => Err(unsupported("NULL for non-Nullable column")), - } - } - - /// Use CH type defaults for non-nullable columns, NULL otherwise - fn append_default(&mut self) { - match self { - Self::Fixed { width, bytes } => bytes.extend(std::iter::repeat_n(0u8, *width)), - Self::String { - offsets, - data, - absent, - } => { - data.extend_from_slice(absent); - offsets.push(data.len() as u64); - } - nullable => nullable.append_null().expect("nullable shape takes NULL"), + Self::Oracle(o) => o.push(OracleCell::Default), } } @@ -1969,11 +1947,9 @@ impl TableEncoder { continue; } match value_of(col).map(|v| col_plan.non_finite.substitute(v)) { - // Absent / NULL coerces: Nullable target takes NULL, - // non-Nullable the type default. Covers key-only delete - // tombstones under non-FULL replica identity and NULL - // source values mapped onto non-Nullable columns - None | Some(ColumnValue::Null) => buf.append_default(), + // Absent coerces like NULL, covers key-only delete + // tombstones under non-FULL replica identity + None => buf.append_default(), Some(v) => encode_value(buf, v, col_plan.decimal, col_plan.datetime64_scale) .map_err(|e| name_column(e, &col.target_name))?, } @@ -2054,28 +2030,25 @@ pub(crate) fn build_leaf( }) } -/// Borrow leaf from storage that outlives returned root +/// Borrow leaf from storage that outlives returned root, `None` for oracle +/// columns which carry no local wire shape pub(crate) fn build_root<'b>( buf: &'b ColumnBuf, leaf: Option<&'b ColumnBuilder<'b>>, n_rows: usize, -) -> Result, EmitterError> { +) -> Result>, EmitterError> { let wrap = |null_map: &'b [u8]| -> Result, EmitterError> { let leaf = leaf.ok_or_else(|| EmitterError::Type("nullable column without leaf".into()))?; Ok(leaf.nullable(null_map)?) }; - Ok(match buf { + Ok(Some(match buf { ColumnBuf::Fixed { width, bytes } => ColumnBuilder::fixed(bytes, *width, n_rows)?, ColumnBuf::String { offsets, data, .. } => ColumnBuilder::string(offsets, data, n_rows)?, ColumnBuf::NullableFixed { null_map, .. } | ColumnBuf::NullableString { null_map, .. } => { wrap(null_map)? } - ColumnBuf::Oracle(_) => { - return Err(EmitterError::Type( - "oracle column has no local wire shape".into(), - )); - } - }) + ColumnBuf::Oracle(_) => return Ok(None), + })) } fn push_fixed(buf: &mut ColumnBuf, le: &[u8]) -> Result<(), EmitterError> { @@ -2086,15 +2059,16 @@ fn push_fixed(buf: &mut ColumnBuf, le: &[u8]) -> Result<(), EmitterError> { /// Peels one `Nullable` layer like [`ColumnBuf::new_for_ast`]. fn decimal_wire_of(ast: &TypeAst) -> Option { let inner = types::strip_nullable(ast.view()); - if !matches!( - inner.kind(), - Some(Kind::Decimal32 | Kind::Decimal64 | Kind::Decimal128 | Kind::Decimal256) - ) { - return None; - } + let width = match inner.kind()? { + Kind::Decimal32 => DecimalWidth::D32, + Kind::Decimal64 => DecimalWidth::D64, + Kind::Decimal128 => DecimalWidth::D128, + Kind::Decimal256 => DecimalWidth::D256, + _ => return None, + }; Some(DecimalWire { scale: u8::try_from(inner.decimal_scale()).ok()?, - width: DecimalWidth::from_elem_size(inner.elem_size())?, + width, }) } @@ -2130,13 +2104,16 @@ fn override_wire(default: &TypeAst, over: &TypeAst) -> Option Some(None), - (WireShape::Fixed(w), Kind::Int32 | Kind::Int64 | Kind::Int128 | Kind::Int256) => { - Some(Some(DecimalWire { - scale: 0, - width: DecimalWidth::from_elem_size(w)?, - })) + (WireShape::Fixed(_), kind) => { + let width = match kind { + Kind::Int32 => DecimalWidth::D32, + Kind::Int64 => DecimalWidth::D64, + Kind::Int128 => DecimalWidth::D128, + Kind::Int256 => DecimalWidth::D256, + _ => return None, + }; + Some(Some(DecimalWire { scale: 0, width })) } - _ => None, }; } match (wire_shape_of(default)?.0, wire_shape_of(over)?.0) { @@ -2352,7 +2329,11 @@ fn encode_value( timestamp_scale: i32, ) -> Result<(), EmitterError> { match v { - ColumnValue::Null => buf.append_null(), + // Non-Nullable target takes type default + ColumnValue::Null => { + buf.append_default(); + Ok(()) + } ColumnValue::Bool(b) => buf.append_fixed_bytes(&[*b as u8]), ColumnValue::Char(c) => buf.append_fixed_bytes(&c.to_le_bytes()), ColumnValue::Int2(n) => buf.append_fixed_bytes(&n.to_le_bytes()), @@ -2592,6 +2573,75 @@ crate::atomic_stats! { } } +/// Comparable view of a buffer's wire content +#[cfg(test)] +#[derive(Debug, PartialEq)] +pub(crate) enum Wire<'a> { + Fixed { + width: usize, + bytes: &'a [u8], + }, + String { + offsets: &'a [u64], + data: &'a [u8], + absent: &'a [u8], + }, + NullableFixed { + width: usize, + null_map: &'a [u8], + inner: &'a [u8], + }, + NullableString { + offsets: &'a [u64], + data: &'a [u8], + null_map: &'a [u8], + absent: &'a [u8], + }, + Oracle(&'a [OracleCell]), +} + +#[cfg(test)] +impl ColumnBuf { + pub(crate) fn wire(&self) -> Wire<'_> { + match self { + Self::Fixed { width, bytes } => Wire::Fixed { + width: *width, + bytes, + }, + Self::String { + offsets, + data, + absent, + } => Wire::String { + offsets, + data, + absent, + }, + Self::NullableFixed { + width, + null_map, + inner, + } => Wire::NullableFixed { + width: *width, + null_map, + inner, + }, + Self::NullableString { + offsets, + data, + null_map, + absent, + } => Wire::NullableString { + offsets, + data, + null_map, + absent, + }, + Self::Oracle(o) => Wire::Oracle(o.cells()), + } + } +} + impl std::fmt::Debug for ColumnBuf { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -2783,10 +2833,8 @@ mod tests { #[test] fn decimal_type_error_wraps_message_in_type_variant() { - match decimal_type_error("scale out of range") { - EmitterError::Type(msg) => assert_eq!(msg, "scale out of range"), - other => panic!("expected Type, got {other:?}"), - } + let e = decimal_type_error("scale out of range"); + assert_eq!(format!("{e:?}"), r#"Type("scale out of range")"#); } #[test] @@ -2810,10 +2858,7 @@ mod tests { #[test] fn emitter_error_converts_into_decoder_observer_error() { let d: DecoderSinkError = EmitterError::Type("nope".into()).into(); - match d { - DecoderSinkError::Observer(msg) => assert!(msg.contains("nope"), "{msg}"), - other => panic!("expected Observer, got {other:?}"), - } + assert_eq!(format!("{d:?}"), r#"Observer("type: nope")"#); } fn mk_mapping() -> TableMapping { @@ -3013,13 +3058,15 @@ mod tests { let mut buf = OracleColumnBuf::string(0, -1); buf.push(OracleCell::Literal(b"a".to_vec())); buf.push(OracleCell::Default); - match literal_column(&buf, 2) { - Some(ColumnBuf::String { offsets, data, .. }) => { - assert_eq!(data, b"a"); - assert_eq!(offsets, [1, 1]); + let lit = literal_column(&buf, 2).expect("renders"); + assert_eq!( + lit.wire(), + Wire::String { + offsets: &[1, 1], + data: b"a", + absent: b"", } - other => panic!("got {other:?}"), - } + ); for reject in ["Array(String)", "LowCardinality(String)", "JSON", "Int32"] { let mut buf = OracleColumnBuf::new( 0, @@ -3102,34 +3149,38 @@ mod tests { ); let (buffers, rows) = enc.take_block().unwrap(); assert_eq!(rows, 2); - let ColumnBuf::Oracle(o) = &buffers[1] else { - panic!("oracle column") - }; - assert!(literal_column(o, rows).is_some()); + assert!(matches!(buffers[1].wire(), Wire::Oracle(cells) if cells.len() == 2)); + let oracle = buffers.iter().find_map(|b| match b { + ColumnBuf::Oracle(o) => Some(o), + _ => None, + }); + assert!(literal_column(oracle.expect("oracle column"), rows).is_some()); } #[test] fn new_for_ast_picks_shape_from_chc_type_kind() { let alloc = Allocator::global(&mimalloc::MiMalloc); let cases = [ - ("Int32", "Fixed"), - ("String", "String"), - ("Nullable(Int64)", "NullableFixed"), - ("Nullable(String)", "NullableString"), - ("FixedString(7)", "Fixed"), - ("Nullable(FixedString(7))", "NullableFixed"), + ("Int32", "Fixed { width: 4, bytes_len: 0 }"), + ("String", "String { rows: 0, data_len: 0 }"), + ( + "Nullable(Int64)", + "NullableFixed { width: 8, rows: 0, inner_len: 0 }", + ), + ( + "Nullable(String)", + "NullableString { rows: 0, offsets_len: 0, data_len: 0 }", + ), + ("FixedString(7)", "Fixed { width: 7, bytes_len: 0 }"), + ( + "Nullable(FixedString(7))", + "NullableFixed { width: 7, rows: 0, inner_len: 0 }", + ), ]; - for (name, tag) in cases { + for (name, shape) in cases { let ast = TypeAst::parse(name, alloc).expect("parses"); let buf = ColumnBuf::new_for_ast(&ast).expect("shape"); - let actual = match buf { - ColumnBuf::Fixed { .. } => "Fixed", - ColumnBuf::String { .. } => "String", - ColumnBuf::NullableFixed { .. } => "NullableFixed", - ColumnBuf::NullableString { .. } => "NullableString", - ColumnBuf::Oracle(_) => "Oracle", - }; - assert_eq!(actual, tag, "{name}"); + assert_eq!(format!("{buf:?}"), shape, "{name}"); } } @@ -3199,13 +3250,13 @@ mod tests { 6, ) .unwrap(); - match &buf { - ColumnBuf::Fixed { width, bytes } => { - assert_eq!(*width, 8); - assert_eq!(bytes.as_slice(), &150i64.to_le_bytes()); + assert_eq!( + buf.wire(), + Wire::Fixed { + width: 8, + bytes: &150i64.to_le_bytes(), } - _ => panic!("expected fixed-shape buffer"), - } + ); let mut buf_nan = ColumnBuf::new_for_ast(&ast).unwrap(); assert!( encode_value( @@ -3227,31 +3278,35 @@ mod tests { }) ); let mut wide_buf = ColumnBuf::new_for_ast(&wide_ast).unwrap(); + let wide_text = "123456789012345678901234567890123456789012345678.12"; encode_value( &mut wide_buf, - &ColumnValue::Numeric(NumericKind::Finite( - "123456789012345678901234567890123456789012345678.12".into(), - )), + &ColumnValue::Numeric(NumericKind::Finite(wide_text.into())), wide_decimal, 6, ) .unwrap(); - match &wide_buf { - ColumnBuf::Fixed { width, bytes } => { - assert_eq!(*width, 32); - assert_eq!(bytes.len(), 32); - assert!(bytes[16..32].iter().any(|b| *b != 0)); + let wide_le = decimal_text_to_scaled_le(wide_text, 2, DecimalWidth::D256).unwrap(); + assert!(wide_le[16..32].iter().any(|b| *b != 0)); + assert_eq!( + wide_buf.wire(), + Wire::Fixed { + width: 32, + bytes: &wide_le, } - _ => panic!("expected fixed-shape buffer"), - } + ); let sast = TypeAst::parse("String", alloc).unwrap(); let mut sbuf = ColumnBuf::new_for_ast(&sast).unwrap(); encode_value(&mut sbuf, &ColumnValue::Numeric(NumericKind::NaN), None, 6).unwrap(); - match &sbuf { - ColumnBuf::String { data, .. } => assert_eq!(data.as_slice(), b"NaN"), - _ => panic!("expected string-shape buffer"), - } + assert_eq!( + sbuf.wire(), + Wire::String { + offsets: &[3], + data: b"NaN", + absent: b"", + } + ); } #[test] @@ -3261,13 +3316,13 @@ mod tests { let ast = TypeAst::parse("Time64(6)", alloc).unwrap(); let mut buf = ColumnBuf::new_for_ast(&ast).unwrap(); encode_value(&mut buf, &ColumnValue::Time(micros), None, 6).unwrap(); - match &buf { - ColumnBuf::Fixed { width, bytes } => { - assert_eq!(*width, 8); - assert_eq!(bytes.as_slice(), µs.to_le_bytes()); + assert_eq!( + buf.wire(), + Wire::Fixed { + width: 8, + bytes: µs.to_le_bytes(), } - _ => panic!("expected fixed-shape buffer"), - } + ); let sast = TypeAst::parse("String", alloc).unwrap(); let mut sbuf = ColumnBuf::new_for_ast(&sast).unwrap(); encode_value( @@ -3280,10 +3335,14 @@ mod tests { 6, ) .unwrap(); - match &sbuf { - ColumnBuf::String { data, .. } => assert_eq!(data.as_slice(), b"12:34:56+02"), - _ => panic!("expected string-shape buffer"), - } + assert_eq!( + sbuf.wire(), + Wire::String { + offsets: &[11], + data: b"12:34:56+02", + absent: b"", + } + ); } #[test] @@ -3503,14 +3562,15 @@ mod tests { let result = encoder.append_row(&row, &mapping, OP_INSERT); if substitute { result.unwrap(); - let ColumnBuf::NullableFixed { - null_map, inner, .. - } = &encoder.buffers[0] - else { - panic!("nullable temporal buffer") - }; - assert_eq!(null_map, &[1]); - assert!(inner.iter().all(|b| *b == 0)); + let width = rel.attributes[0].type_len as usize; + assert_eq!( + encoder.buffers[0].wire(), + Wire::NullableFixed { + width, + null_map: &[1], + inner: &vec![0; width], + } + ); } else { let err = result.unwrap_err().to_string(); assert!( @@ -3549,18 +3609,18 @@ mod tests { )); encoder.append_row(&row, &mapping, OP_INSERT).unwrap(); } - let ColumnBuf::NullableFixed { - null_map, inner, .. - } = &encoder.buffers[0] - else { - panic!("nullable timestamp buffer") - }; - assert_eq!(null_map, &[0, 0, 0]); let expected: Vec = [-1i64, 0, 946_684_800] .into_iter() .flat_map(|s| (s * 10i64.pow(scale)).to_le_bytes()) .collect(); - assert_eq!(inner, &expected); + assert_eq!( + encoder.buffers[0].wire(), + Wire::NullableFixed { + width: 8, + null_map: &[0, 0, 0], + inner: &expected, + } + ); } for us in [-1001, -1, 1, 1001] { let err = timestamp_ticks(us - DATETIME64_PG_EPOCH_US, 3).unwrap_err(); @@ -3665,10 +3725,13 @@ mod tests { ); let mut enc = TableEncoder::new(plan).unwrap(); enc.append_row(&row, &m, OP_INSERT).unwrap(); - let ColumnBuf::Fixed { bytes, .. } = &enc.buffers[0] else { - panic!("fixed-shape buffer") - }; - assert_eq!(bytes.as_slice(), &0i64.to_le_bytes()); + assert_eq!( + enc.buffers[0].wire(), + Wire::Fixed { + width: 8, + bytes: &0i64.to_le_bytes(), + } + ); } #[test] @@ -3725,13 +3788,13 @@ mod tests { .unwrap(); // _is_deleted is the trailing buffer: 0 for insert, 1 for delete let last = enc.buffers.len() - 1; - match &enc.buffers[last] { - ColumnBuf::Fixed { bytes, width } => { - assert_eq!(*width, 1); - assert_eq!(bytes, &[0u8, 1]); + assert_eq!( + enc.buffers[last].wire(), + Wire::Fixed { + width: 1, + bytes: &[0, 1], } - other => panic!("_is_deleted expected Fixed(1), got {other:?}"), - } + ); } #[test] @@ -3758,24 +3821,22 @@ mod tests { Some(ColumnValue::Json(r#"{"a": 1}"#.into())); enc.append_row(&doc, &m, OP_INSERT).unwrap(); enc.append_row(&committed(2, None), &m, OP_INSERT).unwrap(); - match &enc.buffers[1] { - ColumnBuf::String { + let (offsets, data, absent) = (&[8, 10][..], &br#"{"a": 1}{}"#[..], &b"{}"[..]); + let expected = if target == "JSON" { + Wire::String { offsets, data, absent, } - | ColumnBuf::NullableString { + } else { + Wire::NullableString { offsets, data, + null_map: &[0, 1], absent, - .. - } => { - assert_eq!(*absent, b"{}", "{target}"); - assert_eq!(data.as_slice(), br#"{"a": 1}{}"#, "{target}"); - assert_eq!(offsets, &[8, 10], "{target}"); } - other => panic!("{target} took no local shape: {other:?}"), - } + }; + assert_eq!(enc.buffers[1].wire(), expected, "{target}"); } } @@ -3867,13 +3928,14 @@ mod tests { enc.append_row(&committed_delete(3), &m, OP_DELETE).unwrap(); // Insert: genuine NULL mapped onto the non-Nullable column enc.append_row(&committed(4, None), &m, OP_INSERT).unwrap(); - match &enc.buffers[1] { - ColumnBuf::String { offsets, data, .. } => { - assert_eq!(offsets, &[0u64, 0]); - assert!(data.is_empty()); + assert_eq!( + enc.buffers[1].wire(), + Wire::String { + offsets: &[0, 0], + data: b"", + absent: b"", } - other => panic!("name expected String, got {other:?}"), - } + ); } #[test] @@ -3891,10 +3953,15 @@ mod tests { .expect("plan builds"); let mut enc = TableEncoder::new(plan).unwrap(); enc.append_row(&committed_delete(3), &m, OP_DELETE).unwrap(); - match &enc.buffers[1] { - ColumnBuf::NullableString { null_map, .. } => assert_eq!(null_map, &[1u8]), - other => panic!("name expected NullableString, got {other:?}"), - } + assert_eq!( + enc.buffers[1].wire(), + Wire::NullableString { + offsets: &[0], + data: b"", + null_map: &[1], + absent: b"", + } + ); } #[test] @@ -3917,44 +3984,39 @@ mod tests { enc.append_row(&committed(9, Some("nine")), &m, OP_INSERT) .unwrap(); assert_eq!(enc.rows, 3); - match &enc.buffers[0] { - ColumnBuf::Fixed { bytes, width } => { - assert_eq!(*width, 4); - assert_eq!(bytes.len(), 12); - assert_eq!(&bytes[0..4], &7i32.to_le_bytes()); - assert_eq!(&bytes[4..8], &8i32.to_le_bytes()); - assert_eq!(&bytes[8..12], &9i32.to_le_bytes()); + let ids: Vec = [7i32, 8, 9].iter().flat_map(|n| n.to_le_bytes()).collect(); + assert_eq!( + enc.buffers[0].wire(), + Wire::Fixed { + width: 4, + bytes: &ids, } - other => panic!("col 0 expected Fixed, got {other:?} variant tag"), - } - match &enc.buffers[1] { - ColumnBuf::NullableString { - offsets, - data, - null_map, - .. - } => { - assert_eq!(null_map, &[0u8, 1, 0]); - assert_eq!(offsets, &[5u64, 5, 9]); - assert_eq!(&data[..], b"sevennine"); + ); + assert_eq!( + enc.buffers[1].wire(), + Wire::NullableString { + offsets: &[5, 5, 9], + data: b"sevennine", + null_map: &[0, 1, 0], + absent: b"", } - other => panic!("col 1 expected NullableString, got {other:?} variant tag"), - } + ); let off = m.columns.len(); - match &enc.buffers[off] { - ColumnBuf::Fixed { bytes, .. } => { - assert_eq!(bytes.len(), 24); - assert_eq!(&bytes[0..8], &0xCAFEu64.to_le_bytes()); + let lsns = 0xCAFEu64.to_le_bytes().repeat(3); + assert_eq!( + enc.buffers[off].wire(), + Wire::Fixed { + width: 8, + bytes: &lsns, } - other => panic!("_lsn expected Fixed, got {other:?} variant tag"), - } - match &enc.buffers[off + 3] { - ColumnBuf::Fixed { bytes, width } => { - assert_eq!(*width, 1); - assert_eq!(bytes, &[0u8, 0, 0]); + ); + assert_eq!( + enc.buffers[off + 3].wire(), + Wire::Fixed { + width: 1, + bytes: &[0, 0, 0], } - _ => panic!("_is_deleted expected Fixed"), - } + ); } #[test] @@ -4706,8 +4768,8 @@ mod tests { #[test] fn config_backup_s3_static_creds() { - use walrus::config::StorageSettings; - use walrus::storage::s3::CredentialSource; + use walrus::config::StorageSettings::S3; + use walrus::storage::s3::CredentialSource::Static; let c = EmitterConfig::from_toml_str( "[backup]\n\ archive = \"s3://my-bucket/walshadow/prefix\"\n\ @@ -4718,33 +4780,25 @@ mod tests { secret_key = \"SK\"\n", ) .unwrap(); - let s3 = match c.backup.expect("backup set").storage { - StorageSettings::S3(s3) => s3, - other => panic!("expected S3, got {other:?}"), - }; + let storage = c.backup.expect("backup set").storage; + let S3(s3) = storage else { panic!() }; assert_eq!(s3.bucket, "my-bucket"); assert_eq!(s3.prefix, "walshadow/prefix"); assert_eq!(s3.region, "eu-west-1"); assert_eq!(s3.endpoint.as_deref(), Some("https://minio.internal")); assert!(s3.force_path_style); - match s3.creds { - CredentialSource::Static(cr) => { - assert_eq!(cr.access_key, "AK"); - assert_eq!(cr.secret_key, "SK"); - } - other => panic!("expected static creds, got {other:?}"), - } + let Static(cr) = s3.creds else { panic!() }; + assert_eq!(cr.access_key, "AK"); + assert_eq!(cr.secret_key, "SK"); } #[test] fn config_backup_s3_defaults_region_and_imds() { - use walrus::config::StorageSettings; + use walrus::config::StorageSettings::S3; use walrus::storage::s3::CredentialSource; let c = EmitterConfig::from_toml_str("[backup]\narchive = \"s3://b\"\n").unwrap(); - let s3 = match c.backup.unwrap().storage { - StorageSettings::S3(s3) => s3, - other => panic!("expected S3, got {other:?}"), - }; + let storage = c.backup.unwrap().storage; + let S3(s3) = storage else { panic!() }; assert_eq!(s3.bucket, "b"); assert_eq!(s3.prefix, ""); assert_eq!(s3.region, "us-east-1"); @@ -4753,21 +4807,17 @@ mod tests { #[test] fn config_backup_gcs_and_file() { - use walrus::config::StorageSettings; + use walrus::config::StorageSettings::{self, Gcs}; let gcs = EmitterConfig::from_toml_str( "[backup]\narchive = \"gs://gb/pre\"\ncredentials_path = \"/sa.json\"\n", ) .unwrap() .backup .unwrap(); - match gcs.storage { - StorageSettings::Gcs(g) => { - assert_eq!(g.bucket, "gb"); - assert_eq!(g.prefix, "pre"); - assert_eq!(g.credentials_path.as_deref(), Some("/sa.json")); - } - other => panic!("expected GCS, got {other:?}"), - } + let Gcs(g) = gcs.storage else { panic!() }; + assert_eq!(g.bucket, "gb"); + assert_eq!(g.prefix, "pre"); + assert_eq!(g.credentials_path.as_deref(), Some("/sa.json")); let fs = EmitterConfig::from_toml_str("[backup]\narchive = \"file:///var/wal\"\n") .unwrap() .backup diff --git a/src/emit/pipeline/batcher.rs b/src/emit/pipeline/batcher.rs index 4f29c1c4..44281ff4 100644 --- a/src/emit/pipeline/batcher.rs +++ b/src/emit/pipeline/batcher.rs @@ -620,21 +620,17 @@ mod tests { // Drop sender → final flush + graceful exit drop(msg_tx); - let (mut total, mut s0, mut s1) = (0u64, 0u64, 0u64); + let mut total = 0u64; + let mut per_seq = std::collections::BTreeMap::new(); while let Ok(b) = batches_rx.recv().await { total += b.n_rows as u64; for (seq, n) in b.per_seq { - match seq { - 0 => s0 += n, - 1 => s1 += n, - other => panic!("unexpected seq {other}"), - } + *per_seq.entry(seq).or_insert(0) += n; } } handle.await.expect("batcher task"); assert_eq!(total, 5, "all rows sealed exactly once"); - assert_eq!(s0, 3, "seq 0 rows"); - assert_eq!(s1, 2, "seq 1 rows"); + assert_eq!(per_seq, [(0, 3), (1, 2)].into(), "rows per seq"); assert!(fatal.message().is_none(), "no fatal: {:?}", fatal.message()); } @@ -670,21 +666,17 @@ mod tests { .expect("send chunk"); drop(msg_tx); - let (mut total, mut s0, mut s1) = (0u64, 0u64, 0u64); + let mut total = 0u64; + let mut per_seq = std::collections::BTreeMap::new(); while let Ok(b) = batches_rx.recv().await { total += b.n_rows as u64; for (seq, n) in b.per_seq { - match seq { - 0 => s0 += n, - 1 => s1 += n, - other => panic!("unexpected seq {other}"), - } + *per_seq.entry(seq).or_insert(0) += n; } } handle.await.expect("batcher task"); assert_eq!(total, 5, "all rows sealed exactly once"); - assert_eq!(s0, 3, "seq 0 rows"); - assert_eq!(s1, 2, "seq 1 rows"); + assert_eq!(per_seq, [(0, 3), (1, 2)].into(), "rows per seq"); assert!(fatal.message().is_none(), "no fatal: {:?}", fatal.message()); } @@ -1055,10 +1047,10 @@ mod tests { .expect("send row"); let batch = batches_rx.recv().await.expect("budget batch"); assert_eq!(batch.n_rows, 1); + let elapsed = start.elapsed(); assert!( - start.elapsed() < Duration::from_secs(1), - "byte budget sealed at {:?}, not the deadline", - start.elapsed() + elapsed < Duration::from_secs(1), + "byte budget sealed at {elapsed:?}, not the deadline" ); drop(msg_tx); handle.await.expect("batcher task"); diff --git a/src/emit/pipeline/bootstrap.rs b/src/emit/pipeline/bootstrap.rs index adf55740..d38b36d4 100644 --- a/src/emit/pipeline/bootstrap.rs +++ b/src/emit/pipeline/bootstrap.rs @@ -944,14 +944,9 @@ mod tests { /// Flatten the drain's coalesced `Rows` chunks back to a row list async fn collect_rows(rx: &mut mpsc::Receiver) -> Vec { let mut rows = Vec::new(); - while let Some(msg) = rx.recv().await { - match msg { - BatcherMsg::Rows(chunk) => rows.extend(chunk.rows), - BatcherMsg::Row(r) => rows.push(r), - BatcherMsg::FlushAll(reply) => { - let _ = reply.send(()); - } - } + // Any other message ends collection short of the expected rows + while let Some(BatcherMsg::Rows(chunk)) = rx.recv().await { + rows.extend(chunk.rows); } rows } @@ -962,18 +957,12 @@ mod tests { /// read-only end-of-backup store #[tokio::test(flavor = "current_thread")] async fn a_miss_is_fatal_for_a_seeded_mirror_and_superseded_for_a_read_only_store() { - use crate::toast::{ChunkStore, ChunkStoreError, MemChunkStore, ToastRow}; + use crate::toast::{ChunkStore, ChunkStoreError, MemChunkStore}; struct ReadOnly; #[async_trait::async_trait] impl ChunkStore for ReadOnly { - fn accepts_writes(&self) -> bool { - false - } - async fn put(&self, _: &[ToastRow]) -> Result<(), ChunkStoreError> { - Err(ChunkStoreError::ReadOnly("put")) - } async fn fetch_many( &self, _: u32, @@ -983,12 +972,6 @@ mod tests { // Incomplete value is absent from end-of-backup state Ok(vec![FetchedValue::Mismatch { got: 7984 }; values.len()]) } - async fn truncate_mirror(&self, _: u32) -> Result<(), ChunkStoreError> { - Err(ChunkStoreError::ReadOnly("truncate_mirror")) - } - async fn rewrite_barrier(&self, _: u32, _: u64, _: u64) -> Result<(), ChunkStoreError> { - Err(ChunkStoreError::ReadOnly("rewrite_barrier")) - } } let ptr = crate::decode::heap_decoder::ToastPointer { @@ -1024,7 +1007,12 @@ mod tests { let stats = Arc::new(EmitterStats::default()); let read_only = ToastResolver::with_store(Arc::new(ReadOnly), stats.clone()); assert!(!read_only.stores_chunks() && !read_only.fill_on_miss()); - let (column, retained) = apply_fetched(short, &ptr, 25, &rel, "body", &read_only) + let fetched = read_only + .fetch_value(0, 16390, 16402, u64::MAX, 9100) + .await + .unwrap(); + assert_eq!(fetched, short); + let (column, retained) = apply_fetched(fetched, &ptr, 25, &rel, "body", &read_only) .expect("a read-only backend fills instead of failing the load"); assert_eq!(column, ColumnValue::Null); assert_eq!(retained, 0); @@ -1858,16 +1846,6 @@ mod tests { struct FailPrefetch(AtomicU64); #[async_trait::async_trait] impl ChunkStore for FailPrefetch { - async fn truncate_mirror(&self, _: u32) -> Result<(), ChunkStoreError> { - Ok(()) - } - async fn rewrite_barrier(&self, _: u32, _: u64, _: u64) -> Result<(), ChunkStoreError> { - Ok(()) - } - - async fn put(&self, _: &[ToastRow]) -> Result<(), ChunkStoreError> { - Ok(()) - } async fn fetch_many( &self, _: u32, diff --git a/src/emit/pipeline/decode.rs b/src/emit/pipeline/decode.rs index 56162135..5181e517 100644 --- a/src/emit/pipeline/decode.rs +++ b/src/emit/pipeline/decode.rs @@ -216,10 +216,8 @@ mod tests { .expect("place"); assert_eq!(routed, 1, "insert routed, delete dropped"); assert_eq!(stats.deletes_discarded.load(Ordering::Relaxed), 1); - match msg_rx.recv().await { - Some(BatcherMsg::Rows(chunk)) => assert_eq!(chunk.rows.len(), 1), - other => panic!("expected one row chunk, got {}", other.is_some()), - } + let msg = msg_rx.recv().await; + assert!(matches!(msg, Some(BatcherMsg::Rows(c)) if c.rows.len() == 1)); // Default policy keeps the marker, so the DELETE rides through let marked = route(SystemColumns::default()); diff --git a/src/emit/pipeline/inserter.rs b/src/emit/pipeline/inserter.rs index 59f399e2..76c660f4 100644 --- a/src/emit/pipeline/inserter.rs +++ b/src/emit/pipeline/inserter.rs @@ -17,7 +17,7 @@ use tokio::task::JoinHandle; use crate::ch::{ChConn, EmitterError, drain_to_end_of_stream, with_timeout}; use crate::config::DestEmitter; -use crate::emit::ch_emitter::{ColumnBuf, EmitterStats, build_leaf, build_root}; +use crate::emit::ch_emitter::{EmitterStats, build_leaf, build_root}; use crate::emit::pipeline::Fatal; use crate::emit::pipeline::ack::AckHandle; use crate::emit::pipeline::batcher::BatchMeta; @@ -123,10 +123,7 @@ impl Inserter { .buffers .iter() .zip(&leaves) - .map(|(buf, leaf)| match buf { - ColumnBuf::Oracle(_) => Ok(None), - _ => build_root(buf, leaf.as_ref(), batch.n_rows).map(Some), - }) + .map(|(buf, leaf)| build_root(buf, leaf.as_ref(), batch.n_rows)) .collect::>() { Ok(v) => v, diff --git a/src/emit/pipeline/plan_spool.rs b/src/emit/pipeline/plan_spool.rs index 9b467b46..aeeb749e 100644 --- a/src/emit/pipeline/plan_spool.rs +++ b/src/emit/pipeline/plan_spool.rs @@ -606,12 +606,7 @@ mod tests { let mut order = Vec::new(); while let Some(item) = rd.next_item().unwrap() { order.push(match item { - PlanItem::Control(c) => { - let DrainEntry::Catalog(SchemaEvent::Dropped { oid, .. }) = &c.event else { - panic!("unexpected control"); - }; - format!("e{oid}") - } + PlanItem::Control(c) => format!("{:?}", c.event), PlanItem::Heap(h) => { assert!( Arc::ptr_eq(&h.described.descriptor, &plan.descriptors[0].0) @@ -622,14 +617,24 @@ mod tests { assert!(Arc::ptr_eq(route, &plan.routes[0])); } let new = h.described.decoded.new.as_ref().unwrap(); - let Some(ColumnValue::Int4(v)) = new.columns[0] else { - panic!("unexpected column"); - }; - format!("h{v}{}", if h.route.is_some() { "r" } else { "-" }) + let routed = if h.route.is_some() { "r" } else { "-" }; + format!("{:?}{routed}", new.columns[0]) } }); } - assert_eq!(order, ["e1", "h10r", "h20-", "e2", "h30r", "e3"]); + let ev = |oid| format!("{:?}", dropped(oid)); + let heap = |v, routed| format!("{:?}{routed}", Some(ColumnValue::Int4(v))); + assert_eq!( + order, + [ + ev(1), + heap(10, "r"), + heap(20, "-"), + ev(2), + heap(30, "r"), + ev(3) + ] + ); assert!(plan.path().is_none(), "small plan stays memory-resident"); drop(rd); drop(plan); @@ -642,6 +647,7 @@ mod tests { /// shadow-PG oracle resolves after replay, may lag row's catalog state #[test] fn pending_values_round_trip() { + use PlanItem::Heap; let tmp = tempfile::tempdir().unwrap(); let path = tmp.path().join("1.plan"); let mut w = PlanWriter::create(path, 1 << 20, DEFAULT_PLAN_MEM_MAX).unwrap(); @@ -661,9 +667,8 @@ mod tests { w.push_heap(&h, Some(&route())).unwrap(); let plan = w.seal(0x2000, 42).unwrap(); let mut rd = plan.replay().unwrap(); - let Some(PlanItem::Heap(out)) = rd.next_item().unwrap() else { - panic!("expected heap"); - }; + let item = rd.next_item().unwrap(); + let Some(Heap(out)) = item else { panic!() }; assert_eq!(out.described.decoded.new.unwrap().columns, cols); assert!(rd.next_item().unwrap().is_none()); } @@ -682,9 +687,7 @@ mod tests { bytes[mid] ^= 0xFF; fs::write(&path, &bytes).unwrap(); let mut rd = plan.replay().unwrap(); - let Err(err) = rd.next_item() else { - panic!("expected corruption error"); - }; + let err = rd.next_item().err().expect("corruption error"); assert!(matches!(err, PlanSpoolError::Corrupt { .. }), "{err}"); } @@ -701,9 +704,7 @@ mod tests { let mut bytes = fs::read(&path).unwrap(); bytes[4..8].copy_from_slice(&u32::MAX.to_le_bytes()); fs::write(&path, &bytes).unwrap(); - let Err(err) = plan.verify() else { - panic!("expected format error"); - }; + let err = plan.verify().unwrap_err(); assert!(matches!(err, PlanSpoolError::Format { .. }), "{err}"); } @@ -723,9 +724,7 @@ mod tests { let last_body = bytes.len() - 18; // last heap frame body, before 17-byte seal bytes[last_body] ^= 0xFF; fs::write(&path, &bytes).unwrap(); - let Err(err) = plan.verify() else { - panic!("expected corruption error"); - }; + let err = plan.verify().unwrap_err(); assert!(matches!(err, PlanSpoolError::Corrupt { .. }), "{err}"); } @@ -760,9 +759,7 @@ mod tests { matches!(rd.next_item(), Ok(Some(PlanItem::Heap(_)))), "pre-seal frames intact" ); - let Err(err) = rd.next_item() else { - panic!("expected unsealed error"); - }; + let err = rd.next_item().err().expect("unsealed error"); assert!(matches!(err, PlanSpoolError::Unsealed), "{err}"); } @@ -773,9 +770,7 @@ mod tests { let path = tmp.path().join("1.plan"); let mut w = PlanWriter::create(path.clone(), 16, 0).unwrap(); let d = descriptor(16500); - let Err(err) = w.push_heap(&heap(&d, 100, 10), None) else { - panic!("expected budget error"); - }; + let err = w.push_heap(&heap(&d, 100, 10), None).unwrap_err(); assert!(matches!(err, PlanSpoolError::DiskBudget { .. }), "{err}"); drop(w); assert!(!path.exists(), "abandoned plan unlinks"); diff --git a/src/emit/pipeline/planner.rs b/src/emit/pipeline/planner.rs index 1e2dbde0..45e6d892 100644 --- a/src/emit/pipeline/planner.rs +++ b/src/emit/pipeline/planner.rs @@ -479,9 +479,7 @@ mod tests { .await .unwrap() .expect("toast row slice"); - let Err(err) = planner.plan_batch(second).await else { - panic!("expected detoast failure"); - }; + let err = planner.plan_batch(second).await.unwrap_err(); assert!(matches!(err, PlanError::Detoast(_)), "{err}"); drain.finish().await.unwrap(); drop(planner); @@ -517,25 +515,18 @@ mod tests { let mut planned = 0usize; // 1-row slices: valid fanout rows plan before the bad record folds let err = loop { - match drain.next_batch(1, usize::MAX, None).await { - Ok(Some(batch)) => { - planned += batch.heaps.len(); - planner.plan_batch(batch).await.unwrap(); - } - Ok(None) => panic!("expected fold failure before EOF"), + let batch = match drain.next_batch(1, usize::MAX, None).await { + Ok(batch) => batch.expect("fold failure before EOF"), Err(e) => break e, - } + }; + planned += batch.heaps.len(); + planner.plan_batch(batch).await.unwrap(); }; assert!(planned >= 1, "earlier valid rows planned before failure"); + let msg = err.to_string(); assert!( - matches!( - &err, - XactBufferError::OrdinaryFailClosed { - reason: FailClosedReason::ImageOnly, - .. - } - ), - "{err}" + msg.ends_with("failed closed: image-only operation"), + "{msg}" ); drop(planner); assert!(!path.exists(), "abandoned plan unlinks, nothing to execute"); @@ -583,10 +574,8 @@ mod tests { let mut rd = plan.replay().unwrap(); let mut rows = 0usize; while let Some(item) = rd.next_item().unwrap() { - let PlanItem::Heap(h) = item else { - panic!("no controls planned"); - }; - assert!(h.route.is_some()); + // No controls planned, every heap routed + assert!(matches!(item, PlanItem::Heap(h) if h.route.is_some())); rows += 1; } drop(rd); @@ -746,19 +735,14 @@ mod tests { Planner::create(tmp.path().join("1.plan"), 1 << 20, &mut view, &resolver).unwrap(); let mut drain = b.drain_committed(1, 42, 0x2000, &[], false).await.unwrap(); while let Some(batch) = drain.next_batch(8, usize::MAX, None).await.unwrap() { - let is_final = batch.is_final; planner.plan_batch(batch).await.unwrap(); - if is_final { - break; - } } drain.finish().await.unwrap(); let plan = planner.seal(0x2000, 42).unwrap(); assert_eq!(plan.heap_count, 1); let mut rd = plan.replay().unwrap(); - let Some(PlanItem::Heap(h)) = rd.next_item().unwrap() else { - panic!("expected heap"); - }; - assert!(h.route.is_none(), "unmapped discard planned as such"); + // Unmapped discard planned as such + let item = rd.next_item().unwrap(); + assert!(matches!(item, Some(PlanItem::Heap(h)) if h.route.is_none())); } } diff --git a/src/emit/pipeline/resolver.rs b/src/emit/pipeline/resolver.rs index b6ec440d..dc8ab166 100644 --- a/src/emit/pipeline/resolver.rs +++ b/src/emit/pipeline/resolver.rs @@ -169,6 +169,7 @@ mod tests { use crate::decode::heap_decoder::{ ColumnValue, CommittedTuple, DecodedHeap, DecodedTuple, HeapOp, }; + use crate::emit::ch_emitter::Wire; use crate::emit::pipeline::batcher::{BatcherConfig, BatcherMsg, RoutedRow}; use crate::emit::route::RouteSnapshot; use crate::mapping::{ColumnMapping, TableMapping, TableTarget}; @@ -312,18 +313,15 @@ mod tests { let stats = EmitterStats::default(); resolve_local_columns(&mut batch, &stats); assert_eq!(stats.oracle_local_columns.load(Ordering::Relaxed), 1); - let ColumnBuf::NullableString { - offsets, - data, - null_map, - .. - } = &batch.buffers[0] - else { - panic!("column not built locally: {:?}", batch.buffers[0]); - }; - assert_eq!(data, b"POINT(1 2)POINT(3 4)"); - assert_eq!(offsets, &[10, 10, 10, 20]); - assert_eq!(null_map, &[0, 1, 0, 0]); + assert_eq!( + batch.buffers[0].wire(), + Wire::NullableString { + offsets: &[10, 10, 10, 20], + data: b"POINT(1 2)POINT(3 4)", + null_map: &[0, 1, 0, 0], + absent: b"", + } + ); assert!( resolve_oracle(&None, alloc, &batch) diff --git a/src/filter/engine.rs b/src/filter/engine.rs index 9aae7c53..a6bb6e84 100644 --- a/src/filter/engine.rs +++ b/src/filter/engine.rs @@ -184,11 +184,6 @@ impl Filter { self.targets = db_oids.into_iter().collect(); } - /// Followed databases, in the order they were set - pub fn target_dbs(&self) -> &[u32] { - &self.targets - } - /// Route `rels` and relations created at or after `from_lsn` to both paths /// /// `rels` lists files shadow already holds at `from_lsn` @@ -1271,10 +1266,7 @@ mod tests { let mut f = target_filter(); let mut r = xact_assignment(100, &[101]); // Claim two subxids, carry one - match &mut r.main_data { - std::borrow::Cow::Owned(md) => md[4..8].copy_from_slice(&2i32.to_le_bytes()), - _ => unreachable!(), - } + r.main_data.to_mut()[4..8].copy_from_slice(&2i32.to_le_bytes()); assert!(f.decide_record(&r, 150, 0xD116).is_err()); } @@ -1811,10 +1803,7 @@ mod tests { let mut f = target_filter(); let mut r = xact_invals_rec(7, &[(-2, 5, 16400)]); // Claim two messages, carry one - match &mut r.main_data { - std::borrow::Cow::Owned(md) => md[0..4].copy_from_slice(&2i32.to_le_bytes()), - _ => unreachable!(), - } + r.main_data.to_mut()[0..4].copy_from_slice(&2i32.to_le_bytes()); assert!(f.decide_record(&r, 150, 0xD116).is_err()); } @@ -1901,31 +1890,12 @@ mod tests { /// Commit / abort carrying `xl_xact_dbinfo` (`Oid dbId; Oid tsId`), the /// committing backend's database - fn xact_end_dbinfo( - op: u8, - xid: u32, - db_id: u32, - invals: &[(i8, u32, u32)], - ) -> XLogRecord<'static> { - use crate::decode::wal_xact::{XACT_XINFO_HAS_DBINFO, XACT_XINFO_HAS_INVALS}; + fn xact_end_dbinfo(op: u8, xid: u32, db_id: u32) -> XLogRecord<'static> { + use crate::decode::wal_xact::XACT_XINFO_HAS_DBINFO; let mut md: Vec = 0i64.to_le_bytes().to_vec(); - let mut xinfo = XACT_XINFO_HAS_DBINFO; - if !invals.is_empty() { - xinfo |= XACT_XINFO_HAS_INVALS; - } - md.extend_from_slice(&xinfo.to_le_bytes()); + md.extend_from_slice(&XACT_XINFO_HAS_DBINFO.to_le_bytes()); md.extend_from_slice(&db_id.to_le_bytes()); md.extend_from_slice(&1663u32.to_le_bytes()); // tsId - if !invals.is_empty() { - md.extend_from_slice(&(invals.len() as i32).to_le_bytes()); - for &(id, db, rel) in invals { - let mut msg = [0u8; 16]; - msg[0] = id as u8; - msg[4..8].copy_from_slice(&db.to_le_bytes()); - msg[8..12].copy_from_slice(&rel.to_le_bytes()); - md.extend_from_slice(&msg); - } - } let mut r = rec_with_xid(RmId::Xact, &[], xid); r.header.info = op | XLOG_XACT_HAS_INFO; r.main_data = std::borrow::Cow::Owned(md); @@ -1954,7 +1924,7 @@ mod tests { ); let v = f .decide_record( - &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB, &[]), + &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB), 200, 0xD116, ) @@ -1975,7 +1945,7 @@ mod tests { assert!(user.defer_catalog_decode, "target DDL fences its own rows"); let b = f .decide_record( - &xact_end_dbinfo(XLOG_XACT_COMMIT, 8, TARGET_DB, &[]), + &xact_end_dbinfo(XLOG_XACT_COMMIT, 8, TARGET_DB), 400, 0xD116, ) @@ -2000,7 +1970,7 @@ mod tests { .unwrap(); let v = f .decide_record( - &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB, &[]), + &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB), 200, 0xD116, ) @@ -2022,11 +1992,7 @@ mod tests { "filenode tracking stays cluster-wide" ); let v = f - .decide_record( - &xact_end_dbinfo(XLOG_XACT_COMMIT, xid, db, &[]), - 200, - 0xD116, - ) + .decide_record(&xact_end_dbinfo(XLOG_XACT_COMMIT, xid, db), 200, 0xD116) .unwrap(); assert!(!v.catalog_boundary, "db {db} relmap is not target dirt"); } @@ -2041,7 +2007,7 @@ mod tests { .unwrap(); let v = f .decide_record( - &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB, &[]), + &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB), 200, 0xD116, ) @@ -2052,7 +2018,7 @@ mod tests { ); let v = f .decide_record( - &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB, &[]), + &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB), 300, 0xD116, ) @@ -2067,21 +2033,16 @@ mod tests { .unwrap(); let err = f .decide_record( - &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB, &[]), + &xact_end_dbinfo(XLOG_XACT_COMMIT, 7, FOREIGN_DB), 200, 0xD116, ) .unwrap_err(); - assert!( - matches!( - err, - XactPayloadError::ForeignScope { - db_id: FOREIGN_DB, - target: TARGET_DB - } - ), - "{err}" - ); + let want = XactPayloadError::ForeignScope { + db_id: FOREIGN_DB, + target: TARGET_DB, + }; + assert_eq!(err.to_string(), want.to_string()); let v = f .decide_record(&xact_end(XLOG_XACT_COMMIT, 7, &[], None), 300, 0xD116) .unwrap(); diff --git a/src/filter/pg_class_decoder.rs b/src/filter/pg_class_decoder.rs index 4f4f7a96..b2db169d 100644 --- a/src/filter/pg_class_decoder.rs +++ b/src/filter/pg_class_decoder.rs @@ -419,6 +419,16 @@ mod tests { } } + /// Row the tuple helpers build: zero relname and relnamespace + fn decoded(oid: u32, relfilenode: u32) -> DecodeOutcome { + DecodeOutcome::Decoded(PgClassRow { + oid, + relname: [0; NAME_LEN], + relnamespace: 0, + relfilenode, + }) + } + /// Only `flags` matters to the decoder; other fields stay zero. fn xl_heap_update_main_data(flags: u8) -> Vec { let mut md = vec![0u8; SIZE_OF_HEAP_UPDATE]; @@ -430,12 +440,7 @@ mod tests { fn decodes_minimal_pg_class_insert() { let data = pg_class_insert_block(2615, 30000); let rec = record(RmId::Heap, HEAP_INSERT_OP, Vec::new(), data); - let row = match decode_pg_class_tuple(&rec, 0, MAGIC) { - DecodeOutcome::Decoded(r) => r, - other => panic!("expected Decoded, got {other:?}"), - }; - assert_eq!(row.oid, 2615); - assert_eq!(row.relfilenode, 30000); + assert_eq!(decode_pg_class_tuple(&rec, 0, MAGIC), decoded(2615, 30000)); } #[test] @@ -453,12 +458,7 @@ mod tests { v.extend_from_slice(&[0u8; 20]); // cols 3-7 v.extend_from_slice(&77777u32.to_le_bytes()); // relfilenode let rec = record(RmId::Heap, HEAP_INSERT_OP, Vec::new(), v); - let row = match decode_pg_class_tuple(&rec, 0, MAGIC) { - DecodeOutcome::Decoded(r) => r, - other => panic!("expected Decoded, got {other:?}"), - }; - assert_eq!(row.oid, 1234); - assert_eq!(row.relfilenode, 77777); + assert_eq!(decode_pg_class_tuple(&rec, 0, MAGIC), decoded(1234, 77777)); } #[test] @@ -538,13 +538,7 @@ mod tests { xl_heap_update_main_data(0), data, ); - match decode_pg_class_tuple(&rec, 0, MAGIC) { - DecodeOutcome::Decoded(r) => { - assert_eq!(r.oid, 2608); - assert_eq!(r.relfilenode, 40000); - } - other => panic!("expected Decoded, got {other:?}"), - } + assert_eq!(decode_pg_class_tuple(&rec, 0, MAGIC), decoded(2608, 40000)); } #[test] @@ -623,13 +617,7 @@ mod tests { xl_heap_update_main_data(XLH_UPDATE_SUFFIX_FROM_OLD), data, ); - match decode_pg_class_tuple(&rec, 0, MAGIC) { - DecodeOutcome::Decoded(r) => { - assert_eq!(r.oid, 2608); - assert_eq!(r.relfilenode, 40000); - } - other => panic!("expected Decoded, got {other:?}"), - } + assert_eq!(decode_pg_class_tuple(&rec, 0, MAGIC), decoded(2608, 40000)); } #[test] @@ -733,10 +721,7 @@ mod tests { fn inplace_block_data_is_bare_columns() { let tail = pg_class_tuple_tail(2608, 40000, 0); let rec = record(RmId::Heap, HEAP_INPLACE_OP, vec![1, 0], tail[1..].to_vec()); - let DecodeOutcome::Decoded(row) = decode_pg_class_tuple(&rec, 0, MAGIC) else { - panic!("inplace must decode"); - }; - assert_eq!((row.oid, row.relfilenode), (2608, 40000)); + assert_eq!(decode_pg_class_tuple(&rec, 0, MAGIC), decoded(2608, 40000)); } /// Page holding `rows` at offnums 1.., tuples with t_hoff 24 @@ -784,9 +769,6 @@ mod tests { info: 0, }; block.image = std::borrow::Cow::Owned(page_with(&[(2608, 40000), (2615, 41000)])); - let DecodeOutcome::Decoded(row) = decode_pg_class_tuple(&rec, 0, MAGIC) else { - panic!("image must decode"); - }; - assert_eq!((row.oid, row.relfilenode), (2615, 41000)); + assert_eq!(decode_pg_class_tuple(&rec, 0, MAGIC), decoded(2615, 41000)); } } diff --git a/src/lib.rs b/src/lib.rs index c1226052..557bbfc1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,3 +83,10 @@ pub use source::{ pub use toast::toast_retire; #[doc(hidden)] pub use xact::{spill, xact_buffer}; + +/// Enable every callsite so coverage runs evaluate tracing field expressions +#[cfg(test)] +#[ctor::ctor(unsafe)] +fn enable_tracing() { + let _ = tracing::subscriber::set_global_default(tracing_subscriber::registry()); +} diff --git a/src/ops/bridge.rs b/src/ops/bridge.rs index b30fd395..0301f66f 100644 --- a/src/ops/bridge.rs +++ b/src/ops/bridge.rs @@ -1250,26 +1250,17 @@ mod tests { async fn fake_worker(listener: UnixListener, script: Vec>>) { let mut script = script.into_iter(); loop { - let Ok((mut sock, _)) = listener.accept().await else { - return; - }; - loop { - let mut hdr = [0u8; 4]; - if sock.read_exact(&mut hdr).await.is_err() { - break; - } + let (mut sock, _) = listener.accept().await.unwrap(); + let mut hdr = [0u8; 4]; + while sock.read_exact(&mut hdr).await.is_ok() { let mut body = vec![0u8; u32::from_be_bytes(hdr) as usize]; - if sock.read_exact(&mut body).await.is_err() { - break; - } - match script.next() { - Some(Some(resp)) => { - if sock.write_all(&frame(resp)).await.is_err() { - break; - } - } - Some(None) | None => break, + if sock.read_exact(&mut body).await.is_ok() + && let Some(Some(resp)) = script.next() + && sock.write_all(&frame(resp)).await.is_ok() + { + continue; } + break; } } } @@ -1401,16 +1392,13 @@ mod tests { .await .unwrap_err(); assert!( - matches!( - err, - BridgeError::ReplayMismatch { - expected: 0x4000, - end: 0x5000, - .. - } - ), + matches!(err, BridgeError::ReplayMismatch { .. }), "got {err:?}" ); + assert_eq!( + err.to_string(), + "bridge replayed to 4000..5000, expected boundary 4000" + ); assert_eq!(bridge.stats.scan_replay_moved.load(Ordering::Relaxed), 1); } diff --git a/src/ops/control.rs b/src/ops/control.rs index 8ab63860..3190d31d 100644 --- a/src/ops/control.rs +++ b/src/ops/control.rs @@ -649,18 +649,6 @@ fn set_mode_600(path: &Path) -> Result<()> { mod tests { use super::*; - /// Scalar at `[section] key`, for asserting fragment edits - fn str_at(root: &Table, section: &str, key: &str) -> String { - root.get(section) - .and_then(Value::as_table) - .and_then(|t| t.get(key)) - .map(|v| match v { - Value::String(s) => s.clone(), - other => other.to_string(), - }) - .unwrap_or_default() - } - fn cfg(toml: &str) -> Table { if toml.is_empty() { Table::new() @@ -735,7 +723,7 @@ mod tests { "[source]\nhost = \"h\"\npassword = \"p\"\n[table.demo.a]\nreplicate = true\n[table.demo.b]\nreplicate = true\n", ); apply_mask(&mut root, &cfg("[source]\npassword = \"\"")); - assert_eq!(str_at(&root, "source", "host"), "h"); + assert_eq!(root["source"]["host"].as_str(), Some("h")); assert!(root["source"].as_table().unwrap().get("password").is_none()); apply_mask(&mut root, &cfg("[table.demo]\na = \"\"\nmissing = \"\"")); let demo = root["table"].as_table().unwrap()["demo"] diff --git a/src/ops/init.rs b/src/ops/init.rs index 368e8b24..5e776ad5 100644 --- a/src/ops/init.rs +++ b/src/ops/init.rs @@ -88,12 +88,12 @@ pub async fn run(opts: InitOpts) -> Result<()> { let ch_client = crate::ch::connect_client(&ch_cfg) .await .context("connect ClickHouse")?; - match ch_client.server_info() { - Some(i) => println!( + // chc-rs returns None only before handshake + if let Some(i) = ch_client.server_info() { + println!( " ✓ reachable, ClickHouse {}.{}.{}", i.version_major, i.version_minor, i.version_patch - ), - None => println!(" ✓ reachable"), + ); } if created_db { println!(" ✓ created database {}", ch_cfg.database); diff --git a/src/ops/metrics.rs b/src/ops/metrics.rs index 7123668f..3c3202fb 100644 --- a/src/ops/metrics.rs +++ b/src/ops/metrics.rs @@ -656,13 +656,11 @@ impl RateEstimator { pub fn observe(&mut self, now: Instant, received_lsn: u64) { self.samples.push_back((now, received_lsn)); let cutoff = now.checked_sub(self.window); - if let Some(cutoff) = cutoff { - while let Some(&(t, _)) = self.samples.front() - && t < cutoff - && self.samples.len() > 1 - { - self.samples.pop_front(); - } + while let Some(&(t, _)) = self.samples.front() + && cutoff.is_some_and(|c| t < c) + && self.samples.len() > 1 + { + self.samples.pop_front(); } } @@ -1095,7 +1093,6 @@ mod tests { .insert(("Heap".into(), "to_decoder"), 3); let full = render(snap.clone()).len(); - let mut errors = 0; for writes in 0..64 { let mut w = FailAfter { writes, @@ -1104,12 +1101,10 @@ mod tests { let mut registry = Registry::default(); registry.register_collector(Box::new(SnapshotCollector(snap.clone()))); registry.register_collector(Box::new(crate::ops::log_events::LogEventCollector)); - if text::encode(&mut w, ®istry).is_err() { - errors += 1; - assert!(w.out.len() < full, "a failed encode cannot be complete"); - } + let failed = text::encode(&mut w, ®istry).is_err(); + assert!(failed, "prefix length {writes} must surface the failure"); + assert!(w.out.len() < full, "a failed encode cannot be complete"); } - assert_eq!(errors, 64, "every prefix length must surface the failure"); } #[test] diff --git a/src/pos.rs b/src/pos.rs index 85a22caf..10f647bd 100644 --- a/src/pos.rs +++ b/src/pos.rs @@ -128,13 +128,6 @@ impl PartialEq for Pos { } } -#[cfg(test)] -impl PartialOrd for Pos { - fn partial_cmp(&self, other: &u64) -> Option { - Some(self.0.cmp(other)) - } -} - impl PartialEq for Pos { fn eq(&self, other: &Self) -> bool { self.0 == other.0 diff --git a/src/schema.rs b/src/schema.rs index a8fa3f68..4dd2c91d 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -108,16 +108,6 @@ pub enum ReplIdent { } impl ReplIdent { - /// `pg_class.relreplident` - pub fn to_char(&self) -> char { - match self { - ReplIdent::Default { .. } => 'd', - ReplIdent::Nothing => 'n', - ReplIdent::Full { .. } => 'f', - ReplIdent::UsingIndex { .. } => 'i', - } - } - /// Build from `pg_class.relreplident` plus the `pg_index` rows it names: /// the primary key for `d`/`f`, the replica-identity index for `i` pub fn from_parts( diff --git a/src/source/archive.rs b/src/source/archive.rs index 2418bbdd..f1bc5f4c 100644 --- a/src/source/archive.rs +++ b/src/source/archive.rs @@ -508,11 +508,8 @@ mod tests { #[tokio::test] async fn archive_prefetch_drop_cancels_worker() { - let (tx, rx) = tokio::sync::mpsc::channel(1); - let task = tokio::spawn(async move { - let _tx = tx; - std::future::pending::<()>().await; - }); + let (_tx, rx) = tokio::sync::mpsc::channel(1); + let task = tokio::spawn(std::future::pending::<()>()); let abort = task.abort_handle(); drop(ArchiveFeed { wait_nanos: AtomicU64::new(0), diff --git a/src/source/boundary_hold.rs b/src/source/boundary_hold.rs index 1c28e34a..ee292f12 100644 --- a/src/source/boundary_hold.rs +++ b/src/source/boundary_hold.rs @@ -273,25 +273,6 @@ impl RecordSink for BoundaryHoldSink { self.inner.flush().await }) } - - fn on_idle<'a>( - &'a mut self, - ) -> Pin> + Send + 'a>> { - self.inner.on_idle() - } - - fn on_close<'a>( - &'a mut self, - ) -> Pin> + Send + 'a>> { - self.inner.on_close() - } - - fn on_idle_advance<'a>( - &'a mut self, - lsn: u64, - ) -> Pin> + Send + 'a>> { - self.inner.on_idle_advance(lsn) - } } #[cfg(test)] @@ -396,25 +377,20 @@ mod tests { .register_connection(0x1000, 1, None) .expect("current timeline"); let gate = gate_with(s.clone(), Duration::from_secs(5)); + let queued = s.lock().await.queued(); let prodded = tokio::spawn({ let s = s.clone(); async move { - loop { - let drained = s.lock().await.drain_send_queue(id); - if let Some(bytes) = drained { - // 'd' + u32 len + 'k' + wal_end(8) + time(8) + reply(1) - assert_eq!(bytes[5], b'k'); - assert_eq!(*bytes.last().unwrap(), 1, "reply requested"); - s.lock().await.observe_status( - id, - 0x3000, - Pos::new(0x3000), - Pos::new(0x3000), - ); - return; - } - tokio::time::sleep(Duration::from_millis(1)).await; - } + // Wait on listener wake, not runtime task order + queued.advance(Pos::ZERO).await.expect("state alive"); + let bytes = s.lock().await.drain_send_queue(id); + let bytes = bytes.expect("keepalive queued before hold parks"); + // 'd' + u32 len + 'k' + wal_end(8) + time(8) + reply(1) + assert_eq!(bytes[5], b'k'); + assert_eq!(*bytes.last().unwrap(), 1, "reply requested"); + s.lock() + .await + .observe_status(id, 0x3000, Pos::new(0x3000), Pos::new(0x3000)); } }); gate.hold(0x2F00, Pos::new(0x3000), || true) diff --git a/src/source/resume_prefix.rs b/src/source/resume_prefix.rs index 374fce00..7d248b54 100644 --- a/src/source/resume_prefix.rs +++ b/src/source/resume_prefix.rs @@ -216,14 +216,9 @@ mod tests { /// Segment-start page whose first `remaining` bytes continue a record /// that began in the preceding segment fn page(lsn: u64, remaining: u32, seg_size: u64) -> Vec { - let contrecord = if remaining > 0 { - XLP_FIRST_IS_CONT_RECORD - } else { - 0 - }; let mut bytes = vec![0; PAGE_SIZE]; bytes[..2].copy_from_slice(&XLP_PAGE_MAGIC_PG15.to_le_bytes()); - bytes[2..4].copy_from_slice(&(XLP_LONG_HEADER | contrecord).to_le_bytes()); + bytes[2..4].copy_from_slice(&(XLP_LONG_HEADER | XLP_FIRST_IS_CONT_RECORD).to_le_bytes()); bytes[4..8].copy_from_slice(&1u32.to_le_bytes()); bytes[8..16].copy_from_slice(&lsn.to_le_bytes()); bytes[XLP_REM_LEN..XLP_REM_LEN + 4].copy_from_slice(&remaining.to_le_bytes()); diff --git a/src/source/shadow_stream.rs b/src/source/shadow_stream.rs index 5f7dbc0a..61538fd3 100644 --- a/src/source/shadow_stream.rs +++ b/src/source/shadow_stream.rs @@ -387,9 +387,6 @@ impl ShadowStreamState { /// Append contiguous wire bytes; a non-contiguous LSN re-anchors. fn retain_wire(&mut self, start_lsn: u64, bytes: &[u8]) { - if bytes.is_empty() { - return; - } let buf_end = self.wire_buf_start + self.wire_buf.len() as u64; if self.wire_buf.is_empty() || start_lsn != buf_end { self.wire_buf.clear(); diff --git a/src/source/streaming_walker.rs b/src/source/streaming_walker.rs index 6d605ef1..ac92268c 100644 --- a/src/source/streaming_walker.rs +++ b/src/source/streaming_walker.rs @@ -348,10 +348,7 @@ impl StreamingWalker { page_magic: p.page_magic, })); } - if self.page_cursor >= page_end { - self.advance_to_next_page(); - continue; - } + // Fresh page always fits rest of a 24-byte header if take_now == 0 { return None; } diff --git a/src/source/transition.rs b/src/source/transition.rs index 6f19e9ad..a8a5afed 100644 --- a/src/source/transition.rs +++ b/src/source/transition.rs @@ -1156,11 +1156,10 @@ mod tests { TransitionError::Source(anyhow::anyhow!("x")), ]; for e in &errs { + let reason = e.reason(); assert!( - SWITCH_FAILURE_REASONS.contains(&e.reason()), - "{} → unlabelled reason {}", - e, - e.reason(), + SWITCH_FAILURE_REASONS.contains(&reason), + "{e} → unlabelled reason {reason}" ); } } diff --git a/src/source/wal_page.rs b/src/source/wal_page.rs index 635d8526..d42ed8d9 100644 --- a/src/source/wal_page.rs +++ b/src/source/wal_page.rs @@ -141,18 +141,14 @@ mod tests { #[test] fn short_header_valid_data_start_aligned() { let buf = header(XLP_PAGE_MAGIC_PG15, 0, 0); - match parse_page_header(&buf, 0).unwrap() { + assert_eq!( + parse_page_header(&buf, 0).unwrap(), PageHeaderParse::Valid { - magic, data_start, .. - } => { - assert_eq!(magic, XLP_PAGE_MAGIC_PG15); - assert_eq!( - data_start, - align_up(SHORT_HEADER_SIZE, X_LOG_RECORD_ALIGNMENT) - ); + magic: XLP_PAGE_MAGIC_PG15, + data_start: align_up(SHORT_HEADER_SIZE, X_LOG_RECORD_ALIGNMENT), + remaining_data_len: 0, } - other => panic!("expected Valid, got {other:?}"), - } + ); } #[test] diff --git a/src/source/wal_stream.rs b/src/source/wal_stream.rs index 79d8182c..2175820f 100644 --- a/src/source/wal_stream.rs +++ b/src/source/wal_stream.rs @@ -774,10 +774,8 @@ mod tests { .on_record(&rec) .await .expect_err("err propagates from inner sink"); - match err { - SinkError::Other(msg) => assert!(msg.contains("synthetic fail")), - _ => panic!("expected SinkError::Other, got {err:?}"), - } + // `Io` renders with an "io: " prefix + assert_eq!(err.to_string(), "synthetic fail at #1"); assert_eq!(log_before.lock().unwrap().len(), 2); assert_eq!(err_seen.load(Ordering::Relaxed), 2); assert_eq!(log_after.lock().unwrap().len(), 1); diff --git a/src/source_db.rs b/src/source_db.rs index cbd3e9a4..806d29f9 100644 --- a/src/source_db.rs +++ b/src/source_db.rs @@ -22,7 +22,6 @@ use crate::emit::ch_emitter::EmitterConfig; use crate::emit::route::RowPolicy; use crate::mapping::MappingHandle; use crate::ops::bridge::Bridge; -use ahash::{HashMap, HashMapExt}; /// Bridge pool and shadow catalog for one source database, opened before the /// descriptor log and routing map that complete a [`SourceDb`] @@ -174,12 +173,11 @@ impl std::fmt::Debug for SourceDb { } } -/// Look up configured databases by OID from WAL records +/// Databases this daemon follows #[derive(Debug)] pub struct SourceDbs { /// Config order, which is also bridge socket order order: Vec>, - by_oid: HashMap>, /// `[source] dbname`: probe connections, shared settings, and the one /// database a single-database path uses primary: Arc, @@ -188,18 +186,14 @@ pub struct SourceDbs { impl SourceDbs { /// Use config order, fall back to first database if primary OID is missing pub fn new(dbs: Vec>, primary: Oid) -> Self { - let mut by_oid = HashMap::with_capacity(dbs.len()); - for db in &dbs { - by_oid.insert(db.oid, db.clone()); - } - let primary = by_oid - .get(&primary) + let primary = dbs + .iter() + .find(|db| db.oid == primary) + .or_else(|| dbs.first()) .cloned() - .or_else(|| dbs.first().cloned()) .expect("a daemon follows at least one database"); Self { order: dbs, - by_oid, primary, } } @@ -209,11 +203,6 @@ impl SourceDbs { Self::new(vec![db], primary) } - /// Return `None` for databases this daemon does not replicate - pub fn get(&self, oid: Oid) -> Option<&Arc> { - self.by_oid.get(&oid) - } - pub fn primary(&self) -> &Arc { &self.primary } @@ -222,28 +211,7 @@ impl SourceDbs { &self.order } - pub fn len(&self) -> usize { - self.order.len() - } - - pub fn is_empty(&self) -> bool { - self.order.is_empty() - } - pub fn oids(&self) -> impl Iterator + '_ { self.order.iter().map(|db| db.oid) } - - /// Select affected databases, zero means all configured databases - /// PostgreSQL uses `dbId == 0` to invalidate relation caches in every database - pub fn scoped(&self, db_oid: Oid) -> &[Arc] { - if let Some(db) = self.by_oid.get(&db_oid) { - std::slice::from_ref(db) - } else if db_oid == 0 { - &self.order - } else { - // Ignore databases this daemon does not replicate - &[] - } - } } diff --git a/src/toast/resolver.rs b/src/toast/resolver.rs index 4bde42ad..cbe9b775 100644 --- a/src/toast/resolver.rs +++ b/src/toast/resolver.rs @@ -161,10 +161,6 @@ pub struct ToastRow { } impl> ToastRow { - pub fn from_chunk(c: &ToastChunk) -> Self { - Self::with_body(c, c.chunk_data.clone().into()) - } - pub fn tombstone(d: &ToastDelete) -> Self { Self { toast_relid: d.toast_relid, @@ -377,15 +373,17 @@ impl ChunkAssembler { } } -/// Durable TID-keyed chunk store +/// Durable TID-keyed chunk store, read-only unless it overrides the writes #[async_trait] pub trait ChunkStore: Send + Sync { /// Whether backend accepts decoded chunk rows fn accepts_writes(&self) -> bool { - true + false } /// Replay emits byte-identical rows at equal key and version - async fn put(&self, rows: &[ToastRow]) -> Result<(), ChunkStoreError>; + async fn put(&self, _rows: &[ToastRow]) -> Result<(), ChunkStoreError> { + Err(ChunkStoreError::ReadOnly("put")) + } /// Assemble newest live row per sequence at `max_lsn` against each /// pointer's stored size (`va_extsize`) over one mirror's /// `(value_id, expected_size)` batch, results aligned with `values`. @@ -416,17 +414,21 @@ pub trait ChunkStore: Send + Sync { /// /// Owner TRUNCATE orders destination wipe after replayed fills. DROP callers /// wait until persisted replay floor passes dropping commit - async fn truncate_mirror(&self, toast_relid: u32) -> Result<(), ChunkStoreError>; + async fn truncate_mirror(&self, _toast_relid: u32) -> Result<(), ChunkStoreError> { + Err(ChunkStoreError::ReadOnly("truncate_mirror")) + } /// Rewrite-generation residual deaths `O - B`: tombstone at `commit_lsn` /// every TID live as of `marker_lsn` (generation's `XLOG_SMGR_CREATE`) /// with no row past it. Caller puts the generation's births first. /// Missing mirror is a no-op: nothing lived async fn rewrite_barrier( &self, - toast_relid: u32, - marker_lsn: u64, - commit_lsn: u64, - ) -> Result<(), ChunkStoreError>; + _toast_relid: u32, + _marker_lsn: u64, + _commit_lsn: u64, + ) -> Result<(), ChunkStoreError> { + Err(ChunkStoreError::ReadOnly("rewrite_barrier")) + } } /// In-memory implementation of ClickHouse as-of algorithm @@ -443,6 +445,10 @@ impl MemChunkStore { #[async_trait] impl ChunkStore for MemChunkStore { + fn accepts_writes(&self) -> bool { + true + } + async fn put(&self, rows: &[ToastRow]) -> Result<(), ChunkStoreError> { let mut mirrors = self.mirrors.lock().unwrap(); for r in rows { @@ -706,7 +712,8 @@ impl ClickHouseChunkStore { let insert_timeout = self.dest.current().insert_timeout; state .client - .retry( + .retry_when( + |e| is_retryable(e) && !is_missing_mirror(e), |mut client| async move { let result = with_timeout(insert_timeout, async { client.send_query(sql, None).await?; @@ -902,6 +909,10 @@ fn read_value_block( #[async_trait] impl ChunkStore for ClickHouseChunkStore { + fn accepts_writes(&self) -> bool { + true + } + async fn put(&self, rows: &[ToastRow]) -> Result<(), ChunkStoreError> { if rows.is_empty() { return Ok(()); @@ -1780,12 +1791,6 @@ mod tests { #[async_trait] impl ChunkStore for ReadOnly { - fn accepts_writes(&self) -> bool { - false - } - async fn put(&self, _rows: &[ToastRow]) -> Result<(), ChunkStoreError> { - Err(ChunkStoreError::ReadOnly("put")) - } async fn fetch_many( &self, relid: u32, @@ -1794,17 +1799,6 @@ mod tests { ) -> Result, ChunkStoreError> { self.0.fetch_many(relid, values, max_lsn).await } - async fn truncate_mirror(&self, _relid: u32) -> Result<(), ChunkStoreError> { - Err(ChunkStoreError::ReadOnly("truncate_mirror")) - } - async fn rewrite_barrier( - &self, - _relid: u32, - _marker: u64, - _commit: u64, - ) -> Result<(), ChunkStoreError> { - Err(ChunkStoreError::ReadOnly("rewrite_barrier")) - } } let inner = MemChunkStore::new(); @@ -1866,10 +1860,9 @@ mod tests { let shadow = EmitterConfig::from_toml_str("[toast]\nmode = \"shadow\"\n").unwrap(); assert!(shadow.toast.mode.is_shadow()); - let err = match ToastResolver::for_mode(fixed(shadow), stats(), None) { - Err(e) => e, - Ok(_) => panic!("shadow without a bridge must be a config error"), - }; + let err = ToastResolver::for_mode(fixed(shadow), stats(), None) + .err() + .expect("shadow without a bridge must be a config error"); assert!(err.contains("bridge"), "{err}"); let disabled = EmitterConfig::from_toml_str( diff --git a/src/toast/shadow_store.rs b/src/toast/shadow_store.rs index 9ab00878..b9ec3b9c 100644 --- a/src/toast/shadow_store.rs +++ b/src/toast/shadow_store.rs @@ -16,7 +16,7 @@ use async_trait::async_trait; use crate::ops::bridge::{Bridge, BridgeError, FetchedChunks, MAX_FETCH_VALUES}; use crate::toast::xid_ceiling::{XidCeiling, follows}; -use crate::toast::{ChunkStore, ChunkStoreError, FetchedValue, ToastRow}; +use crate::toast::{ChunkStore, ChunkStoreError, FetchedValue}; /// Round-trip payload target. Always allow one value even when it exceeds limit const FETCH_REQUEST_BYTES: usize = 64 << 20; @@ -184,14 +184,6 @@ impl ShadowToastStore { #[async_trait] impl ChunkStore for ShadowToastStore { - fn accepts_writes(&self) -> bool { - false - } - - async fn put(&self, _rows: &[ToastRow]) -> Result<(), ChunkStoreError> { - Err(ChunkStoreError::ReadOnly("put")) - } - /// Treat `max_lsn` as minimum replay position. Chunks precede referring /// record, so reaching this position makes value available async fn fetch_many( @@ -220,19 +212,6 @@ impl ChunkStore for ShadowToastStore { } Ok(out) } - - async fn truncate_mirror(&self, _toast_relid: u32) -> Result<(), ChunkStoreError> { - Err(ChunkStoreError::ReadOnly("truncate_mirror")) - } - - async fn rewrite_barrier( - &self, - _toast_relid: u32, - _marker_lsn: u64, - _commit_lsn: u64, - ) -> Result<(), ChunkStoreError> { - Err(ChunkStoreError::ReadOnly("rewrite_barrier")) - } } /// Detect reused IDs from chunks newer than referring record diff --git a/src/xact/spill.rs b/src/xact/spill.rs index 55e294d0..341a0fd1 100644 --- a/src/xact/spill.rs +++ b/src/xact/spill.rs @@ -507,15 +507,6 @@ impl BodySpoolWriter { Ok(()) } - /// Total appended bytes - pub fn len(&self) -> u64 { - self.len - } - - pub fn is_empty(&self) -> bool { - self.len == 0 - } - /// Reader handle for batch views; outlives writer and unlink pub fn shared(&self) -> &Arc { &self.shared @@ -1576,34 +1567,11 @@ mod tests { let bc = w.byte_count(); assert!(bc > 0); let mut r = w.finish().await.unwrap(); - match r.next().await.unwrap().unwrap() { - SpillEntry::Heap(b) => { - assert_eq!(b.decoded.xid, 42); - assert_eq!(b.decoded.source_lsn, 0x2000); - assert_eq!(b.descriptor_valid_from, 0x1000); - assert_eq!(b.descriptor, sample_descriptor(16385)); - assert_eq!(b.decoded.new.as_ref().unwrap().columns.len(), 5); - let cols = &b.decoded.new.as_ref().unwrap().columns; - assert!(matches!(cols[0], Some(ColumnValue::Int4(7)))); - assert!(matches!(cols[1], Some(ColumnValue::Text(ref t)) if t == "hello")); - assert!(cols[2].is_none()); - assert!(matches!(cols[3], Some(ColumnValue::Null))); - match &cols[4] { - Some(ColumnValue::ExternalToast(p)) => { - assert_eq!(p.va_valueid, 99); - assert_eq!(p.va_rawsize, 1024); - } - other => panic!("expected ExternalToast, got {other:?}"), - } - } - other => panic!("expected Heap, got {other:?}"), - } - match r.next().await.unwrap().unwrap() { - SpillEntry::Chunk(c2) => { - assert_eq!(c2, c); - } - other => panic!("expected Chunk, got {other:?}"), - } + assert_eq!( + r.next().await.unwrap().unwrap(), + SpillEntry::Heap(Box::new(h)) + ); + assert_eq!(r.next().await.unwrap().unwrap(), SpillEntry::Chunk(c)); assert!(r.next().await.unwrap().is_none(), "EOF expected"); r.unlink().await.unwrap(); } @@ -1623,10 +1591,7 @@ mod tests { .await .unwrap(); let mut r = w.finish().await.unwrap(); - match r.next().await.unwrap().unwrap() { - SpillEntry::ToastDelete(d2) => assert_eq!(d2, d), - other => panic!("expected ToastDelete, got {other:?}"), - } + assert_eq!(r.next().await.unwrap().unwrap(), SpillEntry::ToastDelete(d)); assert!(r.next().await.unwrap().is_none()); r.unlink().await.unwrap(); } @@ -1663,28 +1628,26 @@ mod tests { .await .unwrap(); let mut r = w.finish().await.unwrap(); - match r.next().await.unwrap().unwrap() { - SpillEntry::Raw(got) => { - assert_eq!(*got, raw); - let rec = got.to_xlog_record(); - assert_eq!(rec.header.xact_id, 77, "writer xid restored for _xid"); - assert_eq!(rec.header.resource_manager_id, 10); - assert_eq!(rec.header.info, 0x80); - assert_eq!(rec.blocks.len(), 1); - assert_eq!(rec.blocks[0].header.location.block_no, 3); - assert_eq!(rec.blocks[0].header.image_header.hole_length, 7000); - assert_eq!(&*rec.main_data, &[1, 0, 8]); - assert_eq!( - got.rfn(), - Some(RelFileNode { - spc_node: 1663, - db_node: 5, - rel_node: 24680, - }) - ); - } - other => panic!("expected Raw, got {other:?}"), - } + assert_eq!( + r.next().await.unwrap().unwrap(), + SpillEntry::Raw(Box::new(raw.clone())) + ); + let rec = raw.to_xlog_record(); + assert_eq!(rec.header.xact_id, 77, "writer xid restored for _xid"); + assert_eq!(rec.header.resource_manager_id, 10); + assert_eq!(rec.header.info, 0x80); + assert_eq!(rec.blocks.len(), 1); + assert_eq!(rec.blocks[0].header.location.block_no, 3); + assert_eq!(rec.blocks[0].header.image_header.hole_length, 7000); + assert_eq!(&*rec.main_data, &[1, 0, 8]); + assert_eq!( + raw.rfn(), + Some(RelFileNode { + spc_node: 1663, + db_node: 5, + rel_node: 24680, + }) + ); assert!(r.next().await.unwrap().is_none()); r.unlink().await.unwrap(); } @@ -1774,7 +1737,6 @@ mod tests { let b = w.append(b"world!").unwrap(); assert_eq!((a.offset, a.len), (0, 5)); assert_eq!((b.offset, b.len), (5, 6)); - assert_eq!(w.len(), 11); w.flush().unwrap(); let shared = w.shared().clone(); assert_eq!(shared.read(a).unwrap(), b"hello"); @@ -1964,9 +1926,9 @@ mod tests { "sampled tags are not gapless from {VAL_NULL}" ); let probe = decode_value(&mut Cursor::new(&[next])); - let Err(SpillError::Format { detail, .. }) = probe else { - panic!("tag {next} decodes, so it needs a round-trip sample here") - }; + let detail = probe + .expect_err("decodable tag needs a round-trip sample here") + .to_string(); assert!( detail.contains("unknown ColumnValue tag"), "tag {next} is live, so it needs a round-trip sample here: {detail}" @@ -2044,12 +2006,10 @@ mod tests { fn decode_value_rejects_unknown_tag() { let mut cur = Cursor::new(&[99u8]); let err = decode_value(&mut cur).unwrap_err(); - match err { - SpillError::Format { detail, .. } => { - assert!(detail.contains("unknown ColumnValue tag"), "{detail}"); - } - other => panic!("expected Format, got {other:?}"), - } + assert_eq!( + err.to_string(), + "spill format at offset 1: unknown ColumnValue tag 99" + ); } #[test] @@ -2058,12 +2018,10 @@ mod tests { let buf = [20u8, 99u8]; let mut cur = Cursor::new(&buf); let err = decode_value(&mut cur).unwrap_err(); - match err { - SpillError::Format { detail, .. } => { - assert!(detail.contains("unknown NumericKind tag"), "{detail}"); - } - other => panic!("expected Format, got {other:?}"), - } + assert_eq!( + err.to_string(), + "spill format at offset 2: unknown NumericKind tag 99" + ); } #[test] @@ -2073,12 +2031,10 @@ mod tests { buf.push(99u8); let mut cur = Cursor::new(&buf); let err = decode_heap(&mut cur).unwrap_err(); - match err { - SpillError::Format { detail, .. } => { - assert!(detail.contains("unknown HeapOp tag"), "{detail}"); - } - other => panic!("expected Format, got {other:?}"), - } + assert_eq!( + err.to_string(), + "spill format at offset 25: unknown HeapOp tag 99" + ); } #[tokio::test(flavor = "current_thread")] diff --git a/src/xact/xact_buffer.rs b/src/xact/xact_buffer.rs index a1f120cc..f69d7f78 100644 --- a/src/xact/xact_buffer.rs +++ b/src/xact/xact_buffer.rs @@ -2587,31 +2587,26 @@ impl ValueResolution<'_> { type_oid: u32, ) -> std::result::Result { let key = (p.va_toastrelid, p.va_valueid); - let miss = if self.resolver.fill_on_miss() { - Some(StoreMiss::NoStore) + let cached = if self.resolver.fill_on_miss() { + Err(StoreMiss::NoStore) } else { - let cached = self.cache.get(&key).expect("prefetched with the heap"); - cached.as_ref().err().copied() + let uses = self.uses.get_mut(&key).expect("counted in detoast_heap"); + *uses -= 1; + if *uses > 0 { + self.cache.get(&key).cloned() + } else { + self.cache.remove(&key) + } + .expect("prefetched with the heap") }; - if let Some(miss) = miss { - return fill_store_miss(miss, p, self.resolver, MissPolicy::Streaming) - .map_err(XactBufferError::Detoast); + match cached { + Ok(raw) => { + self.retained += raw.len(); + Ok(detoasted_value(raw, type_oid)) + } + Err(miss) => fill_store_miss(miss, p, self.resolver, MissPolicy::Streaming) + .map_err(XactBufferError::Detoast), } - let uses = self.uses.get_mut(&key).expect("counted in detoast_heap"); - *uses -= 1; - let raw = if *uses > 0 { - let Some(Ok(v)) = self.cache.get(&key) else { - unreachable!("matched Ok above") - }; - v.clone() - } else { - let Some(Ok(v)) = self.cache.remove(&key) else { - unreachable!("matched Ok above") - }; - v - }; - self.retained += raw.len(); - Ok(detoasted_value(raw, type_oid)) } } @@ -2629,6 +2624,7 @@ fn first_missing_seq_ref(v: &ValueRef) -> u32 { } /// Chunk coverage outcome, decompression failures remain errors +#[cfg_attr(test, derive(Debug, PartialEq))] pub(crate) enum Reassembled { Bytes(Vec), Missing, @@ -3640,15 +3636,9 @@ mod tests { #[test] fn xact_buffer_error_converts_to_sink_and_decoder_errors() { let s: SinkError = XactBufferError::Observer("boom".into()).into(); - match s { - SinkError::Other(msg) => assert!(msg.contains("boom"), "{msg}"), - other => panic!("expected SinkError::Other, got {other:?}"), - } + assert_eq!(format!("{s:?}"), r#"Other("observer: boom")"#); let d: DecoderSinkError = XactBufferError::Observer("boom".into()).into(); - match d { - DecoderSinkError::Observer(msg) => assert!(msg.contains("boom"), "{msg}"), - other => panic!("expected DecoderSinkError::Observer, got {other:?}"), - } + assert_eq!(format!("{d:?}"), r#"Observer("observer: boom")"#); } #[tokio::test(flavor = "current_thread")] @@ -3949,13 +3939,13 @@ mod tests { ); // Tier 3 lands as PgPending carrying the body so the oracle resolves // it like an inline value, not Unsupported - match detoasted_value(b"\x01body".to_vec(), TSVECTOROID) { - ColumnValue::PgPending { type_oid, raw } => { - assert_eq!(type_oid, TSVECTOROID); - assert_eq!(raw, b"\x01body"); + assert_eq!( + detoasted_value(b"\x01body".to_vec(), TSVECTOROID), + ColumnValue::PgPending { + type_oid: TSVECTOROID, + raw: b"\x01body".to_vec(), } - other => panic!("expected PgPending, got {other:?}"), - } + ); } /// A detoasted jsonb renders through its codec, same as an inline one @@ -4599,13 +4589,15 @@ mod tests { } } + /// Order label of [`dropped_event`] as [`flatten_batch`] renders it + fn dropped_label(oid: u32) -> String { + format!("{:?}", DrainEntry::Catalog(dropped_event(oid))) + } + /// Interleave a batch's events at their local indices with its heaps: - /// `e` / `h` labels for order assertions. + /// event Debug / `h` labels for order assertions. fn flatten_batch(batch: &DrainedBatch) -> Vec { - let label = |e: &DrainEntry| match e { - DrainEntry::Catalog(SchemaEvent::Dropped { oid, .. }) => format!("e{oid}"), - other => panic!("unexpected event {other:?}"), - }; + let label = |e: &DrainEntry| format!("{e:?}"); let mut out = Vec::new(); let mut ev = 0usize; for (i, h) in batch.heaps.iter().enumerate() { @@ -4636,11 +4628,8 @@ mod tests { let mut order: Vec = Vec::new(); while let Some(batch) = drain.next_batch(8, usize::MAX, None).await.unwrap() { order.extend(flatten_batch(&batch)); - if batch.is_final { - break; - } } - assert_eq!(order, ["e7", "h120", "e9"]); + assert_eq!(order, [dropped_label(7), "h120".into(), dropped_label(9)]); drain.finish().await.unwrap(); } @@ -4737,9 +4726,11 @@ mod tests { b.stash_raw(1, raw).await.unwrap(); inject_ordinary(&mut b, rfn, rel); let mut drain = b.drain_committed(1, 42, 0x2000, &[], false).await.unwrap(); - let Err(err) = drain.next_batch(8, usize::MAX, None).await else { - panic!("expected fail-closed error"); - }; + let err = drain + .next_batch(8, usize::MAX, None) + .await + .err() + .expect("fail-closed error"); assert!( matches!( err, @@ -4768,9 +4759,11 @@ mod tests { .unwrap(); inject_ordinary_fenced(&mut b, rfn, rel, None, vec![rfn_ambiguity(rfn, 100, 200)]); let mut drain = b.drain_committed(1, 42, 0x2000, &[], false).await.unwrap(); - let Err(err) = drain.next_batch(8, usize::MAX, None).await else { - panic!("expected fenced record to fail closed"); - }; + let err = drain + .next_batch(8, usize::MAX, None) + .await + .err() + .expect("fenced record to fail closed"); assert!( matches!( err, @@ -4854,9 +4847,11 @@ mod tests { .unwrap(); let mut b = buffer.lock().await; let mut drain = b.drain_committed(1, 42, 0x2F0, &[], false).await.unwrap(); - let Err(err) = drain.next_batch(8, usize::MAX, None).await else { - panic!("expected the log's interval to fence the stashed record"); - }; + let err = drain + .next_batch(8, usize::MAX, None) + .await + .err() + .expect("the log's interval to fence the stashed record"); assert!( matches!( err, @@ -4992,9 +4987,11 @@ mod tests { b.stash_raw(1, multi_insert_raw(1, 100, 16422, &[1])) .await .unwrap(); - let Err(err) = b.drain_committed(1, 42, 0x2000, &[], false).await else { - panic!("expected fail-closed with no resolution installed"); - }; + let err = b + .drain_committed(1, 42, 0x2000, &[], false) + .await + .err() + .expect("fail-closed with no resolution installed"); assert!( matches!(err, XactBufferError::MissingStashResolution { top_xid: 1 }), "{err}" @@ -5031,9 +5028,11 @@ mod tests { b.stash_raw(1, raw).await.unwrap(); inject_ordinary(&mut b, rfn, rel); let mut drain = b.drain_committed(1, 42, 0x2000, &[], false).await.unwrap(); - let Err(err) = drain.next_batch(8, usize::MAX, None).await else { - panic!("expected fail-closed error"); - }; + let err = drain + .next_batch(8, usize::MAX, None) + .await + .err() + .expect("fail-closed error"); assert!( matches!( err, @@ -5060,9 +5059,11 @@ mod tests { .unwrap(); inject_ordinary(&mut b, rfn, rel); let mut drain = b.drain_committed(1, 42, 0x2000, &[], false).await.unwrap(); - let Err(err) = drain.next_batch(8, usize::MAX, None).await else { - panic!("expected foreign-xid error"); - }; + let err = drain + .next_batch(8, usize::MAX, None) + .await + .err() + .expect("foreign-xid error"); assert!( matches!(err, XactBufferError::ForeignXid { xid: 99, top: 1 }), "{err}" @@ -5148,9 +5149,11 @@ mod tests { b.stash_raw(1, raw).await.unwrap(); inject_ordinary(&mut b, rfn, rel); let mut drain = b.drain_committed(1, 42, 0x2000, &[], false).await.unwrap(); - let Err(err) = drain.next_batch(8, usize::MAX, None).await else { - panic!("expected fail-closed error"); - }; + let err = drain + .next_batch(8, usize::MAX, None) + .await + .err() + .expect("fail-closed error"); assert!( matches!( err, @@ -5375,11 +5378,18 @@ mod tests { break; } } + let h = |lsn: u64| format!("h{lsn}"); assert_eq!( order, - ["h100", "h110", "e7", "h120", "h130", "h140", "e8"] - .map(str::to_string) - .to_vec(), + [ + h(100), + h(110), + dropped_label(7), + h(120), + h(130), + h(140), + dropped_label(8) + ] ); assert!(finals.pop().unwrap(), "last slice flags final"); assert!(finals.iter().all(|f| !f), "earlier slices non-final"); @@ -5454,10 +5464,10 @@ mod tests { assert_eq!((v.run_chunks, v.tail.len()), (0, 2)); let spool = b2.chunks.iter().find_map(|g| g.spool()); assert!(spool.is_none(), "no spool below threshold"); - let Reassembled::Bytes(raw) = reassemble_value_ref(&p, spool, v).unwrap() else { - panic!("value visible"); - }; - assert_eq!(raw, b"abcd"); + assert_eq!( + reassemble_value_ref(&p, spool, v).unwrap(), + Reassembled::Bytes(b"abcd".to_vec()) + ); drain.finish().await.unwrap(); } @@ -5553,10 +5563,10 @@ mod tests { let mut rows = 0usize; while let Some(batch) = drain.next_batch(4, usize::MAX, None).await.unwrap() { rows += batch.heaps.len(); + let resident = b.drain_resident_bytes(); assert!( - b.drain_resident_bytes() < read_buffers + total / 4, - "resident {} vs xact {total}", - b.drain_resident_bytes(), + resident < read_buffers + total / 4, + "resident {resident} vs xact {total}" ); if batch.is_final { break; @@ -5564,10 +5574,10 @@ mod tests { } assert_eq!(rows as u64, n); assert!(b.drain_resident_peak() > 0, "gauge saw the merge heads"); + let peak = b.drain_resident_peak(); assert!( - b.drain_resident_peak() < read_buffers + total / 4, - "peak {} vs xact {total}", - b.drain_resident_peak(), + peak < read_buffers + total / 4, + "peak {peak} vs xact {total}" ); drain.finish().await.unwrap(); assert_eq!(b.drain_resident_bytes(), 0, "gauge drains with the drain"); @@ -5685,10 +5695,8 @@ mod tests { .find_map(|g| g.get(&(16400, 50))) .unwrap(); assert_eq!((mem_v.run_chunks, mem_v.tail.len()), (0, 1)); - let Reassembled::Bytes(raw) = reassemble_value_ref(&p(50), spool, mem_v).unwrap() else { - panic!("mem value visible"); - }; - assert_eq!(raw.len(), 512); + let whole = Reassembled::Bytes(body.to_vec()); + assert_eq!(reassemble_value_ref(&p(50), spool, mem_v).unwrap(), whole); let file_v = last .chunks .iter() @@ -5698,11 +5706,10 @@ mod tests { (file_v.run_chunks, file_v.run.len, file_v.tail.len()), (1, 512, 0) ); - let Reassembled::Bytes(raw) = reassemble_value_ref(&p(50 + n - 1), spool, file_v).unwrap() - else { - panic!("file value visible"); - }; - assert_eq!(raw.len(), 512); + assert_eq!( + reassemble_value_ref(&p(50 + n - 1), spool, file_v).unwrap(), + whole + ); // Mirror row refs materialize from the same spool let rows = &held.last().unwrap().new_rows; for r in rows.iter() { @@ -5720,11 +5727,10 @@ mod tests { ); // Held readers survive unlink via open fd let spool = last.chunks.iter().find_map(|g| g.spool()); - let Reassembled::Bytes(raw) = reassemble_value_ref(&p(50 + n - 1), spool, file_v).unwrap() - else { - panic!("read-after-unlink via open fd"); - }; - assert_eq!(raw.len(), 512); + assert_eq!( + reassemble_value_ref(&p(50 + n - 1), spool, file_v).unwrap(), + whole + ); held.clear(); assert_eq!(b.drain_chunk_resident_bytes(), 0); assert_eq!(b.drain_row_resident_bytes(), 0); @@ -5744,9 +5750,11 @@ mod tests { b.on_toast_chunk(chunk(51, 0, 102, b"bb"), 9).await.unwrap(); b.on_heap(heap_with_value(9, 110, 16)).await.unwrap(); let mut drain = b.drain_committed(9, 0, 0x1000, &[], true).await.unwrap(); - let Err(err) = drain.next_batch(usize::MAX, usize::MAX, None).await else { - panic!("cap breach surfaces"); - }; + let err = drain + .next_batch(usize::MAX, usize::MAX, None) + .await + .err() + .expect("cap breach surfaces"); assert!(matches!( err, XactBufferError::ToastIndexOverflow { max, .. } if max == 3 * CHUNK_REF_META diff --git a/tests/bin_stream_e2e.rs b/tests/bin_stream_e2e.rs index 75c6d26a..48053088 100644 --- a/tests/bin_stream_e2e.rs +++ b/tests/bin_stream_e2e.rs @@ -529,8 +529,7 @@ async fn bin_stream_replicates_segments_and_serves_metrics() { })(); if !daemon_killed { - let _ = child.kill(); - let _ = child.wait(); + tools::stop_gracefully(&mut child); } if let Err(e) = result { let stderr = fs::read_to_string(&stderr_path).unwrap_or_default(); @@ -776,8 +775,7 @@ async fn wire_drop_midsegment_shadow_resumes_streaming() { if let Some(mut w) = writer { kill_group(&mut w); } - let _ = child.kill(); - let _ = child.wait(); + tools::stop_gracefully(&mut child); if let Err(e) = result { let stderr = fs::read_to_string(&stderr_path).unwrap_or_default(); let slog = fs::read_to_string(shadow_data.join("startup.log")).unwrap_or_default(); @@ -906,8 +904,7 @@ async fn process_restart_preserves_shadow_postmaster() { } .await; if result.is_err() { - let _ = child.kill(); - let _ = child.wait(); + tools::stop_gracefully(&mut child); } assert!( result.is_ok(), diff --git a/tests/bootstrap_types_e2e.rs b/tests/bootstrap_types_e2e.rs index 5ce30fcb..3c77d212 100644 --- a/tests/bootstrap_types_e2e.rs +++ b/tests/bootstrap_types_e2e.rs @@ -232,10 +232,7 @@ async fn direct_bootstrap_all_types_end_to_end() { Ok(()) })(); - let _ = guard.into_inner().map(|mut c| { - let _ = c.kill(); - let _ = c.wait(); - }); + drop(guard); if bootstrap_shadow_data_dir.join("postmaster.pid").exists() { let mut shadow_cfg = ShadowConfig::new(bootstrap_shadow_data_dir.clone(), shadow_filter_dir.clone()); diff --git a/tests/bootstrap_window_wal_not_landed_raw_e2e.rs b/tests/bootstrap_window_wal_not_landed_raw_e2e.rs index 57fc86d2..1804e598 100644 --- a/tests/bootstrap_window_wal_not_landed_raw_e2e.rs +++ b/tests/bootstrap_window_wal_not_landed_raw_e2e.rs @@ -28,8 +28,11 @@ use anyhow::{Context, Result}; use walshadow::shadow::Shadow; const ROWS: i32 = 30_000; -/// 4 MB/s holds the backup open long enough to write a real window -const MAX_RATE_KIB: &str = "4096"; +/// 8 MB/s holds the backup open long enough to write a real window +const MAX_RATE_KIB: &str = "8192"; +/// Enough to dirty pages in the window. Each segment of WAL they add rides in +/// the tar at the rate limit +const WINDOW_ROUNDS: i32 = 10; /// `Shadow::initdb` plus `-k`: checksums make every hint-bit set log a page /// image, the deployed source's configuration @@ -157,11 +160,11 @@ async fn backup_window_wal_never_materialises_the_shadow() { fx::wait_for_backup_streaming(&source, Duration::from_secs(120)) .context("BASE_BACKUP never opened")?; - // Dirty distinct heap pages for as long as the backup runs. With - // checksums on, each first touch since the checkpoint carries a page - // image, so this is the exact WAL shape that re-materialises a shadow. + // Dirty distinct heap pages while the backup runs. With checksums on, + // each first touch since the checkpoint carries a page image, so this + // is the exact WAL shape that re-materialises a shadow let mut round = 0i32; - while fx::backup_in_progress(&source) { + while round < WINDOW_ROUNDS && fx::backup_in_progress(&source) { let from = (round * 500) % ROWS + 1; source .apply_schema_dump(&format!( diff --git a/tests/common/bootstrap_ch_fixture.rs b/tests/common/bootstrap_ch_fixture.rs index b34260fa..4ea1fd66 100644 --- a/tests/common/bootstrap_ch_fixture.rs +++ b/tests/common/bootstrap_ch_fixture.rs @@ -415,17 +415,11 @@ pub fn wait_for_ch_value(ch: &ChServer, sql: &str, want: &str, timeout: Duration } } -/// Kill the daemon before the shadow, so its supervisor cannot restart the -/// postmaster, then stop whatever SIGKILL left behind and report `result` -/// with the daemon's log attached. +/// Stop the daemon before the shadow, so its supervisor cannot restart the +/// postmaster, then stop whatever it left behind and report `result` with +/// the daemon's log attached. pub fn finish_daemon(guard: ChildGuard, daemon: &DaemonRun, result: Result<()>) { - if let Some(mut child) = guard.into_inner() { - // SIGINT drains and exits, so instrumented builds write their profile - let _ = Command::new("kill") - .args(["-INT", &child.id().to_string()]) - .status(); - let _ = wait_with_timeout(&mut child, Duration::from_secs(15)); - } + drop(guard); daemon.stop_shadow(); if let Err(e) = result { panic!("{e:#}\n--- daemon stderr ---\n{}", daemon.stderr()); @@ -626,9 +620,8 @@ pub fn wait_with_timeout( bail!("walshadow-stream did not exit within {deadline:?}"); } -/// RAII wrapper that SIGKILLs the walshadow-stream subprocess on drop -/// (test failure path). Tests that own a clean exit consume the child -/// via `wait_with_timeout` first. +/// RAII wrapper that stops the walshadow-stream subprocess on drop. Tests +/// that own a clean exit consume the child via `wait_with_timeout` first. pub struct ChildGuard { pub child: Option, } @@ -646,8 +639,7 @@ impl ChildGuard { impl Drop for ChildGuard { fn drop(&mut self) { if let Some(mut c) = self.child.take() { - let _ = c.kill(); - let _ = c.wait(); + tools::stop_gracefully(&mut c); } } } diff --git a/tests/common/tools.rs b/tests/common/tools.rs index 3b5a7e37..5f521661 100644 --- a/tests/common/tools.rs +++ b/tests/common/tools.rs @@ -6,10 +6,31 @@ use std::fmt::Display; use std::path::Path; -use std::process::{Command, Stdio}; +use std::process::{Child, Command, Stdio}; +use std::time::{Duration, Instant}; const REQUIRE: &str = "WALSHADOW_REQUIRE_TOOLS"; +/// Enable every callsite so coverage runs evaluate tracing field expressions +#[ctor::ctor(unsafe)] +fn enable_tracing() { + let _ = tracing::subscriber::set_global_default(tracing_subscriber::registry()); +} + +/// SIGINT, then SIGKILL if still running after 15 s. Instrumented binaries +/// write their coverage profile only on exit, never under SIGKILL +pub fn stop_gracefully(child: &mut Child) { + let _ = Command::new("kill") + .args(["-INT", &child.id().to_string()]) + .status(); + let deadline = Instant::now() + Duration::from_secs(15); + while matches!(child.try_wait(), Ok(None)) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(100)); + } + let _ = child.kill(); + let _ = child.wait(); +} + /// Report a skip and return `false`, panic when CI requires every tool pub fn skip(reason: impl Display) -> bool { assert!( diff --git a/tests/control_plane_e2e.rs b/tests/control_plane_e2e.rs index c23cc254..c8d5857c 100644 --- a/tests/control_plane_e2e.rs +++ b/tests/control_plane_e2e.rs @@ -529,23 +529,8 @@ impl Harness { /// SIGINT → graceful drain so tracing flushes to the stderr file; SIGKILL /// only if it doesn't exit promptly. fn stop_daemon(&mut self) { - let Some(mut c) = self.child.take() else { - return; - }; - let _ = Command::new("kill") - .args(["-INT", &c.id().to_string()]) - .status(); - let deadline = Instant::now() + Duration::from_secs(15); - loop { - match c.try_wait() { - Ok(Some(_)) => break, - _ if Instant::now() >= deadline => { - let _ = c.kill(); - let _ = c.wait(); - break; - } - _ => std::thread::sleep(Duration::from_millis(100)), - } + if let Some(mut c) = self.child.take() { + fx::tools::stop_gracefully(&mut c); } } @@ -905,10 +890,11 @@ async fn ch_password_rotation_via_ctl_keeps_streaming() { // A credential the server rejects must surface as the inserter's own // refusal, which is what proves the dial reads live config rather than - // the value it booted on. + // the value it booted on. No retries, so the refusal surfaces on the + // first dial rather than after the backoff h.ctl_body( &["apply"], - "[ch]\npassword = \"wrong\"\ncompression = \"lz4\"\n", + "[ch]\npassword = \"wrong\"\ncompression = \"lz4\"\nretry_max_attempts = 0\n", )?; h.psql("UPDATE demo.users SET email = 'wrong@x' WHERE id = 1")?; h.wait_log("Authentication failed", Duration::from_secs(30)) @@ -1369,7 +1355,7 @@ async fn pause_and_stop_writes(h: &Harness, target: &Shadow) -> Result<()> { // 3. A paused walshadow sends no standby status, so its walsender only exits // on `wal_sender_timeout` and fast shutdown waits for that h.psql("INSERT INTO demo.users VALUES (2, 'bob', 'below-fork@x')")?; - h.psql("ALTER SYSTEM SET wal_sender_timeout = '5s'")?; + h.psql("ALTER SYSTEM SET wal_sender_timeout = '1s'")?; h.psql("SELECT pg_reload_conf()")?; h.source.stop().context("stop source primary")?; @@ -2092,7 +2078,7 @@ async fn restart_while_paused_refreezes_the_frontier_and_still_crosses() { // The source is down and `[source]` still names it, so boot waits for // the repoint rather than exiting into a supervisor loop h.stop_daemon(); - h.start_daemon(Duration::from_secs(8)) + h.start_daemon(Duration::from_secs(3)) .await .expect_err("pump must not resume while the source it is pointed at is stopped"); ensure!( diff --git a/tests/fpi_user_pages.rs b/tests/fpi_user_pages.rs index d523d0f0..06ac3854 100644 --- a/tests/fpi_user_pages.rs +++ b/tests/fpi_user_pages.rs @@ -229,11 +229,15 @@ async fn user_page_images_never_reach_the_shadow() { DELETE FROM big WHERE id % 7 = 0;\n\ CHECKPOINT;\n\ VACUUM (FREEZE) big;\n\ - VACUUM FULL big;\n\ - SELECT pg_switch_wal();\n", + VACUUM FULL big;\n", ) .expect("dirty hint bits and vacuum"); + // Taken before the switch: the next record past it is background WAL, + // which can be a bgwriter snapshot interval away let target = wal_insert_lsn(&source); + source + .psql_one("SELECT pg_switch_wal()") + .expect("switch wal"); let deadline = Instant::now() + Duration::from_secs(60); while census.max_next_lsn < target && Instant::now() < deadline { diff --git a/tests/kill_restart.rs b/tests/kill_restart.rs index c1ff4358..036af05f 100644 --- a/tests/kill_restart.rs +++ b/tests/kill_restart.rs @@ -608,13 +608,8 @@ async fn run_cycle( format_pg_lsn(ack), ); - // 11. Drain restart daemon for the next cycle. `into_inner` strips - // the guard so we drive the SIGKILL + reap explicitly — letting - // the std::process::Child drop on its own would leak the - // subprocess (Drop doesn't kill). - if let Some(c) = restart_guard.into_inner() { - kill_and_reap(c); - } + // 11. Drain restart daemon for the next cycle + drop(restart_guard); eprintln!("kill-restart: cycle ok strategy={strategy:?} run={run}"); Ok(()) diff --git a/tests/optin_initial_load_none_e2e.rs b/tests/optin_initial_load_none_e2e.rs index bc7b16d5..739248b7 100644 --- a/tests/optin_initial_load_none_e2e.rs +++ b/tests/optin_initial_load_none_e2e.rs @@ -191,10 +191,7 @@ async fn optin_initial_load_none_skips_snapshot_but_streams_cdc() { Ok(()) })(); - let _ = guard.into_inner().map(|mut c| { - let _ = c.kill(); - let _ = c.wait(); - }); + drop(guard); if bootstrap_shadow_data_dir.join("postmaster.pid").exists() { let mut shadow_cfg = ShadowConfig::new(bootstrap_shadow_data_dir.clone(), shadow_filter_dir.clone()); diff --git a/tests/pgbench_acceptance.rs b/tests/pgbench_acceptance.rs index 249e0b8f..315f5031 100644 --- a/tests/pgbench_acceptance.rs +++ b/tests/pgbench_acceptance.rs @@ -494,15 +494,14 @@ async fn run_ddl_intermix( // 14. Force a segment seal so the daemon's pump definitely // reaches every committed row in WAL, then poll the daemon's // `walshadow_emitter_ack_lsn` until it crosses source's - // post-switch `pg_current_wal_lsn`. The switch also moves - // source's insert pointer past its last standby snapshot, - // so bgwriter logs another one within - // `LOG_SNAPSHOT_INTERVAL_MS` — that record is what carries - // the ack over the target. ChildGuard's Drop will SIGKILL - // the daemon once assertions pass. + // pre-switch insert position. The switch record starts there, + // so it carries the ack over the target. A post-switch target + // waits on bgwriter's next standby snapshot instead, up to + // `LOG_SNAPSHOT_INTERVAL_MS`. ChildGuard's Drop stops the + // daemon once assertions pass. + let target_lsn_text = psql_source(&source, "SELECT pg_current_wal_insert_lsn()::text") + .context("read source LSN")?; psql_source(&source, "SELECT pg_switch_wal()").context("pg_switch_wal")?; - let target_lsn_text = - psql_source(&source, "SELECT pg_current_wal_lsn()::text").context("read source LSN")?; let target_lsn = walshadow::pg::parse_pg_lsn(&target_lsn_text).context("parse source LSN")?; wait_for_metric_lsn( @@ -592,12 +591,9 @@ async fn run_ddl_intermix( Ok(()) })(); - // Kill daemon before shadow so supervisor cannot restart it + // Stop daemon before shadow so supervisor cannot restart it // Stop any remaining postmaster before tempdir cleanup - let _ = guard.into_inner().map(|mut c| { - let _ = c.kill(); - let _ = c.wait(); - }); + drop(guard); if bootstrap_shadow_data_dir.join("postmaster.pid").exists() { let mut shadow_cfg = ShadowConfig::new(bootstrap_shadow_data_dir.clone(), shadow_filter_dir.clone()); diff --git a/tests/shadow_stays_catalog_scale_e2e.rs b/tests/shadow_stays_catalog_scale_e2e.rs index caa67676..ae8959f9 100644 --- a/tests/shadow_stays_catalog_scale_e2e.rs +++ b/tests/shadow_stays_catalog_scale_e2e.rs @@ -296,10 +296,7 @@ async fn user_relation_never_materialises_on_the_shadow() { Ok(()) })(); - let _ = guard.into_inner().map(|mut c| { - let _ = c.kill(); - let _ = c.wait(); - }); + drop(guard); if bootstrap_shadow_data_dir.join("postmaster.pid").exists() { let mut shadow_cfg = ShadowConfig::new(bootstrap_shadow_data_dir.clone(), shadow_filter_dir.clone());