Skip to content

adapter: Record replica hydration peaks per process - #38864

Merged
aljoscha merged 1 commit into
MaterializeInc:mainfrom
aljoscha:replica-hydration-history-process
Sep 17, 2026
Merged

aljoscha merged 1 commit into
MaterializeInc:mainfrom
aljoscha:replica-hydration-history-process

Conversation

@aljoscha

@aljoscha aljoscha commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Replica hydration history reports the largest memory and disk peaks across processes. That hides resource skew on multi-process replicas, and the two maxima can come from different processes.

Description

Add process_id (uint8, matching other replica builtins) to mz_internal.mz_replica_hydration_history. Record one row per process for each completed replica-wide episode. Memory and disk peaks belong to that process, while episode timing, status, and object count remain replica-wide and repeat across process rows.

Keep the all-process completeness gate and episode-wide deduplication. Select scratch-filesystem peaks with swap fallback independently for each process, and include the process ID in retention retractions.

Peaks still cover process lifetimes through collection, not just hydration. Summing process peaks does not give a simultaneous replica-wide peak.

Coverage

Adapt the hydration-history restart regression to a two-process replica. It checks one row per process, consistent replica-wide episode fields, preservation across restart, and deduplication across repeated sweeps. Update the existing testdrive assertions for per-process rows and peak comparisons, the catalog documentation golden, and the collector completeness assertion.

Release note

Expose per-process memory and disk peaks in replica hydration history so users can inspect resource skew across replica processes.

Expose process_id in replica hydration history so resource skew remains
visible. Keep episode timing and object counts replica-wide.

Co-authored-by: Amp <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-01a0abb2-a882-711f-b319-58176af0fa84
@aljoscha
aljoscha force-pushed the replica-hydration-history-process branch from 87370d4 to 00bbfdc Compare September 16, 2026 20:51
@aljoscha
aljoscha marked this pull request as ready for review September 16, 2026 20:59
@aljoscha
aljoscha requested review from a team as code owners September 16, 2026 20:59
@aljoscha
aljoscha force-pushed the replica-hydration-history-process branch from 00bbfdc to 93f30f8 Compare September 16, 2026 21:00

@antiguru antiguru left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@def-

def- commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- the cluster-sizing guide still documents one row per episode and a replica-wide peak

doc/user/content/clusters/sizing.md:76

The sizing guide, which mz_replica_hydration_history's own column documentation links to as the way to use these columns, still states that the table "holds one row per replica-wide hydration episode" and that peak_memory "is the highest memory any process on the replica reached". After this change both statements are false on multi-process sizes, and the recipe printed there returns one duplicate-looking row per process with no column that distinguishes them.

Details

The query at sizing.md:79-90 selects no process_id, so on a scale > 1 replica it now emits one row per process per episode, differing only in peak_memory and peak_disk while started_at, hydration_time and object_count repeat. The sample output block at :93-98 shows (1 row) and gives no hint that more are coming, and the prose at :100-103 asserts exactly the max-across-processes reading this PR removed. That is the reading the updated mz_internal.md now warns against ("Do not sum object counts across process rows", "Summing process peaks does not give a simultaneous replica-wide peak"), so the two user-facing pages contradict each other. The multi-process sizes are the ones where this matters most: they are both the motivation for the change and the sizes where a sizing decision is expensive to get wrong.

Fix: add h.process_id to the recipe's select list and ORDER BY, and rewrite :100-103 as a per-process peak, pointing the reader at the largest of the episode's process rows to compare against mz_cluster_replica_sizes.memory_bytes (which the guide already notes is per process).

Separately, doc/developer/design/20260827_durable_replica_hydration_history.md:48-50 still reads "The maximum process peak therefore answers whether any process approached its limit" immediately below the paragraph this PR rewrote to "Each process contributes a row"; no maximum is taken any more.

