Add mz-release-signoff agent skill - #38355
Conversation
Release verification is a weekly rotation across seven Grafana dashboards, and almost all of its cost is rediscovery rather than judgement. Each verifier has to work out which environments run the candidate, when each stack was upgraded, which of a dashboard's panels matter, and which apparent signals are artifacts of the upgrade itself. This skill records that method once, so a run starts from the measurement rather than from the archaeology. The skill is split into an area-agnostic spine and one reference per dashboard. The spine covers finding the candidate version and the upgrade boundaries, selecting the environment set (production pins the canary environments by name because they share the released version with customer environments for part of the week, while staging can join on the release-candidate version), choosing a window that reaches the previous upgrade, batching many metrics into one range query, and the rules for telling a regression from drift. The references name each metric with its type and labels, the hazards specific to that area, and baselines measured on 2026-08-19 for v26.38.0-rc.3 against v26.37.0. The judgement rules are the part worth reviewing. Every upgrade restarts the process, which resets memory downward and flatters the new version, so levels have to be compared at equal age since restart. Zero-downtime upgrades run two generations side by side, so every sum doubles for one bucket and that bucket must be excluded. Arrangement gauges are bimodal and only their base level is comparable. Staging fleet CPU ranged from 2.83 to 5.55 cores over three weeks, so a step below roughly ten percent there carries no information on its own. Characterizing the dashboards surfaced defects worth fixing separately. Four expressions on environmentd-health render empty and therefore read as healthy: two divide a histogram by a counter using a bare metric name that does not exist, one reads mz_query_latency_bucket which went away with the stash, and one plots container_spec_swap_limit_bytes instead of container_spec_memory_swap_limit_bytes. Two adapter p99 panels are pinned by bucket resolution and cannot move while their counters advance. The storage-overview sign-off panel points at a canary environment list in MaterializeInc/cloud that no longer exists; the live list is in MaterializeInc/release. These are recorded in the references rather than fixed here. One measurement is left as an open finding for Persist. On staging us-east-1, compare-and-set mismatches per persist command stepped from 0.0061 to 0.0099 in the first bucket after the v26.38.0-rc.1 upgrade, held there for four days, then fell to 0.0085 with rc.3. The ratio is normalized by command volume, the step lands inside the upgrade bucket and is flat on both sides, and production canary shows no equivalent. No failure counter or latency moved, so this is recorded as something to recheck next release rather than as a blocker. Documentation only, so no tests accompany the change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ed222ed to
fe83e7b
Compare
The per-area baseline tables were the highest-maintenance and lowest-value part of the references. They also duplicated work the method already does: the before-window of a run is derived in the same query as the after-window, so it is the only baseline guaranteed to describe the same fleet, which makes a figure recorded months earlier redundant on the day it is written. Worse, absolute fleet sums move whenever an environment is created, deleted, or resized, so a stale value does not merely age out, it invites a comparison the reader should not make. What was load-bearing in those tables was never the levels. It was the invariants mixed in among them, which hold at any fleet size: that a healthy `mz_source_progress` series contributes exactly 1000 to its rate, so the panel is a series count in disguise; that `environmentd` swap is exactly zero and any reading is the finding; that upsert state is structurally absent from staging, so that dashboard can only be verified from the production sandbox; that rehydration latency is a last-value staircase with one step per upgrade; that error and crashloop series are absent rather than zero when healthy; and that `environmentd_needs_update` is the rollout clock, so a non-zero reading means the deploy under evaluation has not finished. Each reference now carries an invariants section in place of its table, plus a coarse order-of-magnitude block kept for one narrow purpose: catching a mis-scoped selector, such as a missing `container="clusterd"` that inflates a result tenfold. Those figures are dated, given to one significant figure, and labelled as unfit for comparison. The compute reference also gains a known noise classes section, because the environments that are unhealthy independently of any release are a durable fact about the fleet even though their exact restart rate is not. The guidance for characterizing a future dashboard is updated to match, and the calibration rule in step 6 now says to pull the spread live rather than trust a recorded range. The one place dated numbers remain in full is the open Persist finding, which is a claim about a specific moment rather than a reference value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bosconi
left a comment
There was a problem hiding this comment.
Looks good! I will get it running automatically.
Merge each reference's Hazards and Invariants into a single section. The split produced near-verbatim duplication in every reference, between three and five entries each, because almost every hazard is an invariant with a measurement attached. Each entry now leads with the property that holds at any fleet size and gives the measurement inline. Correct the compute reference's response queue row. It claimed mz_compute_controller_response_queue_size exists directly, but no metric! invocation registers that name and Prometheus carries no such series. The response channel is an instrumented_unbounded_channel, which takes a send and a receive counter and exports no depth gauge, so the difference of the two is the only way to read depth. Record the counter-reset hazard that follows, since both counters reset when a pod is replaced. Add scripts/, holding the batched-query builder that step 5 needs and the panel-query slicer that step 4 refers to. Guard the references against metric renames from two directions. At run time, catalog-diff.sh reads doc/user/data/metrics.yml at the release tags on either side of a sign-off boundary, which separates an empty panel that is a finding from one that is a metric arriving or departing with the release. At commit time, ci/test/lint-skill-metrics.sh resolves every mz_* name in the skill against the catalog, glob patterns included, and fails on any that neither resolves nor carries a documented exemption in metrics-allowlist.txt. It also fails on an exemption that has started resolving, so entries cannot outlive their reason. Record three dead dashboard panels found while building the lint. mz_query_latency was added in MaterializeInc#22049 and deleted in MaterializeInc#26647 along with the stash, and environmentd-health still plots its _bucket family. mz_persist_columnar_validation_count and mz_txn_placeholder_schema_apply have never appeared in this repository, yet both are arms of the persist dashboard's `should be small` panel, so two of its sixteen series are permanent zeros that make the panel read healthier than it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The scripts were added and checked in the same working tree, but bin/fmt and bin/lint only visit tracked files, so neither had seen them at the point they were run. Reformat them with black and give metrics-allowlist.txt the copyright header. The allowlist parser strips comments, so the header does not change what the lint reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks for the review! |
QA LLM Review (Post Merge)@antiguru — an automated review of commit 1. MEDIUM --
|
| file | fenced blocks | lint result |
|---|---|---|
persist.md |
no | rc=1, offender named |
adapter.md |
no | rc=1, offender named |
reference-dashboards.md |
no | (all 19 names checked) |
compute.md |
yes | rc=0, silent |
sources-and-sinks.md |
yes | rc=0, silent |
SKILL.md |
yes | rc=0, silent |
Overall the lint resolves 96 of the 166 mz_* names in the skill. compute.md contributes 0 of 26 and sources-and-sinks.md 2 of 40. You can see the desync directly in the token stream for sources-and-sinks.md, where the extracted "code spans" after the first fence are ', ' and ' | counter | Upstream read volume. |\n| ' — the separators between the real code spans, not the spans.
Fix: pull fenced blocks out first and scan them as their own tokens.
FENCED = re.compile(r"^```.*?^```", re.S | re.M)
def referenced():
"""Yield (name, file) for every mz_* token inside backticks in the skill."""
for path in sorted(SKILL.rglob("*.md")):
text = path.read_text()
for token in FENCED.findall(text) + BACKTICKED.findall(FENCED.sub("\n", text)):
...With that change the lint reports exactly three names: mz_storage_upsert_state_rocksdb_autospill_in_use (finding 2), plus mz_metrics_libc_ru_maxrss and mz_query_latency, which the references already document as dead panel arms and which therefore need allowlist entries next to the mz_query_latency_bucket one that is already there. Note that the if "*" in token: continue skip gets coarse when a token is a whole fence, so it is worth applying that test per whitespace-separated word rather than per token.
2. MEDIUM -- Two roster entries name metrics the product does not export, with no "dead" marker
.agents/skills/mz-release-signoff/references/sources-and-sinks.md:59
mz_storage_upsert_state_rocksdb_autospill_in_use and mz_persist_gc_skipped sit in roster tables as live metrics with reading instructions, but neither is exported. A sweep that queries them gets an empty result, which the skill itself names as the most dangerous reading on any dashboard.
Details
mz_storage_upsert_state_rocksdb_autospill_in_use was deleted along with the autospill upsert backend in 18fb1f796d; the surviving mz_storage_upsert_state_* metrics are rehydration_latency, rehydration_total, and rehydration_updates, and autospill appears nowhere in src/ or in doc/user/data/metrics.yml. Its row still explains how to read the percentage panel ("divides sum by count, so it reports the fraction of workers spilling"), so a verifier following the reference concludes no workers are spilling rather than that the concept is gone. The Container Max RSS Memory Usage and Stash (CRDB) Query Latencies entries a few lines away are marked as dead; this one is not, which is what makes it misleading rather than merely stale.
.agents/skills/mz-release-signoff/references/persist.md:35 lists mz_persist_gc_seconds, _started, _finished, _noop, _skipped, _merged_reqs. There is no mz_persist_gc_skipped; the catalog holds _finished, _merged_reqs, _noop, _seconds, _started, _step_seconds. The "already done" case is _noop (src/persist-client/src/internal/metrics.rs:1013), and _skipped exists only as mz_persist_compaction_skipped.
Worth folding into finding 1's fix: bare-suffix continuations such as `_skipped` and `_rehydration_updates` are how most roster rows abbreviate, and they carry no mz_ prefix, so CANDIDATE never sees them and the lint cannot check them even once the fence bug is fixed. Expanding a `mz_foo_bar`, `_baz` sequence against the preceding name's prefix would bring roughly another 40 names under the guard, including this one.
Motivation
Release verification is a weekly rotation across seven Grafana dashboards, and almost all of its cost is rediscovery rather than judgement. Each verifier works out again which environments run the candidate, when each stack was upgraded, which panels matter, and which apparent signals are artifacts of the upgrade itself. This records that method once so a run starts from the measurement.
Description
An area-agnostic spine plus one reference per dashboard.
The spine covers finding the candidate version and the upgrade boundaries, selecting the environment set, choosing a window that reaches the previous upgrade, batching many metrics into one range query, and the rules for separating a regression from drift. Environment selection differs by stack: production pins the canary environments by name, because they share the released version with customer environments for part of the week and a version filter loses them, while staging can join on the release-candidate version.
The judgement rules are the part worth reviewing. Every upgrade restarts the process, which resets memory downward and flatters the new version, so levels have to be compared at equal age since restart. Zero-downtime upgrades run two generations side by side, so every sum doubles for one bucket and that bucket has to be excluded. Arrangement gauges are bimodal and only their base level is comparable. Fleet CPU on staging moved by a factor of two over three weeks, so a few percent across a boundary carries no information on its own.
Each reference names the metrics with their types and labels, then gives one
Hazards and invariantssection. Each entry there leads with the property that holds at any fleet size and gives the measurement it came from inline, because splitting the two produced near-verbatim duplication in every reference.Guarding against metric drift
A reference that names a metric the product no longer exports is worse than no reference, because the sweep reads the empty result as a healthy zero. Two mechanisms cover that, split by when they run.
At run time,
scripts/catalog-diff.shreadsdoc/user/data/metrics.ymlat the release tags on either side of a sign-off boundary. The catalog is checked in, so it is tagged with each release, and the working tree's copy describesmainrather than the build under test. Diffing the two tags separates an empty panel that is a finding from a metric that arrived or departed with the release.At commit time,
ci/test/lint-skill-metrics.shresolves everymz_*name in the skill against the catalog and fails on any that neither resolves nor carries a documented exemption. A rename therefore breaks in the commit that renames rather than during a sign-off weeks later. Matching is not a string comparison: ametric!built withformat!is catalogued with its placeholders globbed, and histograms are catalogued as their expanded families, so both forms have to be resolved through patterns.scripts/also holds the batched-query builder that step 5 needs and the panel-query slicer that step 4 refers to.No recorded baselines
An earlier revision carried per-area baseline tables. They are gone, because they duplicated work the method already does and would need syncing with the product forever. The before-window of a run is derived in the same query as the after-window, which makes it the only baseline guaranteed to describe the same fleet, so a figure recorded months earlier is redundant the day it is written. Absolute fleet sums also move whenever an environment is created, deleted, or resized, and a stale reference value is worse than none because it invites a comparison the reader should not make.
What survives is the part that does not decay. Invariants such as a healthy
mz_source_progressseries contributing exactly 1000 to its rate, so the panel is a series count in disguise;environmentdnever swapping; upsert state being structurally absent from staging, so that dashboard can only be verified from the production sandbox; rehydration latency being a last-value staircase with one step per upgrade; andenvironmentd_needs_updateacting as the rollout clock. Alongside those, a coarse dated order-of-magnitude block per area, kept only to catch a mis-scoped selector such as a missingcontainer="clusterd", and explicitly not for comparison.Dashboard defects found, not fixed here
Recorded in the references so they are not rediscovered:
environmentd-healthrender empty and therefore read as healthy. Two divide a histogram by a counter using a bare metric name that does not exist, one readsmz_query_latency_bucket, and one plotscontainer_spec_swap_limit_bytesinstead ofcontainer_spec_memory_swap_limit_bytes.mz_query_latencywas real: added in adapter: Add more metrics #22049 and deleted in Delete stash files #26647 along with the stash.should be smallpanel resolve nowhere, so the panel carries two permanent zeros and reads healthier than it is.mz_persist_columnar_validation_countduplicates a series thatmz_persist_columnar_op_count{op="validation", result="invalid"}already carries on the same panel, andmz_txn_placeholder_schema_applyhas no counterpart. Neither name has ever appeared in this repository.kubelet_volume_stats_used_bytes, which resolves to five series in a single namespace in both production and staging us-east-1, none of them aclusterdvolume. Those panels are blank in both stacks.storage-overviewsign-off panel points at a canary environment list inMaterializeInc/cloudthat no longer exists. The live list is inMaterializeInc/release.Open finding for Persist
On staging us-east-1, compare-and-set mismatches per persist command stepped from 0.0061 to 0.0099 in the first bucket after the
v26.38.0-rc.1upgrade, held there for four days, then fell to 0.0085 withrc.3. The ratio is normalized by command volume, the step lands inside the upgrade bucket and is flat on both sides, and production canary shows no equivalent. No failure counter or latency moved, so it is recorded as something to recheck next release rather than as a blocker. This is the one place dated numbers remain in full, because it is a claim about a specific moment rather than a reference value.Rechecked at the
v26.39.0-rc.3boundary. The elevation is permanent rather than transient: staging us-east-1 sits at 0.0089 and never returned to 0.0061, while the v26.39 boundary itself is flat in that fleet.Verification
Documentation only. Every claim in the references was measured against the live dashboards rather than estimated, and the skill was then run end to end against the
v26.39.0-rc.3boundary across production canary and staging in us-east-1 and eu-west-1. That run is what surfaced the persist recheck above and the two dashboard defects added to the list.🤖 Generated with Claude Code