Skip to content

Commit f9be4ed

Browse files
antiguruclaude
andauthored
doc: correct the hydration timestamps design against what shipped (#38384)
### 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>
1 parent 1b5caa3 commit f9be4ed

1 file changed

Lines changed: 67 additions & 43 deletions

File tree

‎doc/developer/design/20260817_compute_hydration_timestamps.md‎

Lines changed: 67 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,11 @@ work itself, with no way to separate them.
6060
an environmentd restart and reconnection.
6161
6. Every row satisfies `installed_at <= started_at <= hydrated_at` where the
6262
values are non-NULL, and no row presents a later stage without an earlier one.
63-
7. No existing relation changes shape, semantics, or values.
64-
`mz_compute_hydration_times`, `mz_compute_hydration_statuses`, and the
65-
per-worker relation all keep returning exactly what they return today.
63+
7. No existing relation changes semantics or values.
64+
`mz_compute_hydration_times` and `mz_compute_hydration_statuses` keep
65+
returning exactly what they return today. The per-worker log keeps its
66+
existing columns and their values, and gains three, so a consumer doing
67+
`SELECT *` against it sees a wider row.
6668

6769
## Out of Scope
6870

@@ -165,7 +167,8 @@ acknowledged rather than designed around.
165167

166168
There is no event for hydration start today. Add
167169
`ComputeEvent::HydrationStart { export_id }`, logged from the three places a
168-
dataflow's computation becomes unblocked.
170+
dataflow's computation becomes unblocked. A fourth site fills the column in for
171+
dataflows whose start was never separately observable.
169172

170173
**From `handle_schedule`**, which drops the suspension token. Guard it in the
171174
demux on `started_at` already being set, mirroring the existing guard on
@@ -203,12 +206,27 @@ objects. Emitting the event where they are created is simpler than filtering the
203206
and keeps the state machine total. Nobody should read hydration timings for log
204207
collections, and the relation's documentation should say so.
205208

206-
Between them, the second and third cases cover every path by which a dataflow can
207-
begin computing without a `Schedule`, so `handle_hydration` should not need to
208-
repair anything. If it nonetheless observes a hydration event with `started_at`
209-
still NULL, that is an unanticipated path: stamp `started_at` from `installed_at`
210-
so the invariant holds, and soft-log, so the surprise is visible rather than
211-
silently absorbed.
209+
**And in `handle_hydration`, when the event arrives with `started_at` unset.**
210+
It is tempting to treat this as a repair for something unexpected, on the
211+
reasoning that `StartSignal` gates only imports, so a dataflow with no imports is
212+
the only one that can run before its `Schedule`. That reasoning is wrong. Having
213+
no imports is sufficient to start immediately, but it is not necessary in order to
214+
hydrate early: a dataflow that *does* import can still see its output frontier
215+
pass the as-of while suspended, when the arrangement it imports is already
216+
hydrated. A handful of `mz_catalog_server` indexes do exactly that on every
217+
bootstrap.
218+
219+
So this is a third normal path, not an exotic one, and it needs no diagnostic.
220+
Stamp `started_at` from `installed_at`, which keeps the invariant total and reports
221+
the queueing interval as zero. Stamping from `hydrated_at` would invert it,
222+
charging the whole life of the dataflow to queueing and reporting zero hydration
223+
time for a dataflow that only ever hydrated.
224+
225+
One consequence for consumers. A `started_at` stamped here is exactly equal to
226+
`installed_at`, where one stamped at creation is a separate event a few
227+
microseconds later. That difference is an artifact of how the two are stamped, not
228+
a contract, so nothing should read a zero queueing interval as distinguishing
229+
"never queued" from "queued immeasurably briefly".
212230

213231
### Log relation shape
214232

@@ -238,19 +256,24 @@ variant means no new id and no second log collection producing overlapping data.
238256

239257
### Preserving the existing relations
240258

241-
The builtin log currently named `mz_compute_hydration_times_per_worker`, in
242-
`src/catalog/src/builtin/mz_introspection.rs`, is renamed to
243-
`mz_compute_hydration_timestamps_per_worker` and exposes all six columns. A new
244-
builtin view takes the old name and projects the old three columns:
245-
246-
```sql
247-
SELECT export_id, worker_id, time_ns
248-
FROM mz_introspection.mz_compute_hydration_timestamps_per_worker
249-
```
250-
251-
**This is why `time_ns` is kept rather than replaced.** Because it is retained
252-
rather than derived from the timestamps, that projection is exact: no value
253-
changes, no precision is lost, and no cross-worker arithmetic is introduced.
259+
The three columns are appended to the existing builtin log
260+
`mz_compute_hydration_times_per_worker`, in
261+
`src/catalog/src/builtin/mz_introspection.rs`, which keeps its name, its OID, and
262+
its object kind. A consumer doing `SELECT *` sees three new columns.
263+
264+
Renaming it and leaving a projecting view behind the old name was considered and
265+
rejected. It buys only the `SELECT *` width on an unstable `mz_introspection`
266+
relation, and costs a new OID, a view, and the golden churn that comes with both.
267+
Every consumer of this relation selects columns by name: the introspection
268+
subscribe, `mz-debug`, and the goldens, which churn either way. Nothing reads it
269+
positionally. The consumer that *is* positional,
270+
`arrangement_sizes_snapshot`, decodes the aggregate
271+
`mz_internal.mz_compute_hydration_times` from a persist snapshot, and neither this
272+
change nor the rename would have touched that relation.
273+
274+
**`time_ns` is kept rather than replaced.** It is the reason the existing columns
275+
keep their exact values: retained rather than derived, so nothing is recomputed,
276+
no precision is lost, and no cross-worker arithmetic is introduced.
254277
Deriving `time_ns` as `hydrated_at - installed_at` would have moved it to
255278
microsecond precision, since `timestamptz` caps there, where today it is true
256279
nanoseconds. Deriving it after aggregation would additionally have absorbed
@@ -261,16 +284,15 @@ requires absolute times a duration cannot provide. Two columns with two document
261284
jobs rather than two sources of truth.
262285

263286
Everything downstream is therefore untouched. The introspection subscribe in
264-
`src/adapter/src/coord/introspection.rs` keeps reading the old name and keeps its
265-
current SQL. `mz_internal.mz_compute_hydration_times` and
287+
`src/adapter/src/coord/introspection.rs` keeps its current SQL, since it names the
288+
columns it reads. `mz_internal.mz_compute_hydration_times` and
266289
`mz_internal.mz_compute_hydration_statuses` are not modified at all, so they keep
267290
their shapes, semantics, values, retained-metrics properties, indexes and shards.
268-
`arrangement_sizes_snapshot`, which decodes that collection positionally from a
269-
persist snapshot in `src/adapter/src/coord/message_handler.rs`, needs no change.
270-
Neither does the console query that joins it by name.
291+
`arrangement_sizes_snapshot` in `src/adapter/src/coord/message_handler.rs` needs
292+
no change, and neither does the console query that joins by name.
271293

272-
This is what makes the design purely additive: it adds columns and a relation, and
273-
changes nothing that anything currently reads.
294+
So the change adds columns and changes no value that anything currently reads. The
295+
one thing it is not is invisible: the per-worker log is wider than it was.
274296

275297
### Sequence of events
276298

@@ -415,7 +437,7 @@ for upgrades.
415437

416438
- `src/compute/src/logging/compute.rs`: `ComputeEvent::HydrationStart`, the three
417439
`ExportState` fields, the packer, `handle_export`, `handle_export_dropped`,
418-
`handle_hydration` including the soft-logged repair, and a new
440+
`handle_hydration` including the `started_at` backfill, and a new
419441
`handle_hydration_start`. Also a `CollectionLogging` method alongside
420442
`set_hydrated`.
421443
- `src/compute/src/compute_state.rs`: log the start event from `handle_schedule`,
@@ -424,10 +446,8 @@ for upgrades.
424446
- `src/compute-client/src/logging.rs`: the widened `RelationDesc`.
425447
`LogVariant::desc` is the only exhaustive match a shape change touches, since
426448
the variant itself is unchanged.
427-
- `src/catalog/src/builtin/mz_introspection.rs`: rename the builtin log and add
428-
the compat view.
429-
- `src/catalog/src/builtin.rs`: `BUILTINS_STATIC` registration for the new view.
430-
- `src/pgrepr-consts/src/oid.rs`: an OID for the new view.
449+
- `src/catalog/src/builtin/mz_introspection.rs`: the appended columns on the
450+
existing builtin log. No rename, so no new OID and no `BUILTINS_STATIC` entry.
431451
- Goldens that hardcode this relation's identity, columns, OIDs or indexes:
432452
`test/sqllogictest/oid.slt`, `information_schema_tables.slt`,
433453
`mz_catalog_server_index_accounting.slt`, `cluster.slt`,
@@ -517,7 +537,7 @@ reached persist, or any signal that a replica crashed.
517537

518538
The prototype is the compute and catalog change itself, exercised through
519539
testdrive against a targeted replica. The validating query selects from
520-
`mz_compute_hydration_timestamps_per_worker` on a cluster with a hydration
540+
`mz_compute_hydration_times_per_worker` on a cluster with a hydration
521541
concurrency limit and a handful of indexes, showing objects moving from waiting,
522542
to hydrating, to hydrated, with the queueing interval visible separately from the
523543
hydration interval. A second run after an environmentd restart shows identical
@@ -536,11 +556,12 @@ and not the compute controller".
536556
**Replace `time_ns` with the timestamps rather than keeping both.** Rejected for
537557
the reasons in "Preserving the existing relations".
538558

539-
**Backfill `started_at` at the hydration event** instead of stamping it at
540-
creation for ungated dataflows. Rejected because the row would be wrong in the
541-
interim, reporting an object as queued while it hydrates, and because it defers a
542-
fact the replica already knows at creation time. Retained only as a soft-logged
543-
repair for paths this design does not anticipate.
559+
**Backfill `started_at` at the hydration event only,** instead of also stamping
560+
it at creation for dataflows with no imports. Rejected because the row would be
561+
wrong in the interim, reporting an object as queued while it hydrates, and because
562+
it defers a fact the replica already knows at creation. The backfill is kept as
563+
well, since it covers a case creation-time stamping cannot: see "A new hydration
564+
start event".
544565

545566
**Narrow `mz_compute_hydration_statuses.hydration_time` to the interval its name
546567
claims.** Rejected. Monitoring and analytics depend on the current values, and the
@@ -578,8 +599,11 @@ named.
578599
- **Stamping location.** The replica, not the compute controller. See "Why the
579600
replica and not the compute controller".
580601
- **`time_ns` is kept.** See "Preserving the existing relations".
581-
- **`started_at` for ungated dataflows is stamped at creation,** not backfilled at
582-
hydration. See "A new hydration start event".
602+
- **`started_at` for dataflows with no imports is stamped at creation,** with the
603+
backfill at hydration kept for the cases creation-time stamping cannot see. See
604+
"A new hydration start event".
605+
- **The per-worker log is widened in place,** keeping its name and OID, rather than
606+
renamed behind a projecting view. See "Preserving the existing relations".
583607
- **`hydration_time` is left exactly as it is,** and no `queue_time` column is
584608
added. Future consumers read the timestamps. See "Alternatives".
585609
- **Clock skew.** Accepted and documented rather than designed around.

0 commit comments

Comments
 (0)