@aljoscha
aljoscha force-pushed the replica-hydration-history-process branch from 93f30f8 to 00bbfdc Compare September 17, 2026 10:35
@aljoscha
aljoscha merged commit 9297cd2 into MaterializeInc:main Sep 17, 2026
89 checks passed
@aljoscha
aljoscha deleted the replica-hydration-history-process branch September 17, 2026 12:37
bosconi added a commit that referenced this pull request Sep 17, 2026
…rced ones (#38917)

### Motivation

Since #38864 every 0dt upgrade from a catalog written before v26.41.0 to
`main` crash-loops the new environmentd in read-only mode
([SQL-713](https://linear.app/materializeinc/issue/SQL-713)):

```
environmentd: fatal: cannot migrate builtin schemas from version 26.19.0 to version 26.43.0-dev.0:
missing shard ID for builtin SystemObjectDescription { schema_name: "mz_internal", object_type: Table, object_name: "mz_replica_hydration_history" }
```

Six jobs on
[nightly#18324](https://buildkite.com/materialize/nightly/builds/18324):
all four `Postgres CDC (before source versioning, multi-version upgrade,
...)` shards (from 0.147.0 and 26.19.0) and both `Checks Self-Managed
earliest to latest direct upgrade` shards (from 0.147.0). The same jobs
passed on
[#18320](https://buildkite.com/materialize/nightly/builds/18320), the
previous `main` nightly.

#38864 registers `MigrationStep::evolution("26.43.0-dev.0", ...,
"mz_replica_hydration_history")`. That table first shipped in v26.41.0,
so a source catalog older than that has no shard for it.
`plan_migration` keeps every step above the source version, the plan
reaches `migrate_evolve_one`, the shard lookup finds nothing, and the
read-only branch bails. In leader mode the same case returns `Ok(())`,
which is why the in-place `Checks ... upgrade` scenarios pass on the
same build and only the 0dt-preflight jobs fail.

#36668 fixed this exact bail for the *forced* dev-to-dev plan by
filtering shardless objects out of `plan_forced_migration`: "there is
nothing to evolve or replace for an object that does not yet exist in
persist." The versioned plan never got the filter, so any evolution step
for a builtin younger than the source version reproduces it. #38788's
step for `mz_cluster_replica_metrics_history` did not, only because that
table predates every supported source version.

### Description

Move the filter to the one place both plans pass through.
`Migration::run` now calls `drop_shardless` on the selected plan, forced
or versioned, before executing it; `plan_forced_migration` loses its
private copy of the same filter. A builtin with no registered shard does
not exist in persist yet, whether it was added in the target version or
the source version predates it entirely, and in both cases the leader
creates it at the target schema during bootstrap. The read-only bail in
`migrate_evolve_one` stays as the fail-fast for anything that still
reaches it.

This keeps the semantics #36668 chose rather than adding a new one: the
read-only race the bail guards against, a leader registering the shard
while a read-only process evolves it, cannot involve a builtin the
source version never had.

### Verification

New turmoil test `test_evolution_skips_builtin_without_shard`: one
builtin table with a shard and a stale fingerprint (the pre-evolution
schema), one without a shard at the current fingerprint (the shape
catalog open produces for a builtin new to the durable catalog), steps
at 0.2.0 and 0.3.0 over a 0.1.0 source, run once read-only and once as
leader. Both runs must succeed, evolve the first table, and leave the
second out of `new_fingerprints`.

- Against `main`'s `builtin_schema_migration.rs` the read-only run fails
with `missing shard ID for builtin SystemObjectDescription {
schema_name: "schema", object_type: Table, object_name: "new_table" }
(s2)`, the production message.
- With this change both runs pass, and the existing randomized
`test_builtin_schema_migration` harness passes.

Each turmoil test was run in its own process (`cargo test -p mz-adapter
--lib -- <full path> --exact`), which is what nextest does in CI.
Running the module's tests together in one process fails independently
of this change: `configure_tracing_for_turmoil` installs a global
subscriber under a `Once`, and when
`hydration_history_forced_migration_policy` (`#[mz_ore::test]`) runs
first the install fails and poisons the `Once` for every turmoil test
after it. Pre-existing, not addressed here.

`cargo fmt` clean; `cargo clippy -p mz-adapter --tests -D warnings`
clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5.1 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants