Repository navigation
Release Stage 12 database sessions and keep original failures visible - #789
Merged
Merged
Conversation
Stage 12 coordinators and workers held pooled database sessions open while they calculated or waited, so concurrent runs exhausted the session pooler's 15-client limit (EMAXCONNSESSION). The runtime engine now uses NullPool, closing each physical connection when its operation finishes. Coordinator and worker claims retry, with bounded jittered backoff (60 s budget), only when a database connection could not be opened. No statement reached the database in that case, so the retry cannot duplicate a write. Failed statements and validation errors still fail immediately. Coordinator and worker failure handlers now report the original error to telemetry before writing the failure record, and a failed write no longer replaces the original error. A child failure is reported once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 13, 2026
The claim retry decided whether to retry from the error alone: an OperationalError without a statement. SQLAlchemy raises a failed COMMIT, ROLLBACK or BEGIN the same way, and a COMMIT can fail after the database has applied it. Re-running the coordinator's claim then found its own running row and returned a deduplicated success without dispatching the simulations, leaving the report running. Each persistence store operation now opens its connection through one connection opener, which receives only the connect step. Statements, COMMIT and ROLLBACK run after the opener returns, so no opener can retry them. The coordinator and worker build their store with an opener that retries a refused connection with the same bounded, jittered backoff (1 s doubling to 15 s, 60 s budget) and WARNING telemetry naming the store operation. Every coordinator and worker store operation therefore retries a refused connection, not only the claims. Under NullPool each operation opens its own pooler session, so child-state creation, dispatch bookkeeping and the later updates could each fail a report on one refusal. The Cloud Run entrypoint keeps the default opener and does not retry, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cover the logger fallbacks used when no observability runtime is passed, and check that a failed child dispatch is reported once. Correct the failure-record docstring: the report and simulation handlers raise their stable RuntimeError, so the original cause is visible only in telemetry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
marked this pull request as ready for review
October 14, 2026 17:07
2 tasks done
anth-volk
added a commit
that referenced
this pull request
Oct 14, 2026
#789 landed on main after this stack branched. Group 8a, which this PR enables, flags its new code; each fix follows this PR's conventions: - ARG001/ARG002: open_connection_once and the telemetry-recording test runtime keep keyword parameters their protocols pass, with reasons; the test's random.uniform stand-in prefixes its unused positional bound. - PERF203: the connection retry loop catches each refused attempt by design, with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 14, 2026
The rebase onto #789 reports a closed child-wait window to telemetry before cancellation and report failure records are written, as every other Stage 12 failure is. This test makes those writes fail and checks that the timeout still reached telemetry first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 tasks done
anth-volk
added a commit
that referenced
this pull request
Oct 14, 2026
#789 landed on main after this stack branched. Group 8a, which this PR enables, flags its new code; each fix follows this PR's conventions: - ARG001/ARG002: open_connection_once and the telemetry-recording test runtime keep keyword parameters their protocols pass, with reasons; the test's random.uniform stand-in prefixes its unused positional bound. - PERF203: the connection retry loop catches each refused attempt by design, with a reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 tasks done
anth-volk
added a commit
that referenced
this pull request
Oct 14, 2026
The rebase onto #789 reports a closed child-wait window to telemetry before cancellation and report failure records are written, as every other Stage 12 failure is. This test makes those writes fail and checks that the timeout still reached telemetry first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 14, 2026
The rebase onto #789 reports a closed child-wait window to telemetry before cancellation and report failure records are written, as every other Stage 12 failure is. This test makes those writes fail and checks that the timeout still reached telemetry first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 14, 2026
The rebase onto #789 reports a closed child-wait window to telemetry before cancellation and report failure records are written, as every other Stage 12 failure is. This test makes those writes fail and checks that the timeout still reached telemetry first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk
added a commit
that referenced
this pull request
Oct 14, 2026
The rebase onto #789 reports a closed child-wait window to telemetry before cancellation and report failure records are written, as every other Stage 12 failure is. This test makes those writes fail and checks that the timeout still reached telemetry first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #788
Same-Repository Draft PR Check
PolicyEngine/policyengine-sim-api, not a personal fork.Summary
Stage 12 processes held pooled database sessions while they calculated or waited, so concurrent runs exhausted the session pooler's 15-client limit. From 6 to 13 October 2026, this caused 106 of the 153 production Stage 12 failures.
create_stage12_enginenow usesNullPool, so each physical connection closes when its operation finishes, including after a rollback.Stage12PersistenceStoreoperation now opens its connection in one place, through aConnectionOpener(persistence library). The opener receives only the connect step. The operation's statements, and the COMMIT or ROLLBACK that ends it, run after the opener returns, so they are never retried.OperationalErrorwith no statement. A COMMIT can fail after the database has applied it. An earlier version of this PR retried on that error, so a coordinator whose claim committed but whose reply was lost would re-run its claim, find its ownrunningrow, and return a deduplicated success without dispatching any simulation.open_connection_with_retry(stage12_runtime/database_retry.py). It backs off from 1 s, doubling up to 15 s with jitter, within a 60 s budget for each connection. Each retry is logged at WARNING with the store operation's name.NullPooleach of these opens its own pooler session, so each could otherwise fail a report on a single refused connection. Onmainthey reused the coordinator's pooled connection.stage12_runtime/failure_recording.py).RuntimeErrorwith no cause (from None), as onmain. The original error is therefore visible only in telemetry, not in the exception the Modal call returns.error_summarycontents. Recording a redacted cause is a separate change.Deployment
No environment variables, secrets or Modal function settings change.
STAGE12_DATABASE_URLis read exactly as before. This PR doesn't change any workflow, Dockerfile or Modal configuration.Two services use the changed persistence library. Both deploy through
simulation-deploy.reusable.yml, which runs fromsimulation-deploy.ymlon every push tomain(staging, then production) and from the manualstage12-deploy.yml.deploy_stage12_v2job): they getNullPooland the connection retry described above.deploy_entrypointjob):stage12_backendbuilds one store at startup and reads it on each Stage 12 status request:get_report, thenlist_simulations.main, its pool kept up to 5 sessions open per instance between requests (up to 10 under load). UnderNullPool, each status poll opens and closes 2 new pooler sessions, adding a connection handshake to each read, and the entrypoint holds no idle sessions between polls.main, rather than holding the request for up to 60 s.stage12_infrastructure(theconfigure_stage12_v2job's validation) opens a single connection per run, soNullPooldoesn't change it in practice.Not included
runningrows as failed. This needs a scheduled function; see the follow-ups in the issue.Merge notes
This PR edits the same failure handlers in
coordination.pyandsimulation.pyas #791 (record a redacted failure cause) and #793 (share the child wait deadline). The handlers here passerror_summarythrough a shared helper, so the redacted cause slots into that argument when rebasing. I suggest merging this PR first.Testing
libs/policyengine-stage12-persistence/tests/test_store.py):QueuePoolconfiguration.tests/test_stage12_database_retry.py:tests/test_stage12_runtime.py:tests/test_stage12_failure_recording.py: without an observability runtime, the original failure and an unwritable failure record are each logged at ERROR.uv run --frozen:libs/policyengine-stage12-persistence: 17 passed.projects/policyengine-simulation-executor: 750 passed, 23 skipped.projects/policyengine-simulation-entry: 158 passed.libs/policyengine-simulation-contractwas not run; it is unchanged.ruff format --checkandruff checkpass on the changed files.pyrightreports no errors forstage12_runtimeand the persistence library.🤖 Generated with Claude Code