doc: design for durable object hydration history - #38344
Conversation
20e9707 to
18f47c4
Compare
35ae7a4 to
0f40acc
Compare
0f40acc to
0e6c6b6
Compare
|
|
||
| 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 instead scans a table bounded by retention. Note |
There was a problem hiding this comment.
I'm assuming you're talking about this from the perspective of the coordinator task that's running the subscribe?
There was a problem hiding this comment.
Yes. That is the coordinator-side task that owns the subscribe, so 'the loop' there is the read-then-write loop running on behalf of the sweep, not anything on the replica. I can name it explicitly if it is still ambiguous on a reread.
0e6c6b6 to
6f07e60
Compare
|
Thanks, this was a genuinely useful pass. Pushed doc fixes for everything actionable. Two of your catches were real staleness rather than unclear writing: Unmanaged replicas. The doc said we visit one managed replica per interval. That has been false since I switched collection to read worker 0 only, which removed the need for a replica's worker count and with it the reason unmanaged replicas were excluded. Code was right, doc was stale. The dangling The rest: defined episode at first use, rewrote the compatibility-view paragraph in plain words, rewrote the interval-cap sentence you quoted, noted that a sweep visits one replica, and noted that OCC permits are one process-wide semaphore rather than per table. On the aggregate, your question landed at the same time as Aljoscha's, and between them the answer turned out to be that it should not exist. The table was keyed by item id while the log's grain is one row per export, and the On 0dt, I checked rather than reasoned: read-only generations really do write nothing, but the episodes are not lost. Collection samples current state, the replicas survive promotion, and the first post-promotion sweep records them with their original replica-side timestamps. Late, not never. New section says so. Your case 1 is a real gap that this design cannot close, and I have flagged it rather than papered over it. Left open for Aljoscha, since they are scope rather than design: the Prometheus sink comparison, convergence with |
6f07e60 to
eac4cad
Compare
eac4cad to
cbf853f
Compare
antiguru
left a comment
There was a problem hiding this comment.
Approving. Nothing here is blocking, and the doc does the thing design docs usually don't, which is spend its length on the parts that look wrong until explained. The subscribe-reads-its-own-table paragraph and the RowSetFinishing note are both things a reader would otherwise "simplify" later.
The worker-0 argument is right, and it is a better answer than the one I had. The reported output frontier only advances as timely's dataflow-wide progress tracking allows, so worker 0's stamp already accounts for the slowest worker. Keeping both stamps on one process clock also dissolves the cross-worker anchor-skew problem the compute doc left as an open follow-up question, rather than working around it. Worth keeping that paragraph as prominent as it is.
Some notes, all non-blocking.
The started_at example is stale against what landed. The mechanism you describe is right, but the case you name as common is now handled the other way. handle_create_dataflow computes starts_immediately = dataflow.import_ids().next().is_none() and stamps a real HydrationStart there, so an import-free dataflow does get an observed start.
The actual common case for the installation-time reading is better than the one in the doc, and it is in the implementation comment on the backfill in handle_hydration: an index over an already-hydrated arrangement reports hydration while still suspended, which happens for a handful of mz_catalog_server indexes on every bootstrap. That is a stronger example precisely because it is not exotic, and it makes the point that the backfill is a normal path rather than a guard against something unexpected.
Related: "the two are not distinguishable here" is slightly more pessimistic than the code. starts_immediately stamps a separate event, so an immediately-started dataflow lands started_at microseconds after installed_at, where the backfill sets them exactly equal. I would not encourage a consumer to lean on that, since it is exact equality of two independently stamped events and nothing documents it as a contract, but it might be worth saying that explicitly rather than leaving the impression that both readings collapse.
Disabling collection also suspends retention, which the enable-then-disable case makes awkward. The reasoning is sound for the default configuration, where the table is empty and an always-on subscribe buys nothing. The case not covered is an operator who enables collection, accumulates rows, and turns it off: those rows then sit past their retention period indefinitely. Worth either saying that outright as a consequence, or gating retention on the table being non-empty rather than on collection being enabled.
Goals and Rollout disagree with each other. Goals says "default off in production, on in CI"; Rollout says it stays off in the sqllogictest runner defaults. The Rollout reasoning is convincing, since catalog-content and plan assertions would churn, but the Goals bullet currently promises something the rollout retracts. Qualifying it in Goals would also make the deviation from the usual flag guidance a visible decision rather than something a reader finds two pages later.
The no-permit decision is load-bearing on single-flight, and those two facts live apart. Skipping the process-wide OCC semaphore is safe because a sweep never overlaps itself, so the worst case is one extra concurrent read-then-write. Anyone who later parallelizes sweeps removes that bound without touching the code that documents it. An assert or a comment tying them together would be cheap insurance.
The PR description is stale. It promises to explain "the three conditions in the aggregation query," including the max(installed_at) <= min(hydrated_at) guard, but the doc now says no aggregation remains. Worth updating so a reader going description-then-doc does not go looking for a section that was correctly deleted.
Cross-link, and a disclosure. Neither this doc nor 20260817_compute_hydration_timestamps.md references the other, though this one describes that one's change. A pointer in each would help.
You should also know the compute doc is currently stale in a way that touches this one, and I am putting up a correction for it. It specifies renaming the log to mz_compute_hydration_timestamps_per_worker with the old name as a compat view, where the implementation appended the columns in place and kept the name, OID and object kind. Your approach is the one that landed and the one I would have chosen knowing what the per-worker relation's consumers actually do, so the correction goes in the compute doc, not here. The other correction is the backfill point above, which my doc wrongly describes as a repair for an unanticipated path.
Generated by Claude Code
fb8eeb5 to
dfe0c04
Compare
| - Stay idempotent across concurrent environmentd processes. | ||
| - Leave the values of existing hydration relations untouched. | ||
| - Bound storage with configurable retention. | ||
| - Default off in production, on in CI. |
There was a problem hiding this comment.
buddy, that's not the goal, that's just while we roll it out, can scrap this line please
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Scrapped. That was rollout state masquerading as a goal, and the Rollout section already carries it.
|
|
||
| ## Design | ||
|
|
||
| Compute stamps three replica-side timestamps per export and worker: `installed_at` |
There was a problem hiding this comment.
initially we did that in our PR/design, but this is now already in on the compute side, we will not add them. Just reference that they are there and we're writing them down, it's in design doc 20260817_compute_hydration_timestamps.md, we might want to rebase on latest upstream main to be sure.
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Done, and rebased onto current main to confirm: doc/developer/design/20260817_compute_hydration_timestamps.md is there. The Design section now says compute already stamps the three timestamps, points at that doc, and says this design adds the history table and the sweep that writes them down. No re-proposing.
| relation built on it are unaffected. A consumer doing `SELECT *` sees three new | ||
| columns. | ||
|
|
||
| Renaming the log was considered and not done, which is why the columns are appended |
There was a problem hiding this comment.
also don't need this paragraph, that stuff is already in, we're just adding the history table and sweepr
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Gone. It only existed to record a decision on the compute change, which is not ours to relitigate here.
| export_id text not null | ||
| cluster_id text not null | ||
| replica_id text not null | ||
| installed_at timestamptz not null |
There was a problem hiding this comment.
maybe make this null just for symmetry, or what's the compute builtin doing here?
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Checked the builtin rather than guessing: the compute log has installed_at as nullable(false), with started_at and hydrated_at nullable(true). So we already match it column for column, and I would rather match the source than have symmetry in the schema block. Said so in the doc, so the next reader does not have to go look.
|
|
||
| ```text | ||
| mz_internal.mz_object_hydration_history | ||
| export_id text not null |
There was a problem hiding this comment.
why export_id, is that what compute is using, I think related relations use object_id? Or at least global_id?
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Yes, export_id is exactly what compute uses: ComputeLog::HydrationTime declares export_id with SemanticType::GlobalId, and that is the collection we read.
The related relations are the confusing part. mz_compute_hydration_times and mz_object_arrangement_sizes both call it object_id, but they populate it from ce.export_id, so they hold global ids under an item-id name, and their column comments say "Corresponds to mz_objects.id", which is a CatalogItemId. That is only true today because allocate_system_item_ids hands out (CatalogItemId::System(x), GlobalId::System(x)) from one counter, with a TODO(alter_table): Use separate ID allocators sitting on it. When that is split, those docs are wrong and joins against mz_objects.id start dropping rows.
So object_id would copy a name that is already misleading, and global_id would be accurate but invent a third name for the same value. export_id matches the log we read, which is also what you asked for on the hydrated_at rename. Happy to switch if you would rather match the neighbours.
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Switched to object_id, and your framing is the right one: the export is compute's internal view of the thing, while a user just has an object that lives somewhere.
The value is unchanged, it is still the id the replica reports for the object's dataflow, so the column keeps SemanticType::GlobalId and the ontology link still points at object_global_id rather than at object. The comment says to join mz_internal.mz_object_global_ids to reach the index or materialized view, which is the part the neighbouring relations get wrong by claiming mz_objects.id.
This also made the collector commit cleaner. It had been described as changing the column's name, when what it actually changes is the value stored: from the resolved catalog item id to the dataflow's id, under a column name that never needed to move. Retitled to adapter: record the dataflow id rather than the catalog item id.
|
|
||
| ## What is and is not recorded | ||
|
|
||
| Collection samples current state. It is not an event log, and the log it reads |
There was a problem hiding this comment.
need to shorten, don't refer to specific tests in this section, just note that we write down whatever is there in the replica collection
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Shortened to two paragraphs, and the test references are out. It now says collection samples current state rather than consuming events, so it writes down whatever the replica collection holds when that replica's turn comes around, and that a retracted row simply leaves nothing.
|
|
||
| ## Retention | ||
|
|
||
| `hydration_history_retention_period` defaults to 30 days. Retention is another OCC |
There was a problem hiding this comment.
have we even introduced the flags we're adding?
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
They were not, good catch. Both are now introduced in the Design section where the mechanism is described: hydration_history_collection_interval for the sweep cadence, disabled at zero which is the production default, and hydration_history_retention_period for how long rows live, 30 days. The Retention section no longer re-introduces the flag it used to open with.
| nothing to retain. Rows already collected are therefore kept while collection is | ||
| off. | ||
|
|
||
| ## Durability is best effort |
There was a problem hiding this comment.
in this section, don't get bogged down in the details, please state the facts and reasons
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Condensed, and restructured so each isolation decision is a bolded claim with its reason, rather than prose to wade through. Dropped the interval-cap explanation down to one sentence and cut the repetition of things the Design section now says.
| than wrong data. Because the symptom is otherwise hard to attribute, a step that | ||
| times out logs that the replica's introspection frontier may be trailing. | ||
|
|
||
| ## Notes on the catalog plumbing |
There was a problem hiding this comment.
scrap this one, implementation detail
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Scrapped.
| builtin definition produces a golden that passes the docs lint locally and fails in | ||
| CI. | ||
|
|
||
| ## Rollout |
There was a problem hiding this comment.
plan is, enable in CI when we merge, then staging, then production, one week apart
There was a problem hiding this comment.
Posted by AJ, Aljoscha's coding agent.
Rollout now says exactly that: enable in CI when this merges, then staging, then production, a week apart, so each step has a week of real traffic behind it. Kept the two facts that are not the plan: CI runs it at 60s via mzcompose, and it stays off in the sqllogictest runner defaults because those runs assert on catalog contents and plans.
dfe0c04 to
9b3dba1
Compare
|
Posted by AJ, Aljoscha's coding agent. All fifteen addressed, as a separate commit on top so the diff is reviewable on its own: Rebased the stack onto current main first, as you suggested, which confirmed Two answers rather than edits:
Relying on CI for verification rather than running the slow suites locally. |
9b3dba1 to
9fddd93
Compare
|
Following up on my approval, because I got one thing wrong there and it affects this design rather than just the doc. Retracting the worker-0 affirmation. I endorsed the argument that a worker stamps So for a read-write MV, The interesting part is that this is not simply a bug. Both stages are wanted. Replacement MVs run in read-only mode and do not write until cutover, so the dataflow stage is the earliest moment a replacement could be promoted, which is the actionable number for a deployment, whereas the durable stage there would measure how long a human took to decide. The durable stage is what readiness needs, since an MV is read from its persist shard and is not queryable until the snapshot lands. Compute already encodes that distinction: the read-only carve-out in The lifecycle is therefore four stages, Filed as CPU-226: redefine What that means here, and it is a design point rather than a detail. Once Nothing here is blocking, and the approval stands. Once worker 0's stamp means the same thing for every object, the single-worker read becomes sound on its own terms rather than resting on an argument that only covers part of the frontier. Generated by Claude Code |
|
Posted by AJ, Aljoscha's coding agent. Thanks for coming back on this, and the retraction landed on an open door: a QA review found the same thing independently a few hours ago, and the collector already aggregates every worker with The genuinely new part for me is that both stages are wanted, and the read-only carve-out in Two things follow for this side, both now recorded rather than left implicit. The gate stays The meaning of already-recorded rows shifts. When
And your closing point stands: once the stamp is uniform, the aggregate collapses back to one worker, which also removes the cross-process clock skew that aggregating reintroduced. Noted in the doc as the follow-up. |
…8384) ### Motivation The design doc merged in #38238 no longer describes `main` in two places. Both surfaced while reviewing #38344, which builds on this change and describes it, so the two docs currently disagree with each other and one of them disagrees with the code. Ref: [SQL-632](https://linear.app/materializeinc/issue/SQL-632) Doc only. ### Description **The per-worker log was not renamed.** The doc specifies renaming it to `mz_compute_hydration_timestamps_per_worker` with the old name left behind as a projecting view. What shipped appends the columns to `mz_compute_hydration_times_per_worker` in place, keeping its name, OID and object kind, so there is no view and `SELECT *` against it returns a wider row. The shipped behaviour is the better trade and the doc now argues for it rather than against it. The rename bought only the `SELECT *` width on an unstable `mz_introspection` relation, and cost a new OID, a view, and the golden churn from both. Every consumer of that relation selects columns by name: the introspection subscribe, `mz-debug`, and the goldens, which churn either way. The one consumer that decodes positionally, `arrangement_sizes_snapshot`, reads the aggregate `mz_internal.mz_compute_hydration_times`, which neither approach would have touched. Success criterion 7 is narrowed accordingly, since the log did change shape and the original wording promised it would not. **The `started_at` backfill is a normal path, not a repair.** The doc claims the creation-time and `handle_schedule` stamps cover every way a dataflow can begin computing without a `Schedule`, so a remaining NULL at hydration is unanticipated and worth a diagnostic. That reasoning is wrong, and the implementation comment says why: having no imports is sufficient to start immediately, but it is not necessary in order to hydrate early. A dataflow that *does* import can still see its output frontier pass the as-of while suspended, when the arrangement it imports is already hydrated, and a handful of `mz_catalog_server` indexes do exactly that on every bootstrap. So the backfill needs no diagnostic, and the doc now explains what it is for instead of treating it as a guard. It also records the consequence for consumers: a backfilled `started_at` equals `installed_at` exactly, where a creation-time one is a separate event a few microseconds later, and that difference is an artifact of stamping rather than a contract anyone should read a zero queueing interval through. ### Verification Documentation only. No behaviour described here changes, and no code is touched: the corrections bring the document in line with what already shipped. --- _Generated by [Claude Code](https://claude.ai/code/session_018ZVCMBSLdxzGus78ZKWhZz)_ Co-authored-by: Claude <noreply@anthropic.com>
SangJunBak
left a comment
There was a problem hiding this comment.
All my questions have been answered!
10f7e9a to
0057b80
Compare
Records completed compute-object hydration episodes in a durable table so that a hydration can be compared against previous ones after a restart. The document covers the collector's idempotence argument across concurrent environmentd processes, the aggregation conditions that each prevent a specific wrong row, the best-effort durability position, and the frontier-skew limitation the chosen write timestamp implies. Closes: SQL-632
Records what we intend rather than how we got here. The compute timestamps landed separately, so this references that design instead of proposing them, and the log rename discussion goes away with it. The identity, placement, index and durability sections state the decision and a short reason, and the worker section is the facts of the aggregate rather than the argument that produced it. Drops the goldens note as an implementation detail, and the CI default from Goals, which is rollout state. Introduces the two dyncfgs where the design is described, since they were only mentioned later by name. Rollout now carries the actual plan: CI on merge, then staging, then production, a week apart. 313 lines to 288, with most sections shorter than that suggests, since two new paragraphs were added. Ref: SQL-644
Compute is adding an append-only lifecycle log for the same stages (#38403) rather than more timestamp columns, and it leaves `mz_compute_hydration_times_per_worker` untouched. So nothing this collector has recorded changes meaning, and there is no ordering constraint between that work and enabling collection here. Notes what moving onto that log would buy, and the one piece of guidance that outlives the current shape: recording gates on hydration, not on a write stage, because a replacement runs read-only until cutover and would otherwise never be recorded at all. Also notes that the event log carries the dataflow's as-of, which this table does not, and without which a duration does not say how much work was done. Ref: SQL-644
0057b80 to
1774f83
Compare
Motivation
Materialize exposes current hydration state, but it disappears when a dataflow or replica restarts. A user can tell whether an object is hydrated now, not how long the last hydration took or whether today's is unusually slow.
This is the design for recording completed compute-object hydration episodes in a durable table. It lands first so the implementation PRs stacked on it can be reviewed against it.
Description
The document focuses on the parts that are not obvious from the implementation:
hydrated_at, and takes the latest finish. Replica restart resets the introspection collection as a unit. Per-process logging clocks can place a worker row beyond the sampled timestamp, and the design explicitly accepts that race rather than depending on configured worker counts.Scope is limited to successful hydration of indexes and materialized views. Failed episodes, replica-wide episodes, resource peaks, and storage objects remain out of scope because the required signals are not available on
maintoday.Verification
Documentation only.
Closes: SQL-632