Skip to content

Warm simulation machinery at startup; make /readiness-check honest#3772

Merged
anth-volk merged 3 commits into
masterfrom
fix/startup-warmup-readiness
Jul 23, 2026
Merged

Warm simulation machinery at startup; make /readiness-check honest#3772
anth-volk merged 3 commits into
masterfrom
fix/startup-warmup-readiness

Conversation

@anth-volk

Copy link
Copy Markdown
Collaborator

Fixes #3771. Supersedes #3770 (the CI-only readiness gate), which I'll close — this fixes the cause in the app.

Root cause (from staging boot logs)

Two startup phases, with /readiness-check firing between them:

  1. ~191s — import + tax-benefit-system build (blocks the worker from serving).
  2. ~121s — the first calculate on each worker compiles the per-simulation machinery (parameter tree, formula graph); ~0.5s thereafter.

/readiness-check returned 200 at the end of phase 1, so every consumer (startup probe, health_check.sh, smoke suite) saw 'ready' while the first real request still took ~2 min. That's the cold-start flake behind the staging test_live_calculate_us_1 timeout. And /liveness-check//readiness-check were identical no-ops.

Change

  • policyengine_api/warmup.pyrun_startup_warmup() runs a throwaway calculate per country (single-person household requesting household_net_income; default US, POLICYENGINE_API_WARMUP_COUNTRIES), called from asgi.py before the worker serves. Best-effort: failures are logged, never fatal.
  • policyengine_api/readiness.py — a readiness flag; asgi.py marks not-ready → warmup → ready. Defaults to ready so App Engine/tests/tooling are unaffected.
  • /readiness-check returns 503 until warm, 200 after. /liveness-check unchanged (we deliberately did not move liveness earlier).
  • Startup-probe window raised to the 480s max (initialDelaySeconds 240) since boot-to-ready now includes the warmup; the earlier low-initialDelay trade-off no longer applies (every boot already exceeds 240s). Doc + probe test updated.

Verification

Against policyengine_us directly (bypassing the DB-at-import):

warmup single-person calc:            28.5s
test payload (axes) AFTER warmup:      9.9s   (cold was ~121s)  -> ~12x faster, cross-period

Boot budget: Cloud Run phase-1 ~228s + warmup (~40-55s) ≈ 285s, inside the 480s window.

Unit tests: test_readiness.py, test_warmup.py (fake-country injection for the warmup loop + error-swallowing), updated probe-window assertions. 44 passed for the deploy-script + readiness + warmup suites.

Note: only Cloud Run (asgi.py) warms; App Engine keeps prior behaviour.

🤖 Generated with Claude Code

Building the tax-benefit systems at import does not compile the
per-simulation machinery, so the first calculate on a fresh worker took
~2 minutes (parameter-tree materialisation + formula graph). Meanwhile
/readiness-check returned 200 as soon as the import finished -- before that
cost was paid -- so the Cloud Run startup probe, health checks, and smoke
tests all saw a "ready" instance whose first real request still timed out.

- policyengine_api.warmup runs a throwaway calculate per country (default
  US) from asgi.py before the worker serves, paying the cost off the
  request path. Verified: a cold test-payload calculate drops ~121s -> ~10s.
- policyengine_api.readiness tracks a flag; /readiness-check returns 503
  until the warmup completes, so readiness reflects real readiness.
  /liveness-check is unchanged.
- Startup-probe window raised to the 480s maximum (initialDelay 240) to
  cover the added warmup time.

Fixes the cold-start flake behind the staging smoke-test timeouts at the
source, superseding the CI-only readiness gate in #3770.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.53846% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.63%. Comparing base (e287591) to head (2ffa0b1).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
policyengine_api/warmup.py 85.71% 2 Missing and 2 partials ⚠️
policyengine_api/api.py 33.33% 1 Missing and 1 partial ⚠️
policyengine_api/asgi.py 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3772      +/-   ##
==========================================
+ Coverage   80.03%   80.63%   +0.60%     
==========================================
  Files          70       72       +2     
  Lines        4342     4394      +52     
  Branches      809      818       +9     
==========================================
+ Hits         3475     3543      +68     
+ Misses        648      622      -26     
- Partials      219      229      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

anth-volk and others added 2 commits July 24, 2026 01:25
… boot budget

Follow-ups from reviewing this PR against the same checklist as #3770:

- openapi_spec.yaml: document the 503 /readiness-check now returns while warming
  (previously only 200 was documented).
- tests/unit/test_warmup.py: cheap structural guard that every warmup household's
  group-entity members reference a defined person (catches typos in the fast suite).
- tests/integration/test_warmup_household.py: slow guard (gated on
  RUN_WARMUP_INTEGRATION=1) that builds the real tax-benefit system and confirms
  each warmup household computes a finite result -- so a bad household can't
  silently no-op the warmup. Verified: passes in 191s for US.
- cloud-run-operations.md: state plainly that the warmup eats most of the headroom
  the 480s window buys (p90 ~371s -> ~420s), the far tail can still be killed and
  retried, and the lasting fix is prebuilding the system into the image.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cut the multi-line comment blocks in asgi.py and api.py's readiness_check down
to one-liners pointing at policyengine_api.warmup/readiness, and condense the
readiness/warmup docstrings so the rationale lives in one place.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@anth-volk
anth-volk merged commit d974a1d into master Jul 23, 2026
10 checks passed
@anth-volk
anth-volk deleted the fix/startup-warmup-readiness branch July 23, 2026 22:48
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.

/readiness-check reports ready before the first calculate is fast (cold-start warmup missing)

1 participant