Skip to content

catalog: add mz_object_hydration_history - #38346

Open
aljoscha wants to merge 1 commit into
aljoscha/hydration-02-computefrom
aljoscha/hydration-03-catalog
Open

catalog: add mz_object_hydration_history#38346
aljoscha wants to merge 1 commit into
aljoscha/hydration-02-computefrom
aljoscha/hydration-03-catalog

Conversation

@aljoscha

@aljoscha aljoscha commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Stacked on #38345.

Motivation

The durable surface that hydration episodes are recorded into. Nothing writes it yet, the collector arrives in the next PR, so this can be reviewed purely as a catalog change.

Design doc: 20260817_durable_object_hydration_history.md

Description

Adds mz_internal.mz_object_hydration_history. No index, see below.

The table is in mz_internal rather than mz_catalog because its contents are best effort and its status column and nullable timestamps will gain meanings as more hydration events become observable. Its closest sibling, mz_internal.mz_object_arrangement_size_history, sits there for the same reason.

An episode is identified by (object_id, replica_id, installed_at). The replica-stamped installation time is used because it is stable across an environmentd restart, and because started_at is null while an object waits to run.

That identity is not declared as a key on the relation. A key is a promise to the optimizer, which may then elide a DISTINCT or assume a join cardinality, so one duplicate from a best-effort sampler becomes a silently wrong answer rather than a duplicate row. The collector's anti-join keeps the identity unique, and none of the comparable history tables (mz_object_arrangement_size_history, mz_cluster_replica_metrics_history, mz_cluster_replica_status_history, mz_storage_usage_by_shard) declare a key either.

There is also no index. An arrangement on the catalog server would hold the whole table, which grows with objects times replicas times re-hydrations, and nothing queries this table by key yet. A user query scans a table bounded by retention, and an index would not help the collector anyway, since its anti-join runs on the targeted user replica.

Adding one later is not only an index: make_mz_indexes inlines the builtin index set as VALUES, so a new index changes the mz_indexes fingerprint and needs a MigrationStep::replacement pinned to the then-current dev version, or the fingerprint check panics at catalog open. That is called out in the commit message for whoever does it.

Two things about durability are worth pulling out of the diff:

  • The contents are exempt from the bootstrap reset and from forced schema migrations, because a sampled history cannot be rebuilt from anything else once it is gone. Those two exemptions are the entire promise. The assert added in plan_migration is a tripwire, not a wall: there is no correctness hazard in truncating this table, so the comment there says how to give the exemption up deliberately and to note in the release notes that the history restarts. The user-facing docs say the matching thing, that this is a record to look at and not a data source to build on.
    The bootstrap-reset filter is also converted from comparing names and schema specifiers to a set of resolved ids, which is what adding a third table to it made worth doing.

Verification

Catalog goldens (builtin counts, OIDs, index accounting, information_schema, the autogenerated mz_internal relation spec) are updated. test_builtin_schema_migration exercises the migration path.

This release will add the mz_internal.mz_object_hydration_history table.

Ref: SQL-644

@linear-code

linear-code Bot commented Aug 19, 2026

Copy link
Copy Markdown

SQL-632

SQL-644

| `created_at` | [`timestamp with time zone`] | Wall-clock timestamp of when the object was created. `NULL` for built in system objects. |
| `dropped_at` | [`timestamp with time zone`] | Wall-clock timestamp of when the object was dropped. `NULL` for built in system objects or if the object hasn't been dropped. |

## `mz_object_hydration_history`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this needs to be cut down quite a bit and focus on the facts, no handwaving about support and whatnot, when we ship the feature this will be on. Also cut the stuff about surviving restarts, this is expected of tables.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Cut to two paragraphs, matching the length of siblings like mz_cluster_replica_metrics_history. Dropped the restart sentence and the whole "contact support to enable" paragraph. What's left is what it records, 30 day retention, that ids may name dropped objects, and that recording is best effort with only successful hydration captured.

is_retained_metrics_object: true,
});

/// Completed hydration episodes, one row per object, replica, and installation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

cut this down to teh facts please

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, down to three lines: exempt from the bootstrap reset and forced migrations because the contents can't be rebuilt, and clearing them for a schema change is still allowed with a pointer to the tripwire. The reasoning about why that trade is acceptable lives in the design doc instead.

Comment thread src/catalog/src/builtin/mz_internal.rs Outdated
ontology: None,
});

pub static MZ_OBJECT_HYDRATION_HISTORY_IND: LazyLock<BuiltinIndex> =

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

are we sure we want the index? what do comparable internals have?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Checked the closest comparable. mz_internal.mz_object_arrangement_size_history is the same kind of thing, a durable retention-bounded internal history table, and it carries two builtin indexes: mz_object_arrangement_size_history_object_ind on (object_id) and mz_object_arrangement_size_history_ts_ind on (collection_timestamp), both with is_retained_metrics_object: true.

So one index on (object_id) with is_retained_metrics_object: true is exactly the sibling's first index, and I'd keep it. I deliberately did not copy the sibling's second one: our retention deletes a bounded batch per sweep rather than being something users scan, so indexing finished_at would cost every environment an arrangement to speed up a background job that is already cheap.

This also settles a question I had flagged separately, whether is_retained_metrics_object should be false here. The sibling precedent is unambiguous, so true stays.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Following up on my own answer: you are right, and my precedent argument was the wrong test. is_retained_metrics_object: true resolves to a 30 day logical compaction window (metrics_retention), which holds the since back so the shard, and for the index the arrangement in memory, retain 30 days of update history rather than just current state.

Our history lives in the rows, and the retention sweep retracts them on its own schedule, so nothing here needs to be readable at an old timestamp. The window would buy nothing and cost every environment memory on the catalog server. Set to false on both the table and the index, with the reasoning recorded at the definition.

That the sibling sets true while also pruning explicitly looks like the same redundancy, but I left it alone rather than widen this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ahh, I really think we shouldn't add the index by default, the history collection can grow quite a bit, so this index might grow out of hand, no?

Comment thread src/catalog/src/builtin.rs Outdated
Builtin::View(&MZ_INDEX_ADVICE),
Builtin::View(&MZ_MCP_DATA_PRODUCTS),
Builtin::View(&MZ_MCP_DATA_PRODUCT_DETAILS),
// NOTE: This list is a dependency order, and in a fresh environment it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

why did we add this comment? is it not clear from how things work so far? Might just want to drop this and spare the noise?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair, dropped. It was explaining a property of the list that holds regardless of this change.

@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch 3 times, most recently from 49e130d to 9783a3e Compare August 19, 2026 16:45
## `mz_object_hydration_history`

The `mz_object_hydration_history` table records completed hydration of indexes and
materialized views, one row per object, replica, and installation. Rows are retained

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

what's the "installation" here?

Comment thread src/catalog/src/builtin/mz_internal.rs Outdated
// its own schedule. Nothing reads this table at an old timestamp.
is_retained_metrics_object: false,
access: vec![PUBLIC_SELECT],
// No ontology links: a history row deliberately outlives the object and the

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

are we sure about the ontology links? stll helpful to say what this points at, no?

Comment thread src/catalog/src/builtin/mz_internal.rs Outdated
ontology: None,
});

pub static MZ_OBJECT_HYDRATION_HISTORY_IND: LazyLock<BuiltinIndex> =

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ahh, I really think we shouldn't add the index by default, the history collection can grow quite a bit, so this index might grow out of hand, no?

@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 9783a3e to 950d1d0 Compare August 20, 2026 11:11
@aljoscha
aljoscha marked this pull request as ready for review August 20, 2026 11:40
@aljoscha
aljoscha requested review from a team as code owners August 20, 2026 11:40
@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 950d1d0 to 4fc49af Compare August 20, 2026 11:55
@aljoscha
aljoscha requested a review from a team as a code owner August 20, 2026 12:08
@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 4fc49af to 5dc9944 Compare August 20, 2026 12:08
@def-

def- commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- promised object_id index and mz_indexes replacement step are not in the diff

src/catalog/src/builtin/mz_internal.rs:4973

