Repository navigation
feat(relay): alert on relay cell table lock convoys - #22446
Conversation
Adds a log-based metric and alert for cell-inventory lock holds of at least 1,000 ms, and a Cloud SQL log metric and alert for relay-only lock timeout cancels at 20 or more per minute. NOWAIT refusals are excluded: background sweeps produce about 160 per minute even with rehome paused. Replayed over 2026-09-20 14:00 to 2026-09-22 15:00 UTC: the hold filter matches all 93 asia-east2 rehome holds plus 9 director holds, and every one of the 88 cancel burst minutes overlaps an asia-east2 hold. Claude-Session: ced32ebb-7155-4413-adad-1eccd14c2010
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues. One doc-accuracy nit inline.
Reviewed changes
- Long-hold log metric + alert — new
google_logging_metric.relay_long_cell_inventory_holdthresholds the existingcellInventoryHoldMsMaxruntime field at>=1000in the log filter, with a newgoogle_monitoring_alert_policy.relay_long_cell_inventory_holdsplittingcloud_run_revision/gce_instanceconditions and grouping bycell_id. - Lock-timeout counter + burst alert —
local.relay_incident_metrics.cloud_sql_lock_timeoutsreads Postgres cancels fordb=orca_relay,only, andgoogle_monitoring_alert_policy.relay_lock_timeout_burstfires at ≥20/min viaALIGN_SUM/REDUCE_SUM. - Tests, fixture, docs — new
relay-lock-contention-alerts.test.mjspins the wiring,families.jsongains the three new families, andrelay-incident-monitor.mddocuments the response.
Verified the emitted metric type resolves to the same name the burst policy hardcodes (orca_relay_${each.key} → orca_relay_cloud_sql_lock_timeouts), that RelayProcessCounts spreads CellInventoryHoldCounts into the orca_relay_runtime_metrics payload, and that the new test plus terraform-root-partition.test.mjs both pass locally. The percentile + REDUCE_MAX + threshold-0 hold condition matches the existing relay_gce_connection_headroom pattern, which is the only valid scalar aligner for a DELTA DISTRIBUTION in condition_threshold.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| rehome commits (about 3.6 s each) and 9 director holds of 1.0-1.8 s, and every | ||
| one of the 88 burst minutes overlapped an asia-east2 hold. First response: if | ||
| the hold names an asia-east2 cell, pause regional rehoming through | ||
| `Operate Relay Production Rehome` with action `pause`. If rehoming is already |
There was a problem hiding this comment.
The Operate Relay Production Rehome dispatch input is named mode, not an action, so "action pause" sends on-call looking for a field that doesn't exist. The same phrasing is repeated in both new alert documentation.content strings.
Technical details
# Workflow input name mismatch
## Affected sites
- `cloud/docs/relay-incident-monitor.md:286` — "`Operate Relay Production Rehome` with action `pause`"
- `cloud/infra/terraform/relay-observability.tf:872` — hold-policy documentation: "workflow, action `pause`"
- `cloud/infra/terraform/relay-observability.tf:906` — burst-policy documentation: "workflow, action `pause`"
## Required outcome
- The three strings name the actual workflow input (`mode: pause`) so the first-response instruction is directly actionable.
## Suggested approach (optional)
- Replace "action `pause`" with "mode `pause`". Optionally add the required typed `confirmation: PAUSE_REGIONAL_REHOMING` that `cloud/docs/relay-workflows.md:508` documents, since the alert text is what on-call reads first.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds Cloud SQL and runtime metrics for relay lock-timeout cancellations and long cell-inventory holds. It adds alert policies for lock-timeout bursts and long holds, registers the Terraform resources in the fixture family list, and adds tests to the pretest script. The incident monitor documentation describes policy thresholds, replay results, and response procedures. Priority: ⬆️ High Merge Risk: ⚪ Minimal · up to This change adds alerts for relay lock convoys and long cell-inventory table holds. It does not change runtime relay behavior, so a mistake would at most cause missed or noisy alerts, not user-facing failures. It is ready to merge. Before on-call relies on the new pause guidance, update it to name the rehome workflow's 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Director holds of 1-2.5 s recur several times a day with rehoming paused, and pausing rehome does not stop them. The paging hold policy now selects role=cell samples only; a separate policy with no notification channel keeps director holds visible. The burst documentation no longer claims no burst happens while paused, and the runbook points a burst with no cell hold at the director policy. Replayed cell-only: 93 of 93 asia-east2 holds, 0 director holds over 2026-09-20 14:00 to 2026-09-22 15:00 UTC; 0 from then to 2026-09-23 07:30. Claude-Session: ced32ebb-7155-4413-adad-1eccd14c2010
ELI5
Sometimes one relay process locks the table of relay cells for several seconds. While it holds that lock, every host connect and reservation change in the whole fleet waits and then fails. Nothing paged on this. This PR adds two paging alerts: one fires when a cell holds that lock for a second or longer, and one fires when those failures pile up to 20 or more in a minute. Holds by the director are shown on a third policy that does not page.
What Changed
Before: the stalls only showed up when someone went digging in logs. On-call had no signal and no first step.
After: two new Cloud Monitoring alert policies page the existing relay channel, and a third one records director holds without paging.
cellonly.directorand has no notification channel. Director holds of 1 to 2.5 seconds happen a few times a day even with rehoming paused, and pausing rehome does not stop them. Paging on them would send on-call a runbook that says do nothing.Both paging policies tell the reader what to do. If an asia-east2 cell held the lock, pause regional rehoming with the
Operate Relay Production Rehomeworkflow, actionpause. If the burst alert fires with no cell hold in the same minute, look at the director policy instead of pausing rehome, and do not drain or restart cells. The runbook section is incloud/docs/relay-incident-monitor.md.The two metric filters, as rendered by the plan. The paging hold policy adds
metric.label."role"="cell"on top of the first.Why
The lock hold is the cause, and the cancels are the damage. An alert on each lets on-call see both the holder and the blast radius.
The cancel counter reads the Postgres log, not the relay's own lock-timeout counter. The relay counter misses most of the waiters: row-level waits and the reservation updates were 122 of 134 cancels in one recorded episode.
Fail-fast lock refusals are left out of the cancel count on purpose. The brief asked to consider them, but background sweeps produce them by design. They ran at 9,690 in one hour with rehoming paused, about 160 per minute. Counting them would keep a 20-per-minute alert permanently firing.
db=orca_relay,keeps the auth database on the same instance out. The push database is on a different instance and never matches.Linked Issue
No issue. This is step 0 of the relay lock convoy fix plan.
Visual Proof
N/A. This change adds monitoring configuration and no UI.
Testing
Replay, not waiting. Log-based metrics count only entries written after they exist. So both exact filter strings were replayed with read-only
gcloud logging readover the root-cause window, 2026-09-20 14:00 to 2026-09-22 15:00 UTC, and over the paused period since then.Paging lock hold filter, cell samples only, 49 h window:
Cancel filter, 49 h window: 6,104 cancels, which matches the investigation exactly. "Peak" below is the highest count in any rolling 60-second window.
The one burst while paused is at 2026-09-23 04:23 UTC. It had 25 cancels behind a 2,457 ms director hold. Rehoming was paused, and no rehome commits were logged in that period. The filter was not tuned to hide it. It is a real director-held stall, the residual the investigation left open. The burst documentation and the runbook now say to check the director policy when no cell hold appears in the same minute.
Plan census. The Terraform root that owns these resources also owns the 29 cell templates. All plans ran with
-refresh=false -lock=falseagainst production state; no apply was run.-targeton the five new addresses, showsPlan: 5 to add, 0 to change, 0 to destroy:google_logging_metric.relay_incident["cloud_sql_lock_timeouts"]google_logging_metric.relay_long_cell_inventory_holdgoogle_monitoring_alert_policy.relay_lock_timeout_burstgoogle_monitoring_alert_policy.relay_long_cell_inventory_holdgoogle_monitoring_alert_policy.relay_director_cell_inventory_hold52 to add, 66 to change, 35 to destroy. The same plan at base51c3434851is47 to add, 66 to change, 35 to destroy.Do not apply this root untargeted. Applying is a separate operator step. Use the five
-targetflags above and confirm the plan shows 5 to add and nothing else.Checks run:
terraform fmt -check -recursiveandterraform validatepass.A new test,
cloud/dev/scripts/relay-lock-contention-alerts.test.mjs, pins the pieces a rename on either side would silently break. It checks that the hold field exists in the relay emitter and that the cancel filter matches the relay database only. It checks that the cancel filter excludes fail-fast refusals and thelock_timeoutstatement echo. It also checks the thresholds, the bucket floor and the notification channel. It checks that the paging hold policy selects only thecellrole and that the director policy has no notification channel. Changing a paging condition to the director role makes the test fail. The test is added topretest.The existing
terraform-root-partitionfixture now includes the four new resource families. That test, the region-hint test and the connection-budget test all pass.I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
The two paging policies are expected to be noisy while regional rehoming runs with asia-east2 sources. That is intended, because each firing is a fleet-wide stall of several seconds. A cell that has stopped reporting emits no hold sample, so these alerts catch lock convoys, not outages. The existing liveness checks stay in place.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)