Skip to content

fix(cloud): compare the Asia topology budget gate against the measured 500-connection default - #22386

Merged
Jinwoo-H merged 1 commit into
mainfrom
relay-budget-gate-live-max-500
Sep 23, 2026
Merged

Jinwoo-H merged 1 commit into
mainfrom
relay-budget-gate-live-max-500

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7 $\color{#cf222e}{\Huge{\mathbf{−}}}$​4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3

ELI5

The workflow that adds a relay cell in Asia first checks that the shared database has room for the new cell's connections. That check had two copies of the database's connection limit that disagreed (400 in the workflow, 500 in the contract file). The workflow refused to run, even though there is plenty of room. This makes the two copies agree on the measured number.

What Changed

Before: dispatching Deploy Relay Asia Topology for production failed in the step "Require the checked Cloud SQL connection budget" with no output, because test 400 = 500 exits non-zero silently. Run 35815654836 (mode plan, cell production-gce-c30) was the first production run of this workflow and hit it.

After: the same step passes when the instance keeps its verified shape (db-custom-4-15360, POSTGRES_17, no max_connections flag) and the contract says 500.

Mechanism: the verified default moves out of the script body into VERIFIED_DEFAULT_MAX_CONNECTIONS: '500' beside the tier and database version it is verified for. The workflow test pins that source and forbids a bare numeric live_max= literal. One line in relay-incident-monitor.md that called 400 "verified" now says it was the assumption at the time and that the live instance measured 500.

Why

The 400 dates from the 2026-09-03 import and was never measured. #21163 measured SHOW max_connections on the live instance at 500 and corrected the contract, but not this workflow, and nothing exercised the gate in production until now.

Alternatives: making the workflow read the number from the contract would remove the drift but also remove the cross-check, since checked_max already comes from the contract and the comparison would become a tautology. Keeping two named records of one measurement keeps the gate meaningful. Editing the contract back to 400 would record a number the instance does not report.

No production resource changes. The budget itself is green (operating 419 of 490 usable).

Linked Issue

N/A — follow-up to #22375 and #21163.

Visual Proof

N/A — CI workflow and test only.

Testing

  • node --test cloud/dev/scripts/relay-asia-topology-workflow.test.mjs: 10 pass, 0 fail.

  • Read-only gcloud sql instances describe orca-cloud-auth-db: tier db-custom-4-15360, POSTGRES_17, flags max_wal_size, wal_compression only, so the no-flag branch is the one production takes.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

…d 500-connection default

The topology workflow's Cloud SQL gate carried a hard-coded 400 for the
instance's tier default while the consumer contract records the value
measured on the live instance (SHOW max_connections = 500, 2026-09-16,
#21163). The gate compares the two and the first production plan run
(35815654836) failed silently on that mismatch before Terraform ran.

The verified default now lives beside the tier and version it is verified
for, as VERIFIED_DEFAULT_MAX_CONNECTIONS, so the contract and the workflow
are two independent records of the same measurement and the gate keeps
its cross-check. The test pins the new source and forbids a bare literal.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Budget gate now reads the measured default — the no-flag production branch of Require the checked Cloud SQL connection budget uses live_max="${VERIFIED_DEFAULT_MAX_CONNECTIONS}" ('500') instead of a hardcoded 400, so test "${live_max}" = "${checked_max}" passes against the contract's maxConnections: 500 while the tier/version shape checks still fail closed.
  • Test pins the source and forbids the literal — the workflow test asserts the env var value and interpolation, and rejects a bare numeric live_max= literal; all three assertions fail against the pre-fix code.
  • Doc correction — the 250-backends entry no longer calls 400 "verified"; it was the assumption at the time, and the live instance measured 500 on 2026-09-16.

I traced the production path end to end: checked_max comes from relay-cloud-sql-connection-budget.mjs (contract 500), the live instance has no max_connections flag, and the tier (db-custom-4-15360) / version (POSTGRES_17) checks gate the default. The explicit-flag branch and the staging path are unchanged. The added assertions are not theatre — each fails with the old live_max=400. Ran node --test dev/scripts/relay-asia-topology-workflow.test.mjs from cloud/: 10 pass, 0 fail.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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 configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 76bb2fcb-b113-4026-a96a-524586f12335

📥 Commits

Reviewing files that changed from the base of the PR and between bf3f952 and 1ee8ec9.

📒 Files selected for processing (3)
  • .github/workflows/cloud-deploy-relay-asia-topology.yml
  • cloud/dev/scripts/relay-asia-topology-workflow.test.mjs
  • cloud/docs/relay-incident-monitor.md

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The workflow sets VERIFIED_DEFAULT_MAX_CONNECTIONS to 500 and uses it for the production live_max fallback. The test checks the configured value and assignment. The incident-monitor entry now states that the 400-connection ceiling was assumed and records a live measurement of 500 on 2026-09-16.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 1ee8e

The workflow uses the recorded 500-connection default for its guarded fallback. The alternate documented shape would increase available capacity rather than make the gate permissive, so no actionable merge risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Asia topology budget gate and its update to the measured 500-connection default.
Description check ✅ Passed The description explains the problem, change, rationale, and tests. It includes visual-proof and testing details. Although the Linked Issue section says N/A, it identifies follow-up issues #22375 and …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Jinwoo-H
Jinwoo-H merged commit 483fa0a into main Sep 23, 2026
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant