Repository navigation
[pull] main from MaterializeInc:main - #1256
Merged
Merged
Conversation
Builtin clusters became managed clusters in `76ba9e793f`, which gave them a `replication_factor` and made the hardcoded `BUILTIN_CLUSTER_REPLICAS` list redundant. The list stayed, and the catalog-open migration that materializes builtin replicas was never rewired to the factor. It kept reconciling against the list, using the bootstrap replication-factor flag only as an on/off gate and never reading the cluster's own row. Every other part of a builtin cluster's shape follows one rule: a const or a bootstrap flag seeds it at creation, and the durable row plus `ALTER CLUSTER` own it afterwards. The replica set was the sole exception, and the only part that drifted: - `replication_factor > 1` was silently truncated to one replica. The flags accept `0..=2`, and nothing else filled the gap because the cluster controller excludes system clusters. - `ALTER CLUSTER mz_support SET (REPLICATION FACTOR 1)` had its replica reaped on every restart, since `mz_support` and `mz_analytics` are absent from the list. That is the support break-glass path, and `--bootstrap-builtin-support-cluster-replication-factor` was inert for the same reason. - `ALTER CLUSTER mz_system SET (REPLICATION FACTOR 0)` was undone on the next boot, leaving durable factor 0 with a live replica. That state wedges later factor `ALTER`s on the `(cluster_id, name)` uniqueness constraint with `DuplicateReplica`. It needs no SQL to reach: bootstrap at 0, then restart without the flag. The helm chart ships `defaultReplicationFactor.system: 0`, so "install, then raise the factor" walked into it. - `ALTER CLUSTER mz_system SET (REPLICATION FACTOR 2)` had `r2` reaped, likewise. - A created replica took only `size` from its cluster, hardcoding a 1s introspection interval, no arrangement compression and no availability zones, so it could disagree with its own cluster's config. ## The change One rule: for a builtin cluster, the durable managed config is the single source of truth for its replica set. `reconcile_builtin_cluster_replicas` replaces the migration. A managed builtin cluster converges on exactly `replication_factor` replicas named `r1..rN`, shaped from the cluster's size, availability zones, logging and arrangement compression, and owned by the cluster's owner. Everything else on that cluster is dropped, except internal replicas, which are not derived from the factor and are left alone. An unmanaged one has no factor to converge on, so its replicas are left to the operator. The bootstrap flags now only seed the cluster row at creation, matching what `--bootstrap-builtin-*-cluster-replica-size` already did. A flag that disagrees with an existing row logs a warning naming both values and the `ALTER CLUSTER` that would apply it, so the deliberate no-op is not silent. `BUILTIN_CLUSTER_REPLICAS` and friends are deleted. Each entry said only "this cluster has a replica named r1", which the factor already says and says better. `managed_cluster_replica_name` moves to `mz_catalog::durable` so the reconciler and the `ALTER` path derive names from one rule, since two places disagreeing on names is what let `ALTER`'s create-and-drop-by-name miss replicas. Owning a created replica by `cluster.owner_id` rather than `mz_system` matters now that all five clusters are in scope, because `mz_support` and `mz_analytics` own theirs and ownership is checked against the replica's own `owner_id`. ## Behavior changes - `replication_factor > 1` takes effect and persists. An environment already carrying factor 2 converges on the first boot by gaining a replica. That would have been broad in CI, where the mzcompose harness tied the builtin system and probe factors to `default_replication_factor` and 24 compositions set it to 2. The coupling was incidental, so `materialized.py` pins them to 1 and CI's resource profile is unchanged. - An environment whose `mz_support` or `mz_analytics` factor is above 0 gains a replica that now sticks. - The bootstrap replication-factor flags become create-only, so changing `defaultReplicationFactor.*` in helm on an existing deployment no longer does anything. It previously appeared to work while leaving the wedged state above, so the working path (`ALTER CLUSTER`) is unchanged and the broken one is gone. Making the flag genuinely declarative needs a durable last-applied value and belongs in its own change. - An unmanaged builtin cluster is no longer repaired at boot. `ALTER CLUSTER mz_catalog_server SET (MANAGED = false)` plus dropping its replica used to be undone on the next open. That already breaks introspection immediately, so the repair was papering over a self-inflicted outage. Rejecting `MANAGED = false` on a builtin cluster would remove the footgun and is worth doing separately. ## Testing `src/environmentd/tests/bootstrap_builtin_clusters.rs` restarts against the same durable catalog. Five tests fail on `main`: a bootstrap factor above one is realized, the `mz_support` replica survives, `ALTER` to 0 sticks and a stale flag does not resurrect it, `ALTER` to 2 keeps `r2`, and a created replica is owned by its cluster's owner. Two more are guards that pass on both. Replica logging and arrangement compression are not exposed per replica in the catalog, so a unit test on `managed_replica_config` pins that every shared field comes from the cluster config. `builtin_cluster_replication_factor.py` covers restart survival as a platform check, gated to scenarios whose boots are all the current version. Released binaries reap the replica, and both `manipulate` and `validate` run on them in the upgrade scenarios. ## Follow-up The helm chart documents `operator.clusters.defaultReplicationFactor.*` with no description, and those knobs are now bootstrap-only. That needs a description and a release note pointing operators at `ALTER CLUSTER`. Not done here because `helm-docs` generates the chart docs and CI lints them for being in sync.
…alized views (#37679) ### Motivation `mz_postgres_source_tables`, `mz_mysql_source_tables`, `mz_sql_server_source_tables`, and `mz_kafka_source_tables` were builtin tables filled by Rust packers that ran on the leader `environmentd` on every catalog change. This is the next step in the ongoing effort to convert catalog builtin tables to materialized views. It follows the same pattern already landed for `mz_postgres_sources` and `mz_kafka_sources`. ### Solution There are two parts: The first add parse_source_export_details SQL helper`. A new SQL helper that parses a catalog `create_sql` and, for a `CREATE TABLE ... FROM SOURCE` or a non-progress `CREATE SUBSOURCE ... OF SOURCE` statement, returns the parent source id, the external reference parts, and the envelope and key/value formats. The second, converts all four tables to `BuiltinMaterializedView`. Each view reads `Item` rows from `mz_catalog_raw`, extracts the parent source id and external reference (plus envelope and formats for kafka) via `parse_source_export_details`, and joins `mz_sources` to keep only exports whose parent matches the connection type. ### Testing - New behavioural-lockdown SLTs (`test/sqllogictest/mz_*_source_tables.slt`) pin the column shape and nullability, confirm the views are empty on a fresh env, and verify that a LOAD GENERATOR source's subsources are excluded by the parent-type filter. - Rows with real upstream data are exercised by the existing pg-cdc, mysql-cdc, sql-server-cdc, and mz-sources testdrive files, which query these views unchanged. - Unit tests in `src/expr/src/scalar/func/impls/jsonb.rs` cover both syntaxes: the multi-part postgres-style reference, kafka bare/key-value/registry formats across envelopes, progress subsources, non-exports, and unresolved item names.
### Motivation The envd CPU sweeps have been failing against staging with `failed to apply Cloud CPU override for environmentd via 'mz region': never got correct result ... FATAL: upstream server not available` ([SQL-596](https://linear.app/materializeinc/issue/SQL-596)). Nothing is wrong with the environment. `mz region enable --environmentd-cpu-allocation` on a region that still has durable state is a 0dt rollout, so the new envd comes up read-only and only promotes after the caught-up stability period: ``` 06:25:01 cluster is caught up but not yet stable for the required period cluster_id=s1/s2/s3/u2 all_healthy=true restart_total=0 stable_for_ms=290025 required_period_ms=600000 ``` Enabled at 06:19:48, caught up and healthy by 06:20:03, would have promoted at ~06:30. We wait 300s and give up at 06:25, and nothing serves SQL in between because the soft disable took the old envd away. The `with_0dt_caught_up_check_stability_period=0s` we pass via `--environmentd-extra-arg` isn't in effect there (600000 is the compiled-in default, and the region is hard-disabled at job start so there's no stale `ALTER SYSTEM SET`). That's being followed up separately, this PR makes the test not depend on it. ### Description Hard-disable instead of soft-disable: with no state left there's no predecessor generation to catch up with, so the new envd promotes as soon as it has booted and the gate never applies. The sweep re-runs `prepare` per scale point, since the wipe takes its state with it (one table, one insert, both envd workloads have an empty `setup()`). Teardown's reset to the default allocation goes through the same recreate, so it can't leave the region in read-only limbo for a following scenario. Readiness gets an honest check, which is what let this hide. After the recreate we assert the region has no user tables, which can only hold for a fresh region and so rules out a leftover envd, and `wait_for_envd` now wants several consecutive probes instead of one, reports how long it waited, and allows 600s instead of 300s. Neither `mz region enable` (which only checks that something answers on the SQL port) nor a single query can tell old envd from new, and before this change the wait was returning one to two seconds after the re-enable, which is well inside a real bring-up. The second commit retries the disable/enable pair, because a sweep makes a dozen region API calls and staging returns the occasional 502 that the CLI does not retry itself. ### Verification Test-only. Ran on staging in spec-sheet build 29, where both envd sweeps pass: all six scale points plus the teardown reset, each region recreate serving SQL after 6-9s, roughly two minutes per point end to end. Those scenarios have been failing since build 27. `copy_from_stdin_envd_strong_scaling` needed a job retry because `mz region enable` hit a transient 502 from the Cloud API, which is what the second commit retries in-test. Throughput matches build 26, the last green run, within a few percent at every point but the 32-core QPS one, which is a plateau in the benchmark unrelated to this change, see [SQL-596](https://linear.app/materializeinc/issue/SQL-596) for that. The Docker target was exercised locally as well, where it reaches dbbench and then hits a pre-existing docker-target failure (`permission denied for CLUSTER "c"` in dbbench setup, present before this change too).
… timeout (DEX-92) (#37986) `MCP_REQUEST_TIMEOUT` only wrapped the handler body, so a stalled `catalog_snapshot` could hang a request indefinitely. Since the configured value lives in that same snapshot, this bounds the fetch with the dyncfg's compiled-in default and returns 503, matching the existing pre-flight failure path.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…med input (#37971) Three unchecked-arithmetic and unchecked-decode fixes in `mz-persist-client`. Each one is on a path that reads durable state, where the bytes are whatever consensus or blob happened to hold, so none of the inputs are trusted. * **`ProtoStateFieldDiffs::validate` summed `data_lens` with unchecked arithmetic.** Overflow checks are off in the release and optimized profiles, so `data_lens = [u64::MAX, 1]` with empty `data_bytes` wrapped the sum to 0, matched the real byte length, passed validation, and left the iterator slicing `0..u64::MAX` out of an empty slice. Sums with `checked_add` now and rejects on overflow, so a malformed diff becomes the same decode error as every other inconsistency `validate` catches. * **`LazyPartStats`'s `Debug` panicked on a malformed encoding.** The proto is held undecoded and stored without validation, and `Debug` called a `decode()` that `expect`s it to be well formed. That is reachable on the state-load *rejection* path: `Spine::validate` formats the spine into the message it rejects with, so a rollup whose trace fails validation *and* carries malformed part stats panicked while building the error meant to reject it. The shard then failed to load by panic rather than by error, on every attempt. Splits out `try_decode` and renders an `<undecodable: ...>` placeholder instead. * **The spine's fuel computation could wrap.** `(8 << batch_index) * effort` was unchecked in both steps, with `batch_index` derived from the inserted batch's `len`. Saturates now. One `NOTE:` is left in the code deliberately: the read-path callers of `LazyPartStats::decode()` (`fetch.rs`, filter pushdown in `operators/shard_source.rs`) run on the same untrusted bytes and still panic on a malformed encoding. Each needs its own decision about what a missing-stats fallback should do, so they are out of scope here. ### Tests Adds `proto_state_diff_data_lens_overflow_is_error` and `lazy_part_stats_debug_does_not_panic_on_garbage`, both of which panic without the corresponding fix, plus coverage for the saturating fuel computation. The `LazyPartStats` panic was found by the `rollup_proto_roundtrip` cargo-fuzz target in release-qualification 1330 (v26.35.0-rc.1). That target is sharpened in #37979, which should land after this. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ending an implicit transaction can change session parameters, but pgwire's `end_transaction` threw away the whole `ExecuteResponse` and only inspected the error case. Explicit COMMIT/ROLLBACK announced those changes already, so only the implicit paths were silent. `SET LOCAL` outside an explicit transaction is what makes this observable. The SET announces the new value and the implicit commit reverts it, so a client that caches ParameterStatus (pgjdbc, psycopg) serves the reverted value for the rest of the session. The same asymmetry hit a plain SET whose staged value a later statement error rolled back. Closes: [SQL-490](https://linear.app/materializeinc/issue/SQL-490) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…37972) Two fixes to the Avro object-container-file reader. Both are on the path that decodes an OCF supplied by a source, so the bytes are untrusted. * **A block could read the previous block's bytes.** `Reader` reuses one buffer across blocks and `fill_buf` only ever grew it, so a block shorter than its predecessor left that predecessor's tail readable past its own payload, because everything downstream reads the buffer by its length (`read_next` slices `self.buf[self.buf_idx..]`, and `Codec::decompress` hands the whole buffer to the decompressor). A block whose declared object count outran its own bytes then decoded stale bytes into values instead of running out of input. Truncating to the payload after reading fixes it. `Vec::truncate` keeps the allocation, so the buffer is still reused. * **A block's object count was effectively unbounded.** The count is read straight off the wire and `safe_len` capped it only at `MAX_ALLOCATION_BYTES`, a sensible ceiling for a byte length, an enormous one for a count, and unrelated to the block it describes. A zero-width schema (an empty record, or a record of only `null` fields) encodes every object to no bytes, so a handful of wire bytes could claim hundreds of millions of objects and the reader would decode every one, allocating a `Vec` and per-field `String` each time. Measured: a ~30-byte file spends 47s on 100M objects, minutes at the old ceiling. The count is now bounded against the block that is supposed to hold those objects, choosing the bound the way `decode.rs` already does for an array block: the byte floor when the object schema has a proven positive one, otherwise the node-weighted `MAX_VALUE_NODES` cap, since no payload length can constrain a count of zero-width objects. Unlike an array block the two are alternatives rather than both applied. An array materializes all its elements at once so its cap is about retained memory, while a block's objects are yielded and dropped one at a time, so this cap is about work amplified out of a few bytes. Once the byte floor holds, the work is linear in the file, and capping nodes as well would reject a legitimate large block of small records. The bound is applied after decompression, since the declared byte size is the compressed size. ### Tests Both are covered by the `reader_decode` cargo-fuzz target, which timed out within seconds of drawing block counts from the range `safe_len` accepted. That target is sharpened in #37979, which should land after this. --------- Co-authored-by: Claude Opus 5 (1M context) <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 : )