Compute surface suite - #38058
Draft
def- wants to merge 11 commits into
Draft
Conversation
`CollationPlan` and its `as_monotonic` were unreferenced: nothing in the tree constructs one, because `ReducePlan` has no collation variant to hold it. Collating was how the renderer used to execute a `Reduce` whose aggregates span several `ReductionType`s. That job now belongs to `ReduceReduction`, which splits such a reduce into one reduce per type and joins the results back together, so the renderer never sees a mixed reduce and has nothing to collate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
…feature
The random `MirRelationExpr` generators and the `FoldConstants`
result-equivalence oracle lived in the `mz-transform-fuzz` crate, which
sits in the nightly-only cargo-fuzz workspace and so cannot be depended
on from the main workspace. A second consumer now needs them: a suite
that renders the same generated plans on a real replica and checks them
against the same oracle, testing MIR-to-dataflow where the fuzz targets
test MIR-to-MIR.
Move them to `mz_transform::mirgen`, behind a `mirgen` feature that is
off by default. Generation draws from an `Entropy` trait rather than
libFuzzer's `Unstructured` directly, so a byte-driven fuzz target and a
seeded PRNG can both drive it. The fuzz crate keeps its public API and
contributes the `Unstructured` implementation.
`FuzzEntropy` delegates one-to-one, in call order, so byte consumption
is unchanged. That is load-bearing rather than incidental: a corpus
entry is a byte string, which plan it denotes depends on how many bytes
each draw consumes, and release qualification carries a minimized corpus
between runs. A different draw sequence would silently remap every
stored entry and discard the accumulated coverage. Verified by running
both generators over 4000 identical inputs and comparing the produced
plan and the bytes left unconsumed.
Also split the oracle's verdict into `FoldOutcome::{Rows, Error,
Unfoldable}`. `fold_to_multiset` returned `None` both for a plan that
folded to an `EvalError` and for one that did not fold at all, and
callers skipped both. An erroring plan has a definite expected result,
namely the same error, so collapsing the two dropped every
error-propagating plan out of the oracle's reach. This matters for the
generators specifically, since `gen_scalar` emits `DivisionByZero`
poison literals precisely to exercise error propagation.
`fold_to_multiset` keeps its old signature and behaviour.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
… oracles
Adds a generated test suite for the compute rendering surface, on top of
the existing headless driver. The hand-written `.spec` scenarios cover
shapes somebody thought to write down; this covers the surface as an
enumerated space, and measures what it reached.
`surface` enumerates the rendering surface as `SurfaceCell`s: an LIR
operator plus the variant choices that select a distinct path through
`mz_compute::render`. Every match is exhaustive with no wildcard arm, so
adding an LIR variant fails to compile until it is classified. That is
what keeps the taxonomy honest as the surface grows, and it is why cells
are defined at the granularity the plan distinguishes rather than by
concrete keys or literals, which would make the space unbounded.
`workload` is the JSON format: inputs as timestamped batches with
retractions, the MIR to render (embedded directly, since
`MirRelationExpr` is serde), the exports, the dyncfg matrix, the surface
cells claimed, and the oracles to apply. `runner` executes it and applies
them:
* fold-constants, the primary oracle: substitute each input's actual
updates back as literal `Constant`s and evaluate with the optimizer's
constant folder, an implementation independent of the renderer.
* export-invariance: an index, a materialized view, and a subscribe
over the same computation must agree.
* incremental: the maintained collection must equal a dataflow freshly
created at the same `as_of`. Redundant where folding is live, and the
only oracle that speaks where it is not.
* strategy-invariance: the same workload under different compute
strategy dyncfgs must produce the same result. Whatever the right
answer is, it cannot depend on which strategy computed it.
An oracle that declines to answer is indistinguishable from one that
passes, so the fold oracle treats an unfoldable plan as a failure rather
than a skip, and the generator only requests it once the plan is known to
fold. Errors are results, not failures: `peek_result` and
`await_subscribe_result` report a collection error as a value, so a plan
that should error is required to produce that error rather than rows.
Each export is rendered as its own dataflow, which is how the real system
renders them. Building one dataflow with all three exports does not work:
a subscribe carries a finite `up_to` while a persist sink requires an
empty one.
`generate` builds the corpus by greedy set cover: draw random MIR from
the generator shared with the `mz-transform` fuzz targets, lower it, ask
which cells came out, and keep the workload only if it covers something
new. Coverage is therefore measured against real lowering rather than
asserted by construction. The corpus is committed for deterministic,
bisectable nightly runs, with a test that regenerates and diffs so the
generator and the committed files cannot drift apart.
The cells random MIR cannot reach are enumerated in `KNOWN_GAPS` with
their causes, because a suite that reports only what it covered reads the
same as one that covered everything.
Known issue: the runner intermittently times out waiting for a frontier
when several dataflows run concurrently. Identical workloads pass and
fail across runs, so this is a race in the harness, not a product
defect; the hand-written scenarios stay green throughout.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
… aborting `DataflowBuilder::finish` documents that it reports a malformed plan as an error rather than panicking, so a caller lowering MIR straight from external input can fail one scenario with a readable message. That did not hold for a `Reduce` whose aggregates span more than one `ReductionType`: `ReducePlan::create_from` asserts they all share a type, so lowering aborted the process and took the whole script run with it. The assertion is sound for the real optimizer, where `ReduceReduction` always runs first and splits such a reduce into one per type joined back together, so this is unreachable from SQL. It is reachable from a `.spec` script, which is the case this driver exists to serve. Screen for the shape in `finish` and return an error naming both types and the two ways out. Keeping the fix local to the driver avoids making `create_from` fallible on the optimizer's hot path for a case it cannot hit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
`watch::Sender::send` fails when no receiver exists yet, and leaves the stored value unchanged. The response pump discarded that failure with `let _ =`, for both frontier updates and subscribe uppers, so any response the replica sent before something subscribed was lost. A later `expect_frontier` or `await_subscribe` then blocked until its timeout on a frontier the replica had already reported. `send_replace` always stores, so a late subscriber sees the latest value. The ordering this needs is routine rather than exotic: a dataflow can hydrate and report while the caller is still reading a different export, and a subscribe with a finite `up_to` can complete before anything awaits it. It stayed latent because the hand-written scenarios always `await-frontier` immediately after `schedule`, which puts the waiter in place first. It surfaced with several exports read in sequence, where it looked like intermittent hydration failure and moved around under every unrelated change. The existing dispatch test creates the receiver before dispatching, which is the ordering that works. The two added tests dispatch first and subscribe afterwards; both fail against `send` and pass against `send_replace`. Also make a frontier timeout report the last output frontier it observed, or say that none was ever reported. The two have different causes and the bare timeout could not distinguish them, which is what made this take as long as it did to localize. `DRIVER_DEBUG_RESPONSES` logs the raw response stream for the same reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
…d verdicts Two corrections to the fold oracle, both found by the first corpus run that got far enough to compare results. Errors were compared against `EvalError`'s own `Display`, but the renderer surfaces an `EvalError` wrapped in a `DataflowError`, whose `Display` prepends "Evaluation error: ". Every erroring plan therefore reported a mismatch on the spelling alone. Build the expected string by wrapping the same way, so the comparison stays exact; a substring match would have hidden the case this is meant to catch, a genuinely different error that happens to share a prefix. Rows on one side and an error on the other is no longer a failure. Errors travel in a dataflow's `err` collection, which is unioned through operators independently of the `ok` collection, so a join with an empty input still forwards its inputs' errors while constant folding computes the join, gets no rows, and drops the error with them. Neither side is wrong, and Materialize does not promise that optimization preserves errors exactly, so failing on it would bury the rows-versus-rows disagreements the oracle exists to catch. Such a comparison is now reported as inconclusive: counted, named with its reason, and printed in the run summary. Not skipped. A check that quietly stops answering is indistinguishable from one that agrees, which is the failure mode this suite is built around. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
Adds hand-written plan shapes for the surface cells the shared generator
does not produce, and raises the input volume so the volume-sensitive
dyncfgs in the matrix are actually exercised. Coverage goes from 26 cells
to 35.
The shared generator has no arm for a table function or a recursive
binding, never marks an input monotonic, and draws only accumulable and
hierarchical aggregates. Those are not omissions to fix there: its draw
sequence determines what a stored fuzz corpus entry decodes to, and the
release-qualification corpus is carried between runs, so widening it would
remap every entry and discard that coverage. `shapes` closes the gaps
alongside the draws instead, and each shape is held to its claim by the
runner like any other workload.
New cells: Constant/Error, FlatMap/Stream/{NoMfp,MfpAfter},
Reduce/Monotonic, TopK/{MonotonicTop1,MonotonicTopKLimited}, and
LetRec/{Unbounded,Limited,LimitedReturnAt}.
The monotonic shapes declare append-only inputs. A monotonic operator over
a retracting collection is simply incorrect, and the oracles would report
the wrong answers as a divergence, which would be the suite flagging a bug
in its own test data. A test enforces the pairing rather than trusting it.
The LetRec shapes are where the incremental oracle stops being redundant:
the constant folder cannot see through a recursive binding, so with no
independent reference it is the only check that a maintained recursive
collection matches a freshly computed one.
Inputs now span four timestamps instead of two, with rows repeated, so
arrangements compact mid-stream and the correction buffers see more than
one round. Still modest: the configuration matrix multiplies per-workload
cost by eight, and genuinely large data belongs in a load-oriented test.
`KNOWN_GAPS` entries are now matchable cell-name prefixes, and
`known_gaps_are_still_gaps` fails if any names a covered cell. A stale gap
list is worse than none, because it argues against work already done.
Verified against a real clusterd: 23/23 workloads pass all 8
configurations, 35 cells exercised, in 57s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
…itting it
The corpus was committed as 23 JSON files with a test that regenerated and
diffed them. That made the generator and the files two sources of truth for
what the suite runs, kept in step by a lint, and drifting the moment
somebody changed generation without regenerating.
Generate it in process instead. `headless-driver` takes
`DRIVER_WORKLOAD_SEED` and builds the corpus from the same fixed seed the
committed files came from, so a run executes exactly what it did before,
with nothing on disk to fall behind. Reproducibility is unchanged and is
pinned by `generation_is_reproducible`.
The two debugging paths that made a file-based corpus useful are kept, and
both still work:
* `gen-workloads --out <dir>` dumps a corpus for reading, which is what
you want when a workload fails and you need its plan and inputs in a
readable form. Its output is now purely an artifact, so nothing has to
keep it in step.
* `DRIVER_WORKLOADS=<dir>` runs JSON workloads from disk, for replaying a
dumped or hand-written one.
`corpus_matches_committed_files` is gone with the files it checked.
`committed_workloads_are_self_consistent` becomes
`default_corpus_is_self_consistent` over the generated corpus, which is
strictly better: it now checks what a run will actually execute rather than
what happened to be committed. Added a JSON round-trip test, since the
format is no longer exercised by a run and would otherwise break silently
in the two debugging paths that still depend on it.
Verified: 23/23 workloads pass all 8 configurations, 35 surface cells,
generated from seed 24301 with nothing read from disk.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
Nightly 17744 failed on `error_behavior.spec`: making a frontier timeout report what it observed changed a message the scenario asserts on, and the golden was not updated. The golden was doing its job. The new text is a stronger assertion for that scenario. It waits on a deliberately unscheduled dataflow, so "the replica never reported an output frontier" is precisely the expected state, and it now pins the distinction from a frontier that was reported and stopped short. The comment says so, since the message now carries meaning rather than being incidental. The gap that let it through is that `run-local.py` runs one scenario per invocation, so no single command checks every golden, and every local run during that work exercised the generated workloads instead. `SCRIPT=all` now runs all of them, each against a fresh clusterd (they reuse global ids and would otherwise collide), mirroring the mzcompose `scripts` workflow. Verified: all 13 scenarios pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
…step
The workloads suite reached CI only through the composition's `default`
workflow, chained after the scenarios. Nightly 17744 showed why that is
not enough: a scenario golden failed, the step stopped, and the workloads
suite never ran at all. Neither half should be able to hide the other.
Split it into `clusterd-test-driver` (run: scripts) and
`clusterd-test-driver-workloads` (run: workloads).
The new step's `inputs` are wider than the old step's, which listed only
`test/clusterd-test-driver`. The corpus is generated in process from the
LIR surface and the shared MIR generator, so a change to `src/compute-types`
or to `mirgen.rs` changes what this suite runs while touching nothing under
`test/`. With the old inputs, step trimming would skip the step for exactly
the changes most likely to break it.
`workflow_default` now loops over `c.workflows` with `c.test_case`, which
`check_default_workflow_references_others` requires and which keeps the
workflows independent for a developer running the lot at once.
Two hazards that only appear once `workloads` runs standalone:
* It killed clusterd before the first pass, which previously worked
because the scenarios had already started one. Skip the kill on the
first pass, as `workflow_scripts` already does.
* `config_base(0)` was 1000, so a workload's index was `u1001`, the same
id `index.spec` uses. Reconciliation matches the dataflows it is asked
to keep by id, so a leftover scenario dataflow could stand in for a
workload's and the workload would check the wrong collection. Workload
ids now start at 100_000, well clear of the scenarios' 1000-2001.
Removing the overlap is better than relying on teardown ordering.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
… imports
`Get/ArrangementLookup` was listed as needing index imports in the workload
format, which the format does not have. It does not need them.
Lowering keeps its own literal-constraint path alongside the
`LiteralConstraints` transform, because that transform handles only `Get`s
of *global* ids. A `Let`-bound `ArrangeBy` read by a filter that pins the
key to a literal takes the local-id path and lowers to a seek:
Let l = ArrangeBy(input, [#0]) in Filter(Get(l), #0 = k)
The shape carries export-invariance and the incremental check but not the
fold oracle: the constant folder does not see through a `Let`, so
generation drops it, as it already does for the `LetRec` shapes.
Two gap entries are corrected rather than removed, because attempts to
close them failed and the reasons are worth recording. `FlatMap/Arranged`
is not reached by putting a `FlatMap` over a `Let`-bound `ArrangeBy`:
lowering still hands the table function the raw collection, so the
arrangement goes to the `Get` and the FlatMap stays streamed. That shape
was written, measured, found to reach nothing new, and deleted rather than
left in claiming a cell it does not cover. `Mfp/Plain/Lookup` is likewise
still open: in this shape the filter fuses into the `Get` instead of
surviving as its own `Mfp` node.
Verified: 24/24 workloads pass all 8 configurations, 36 surface cells.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DxzVt3WrhiPPfUKwp7oNZ1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Compute's rendering path has no generated correctness suite. sqllogictest checks
SQL semantics end to end, and the
mz-transformfuzz targets check MIR-to-MIRequivalence under the optimizer, but nothing draws a plan, renders it on a real
replica, and compares the result against an independent reference. This adds
that, on top of the existing
clusterd-test-driver.The suite draws MIR from the same generator the fuzz targets use, lowers it, and
renders it against a real
clusterdwith noenvironmentdin the picture.Description
Four oracles, layered so each covers where the previous goes blind, all applied
at every timestamp the workload writes:
into the plan as literals and evaluate with the optimizer's constant folder, an
implementation independent of the renderer. Blind to
LetRec.same computation, rendered as three separate dataflows, must agree. Needs no
reference implementation.
its
as_ofalready at that timestamp. The only oracle that can judge arecursive collection.
must not change the answer.
Coverage is measured rather than asserted.
surface.rsclassifies a loweredplan into cells, the generator keeps a draw only if it covers something new, and
the runner re-derives the cells at run time and fails if they differ from what
the workload claimed.
KNOWN_GAPSrecords what is not reached and why, and atest fails if an entry names a cell that is now covered.
src/transform/src/mirgen.rs(behind amirgenfeature) holds the generator andthe fold oracle, extracted from
src/transform/fuzzso the main workspace canuse them.
FuzzEntropydelegates one-to-one so byte consumption is unchanged:that is what decides which plan a stored fuzz corpus entry decodes to, and
release qualification carries a minimized corpus between runs.
Design notes and the full findings list are in
doc/developer/compute-surface-suite.md.What the review of the first draft changed
The suite was green and had found nothing. The reasons were all in the corpus and
the observation points, and the last commit fixes them:
as_ofalready at the assertion timestamp, so each read a snapshot at thefinal timestamp and stopped. The incremental oracle compared two snapshot
dataflows. Exports now start at
as_of = 0, and every oracle runs at everytimestamp, so a collection that passes through a wrong intermediate state fails
even when it converges.
values never coincide, so equi-join keys never met, and a fifth of all leaves
drew zero rows and annihilated everything above them. Of six join workloads,
four had an empty input, one was a cross product, one had a single row. Values
are now folded into a small domain, empty leaves are redrawn, and a liveness
filter rejects a candidate that computes nothing.
workloadskeeps it, because aregression suite wants the same plans every night. The new
soakstep seedsfrom the build number, keeps every draw, and prints the seed to replay it.
stops computing anything reads exactly like a passing one. There is now a test
that the corpus computes something, a test that every dyncfg in the matrix
still exists (
ConfigUpdates::applyskips unknown names silently, so a renamewould degenerate the matrix into eight identical runs), and a run reports every
cell it only ever rendered over an empty collection.
and eleven shapes cover surfaces nothing reached: a recursion that iterates,
mutual recursion, an error appearing and being retracted mid-stream, a
hierarchical reduce losing its running extreme, a collection emptying out, a
join whose keys meet, a lookup whose key exists, a literal collection, and
array_agg, which turns out to be the one Basic aggregate expressible overinteger columns and so closes both
Reduce/Basicgaps and therender_basic_aggregatespath with them.The finding it then produced
An index peek and a materialized-view read of the same collection at the same
timestamp reported different errors:
An error is emitted with the multiplicity of the row it was raised on, and
Negatenegates its input's rows while passing its errors through, so anexpression evaluated above a
Negateraises errors with negative multiplicity.EXCEPT ALLplans toThreshold(Union(lhs, Negate(rhs))), and predicatepushdown deliberately leaves a literal-error predicate above the
Negateso thatthose errors cancel against the other branch's. That cancellation is
load-bearing: it is what keeps the null-extended branch of an outer join from
raising spurious errors, and database-issues#5691's regression test fails if the
predicate is pushed through instead (I tried).
So errors cancel between unrelated rows, and the row counts decide the outcome of
Invalid data in source errorsand logged aterror!, i.e. paging on a plain user queryNone of that is a claim about what should happen. Materialize has no error
semantics, they are a byproduct of
render.rs, and STG-54 is the standingaccount of what has to be decided. This case is its 1.8.1, which leans towards
errors from different records never cancelling while noting that we rely on the
cancellation internally, and the
Negatemechanism is its own entry there, downto needing error provenance to resolve.
test/sqllogictest/error_semantics.sltnow records all three cases rather than asserting one, which is what that file is
for.
What is a defect either way, and all the second commit changes, is narrower: two
read paths over one collection disagreed at one timestamp, and a condition a
plain query can reach was reported in the storage layer's vocabulary (the message
of nearly the same name really does mean an upstream source sent retractions for
rows it never sent, and pg-cdc asserts on it) and logged at
error!. It nowreports the error itself, which is what the persist and subscribe paths of the
same collection already do, and logs at
warn. No semantics change.Verification
cargo nextest run -p mz-clusterd-test-driver: 49 tests, including the corpusself-consistency, non-vacuity, gap-list and dyncfg-existence guards.
clusterd: 31 workloads x 8 configurationsx up to 4 timestamps each, 38 surface cells, green at one, two and four timely
workers.
.specscenarios (SCRIPT=all), which assert on driveroutput including error text.
bin/sqllogictest -- test/sqllogictest/error_semantics.slt test/sqllogictest/transform/predicate_pushdown.slt, the latter because itholds database-issues#5691's regression test.
two configurations each. This is what found the bug above; the seed that found
it now runs 163/163 clean.
Nightly runs three steps, separate so one cannot mask another:
scripts,workloads(fixed seed, one and two workers), andsoak(new plans each run).🤖 Generated with Claude Code
https://claude.ai/code/session_01Dz1B3CLa3AeEn94Ln2fASF