diff --git a/.github/scripts/cloud_run_env.sh b/.github/scripts/cloud_run_env.sh index f5388559a..17c70e127 100755 --- a/.github/scripts/cloud_run_env.sh +++ b/.github/scripts/cloud_run_env.sh @@ -20,6 +20,10 @@ cloud_run_set_defaults() { CLOUD_RUN_CONCURRENCY="${CLOUD_RUN_CONCURRENCY:-6}" CLOUD_RUN_WEB_CONCURRENCY="${CLOUD_RUN_WEB_CONCURRENCY:-2}" CLOUD_RUN_PORT="${CLOUD_RUN_PORT:-8080}" + # Readiness gate: a TCP probe passes at port-bind, minutes before the app can + # serve. Window is 180s + 24x10s = 420s; both halves are capped at 240s. + # Sizing rationale in docs/migration/cloud-run-operations.md. + CLOUD_RUN_STARTUP_PROBE="${CLOUD_RUN_STARTUP_PROBE:-httpGet.path=/readiness-check,httpGet.port=${CLOUD_RUN_PORT},initialDelaySeconds=180,periodSeconds=10,failureThreshold=24,timeoutSeconds=5}" CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET="${CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET:-policyengine-api-prod-db-password:latest}" CLOUD_RUN_GITHUB_MICRODATA_TOKEN_SECRET="${CLOUD_RUN_GITHUB_MICRODATA_TOKEN_SECRET:-policyengine-api-prod-github-microdata-token:latest}" CLOUD_RUN_ANTHROPIC_API_KEY_SECRET="${CLOUD_RUN_ANTHROPIC_API_KEY_SECRET:-policyengine-api-prod-anthropic-api-key:latest}" @@ -50,6 +54,7 @@ cloud_run_set_defaults() { export CLOUD_RUN_CONCURRENCY export CLOUD_RUN_WEB_CONCURRENCY export CLOUD_RUN_PORT + export CLOUD_RUN_STARTUP_PROBE export CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET export CLOUD_RUN_GITHUB_MICRODATA_TOKEN_SECRET export CLOUD_RUN_ANTHROPIC_API_KEY_SECRET diff --git a/.github/scripts/deploy_cloud_run_candidate.sh b/.github/scripts/deploy_cloud_run_candidate.sh index 312973bea..c06373987 100755 --- a/.github/scripts/deploy_cloud_run_candidate.sh +++ b/.github/scripts/deploy_cloud_run_candidate.sh @@ -54,5 +54,6 @@ cloud_run_run gcloud run deploy "${CLOUD_RUN_SERVICE}" \ --min-instances "${CLOUD_RUN_MIN_INSTANCES}" \ --max-instances "${CLOUD_RUN_MAX_INSTANCES}" \ --concurrency "${CLOUD_RUN_CONCURRENCY}" \ + --startup-probe "${CLOUD_RUN_STARTUP_PROBE}" \ --set-env-vars "${set_env_vars}" \ --set-secrets "${set_secret_vars}" diff --git a/changelog.d/cloud-run-http-startup-probe.fixed.md b/changelog.d/cloud-run-http-startup-probe.fixed.md new file mode 100644 index 000000000..76006ccac --- /dev/null +++ b/changelog.d/cloud-run-http-startup-probe.fixed.md @@ -0,0 +1 @@ +Gate Cloud Run traffic on an HTTP readiness startup probe so instances that are still importing no longer receive requests. diff --git a/changelog.d/cloud-run-warm-capacity-2.changed.md b/changelog.d/cloud-run-warm-capacity-2.changed.md new file mode 100644 index 000000000..b539a4b50 --- /dev/null +++ b/changelog.d/cloud-run-warm-capacity-2.changed.md @@ -0,0 +1 @@ +Raise Cloud Run service-level warm capacity to 2 instances and correct the runbook flag. diff --git a/docs/migration/cloud-run-operations.md b/docs/migration/cloud-run-operations.md index 7f53c5a53..4dd8b0e97 100644 --- a/docs/migration/cloud-run-operations.md +++ b/docs/migration/cloud-run-operations.md @@ -71,19 +71,95 @@ Values measured and justified in gcloud inherits unspecified template fields (a mid-campaign CI deploy once shipped an inherited test concurrency), and `--concurrency default` resolves to the *platform* default (640), not the historical 80. +- **`--startup-probe httpGet.path=/readiness-check` — pinned on every deploy.** + Cloud Run's default is a *TCP* probe, which passes the instant gunicorn's master + binds the port, long before a worker finishes importing (the import runs + post-fork; `--preload` is deliberately unset). Cloud Run therefore treated a + booting instance as "started" and routed live traffic onto it, where requests + queued until the 300s timeout — the "no available instance" 500s and 504s seen on + scale-out bursts. Probing `/readiness-check` over HTTP makes Cloud Run withhold + traffic until the app can actually serve. + + **Sizing it is constrained by measured boot times.** Boot-to-ready across 48 + boots over 7 days (2026-07-14→21), measured from each instance's + `Starting gunicorn` to its last `Application startup complete`: + + | p50 | p90 | p95 | max | + |---|---|---|---| + | 201s | 371s | 417s | 503s | + + The tail is worst under CPU contention — i.e. during the very bursts that trigger + scale-out. (Stage 2's "~161s import" measures only the import step on an + uncontended instance; it is **not** boot-to-ready and must not be used to size + this.) + + Cloud Run caps `failureThreshold x periodSeconds` at 240s **and** + `initialDelaySeconds` at 240s, and shuts the container down past their sum. We run + `initialDelaySeconds 180 + 24 x 10s = 420s`. The threshold/period half is already + at its ceiling, so `initialDelaySeconds` is the only way to widen the window; it + is additive (no probe runs during it) but also delays availability for instances + that boot faster than it. Trade-off against the distribution above: + + | initialDelay | window | boots killed | boots delayed | median penalty | + |---|---|---|---|---| + | 120 | 360s | 12.5% | 8.3% | 31s | + | **180 (current)** | **420s** | **6.2%** | **22.9%** | **25s** | + | 240 (max) | 480s | 2.1% | 77.1% | 50s | + + A delayed instance loses tens of seconds; a **killed** one loses its whole boot + plus a retry (400s+) and fails the deploy if it happens in CI — so the asymmetry + favours a wider window. 240 was rejected because it holds 77% of instances to a + full 240s, slowing every scale-out. + + **Residual risk: ~6% of boots still exceed 420s and will be killed and retried**, + which can fail a deploy. That is accepted deliberately — the alternative (TCP) + served real users 5xx from instances that were never ready. Qualified 2026-07-22 + on a `--no-traffic` revision: Cloud Run stored the config exactly as specified and + the revision reached `Ready` on a 181.6s boot. + + The real fix is cutting boot time — **~82% of it is constructing the US + tax-benefit system** (`policyengine_api.country` builds all five countries at + import; US alone is ~90% of that, and `CountryTaxBenefitSystem()` is ~91% of the + per-country cost). At a 20s boot this would be `initialDelay 0` with a 240s window + covering every boot. Note that lazily deferring the build does **not** help: it + relocates the cost onto the first request, where readiness would lie and a user + would absorb it. - **Scaling pins live in `push.yml` per job**: production `max-instances 4` (peak real traffic ~11 RPS, mostly cached/light; ~1–2 concurrent uncached calculates per instance), staging `min 0 / max 1`. - **Warm capacity is a service-level setting made manually, once** — CI keeps revision-level `--min-instances 0`, because revision-level minimums keep a warm 16Gi - instance alive per accumulated `stage3-*` tag: + instance alive per accumulated `stage3-*` tag. + + **The flag names differ by one word and mean opposite things:** service-level + warm capacity is `--min` (or the `run.googleapis.com/minScale` annotation on the + *service* metadata); `--min-instances` is the *revision-level* setting and is the + per-tag cost bomb. `--min` requires a recent gcloud (it does not exist in 461). ```bash gcloud run services update policyengine-api \ --project policyengine-api --region us-central1 \ - --min-instances 1 + --min 2 ``` + On older gcloud, set it by exporting the service YAML and adding + `run.googleapis.com/minScale: "2"` to the **service** `metadata.annotations` + (never under `spec.template`), then `gcloud run services replace` — which + requires the Cloud Resource Manager API enabled on the project: + + ```bash + gcloud run services describe policyengine-api --project policyengine-api \ + --region us-central1 --format export > svc.yaml + # edit ONLY metadata.annotations."run.googleapis.com/minScale", then diff to + # confirm a single changed line before applying + gcloud run services replace svc.yaml --project policyengine-api --region us-central1 + ``` + + `replace` re-applies the whole spec and prints "Creating Revision", but Cloud Run + deduplicates an unchanged template — the serving revision and its tag are + preserved. Verify that: `status.latestReadyRevisionName` and the 100%-traffic + entry should be unchanged afterwards. + **When:** once, immediately after the Stage 3 PR merges — before evaluating the Stage 3 exit gates (the idle-readiness gate cannot pass without it) and before any public traffic. **Verify** it took, and re-verify during ramp incident response: @@ -91,10 +167,20 @@ Values measured and justified in ```bash gcloud run services describe policyengine-api \ --project policyengine-api --region us-central1 --format yaml | grep -i minscale - # expect a service-level minScale annotation of 1; a minScale under + # expect a service-level minScale annotation; a minScale under # spec.template (revision-level) would be the per-tag cost bomb — remove it. ``` + **Current value: 2.** Originally set to 1 on 2026-07-08 (via the YAML-replace + fallback, since the gcloud in use lacked `--min`), raised to 2 on 2026-07-22 by + the same route. One warm instance meant every + burst beyond a single instance's capacity landed on a cold boot; two warm + instances carry the burst during the ~161s a scaled-out instance takes to become + ready. Cost is ~$131/month per warm instance at 4 vCPU / 16Gi (idle CPU + $0.0000025/vCPU-s, idle memory $0.0000025/GiB-s — memory gets no idle discount and + is ~80% of it), so trimming the memory allocation is the lever if this needs to + get cheaper. + Rationale: the user-facing scale-from-zero wake was measured at 282.8s — 17s under the 300s request timeout — so a cold start must never sit on a public request path. diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index 32895da97..d05c058d4 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -376,6 +376,58 @@ def test_deploy_cloud_run_candidate_pins_runtime_shape(): assert "--min-instances 0 " in result.stdout +def test_deploy_cloud_run_candidate_pins_http_startup_probe(): + """The startup probe must poll readiness over HTTP, never TCP. + + gunicorn's master binds the port long before a worker finishes importing, + so a TCP probe reports "started" while the app cannot answer and Cloud Run + routes live traffic onto it. Probing /readiness-check makes Cloud Run + withhold traffic until the app can actually serve. + """ + result = _run_script( + ".github/scripts/deploy_cloud_run_candidate.sh", + _script_env( + **_required_runtime_env(), + CLOUD_RUN_IMAGE_URI="us-central1-docker.pkg.dev/project/repo/api:sha", + CLOUD_RUN_TAG="stage3-test", + ), + ) + + assert result.returncode == 0, result.stderr + assert "--startup-probe " in result.stdout + assert "httpGet.path=/readiness-check" in result.stdout + assert "httpGet.port=8080" in result.stdout + # A tcpSocket probe would reintroduce routing-before-ready. + assert "tcpSocket" not in result.stdout + + probe = next( + part + for part in result.stdout.split() + if "httpGet.path=/readiness-check" in part + ) + # The dry-run echoes the command shell-escaped, so commas arrive as "\,". + settings = dict( + item.split("=", 1) for item in probe.replace("\\", "").split(",") if "=" in item + ) + period = int(settings["periodSeconds"]) + threshold = int(settings["failureThreshold"]) + initial_delay = int(settings["initialDelaySeconds"]) + + # Cloud Run caps EACH half at 240s and shuts the container down past the + # total, so both halves and their sum are load-bearing. + assert threshold * period <= 240, "failureThreshold x periodSeconds > 240s cap" + assert initial_delay <= 240, "initialDelaySeconds > 240s cap" + # initialDelaySeconds is additive (no probe runs during it), so the real + # deadline is the sum. Keep it wide enough to cover the measured p90 boot + # (371s at time of writing) — see cloud-run-operations.md for the data. + assert initial_delay + threshold * period >= 340 + # ...but initialDelaySeconds also delays availability, since no probe can + # succeed before it elapses. Keep it under the measured p50 boot (201s) so + # it does not needlessly slow down every scale-out. + assert initial_delay <= 200 + assert int(settings["timeoutSeconds"]) <= period + + def test_push_workflow_pins_cloud_run_scaling_per_job(): workflow = _push_workflow() staging_deploy = _workflow_job_block(workflow, "deploy-cloud-run-staging")