The commit message and PR body both state that this change adds a builtin index on object_id and "declares the required replacement migration step" for mz_indexes. Neither exists in the diff: there is no BuiltinIndex for the new table and no new entry in MIGRATIONS. The two are consistent with each other (no index means no mz_indexes fingerprint change), so CI is green, but the record now says a mz_indexes step was declared when it was not, which is exactly the belief that makes the follow-up catalog-open panic.

Details

Evidence that nothing was added:

  • git grep MZ_OBJECT_HYDRATION_HISTORY returns only the table, its SystemObjectDescription, the BUILTINS_STATIC entry, and the two migration/reset call sites.
  • test/sqllogictest/mz_catalog_server_index_accounting.slt gains no index row (only the s835s836 shift of mz_notices), test/testdrive/catalog.td's SELECT COUNT(id) FROM mz_indexes WHERE id LIKE 's%' is unchanged, and catalog_server_explain.slt — whose query list is generated by enumerating catalog-server indexes — gains no EXPLAIN INDEX block.
  • The newest MIGRATIONS entry is still the 26.39.0-dev.0 mz_audit_events replacement.

Two consequences worth deciding between:

  1. If the index was meant to ship, it is missing, and the user query the PR describes ("how long did this object take to hydrate") full-scans up to 30 days of history on mz_catalog_server.
  2. If it was deliberately dropped, the commit message needs the same edit. It matters beyond tidiness: make_mz_indexes (src/catalog/src/builtin/mz_catalog.rs:754) inlines every builtin index as a VALUES row, so whoever adds the index later must add a MigrationStep::replacement for mz_catalog.mz_indexes pinned to the then-current dev version. The NOTE above the 26.34.0-dev.0 step spells out the failure mode: a step at a stale version is skipped and "the fingerprint check then panics at catalog open". A follow-up that trusts this commit message and skips the step reproduces that panic on every upgrading environment.

2. LOW -- unique key declared on the new table is not enforced by anything, and cannot be corrected without clearing the table

src/catalog/src/builtin/mz_internal.rs:4994

.with_key(vec![0, 2, 3]) tells the optimizer that (object_id, replica_id, installed_at) is unique, so DISTINCT/Reduce over those columns can be elided and join cardinalities assumed. Unlike the other keyed builtin tables, whose rows are derived one-per-catalog-object and are therefore unique by construction, this table's rows will come from a best-effort sampling collector, and this diff removes the two mechanisms that would otherwise paper over a duplicate: the bootstrap reset (src/adapter/src/coord.rs:3068) and forced schema migration (src/adapter/src/catalog/open/builtin_schema_migration.rs:812) both now skip it.

Details

That combination also makes the key hard to walk back. Changing it is a RelationDesc change, and the tripwire added at builtin_schema_migration.rs:735 forbids a migration step for this table, so the documented escape hatch is to remove the tripwire and the exemption, which discards the history the exemptions exist to protect.

Worth confirming before the collector lands that a re-recorded episode (same object, same replica, same replica-stamped installed_at, written twice across an environmentd restart) is impossible rather than merely unlikely. If it is not, the row multiplicity is silently wrong and so is any query the optimizer simplifies using this key.

@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 5dc9944 to 925bc3a Compare August 20, 2026 12:39
@aljoscha

Copy link
Copy Markdown
Contributor Author

Both valid, thanks. Fixed.

1. Stale record. Correct, and the reason it matters is the reason I fixed it rather than shrugged. The index was dropped deliberately after review (an arrangement holding the whole table, growing with objects times replicas times re-hydrations, for a query nobody runs yet), and the mz_indexes step went with it because there is no longer a fingerprint change. The commit message and PR body still claimed both. A future contributor trusting that message would skip declaring the step when adding an index and reproduce exactly the catalog-open panic the 26.34.0-dev.0 NOTE warns about.

Commit message and PR body now say there is no index and no step, and the commit message carries the forward-looking warning so it is attached to the thing someone will read when they add one:

NOTE: Adding one later is not only an index. make_mz_indexes inlines the builtin index set as VALUES, so a new index changes the mz_indexes fingerprint and needs a MigrationStep::replacement for it pinned to the then-current dev version. A step at a stale version is skipped and the fingerprint check panics at catalog open.

2. The key. Dropped it. Your asymmetry argument is the decisive one: the key buys a DISTINCT elision nobody needs, while being wrong once turns into a silently wrong answer rather than a visible duplicate row, and correcting it costs a descriptor change, which for this table means giving up the exemptions that protect its contents.

I also checked what comparable tables do, and none of them declare a key: mz_object_arrangement_size_history (same shape, background collector plus its own pruner), mz_cluster_replica_metrics_history, mz_cluster_replica_status_history, mz_storage_usage_by_shard. Mine was the outlier.

On whether a duplicate is actually possible: I believe it is prevented by construction, since the collector's subscribe reads the history table, so a second write for the same episode would have to be computed from a frontier that does not yet include the first, and the OCC write then fails with TimestampPassed. The outer GROUP BY collapses to the identity within one batch, and collection applies the retention cutoff so a retracted row is not resurrected. But "I believe it is prevented" is exactly the wrong footing for a promise to the optimizer, so the identity is now enforced operationally by the anti-join and documented in the design doc instead of declared on the relation.

@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 925bc3a to 025d91f Compare August 20, 2026 13:11
@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 025d91f to 58e4a1e Compare August 20, 2026 13:39
@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 58e4a1e to 4ab26a2 Compare August 20, 2026 13:46

@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.

Seems good, thank you!

@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 4ab26a2 to 2ba3fb3 Compare August 20, 2026 14:59
@aljoscha
aljoscha requested review from a team as code owners August 20, 2026 14:59
@aljoscha
aljoscha requested a review from Alphadelta14 August 20, 2026 14:59
@Alphadelta14
Alphadelta14 removed their request for review August 20, 2026 15:12
@def-

def- commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- the migration guards block Evolution, the one mechanism that would preserve the history they exist to protect

src/adapter/src/catalog/open/builtin_schema_migration.rs:761

The new assert_ne! rejects any MigrationStep for this table, and the plan_forced_migration exemption excludes it from the forced plan. Both are justified by truncation, but only Mechanism::Replacement truncates: Evolution "Keeps existing contents" and migrate_evolve_one registers the new schema on the existing shard_id without allocating one. The result is that the next schema change to this table has no non-destructive path, and on the dev-upgrade path it cannot start at all.

Details

Two specific consequences.

The comment added at builtin_schema_migration.rs:830 states that "A forced migration allocates a fresh shard, which discards the table's contents". plan_forced_migration is reached with Mechanism::Evolution on every dev-version upgrade (builtin_schema_migration.rs:671-685 forces "evolution" whenever source_version.pre starts with dev), and that path allocates nothing. So the exemption protects against something Evolution does not do, while suppressing the schema registration Evolution exists to perform.

That leaves a developer who changes this table's desc boxed between two panics. Adding MigrationStep::evolution(...) trips the new assert at catalog open. Not adding a step means the table is absent from migrated_items, so update_fingerprints reaches its panic!("fingerprint mismatch for builtin ...") at builtin_schema_migration.rs:1227 — a Table is neither ephemeral nor runtime-alterable, and the forced Evolution plan that would otherwise have covered it now skips it. The only exit is the one the comment recommends: delete the assert, add a Replacement step, and announce that the history restarts. An additive nullable column, the most likely change to a history table and exactly what backward_compatible accepts, then costs the whole table.

mz_storage_usage_by_shard carries the same forced-plan exemption, but its desc has been stable for years; this PR states the opposite intent for this table ("a schema change to this table in a future release may clear its contents", status gaining values).

Scoping both guards to Mechanism::Replacement fixes it: permit MigrationStep::evolution for this table, and drop the name from the plan_forced_migration filter when the mechanism is Evolution.

Separately, the two comments that point a future reader at the tripwire (builtin_schema_migration.rs:836 and src/catalog/src/builtin/mz_internal.rs:5032) name plan_migration; the assert is in validate_migration_steps.

2. LOW -- started_at is declared nullable but the documented contract has no NULL case

src/catalog/src/builtin/mz_internal.rs:5046

The column comment and the user docs now say started_at is "Equal to installed_at when the replica observed no start", so every row carries a real timestamp, while the RelationDesc still declares it nullable(true). The relation ships with two contradictory NULL contracts, and information_schema.columns.is_nullable reports the one the docs deny.

Details

The earlier revision documented the no-start case as NULL, which matched nullable(true); reassigning that case to an installed_at sentinel left no documented circumstance under which the column is NULL. Either the column should be nullable(false), or the comment should say when NULL does appear.

finished_at is also nullable(true) with a comment ("When hydration finished") that names no NULL case. That one is defensible as headroom for the future status values the PR body anticipates, since a non-terminal or failed episode would have no finish time — but then the comment is the half that is wrong, and saying so costs nothing while the desc is still free to change.

@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 2ba3fb3 to 32cbb0a Compare August 20, 2026 15:26
@aljoscha

Copy link
Copy Markdown
Contributor Author

Both valid, thanks. Fixed in the latest push.

1, the guards blocking Evolution. Confirmed the whole chain, including the part that makes it worse than "boxed in later": dev upgrades default to Some("evolution"), plan_forced_migration is reached with Mechanism::Evolution, my filter dropped the table from that plan, so the table never reaches migrated_objects and update_fingerprints panics at open the moment the desc changes. My guards were aimed at truncation but keyed on migration in general, and Evolution keeps the shard and the rows, so I was blocking the one path that preserves what the guards exist to protect.

Both guards are now scoped to Mechanism::Replacement. The assert only fires for a replacement step, and the forced-plan filter only excludes the table when the mechanism is Replacement, so a forced Evolution covers it like any other table. An additive nullable column now costs nothing. I deliberately did not touch mz_storage_usage_by_shard's side of that filter, same argument probably applies to it, but it is not mine to change here.

Also corrected the two comments that said plan_migration where they meant validate_migration_steps.

2, started_at nullable with no documented NULL case. Correct, and self-inflicted: I rewrote that comment yesterday when #38246 landed its backfill, and in reassigning the no-start case to an installed_at sentinel I left the NULL case undocumented. The comment now reads "or NULL if the replica reported none", which is the honest contract for a column the collector passes straight through from a nullable log column. I would rather keep it nullable than coalesce in the collector: baking the current backfill into this table would also remove the way to record a genuinely unstarted episode once status grows meanings.

On finished_at, I left it as is. It is nullable for the same headroom, and today every row we write has it set, so I did not want to document a NULL case for rows that do not exist yet. Happy to add a clause if you would rather have it stated.

@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 32cbb0a to f34d18e Compare August 20, 2026 15:38
@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from f34d18e to da15f27 Compare August 20, 2026 16:12
@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from da15f27 to 3f16731 Compare August 20, 2026 16:39
@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from 3f16731 to cfcedc3 Compare August 20, 2026 17:11
Adds the durable table that hydration episodes are recorded into. Nothing writes
it yet, the collector arrives separately.

The table is in `mz_internal` because its contents are best effort and its
`status` column will gain values as more hydration events become observable. An
episode is identified by `(object_id, replica_id, installed_at)`, using the
replica-stamped installation time because it is stable across an environmentd
restart. That identity is not declared as a key on the relation: the collector's
anti-join is what keeps it unique, and telling the optimizer a best-effort
sampler's output is unique would turn any duplicate into a silently wrong query
result. None of the comparable history tables declare one either.

No index. An arrangement on the catalog server would hold the whole table, which
grows with objects times replicas times re-hydrations, and nothing queries this
table by key yet.

NOTE: Adding one later is not only an index. `make_mz_indexes` inlines the
builtin index set as VALUES, so a new index changes the `mz_indexes` fingerprint
and needs a `MigrationStep::replacement` for it pinned to the then-current dev
version. A step at a stale version is skipped and the fingerprint check panics at
catalog open.

Contents are exempt from the bootstrap reset and from forced schema migrations,
since a sampled history cannot be rebuilt from anything else once it is gone.
Durability is best effort in both directions, and the assert added here is a
tripwire so that clearing the table is chosen rather than stumbled into.

Ref: SQL-644
@aljoscha
aljoscha force-pushed the aljoscha/hydration-03-catalog branch from cfcedc3 to 64d8df3 Compare August 20, 2026 19:00
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