Repository navigation
[pull] main from MaterializeInc:main - #1306
Merged
Merged
Conversation
Adds `mz_internal.mz_object_graph_edges`, a maintained view (indexed on `mz_catalog_server`) that exposes object dependency edges. Each row `(object_id, dependency_id)` means `object_id` depends on `dependency_id`. The view unions the dataflow-layer dependencies between maintained objects (indexes, materialized views, sinks, sources, tables) with the source-to-subsource and source-to-table edges that stitch a source to the children carrying its data. ## Console surfaces The console reads these edges to walk object dependencies from a single maintained arrangement, instead of recomputing a per-view union of the dependency relations on every request. Surfaces that consume the DAG: - The object **workflow graph**, the dependency graph on an object's detail page. - **Critical-path freshness analysis** for maintained objects, which walks upstream to find the slowest input. - **Impact and dependents** views, the blast radius of changing or dropping an object. ## Operational impact Adds one maintained arrangement to `mz_catalog_server` (~200kB of memory). ## Tests - Existence and schema goldens: `information_schema_tables.slt`, `oid.slt`, `mz_catalog_server_index_accounting.slt`, `autogenerated/mz_internal.slt`, `catalog.td`, `indexes.td`. - A functional edge test added to `dataflow-dependencies.td` covering all three union branches: the maintained dependency chain (materialized view, index, and sink), the source-to-subsource stitching, and the source-to-table stitching. ## Release notes This release will add the `mz_internal.mz_object_graph_edges` system catalog view, which lists the dependency edges between maintained objects and the sources of their subsources and tables. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to #38118, which moved the sync MV sink's correction maintenance to a blocking thread but left the read-back that follows it on the async task, with a comment claiming the batch builder's part flushes bound how long that occupies a Tokio worker. They do not. `BatchBuilder::add` only awaits once a part fills, and the part writes are spawned, so the builder awaits nothing until more than `persist_batch_builder_max_outstanding_parts` (2) writes are in flight. Feeding it therefore pulls on the order of two parts' worth of updates, `persist_blob_target_size` (128 MiB) each, before the first real yield. Every pull clones a row out of a correction chunk and, with the pager enabled, can page that chunk back in, so the worker ends up blocked in the kernel for the same reason the maintenance pass was. `emitted` holds every update below `upper`, so the read-back sweeps a buffer as large as the pass that already runs on a blocking thread. Move it there too: the blocking closure keeps ownership of the buffers, consolidates, and streams the consolidated updates back over a bounded channel in chunks, and the async task feeds those chunks to the persist batch builder. The row clones and page-ins land on a preemptible thread, and the handoff adds at most a few thousand buffered updates. `writer` no longer needs an exclusive borrow, because the builder is created with `WriteHandle::builder` rather than `WriteHandle::batch`. Based on QA LLM review: https://github.com/MaterializeInc/qa-llm-review/blob/master/commit-bugs/done/analysis-commit-0f7e5b187226c54a5feefd91feb929af6642c8b8.md ### Release notes This release will not include user-visible changes. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…me (#38185) Fixes [CLO-117](https://linear.app/materializeinc/issue/CLO-117/invalid-service-name-used-during-connection-reports-availability-zone). ### Motivation A user created an AWS PrivateLink connection with an ELB DNS hostname in `SERVICE NAME` instead of a VPC endpoint *service* name: ```sql CREATE CONNECTION c TO AWS PRIVATELINK ( SERVICE NAME 'my-db-lb-0123456789abcdef.elb.eu-central-1.amazonaws.com', AVAILABILITY ZONES () ); -- CREATE CONNECTION ``` `CREATE CONNECTION` accepted it, and the mistake only surfaced later as ``` Error: The Endpoint cannot be created due to missing availability zones ``` which points at the wrong option. The environment controller cannot parse that value as a cross-region service name, so it treats the endpoint as same-region and reports the empty `AVAILABILITY ZONES` list as the failure. ### Changes * `AwsPrivatelinkConnection::check_service_name` rejects values that cannot be AWS VPC endpoint service names. It is deliberately permissive: it requires only the `com.amazonaws.` prefix, so both customer-owned (`com.amazonaws.vpce.<region>.vpce-svc-<id>`) and AWS-managed (`com.amazonaws.<region>.<service>`) services pass. * A new `PlanError::InvalidPrivatelinkServiceName` carries a hint that gives the expected shape and says the value is not the DNS name of the target. * `CREATE CONNECTION` and `ALTER CONNECTION` now fail up front with that error. * `AwsPrivatelinkConnection::validate` performs the same check before reading endpoint status, so `VALIDATE CONNECTION` on a connection that already stores an offending value reports the true cause instead of the availability zone message. This is the exact symptom in the report, and it also removes the need for a new `VpcEndpointState` variant in the cloud repo. * Docs: the `SERVICE NAME` syntax element now says what the value is and is not. ### Why the sequencer and not the planner The obvious home for this is `plan_connection`, next to the existing `AVAILABILITY ZONES` validation. That is not safe. Catalog items are re-planned on boot via `CatalogState::deserialize_item`, and a failure there panics (`src/adapter/src/catalog/apply.rs`). Any environment that already stores an offending service name would crash-loop on upgrade, which is precisely the population this change is about. `with_enable_for_item_parsing` does not help either, since it force-*enables* flags during item parsing, so a flag-gated planner rule would still fire on boot. The sequencer runs only for statements a client issues, so the rule cannot stop an existing item from loading. For `CREATE` the check precedes id allocation, secret creation, resource-limit validation, and the catalog transaction, so nothing leaks before the rejection. Two accepted behavior changes for environments that already store an offending value: * `CREATE CONNECTION IF NOT EXISTS` with the offending name now errors instead of being a no-op. This matches how existing option validation already behaves (an invalid `AVAILABILITY ZONES` entry errors at plan time regardless of `IF NOT EXISTS`). * `ALTER CONNECTION` re-plans the full `create_sql`, so it reports the stored offending name even when the statement does not touch `SERVICE NAME`. Setting a valid `SERVICE NAME` in the same `ALTER` clears it. ### Tips for reviewer The permissiveness of the rule is the main thing to sanity check. It is prefix-only on purpose. Worth confirming with the cloud team that no live connection uses a service name outside `com.amazonaws.*` before this ships, since such a connection would start failing `VALIDATE CONNECTION` and `ALTER`. ### Checklist - [x] This PR has adequate test coverage / QA involvement has been duly considered. ([trigger-ci for additional test/nightly runs](https://trigger-ci.dev.materialize.com/)) - [ ] This PR has an associated up-to-date [design doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md), is a design doc ([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)), or is sufficiently small to not require a design. - [ ] If this PR evolves [an existing `$T ⇔ Proto$T` mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md) (possibly in a backwards-incompatible way), then it is tagged with a `T-proto` label. - [ ] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label ([example](MaterializeInc/cloud#5021)). - [x] This PR includes the following [user-facing behavior changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note): - `CREATE CONNECTION ... TO AWS PRIVATELINK` and `ALTER CONNECTION` now reject a `SERVICE NAME` that is not an AWS VPC endpoint service name, and `VALIDATE CONNECTION` reports that as the cause instead of a missing availability zone. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Closes: [SQL-643](https://linear.app/materializeinc/issue/SQL-643) Related to #37988
…M (SQL-428, SQL-423) (#37905) Problem: A single query can drive environmentd to OOM, even when the client read as fast as it could, because two paths buffered unbounded output in the shared process. - **HTTP `SELECT`.** The peek path already streams results out of the replica in bounded batches, but the HTTP SQL layer re-buffered every batch into a `serde_json::Value` tree roughly 10-15x the wire size. A large `SELECT` over `/api/ws` or `/api/sql` grew memory without limit. The `/api/sql` size guard was a stale `TODO` and was never enforced. - **SUBSCRIBE / COPY-TO.** The replica -> controller -> coordinator response path is unbounded at every hop, and the subscribe producer runs on the non-blockable coordinator main loop, so it cannot apply backpressure to a slow client. One slow or stopped SUBSCRIBE consumer could make the coordinator buffer the entire subscribe output in memory. Solution: **HTTP SQL layer ([SQL-428](https://linear.app/materializeinc/issue/SQL-428)).** - WebSocket `/api/ws` streams rows straight through. `SELECT` now flows through a new `StatementResult::Rows` variant whose sender emits `Rows(desc)` plus per-row `Row` messages, flushing between stash batches so a slow client applies backpressure and only one batch is resident. `max_result_size` is enforced incrementally. Large results over WebSocket now get the same bounded memory profile as pgwire `SELECT`. - Plain JSON `/api/sql` is a single buffered document. Rows are accumulated as pre-serialized `RawValue`s instead of a `Value` tree, and `max_result_size` is enforced on the actual serialized byte count. An oversized result fails fast with `ResultSize`. - MCP and prometheus consumers of `SqlResult::Rows` parse each `RawValue` on demand. **SUBSCRIBE / COPY-TO buffer ([SQL-423](https://linear.app/materializeinc/issue/SQL-423)).** Bound the terminal buffer and retire the sink when it overflows. `ActiveSubscribe` carries a shared byte counter and a per-sink budget. Each send adds the batch's byte size and a receiver wrapper subtracts it as the client writer drains, so the counter tracks the currently buffered depth. After each response the coordinator retires the subscribe if the depth exceeds the budget, surfacing `AdapterError::SubscribeFellBehind` with a hint to use a non-buffering client or `COPY (SUBSCRIBE ...) TO STDOUT`. The budget is a new dyncfg `subscribe_max_buffered_bytes` (default 128 MiB). `COPY (SUBSCRIBE ...) TO STDOUT` shares the channel and is covered too. Testing: New integration tests in `src/environmentd/tests/server.rs`: - `test_ws_select_streams_rows`: a `SELECT` over `/api/ws` emits one `Row` message per row before `CommandComplete`, confirming the non-buffering path. - `test_http_sql_result_size_limit`: with `max_result_size` lowered to 1 MB, an oversized `/api/sql` result fails with a result-size error while a small one succeeds. - `test_subscribe_buffer_bound`: with `subscribe_max_buffered_bytes` set to 1 KiB, a COPY-wrapped SUBSCRIBE whose client never reads surfaces the "fell behind" error instead of growing without bound.
### Motivation Closes: [CPU-204](https://linear.app/materializeinc/issue/CPU-204) Follow-up: [CPU-209](https://linear.app/materializeinc/issue/CPU-209) An error collection carries a multiplicity almost nothing reads. A dataflow is in an error state if its error collection is non-empty, and the error surfaced to the user is chosen arbitrarily, so an error's diff conveys nothing about the query's answer. Several rendering patterns nonetheless grow that diff multiplicatively rather than additively, and because every error in a dataflow consolidates onto one of a handful of distinct error values, those diffs pile onto single records and reach `Diff` overflow on plans of quite ordinary size. The reported shape is plan-level sharing: a chain of diamond-shaped CTEs over a dataflow that has any `EvalError`. The asymmetry that makes this an error-only problem is worth stating plainly, since it explains why the ok side of the same plan is fine: the branches of a diamond compute *different* ok rows, so ok multiplicity does not compound, but they propagate the *identical* upstream error collection, so error multiplicity doubles at every reconvergence. ### Description Three multipliers. Two are fixed here, the third only for materialized views. **Delta join, N copies per join.** `render_delta_join` propagated each input's pre-existing errors once per delta path: `build_update_stream` concatenated the source's errors and every stage's `build_halfjoin` concatenated a lookup arrangement's errors, inside the per-path region. With N inputs that is N copies of every input's errors assembled over N² concat edges, and since a join's output is another join's input those factors compound through a nested plan. Input error collections are now gathered once, outside the path loop, and the two helpers return only the errors they themselves produce. No operator added, no semantic change: the same union of streams, formed once instead of N times. **Sharing within a dataflow, f copies per level.** Every reader of a binding propagates that binding's errors independently, so a binding read f times contributes its errors f times, and the factors apply again at each further level of sharing. `CollectionBundle::distinct_errs` collapses a binding's error multiplicities to one, applied at every `Let` and rec binding definition. This is the multiplier the reported incidents were made of, and the one the regression test drives to overflow. **Sharing across objects, f copies per level.** A level of sharing expressed as an indexed view rather than a CTE reads `Get(Global)`, which a binding collapse never sees, so the importing dataflow accumulates the exported diffs. Only half of this is closed here, at the persist sink, whose shard another dataflow reads back verbatim and whose contents are durable. Subscribes and one-shot copies are read once by a client and never re-imported, so they do not pay for the arrangement. The sink normalizes after the null assertions, whose errors follow the ok row's multiplicity rather than the error collection's. The index half is **not** fixed, and [CPU-209](https://linear.app/materializeinc/issue/CPU-209) records why: collapsing at the export gives the exported arrangement a second trace reader, which pins the shared spine's physical frontier and makes importing dataflows panic in `cursor_through` with `upper` straddles batch. That was measured four ways, including a variant that builds the reduce, discards its output, and exports the original trace, which panics too. So the fault is the reader's existence, not its output or the frontier probe. Cross-object multiplicity through indexes therefore stays unbounded. In proportion: object-graph path counts for the reported incidents top out at 8, against roughly 7×10^30 within one dataflow, so this half is reachable synthetically but is not what those incidents were. Two decisions the diff cannot explain: *Why a reduce and not arithmetic on the diffs.* Saturating the add, or collapsing to a sign, looks cheaper and does not work: neither is additive, so neither cancels when the errors retract, and a partially-retracted error would persist forever as a phantom. Bounding multiplicity requires reading the accumulated collection, which means a reduce over an arrangement. `LetRec` already collapses its error variable this way and for this reason. *Why every representation in a bundle is collapsed, not just one.* A bundle's raw collection and each of its arrangements carry their own independent error stream, and they are not even the same content: an arrangement's errors include the key-formation errors that the raw collection's do not. Which form a consumer reads is the consumer's choice, and a delta join reads both in one operator, so the binding definition cannot know that choice. ### Behavior and rollout `enable_compute_error_distinct` gates the per-binding collapse, and now defaults **on**. The divergence that argued for shipping dark is closed: the sink normalization is ungated, so nothing this flag decides reaches durable state and two replicas rendering under different values write the same thing. A default change also arrives with a new binary that every replica is recreated on, whereas a runtime flip is what leaves two live replicas rendering differently. Production LaunchDarkly still serves off, so the flag is allowlisted in `launchdarkly-flag-consistency` until that is reconciled. Two parts are deliberately **not** gated, so "behavior is unchanged with the flag off" would be wrong: - The delta-join rewiring is unconditional. It is equivalence-preserving on the set of errors and strictly reduces copies, and the failure mode worth guarding against is `bundle_errs` missing a form and silently dropping a dataflow's errors, which a test catches and a flag does not. - The sink normalization is unconditional, because gating it would leave the durable content of a materialized view's shard dependent on the flag. Error counts change, in two places. `mz_compute_error_counts_raw_unified` sums error diffs, so a diamond over one failing row previously reported 8 and now reports 1; unshared plans are unaffected. And anything reading a materialized view's shard now sees one error per distinct error rather than one per failing row, which is what the `idx2_div_by_zero` golden moves record. Neither number is promised: an object is tainted by having any error at all, and that is the part that is preserved. The Console does surface these counts, so the change is visible. ### Verification `test/sqllogictest/error_semantics.slt` gains a 70-level diamond CTE chain over an erroring expression, asserting the error still comes back, where an overflow is a panic CI fails on. Measured both directions: | Flag on | Flag off | | --- | --- | | PASS | `Overflow: 4611686018427387904 + 4611686018427387904` | Full file with the flag on: 153/153. With the flag off it panics in `consolidate_updates_slice_slow` over `DataflowErrorSer` and the replica dies. `union` rather than `union all` is what makes the chain reach the overflow: the distinct arranges the error collection so duplicates consolidate into one record with a doubling diff. Under `union all` the records duplicate physically instead and the query exhausts memory, 34 GB with no completion, without ever overflowing. A cross-object chain of indexed views is deliberately not in the suite, since nothing here bounds it. It belongs with [CPU-209](https://linear.app/materializeinc/issue/CPU-209). Thanks to @ggevay for measuring that the first version of that test did not reproduce the bug at all, for the `union all` to `union` diagnosis, for finding that the cross-object half was still unfixed, and for the reference-count measurement that removed a gate this PR did not need; and to @def- for the mzcompose workflow showing two replicas of one cluster correcting each other's writes to an errored materialized view's shard indefinitely, which the sink normalization closes. Operator cost, measured on `mz_catalog_server` in an empty environment: +1,014 dataflow operators (+6.4%) with the flag on, measured before the sink normalization was added. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01GbtepuqcxmAffaM2qXZ4uA --------- Co-authored-by: Claude <noreply@anthropic.com>
This PR is meant to be merged with everything else in the stack and is split for review purposes ### Motivation sql-150 ### Description ## catalog: convert mz_tables and mz_views to materialized views - With temporary items durable in the catalog shard, `mz_tables` and `mz_views` can be derived from `mz_internal.mz_catalog_raw`. Both show every item including temporary ones, matching the previous builtin tables. Temporary rows keep the temporary schema sentinel `"0"` in `schema_id`. - Builtin tables and views are reported through two new generated constant views, `mz_internal.mz_builtin_tables` and `mz_internal.mz_builtin_views`, following the `mz_builtin_materialized_views` pattern. - The generated `mz_builtin_*` reporter views are listed in `mz_views` with a short placeholder instead of their real SQL. `mz_builtin_views` cannot textually contain itself, and the other reporters embed metadata about every builtin object, so re-embedding their SQL would produce enormous rows. - A deliberate cosmetic delta: builtin view rows' `create_sql`/`redacted_create_sql` used to be the raw in-memory text (`CREATE VIEW mz_catalog.x AS ...`, unquoted idents) and are now stable reprints (quoted idents). `definition` is unchanged either way. - `parse_catalog_create_sql` now exposes `'definition'` for views and `'source_id'` for tables created from sources. ## test: update tests for mz_tables/mz_views as materialized views Updates various tests to accommodate the new builtin objects, and updates the descriptions of other tests related to temporary objects. ## expr: test parse_catalog_create_sql `mz_tables` and `mz_views` select rows by `parse_catalog_create_sql(...)->>'type'` and read `'definition'` and `'source_id'` out of the same call, but the function had no tests. Pins: - the reported type for every statement kind an Item record can hold - the exact `mz_views.definition` rendering (which `pg_views` exposes and which moved here out of the deleted `pack_view_update`) - its idempotence under re-parsing - `source_id` presence for `CREATE TABLE FROM SOURCE` and absence for plain and webhook tables - the four error paths The error paths matter more than they used to: the MVs call this inside their `WHERE` clause, so an item the parser rejects makes the whole relation unreadable rather than breaking one row's packing. ## test: add mz_tables and mz_views lockdown slts `mz_tables` and `mz_views` became `BuiltinMaterializedView`s over `mz_internal.mz_catalog_raw`, deriving every column from durable catalog JSON, but the conversion added no test file. Every earlier conversion (`mz_indexes`, `mz_audit_events`, `mz_postgres_sources`) got one. Follows the established lockdown shape: one section per union branch, plus the temporary-item sentinel schema id, the exactly-once property across the user and builtin branches, and the `ASSERT NOT NULL` columns. Two checks are independent of the MV rather than golden values: - `oid` is compared against a `regclass` cast, which resolves through the in-memory catalog - `mz_views.definition` is compared against the definition of a view planned from it, so the rendering is verified to be a fixed point end to end `create_sql` cannot be compared against `SHOW CREATE`, which humanizes item ids and pretty-prints by design. Also pins the one known difference from the old builtin tables: the generated `mz_builtin_*` reporter views appear in `mz_views` with a placeholder definition, since `mz_builtin_views` cannot textually contain itself and the other reporters' SQL would be enormous. ### Verification - Added SLTs and jsonb tests following the other builtin table -> mv conversions --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keeps failing in CI, see for example https://buildkite.com/materialize/test/builds/131939#01a01a66-ff4c-43bd-a5b4-f53b0d57c626
…ange (#38363) ### Motivation The MySQL RDS instance backing SourceIngestionScenario is being upsized, which makes new results incomparable with prior runs. ### Description Bumping the scenario VERSION so the analytics data reflects the discontinuity. ### Verification Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )