From 8c47c50d60cc1951703657661ecd5a807a39cb5f Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:37:19 +0400 Subject: [PATCH 01/17] Let the report run store find and fail runs whose Modal call ended The store can now list the report runs and simulation runs that are running under a recorded Modal call and started before a given time, and mark one failed only while it is still running under the call it was listed with. A run that finished, or that a restarted coordinator moved to a new call since it was listed, is left as it is, so two sweeps at once fail each run once. Co-Authored-By: Claude Opus 5.5 --- .../__init__.py | 4 + .../report_run_store.py | 78 +++++++ .../tests/test_report_run_sweep_store.py | 197 ++++++++++++++++++ 3 files changed, 279 insertions(+) create mode 100644 libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/__init__.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/__init__.py index 669f4ceb5..aec46b6c8 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/__init__.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/__init__.py @@ -7,6 +7,8 @@ ) from policyengine_stage12_persistence.report_run_store import ( ReportRunStore, + RunKind, + RunningRun, RunStatus, RunWrite, ) @@ -26,8 +28,10 @@ "DistrictCodes", "ReportRunResult", "ReportRunStore", + "RunKind", "RunStatus", "RunWrite", + "RunningRun", "Stage12PersistenceStore", "Stage12RuntimeAccessError", "create_stage12_engine", diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py index da0eeaeb4..e5bfcd640 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py @@ -51,6 +51,25 @@ class RunStatus(StrEnum): ACTIVE_STATUSES = (RunStatus.PENDING.value, RunStatus.RUNNING.value) +class RunKind(StrEnum): + """Which run table a run is in.""" + + REPORT_RUN = "report_run" + SIMULATION_RUN = "simulation_run" + + +RUN_TABLES = {RunKind.REPORT_RUN: report_runs, RunKind.SIMULATION_RUN: simulation_runs} + + +@dataclass(frozen=True) +class RunningRun: + """A run recorded as running, and the Modal call recorded as running it.""" + + kind: RunKind + run_id: UUID + modal_call_id: str + + @dataclass(frozen=True) class RunWrite: """One conditional write's outcome: whether it changed the run, and its status.""" @@ -298,6 +317,65 @@ def fail_simulation_run( simulation_runs, simulation_run_id, statement, "fail_simulation_run" ) + def list_running_runs( + self, *, started_before: datetime, limit: int = 500 + ) -> tuple[RunningRun, ...]: + """Return up to ``limit`` runs of each kind running since before a time. + + Only runs with a recorded Modal call are returned, oldest first; a run + without a recorded start counts from its last update. + """ + running: list[RunningRun] = [] + with self._transaction("list_running_runs") as connection: + for kind, table in RUN_TABLES.items(): + started = func.coalesce(table.c.started_at, table.c.updated_at) + statement = ( + select(table.c.id, table.c.modal_call_id) + .where( + table.c.status == RunStatus.RUNNING.value, + table.c.modal_call_id.is_not(None), + started < started_before, + ) + .order_by(started, table.c.id) + .limit(limit) + ) + running.extend( + RunningRun( + kind=kind, run_id=row.id, modal_call_id=row.modal_call_id + ) + for row in connection.execute(statement) + ) + return tuple(running) + + def fail_run_of_ended_call( + self, + run: RunningRun, + *, + error_message: str, + failed_at: datetime, + ) -> RunWrite: + """Mark ``run`` failed, only while it is running under ``run.modal_call_id``. + + A run that finished, or that moved to another call since it was + listed, is left as it is, so concurrent sweeps change it once. + """ + table = RUN_TABLES[run.kind] + statement = ( + update(table) + .where( + table.c.id == run.run_id, + table.c.status == RunStatus.RUNNING.value, + table.c.modal_call_id == run.modal_call_id, + ) + .values( + status=RunStatus.FAILED.value, + error_message=error_message, + completed_at=failed_at, + updated_at=failed_at, + ) + ) + return self._write(table, run.run_id, statement, "fail_run_of_ended_call") + def _write( self, table: Table, diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py new file mode 100644 index 000000000..c00929e87 --- /dev/null +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -0,0 +1,197 @@ +"""The stuck-run sweep's reads and writes, as the Modal worker role. + +These run against a disposable Postgres that the Public API's v2 migrations +created (see ``report_run_database``). +""" + +from __future__ import annotations + +from concurrent.futures import ThreadPoolExecutor +from datetime import timedelta +from threading import Barrier + +import pytest +from policyengine_stage12_persistence import ( + ReportRunStore, + RunKind, + RunningRun, + RunStatus, + RunWrite, +) +from report_run_database import NOW, SeededReportRun, worker_database_url + +CUTOFF = NOW + timedelta(minutes=30) +ENDED_MESSAGE = "modal_call_ended: the run's Modal call ended without recording it" + + +@pytest.fixture +def store(): + run_store = ReportRunStore(worker_database_url()) + try: + yield run_store + finally: + run_store.engine.dispose() + + +def _set_run(seeder, table_name, run_id, **values): + table = seeder.table(table_name) + with seeder.engine.begin() as connection: + connection.execute(table.update().where(table.c.id == run_id).values(**values)) + + +def _running(seeder, seeded_run: SeededReportRun): + """Mark the report run and baseline run running under calls started at NOW.""" + run_ids = seeded_run.run_ids + _set_run( + seeder, + "report_runs", + run_ids.report_run_id, + status="running", + modal_call_id="fc-coordinator", + started_at=NOW, + ) + _set_run( + seeder, + "simulation_runs", + run_ids.baseline_simulation_run_id, + status="running", + modal_call_id="fc-baseline", + started_at=NOW, + ) + return ( + RunningRun( + kind=RunKind.REPORT_RUN, + run_id=run_ids.report_run_id, + modal_call_id="fc-coordinator", + ), + RunningRun( + kind=RunKind.SIMULATION_RUN, + run_id=run_ids.baseline_simulation_run_id, + modal_call_id="fc-baseline", + ), + ) + + +def _listed_for(store, seeded_run): + ids = { + seeded_run.run_ids.report_run_id, + seeded_run.run_ids.baseline_simulation_run_id, + seeded_run.run_ids.reform_simulation_run_id, + } + return { + run + for run in store.list_running_runs(started_before=CUTOFF) + if run.run_id in ids + } + + +def test_the_sweep_lists_runs_running_under_a_call_since_before_the_cutoff( + store, seeder, seeded_run: SeededReportRun +) -> None: + report_run, baseline_run = _running(seeder, seeded_run) + # The reform run is running, but its call has no ID recorded yet. + _set_run( + seeder, + "simulation_runs", + seeded_run.run_ids.reform_simulation_run_id, + status="running", + started_at=NOW, + ) + + assert _listed_for(store, seeded_run) == {report_run, baseline_run} + assert _listed_for_cutoff(store, seeded_run, NOW) == set() + + +def _listed_for_cutoff(store, seeded_run, cutoff): + ids = { + seeded_run.run_ids.report_run_id, + seeded_run.run_ids.baseline_simulation_run_id, + } + return { + run + for run in store.list_running_runs(started_before=cutoff) + if run.run_id in ids + } + + +def test_the_sweep_fails_a_run_once_while_its_ended_call_still_holds_it( + store, seeder, seeded_run: SeededReportRun +) -> None: + report_run, baseline_run = _running(seeder, seeded_run) + + first = store.fail_run_of_ended_call( + report_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ) + again = store.fail_run_of_ended_call( + report_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ) + simulation = store.fail_run_of_ended_call( + baseline_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ) + + assert first == RunWrite(changed=True, status=RunStatus.FAILED) + assert again == RunWrite(changed=False, status=RunStatus.FAILED) + assert simulation == RunWrite(changed=True, status=RunStatus.FAILED) + row = seeder.row("report_runs", seeded_run.run_ids.report_run_id) + assert row["error_message"] == ENDED_MESSAGE + assert row["completed_at"] == CUTOFF + + +def test_the_sweep_leaves_a_run_that_moved_to_another_call_or_finished( + store, seeder, seeded_run: SeededReportRun +) -> None: + report_run, baseline_run = _running(seeder, seeded_run) + # After the sweep listed them, Modal restarted the baseline's worker + # under a new call, and the report run succeeded. + _set_run( + seeder, + "simulation_runs", + seeded_run.run_ids.baseline_simulation_run_id, + modal_call_id="fc-baseline-restarted", + ) + _set_run( + seeder, + "report_runs", + seeded_run.run_ids.report_run_id, + status="succeeded", + completed_at=CUTOFF, + ) + + moved = store.fail_run_of_ended_call( + baseline_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ) + finished = store.fail_run_of_ended_call( + report_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ) + + assert moved == RunWrite(changed=False, status=RunStatus.RUNNING) + assert finished == RunWrite(changed=False, status=RunStatus.SUCCEEDED) + + +def test_two_sweeps_at_once_fail_each_run_exactly_once( + seeder, seeded_run: SeededReportRun +) -> None: + runs = _running(seeder, seeded_run) + stores = [ReportRunStore(worker_database_url()) for _ in range(2)] + start_together = Barrier(len(stores)) + + def sweep(run_store): + start_together.wait() + return [ + run_store.fail_run_of_ended_call( + run, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ).changed + for run in runs + ] + + try: + with ThreadPoolExecutor(max_workers=len(stores)) as pool: + changes = list(pool.map(sweep, stores)) + finally: + for run_store in stores: + run_store.engine.dispose() + + assert [sum(column) for column in zip(*changes, strict=True)] == [1, 1] + assert seeder.row("report_runs", seeded_run.run_ids.report_run_id)["status"] == ( + "failed" + ) From e2628a5e418477eca61c6065230a69e444177f25 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:39:02 +0400 Subject: [PATCH 02/17] Add a sweep that fails runs left running after their Modal call ended The sweep lists the report runs and simulation runs that have been running under a recorded Modal call for at least ten minutes, asks Modal for each call's call graph once, and marks a run failed only when every input of its call has finished: succeeded, failed, terminated, timed out or failed to start. A call with a pending input is running; Modal reports a pre-empted call pending until it runs again under the same call ID. A call Modal cannot describe, or any error asking, leaves the run as it is. Co-Authored-By: Claude Opus 5.5 --- .../stage12_runtime/run_sweep.py | 144 +++++++++++++++ .../tests/test_stage12_run_sweep.py | 168 ++++++++++++++++++ 2 files changed, 312 insertions(+) create mode 100644 projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py create mode 100644 projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py new file mode 100644 index 000000000..c4b58d5b5 --- /dev/null +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py @@ -0,0 +1,144 @@ +"""The sweep that fails report runs and simulation runs whose Modal call ended. + +A coordinator or worker records the Modal call running each run. If that call +ends without recording the run's outcome, for example because its container +ran out of memory, the run stays running. The sweep asks Modal for the status +of each recorded call and marks the run failed only once the call has ended. +Modal restarts a pre-empted call under the same call ID, and reports it +pending meanwhile, so a pre-empted run is never failed. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from datetime import UTC, datetime, timedelta +from enum import StrEnum +from importlib import import_module +from typing import TYPE_CHECKING, Any, Protocol + +if TYPE_CHECKING: + from collections.abc import Callable + + from policyengine_stage12_persistence import RunningRun, RunWrite + +logger = logging.getLogger(__name__) + +# A run must have been running at least this long before the sweep checks +# its call, so it never races the dispatch that records the call. +DEFAULT_MINIMUM_AGE = timedelta(minutes=10) +ENDED_CALL_MESSAGE = ( + "modal_call_ended: The Modal call recorded for this run ended without " + "recording the run's outcome." +) +# Modal's input statuses (``modal.call_graph.InputStatus``) by name; only a +# pending input is still running or waiting to run again. +RUNNING_INPUT_STATUSES = frozenset({"PENDING"}) +ENDED_INPUT_STATUSES = frozenset( + {"SUCCESS", "FAILURE", "TERMINATED", "TIMEOUT", "INIT_FAILURE"} +) + + +class CallState(StrEnum): + """What Modal reports about a recorded call.""" + + RUNNING = "running" + ENDED = "ended" + UNKNOWN = "unknown" + + +class StuckRunStore(Protocol): + """The sweep's reads and writes; ``ReportRunStore`` implements them.""" + + def list_running_runs(self, *, started_before: datetime) -> tuple[RunningRun, ...]: + """Return the runs running under a recorded call since before a time.""" + ... + + def fail_run_of_ended_call( + self, run: RunningRun, *, error_message: str, failed_at: datetime + ) -> RunWrite: + """Mark ``run`` failed while it is still running under its listed call.""" + ... + + +@dataclass(frozen=True) +class SweepSummary: + """How many listed runs the sweep checked, failed, and left for each reason.""" + + checked: int + failed: int + running: int + unknown: int + + +def modal_call_state(modal_call_id: str, *, modal_module: Any = None) -> CallState: # noqa: ANN401 # the modal module, or a stand-in for it + """Return whether Modal reports the call as running or ended. + + A call whose inputs are all finished has ended; one with any pending input + is running. A call Modal cannot describe, or any error asking, is unknown, + so the sweep leaves its run alone. + """ + modal = modal_module if modal_module is not None else import_module("modal") + try: + inputs = modal.FunctionCall.from_id(modal_call_id).get_call_graph() + statuses = {input_info.status.name for input_info in inputs} + except Exception as error: # noqa: BLE001 # any failure to ask leaves the run as it is + logger.warning( + "Could not read the status of Modal call %s (%s)", + modal_call_id, + type(error).__name__, + ) + return CallState.UNKNOWN + if statuses & RUNNING_INPUT_STATUSES: + return CallState.RUNNING + if statuses and statuses <= ENDED_INPUT_STATUSES: + return CallState.ENDED + return CallState.UNKNOWN + + +def sweep_stuck_runs( + store: StuckRunStore, + *, + call_state: Callable[[str], CallState] = modal_call_state, + now: datetime | None = None, + minimum_age: timedelta = DEFAULT_MINIMUM_AGE, +) -> SweepSummary: + """Fail every listed run whose recorded Modal call has ended. + + Each call is asked about once. A run whose call is running, or whose state + is unknown, is left as it is; so is one that finished or moved to another + call since it was listed, which makes two sweeps at once safe. + """ + now = now or datetime.now(UTC) + runs = store.list_running_runs(started_before=now - minimum_age) + states: dict[str, CallState] = {} + failed = running = unknown = 0 + for run in runs: + if run.modal_call_id not in states: + states[run.modal_call_id] = _safe_call_state(call_state, run.modal_call_id) + state = states[run.modal_call_id] + if state is CallState.RUNNING: + running += 1 + elif state is CallState.UNKNOWN: + unknown += 1 + elif store.fail_run_of_ended_call( + run, error_message=ENDED_CALL_MESSAGE, failed_at=now + ).changed: + failed += 1 + return SweepSummary( + checked=len(runs), failed=failed, running=running, unknown=unknown + ) + + +def _safe_call_state( + call_state: Callable[[str], CallState], modal_call_id: str +) -> CallState: + try: + return call_state(modal_call_id) + except Exception as error: # noqa: BLE001 # a failed lookup must never fail the run + logger.warning( + "Could not read the status of Modal call %s (%s)", + modal_call_id, + type(error).__name__, + ) + return CallState.UNKNOWN diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py new file mode 100644 index 000000000..7a00dbc96 --- /dev/null +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py @@ -0,0 +1,168 @@ +"""The sweep that fails runs left running after their Modal call ended.""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from enum import Enum +from types import SimpleNamespace +from uuid import uuid4 + +import pytest +from policyengine_simulation_executor.stage12_runtime.run_sweep import ( + CallState, + SweepSummary, + modal_call_state, + sweep_stuck_runs, +) +from policyengine_stage12_persistence import RunKind, RunningRun, RunStatus, RunWrite + +NOW = datetime(2026, 11, 4, 12, tzinfo=UTC) + + +class FakeInputStatus(Enum): + """Stands in for ``modal.call_graph.InputStatus``; the sweep reads names.""" + + PENDING = 0 + SUCCESS = 1 + FAILURE = 2 + TERMINATED = 3 + TIMEOUT = 4 + INIT_FAILURE = 5 + + +class FakeSweepStore: + def __init__(self, runs): + self.runs = list(runs) + self.cutoffs = [] + self.failed = [] + + def list_running_runs(self, *, started_before): + self.cutoffs.append(started_before) + return tuple(self.runs) + + def fail_run_of_ended_call(self, run, *, error_message, failed_at): + self.failed.append((run, error_message, failed_at)) + return RunWrite(changed=True, status=RunStatus.FAILED) + + +def _run(call_id, kind=RunKind.SIMULATION_RUN): + return RunningRun(kind=kind, run_id=uuid4(), modal_call_id=call_id) + + +def test_only_runs_whose_call_ended_are_failed() -> None: + ended = _run("fc-ended") + alive = _run("fc-alive") + unknown = _run("fc-unknown") + store = FakeSweepStore([ended, alive, unknown]) + states = { + "fc-ended": CallState.ENDED, + "fc-alive": CallState.RUNNING, + "fc-unknown": CallState.UNKNOWN, + } + + summary = sweep_stuck_runs(store, call_state=states.__getitem__, now=NOW) + + assert [run for run, _, _ in store.failed] == [ended] + (_, message, failed_at) = store.failed[0] + assert message.startswith("modal_call_ended: ") + assert failed_at == NOW + assert summary == SweepSummary(checked=3, failed=1, running=1, unknown=1) + + +def test_the_sweep_lists_runs_older_than_its_minimum_age() -> None: + store = FakeSweepStore([]) + + sweep_stuck_runs( + store, + call_state=lambda _: CallState.ENDED, + now=NOW, + minimum_age=timedelta(minutes=15), + ) + + assert store.cutoffs == [NOW - timedelta(minutes=15)] + + +def test_a_call_running_several_runs_is_asked_once() -> None: + report_run = _run("fc-coordinator", RunKind.REPORT_RUN) + other = _run("fc-coordinator", RunKind.REPORT_RUN) + asked = [] + + def call_state(call_id): + asked.append(call_id) + return CallState.ENDED + + sweep_stuck_runs( + FakeSweepStore([report_run, other]), call_state=call_state, now=NOW + ) + + assert asked == ["fc-coordinator"] + + +def test_a_failed_status_lookup_never_fails_the_run() -> None: + run = _run("fc-error") + store = FakeSweepStore([run]) + + def call_state(_call_id): + msg = "Modal is unreachable" + raise ConnectionError(msg) + + summary = sweep_stuck_runs(store, call_state=call_state, now=NOW) + + assert store.failed == [] + assert summary == SweepSummary(checked=1, failed=0, running=0, unknown=1) + + +def test_a_run_another_sweep_failed_first_is_not_counted_again() -> None: + run = _run("fc-ended") + + class AlreadyFailedStore(FakeSweepStore): + def fail_run_of_ended_call(self, run, *, error_message, failed_at): + super().fail_run_of_ended_call( + run, error_message=error_message, failed_at=failed_at + ) + return RunWrite(changed=False, status=RunStatus.FAILED) + + summary = sweep_stuck_runs( + AlreadyFailedStore([run]), call_state=lambda _: CallState.ENDED, now=NOW + ) + + assert summary.failed == 0 + + +def _fake_modal(statuses=None, error=None): + def from_id(call_id): + if error is not None: + raise error + assert call_id == "fc-1" + return SimpleNamespace( + get_call_graph=lambda: [ + SimpleNamespace(status=status) for status in (statuses or []) + ] + ) + + return SimpleNamespace(FunctionCall=SimpleNamespace(from_id=from_id)) + + +@pytest.mark.parametrize( + ("statuses", "expected"), + [ + ([FakeInputStatus.PENDING], CallState.RUNNING), + ([FakeInputStatus.SUCCESS], CallState.ENDED), + ([FakeInputStatus.FAILURE], CallState.ENDED), + ([FakeInputStatus.TERMINATED], CallState.ENDED), + ([FakeInputStatus.TIMEOUT], CallState.ENDED), + ([FakeInputStatus.INIT_FAILURE], CallState.ENDED), + ([FakeInputStatus.FAILURE, FakeInputStatus.PENDING], CallState.RUNNING), + ([], CallState.UNKNOWN), + ], +) +def test_modal_call_state_reads_the_call_graph(statuses, expected) -> None: + assert modal_call_state("fc-1", modal_module=_fake_modal(statuses)) is expected + + +def test_modal_call_state_is_unknown_when_modal_cannot_answer() -> None: + state = modal_call_state( + "fc-1", modal_module=_fake_modal(error=ConnectionError("unreachable")) + ) + + assert state is CallState.UNKNOWN From 7aa6ac784337cc35257dbe72e746419f9714174b Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:40:27 +0400 Subject: [PATCH 03/17] Run the stuck-run sweep every ten minutes as its own Modal app The new Modal application policyengine-simulation-run-sweep runs the sweep on a cron schedule every ten minutes, one container at a time, with the Modal worker role's database secret and the locked packages of the v2 images, without the bundle's data. It is not versioned with the .py bundle, so one copy runs per Modal environment; an operator deploys it. A sweep that overlaps another is still safe. Co-Authored-By: Claude Opus 5.5 --- .../src/modal/run_sweep_app.py | 78 +++++++++++++++++++ .../tests/test_stage12_run_sweep_app.py | 76 ++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 projects/policyengine-simulation-executor/src/modal/run_sweep_app.py create mode 100644 projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py diff --git a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py new file mode 100644 index 000000000..9f652e031 --- /dev/null +++ b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py @@ -0,0 +1,78 @@ +"""A scheduled Modal function that fails runs left running after their call ended. + +Every ten minutes it lists the Public API's report runs and simulation runs +that have been running under a recorded Modal call for at least ten minutes, +asks Modal for each call's status, and marks a run failed once its call has +ended. It connects as the Modal worker role, from the same Modal secret the +v2 coordinator and workers use. This application is not versioned with the +.py bundle: one copy runs per Modal environment. + +Deploy it, once per Modal environment, from this project's directory: + + uv run --frozen modal deploy --env=staging src/modal/run_sweep_app.py + uv run --frozen modal deploy --env=main src/modal/run_sweep_app.py +""" + +from __future__ import annotations + +from dataclasses import asdict +from pathlib import Path + +from policyengine_simulation_executor.stage12_runtime.report_run_access import ( + MODAL_WORKER_DATABASE_SECRET_NAME, + MODAL_WORKER_DATABASE_URL_ENV, +) +from src.modal.ca_certificates import GRPC_CA_BUNDLE_ENV, OPERATING_SYSTEM_CA_PACKAGE + +import modal + +APP_NAME = "policyengine-simulation-run-sweep" +_UV_PROJECT_DIR = str(Path(__file__).resolve().parents[2]) if modal.is_local() else "." + +app = modal.App(APP_NAME) +modal_worker_database_secret = modal.Secret.from_name( + MODAL_WORKER_DATABASE_SECRET_NAME, + required_keys=[MODAL_WORKER_DATABASE_URL_ENV], +) +sweep_image = ( + modal.Image.debian_slim(python_version="3.13") + # The same CA store and locked packages as the v2 images, without the + # bundle's data, which the sweep never reads. + .apt_install(OPERATING_SYSTEM_CA_PACKAGE) + .env(GRPC_CA_BUNDLE_ENV) + .uv_sync( + uv_project_dir=_UV_PROJECT_DIR, + frozen=True, + extra_options="--only-group modal-simulation-image", + # The repository's uv release, from .uv-version. + uv_version="0.13.6", + ) + .add_local_python_source( + "policyengine_simulation_executor", + "policyengine_simulation_observability", + "policyengine_simulation_contract", + "policyengine_stage12_persistence", + copy=True, + ) +) + + +@app.function( + image=sweep_image, + schedule=modal.Cron("*/10 * * * *"), + cpu=0.25, + memory=512, + timeout=600, + retries=0, + max_containers=1, + secrets=[modal_worker_database_secret], +) +def sweep_stuck_runs() -> dict: + """Fail the runs whose Modal call ended; return how many were checked and why.""" + from policyengine_simulation_executor.stage12_runtime import ( # noqa: PLC0415 # Modal function body: loaded in the container, not at deploy time + report_run_access, + run_sweep, + ) + + summary = run_sweep.sweep_stuck_runs(report_run_access.report_run_store()) + return asdict(summary) diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py new file mode 100644 index 000000000..99ca0bc39 --- /dev/null +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py @@ -0,0 +1,76 @@ +"""Declaration tests for the Modal app that runs the stuck-run sweep on a schedule.""" + +from __future__ import annotations + +import importlib +import sys + +from fixtures.fake_modal import install_fake_modal +from policyengine_simulation_executor.stage12_runtime import ( + report_run_access as report_run_access_module, +) +from policyengine_simulation_executor.stage12_runtime import run_sweep + + +def _load(monkeypatch): + install_fake_modal(monkeypatch) + sys.modules.pop("src.modal.run_sweep_app", None) + return importlib.import_module("src.modal.run_sweep_app") + + +def test_the_sweep_runs_every_ten_minutes_with_the_worker_database_secret( + monkeypatch, +) -> None: + module = _load(monkeypatch) + ((name, options),) = module.app.function_calls + + assert module.app.name == "policyengine-simulation-run-sweep" + assert name == "sweep_stuck_runs" + assert options["schedule"] == {"schedule": "*/10 * * * *"} + assert options["secrets"] == [ + { + "args": ("policyengine-modal-worker-database",), + "kwargs": {"required_keys": ["MODAL_WORKER_DATABASE_URL"]}, + } + ] + # One sweep at a time in this app; two at once would still be safe. + assert options["max_containers"] == 1 + assert options["retries"] == 0 + assert options["image"] is module.sweep_image + + +def test_the_sweep_image_installs_the_locked_group_and_the_runtime_sources( + monkeypatch, +) -> None: + module = _load(monkeypatch) + calls = {call[0]: call[1:] for call in module.sweep_image.calls} + + assert calls["uv_sync"][1]["frozen"] is True + assert calls["uv_sync"][1]["extra_options"] == "--only-group modal-simulation-image" + assert calls["add_local_python_source"][0] == ( + "policyengine_simulation_executor", + "policyengine_simulation_observability", + "policyengine_simulation_contract", + "policyengine_stage12_persistence", + ) + + +def test_the_scheduled_function_sweeps_with_the_report_run_store(monkeypatch) -> None: + module = _load(monkeypatch) + store = object() + swept = [] + monkeypatch.setattr(report_run_access_module, "report_run_store", lambda: store) + + def recording_sweep(received_store): + swept.append(received_store) + return run_sweep.SweepSummary(checked=3, failed=1, running=1, unknown=1) + + monkeypatch.setattr(run_sweep, "sweep_stuck_runs", recording_sweep) + + assert module.sweep_stuck_runs() == { + "checked": 3, + "failed": 1, + "running": 1, + "unknown": 1, + } + assert swept == [store] From 5d1b20173694d644efb83923dc06cea4a17dc3f3 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:40:27 +0400 Subject: [PATCH 04/17] Add a changelog fragment for the stuck-run sweep Co-Authored-By: Claude Opus 5.5 --- changelog.d/stuck-run-sweep.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/stuck-run-sweep.added.md diff --git a/changelog.d/stuck-run-sweep.added.md b/changelog.d/stuck-run-sweep.added.md new file mode 100644 index 000000000..286f915b7 --- /dev/null +++ b/changelog.d/stuck-run-sweep.added.md @@ -0,0 +1 @@ +A scheduled Modal function, in the new Modal application `policyengine-simulation-run-sweep`, marks the Public API's report runs and simulation runs failed when they are still running but the Modal call recorded for them has ended. It checks every ten minutes, never fails a run whose call is still running or whose state Modal cannot report, and is safe to run twice at once. An operator deploys it once per Modal environment. From fa35739cc0a4246411780b1c6d7cf82c2174c3df Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:03:58 +0400 Subject: [PATCH 05/17] Mount src.modal in the stuck-run sweep's image The sweep's app module imports src.modal at module level, and Modal imports that module again in the container, so the image now adds the src.modal sources as the project's other Modal apps do. Without them every scheduled run failed with No module named 'src'. Co-Authored-By: Claude Opus 5.5 --- .../policyengine-simulation-executor/src/modal/run_sweep_app.py | 2 ++ .../tests/test_stage12_run_sweep_app.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py index 9f652e031..c32655659 100644 --- a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py +++ b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py @@ -48,6 +48,8 @@ uv_version="0.13.6", ) .add_local_python_source( + # This module imports src.modal, so the container needs it too. + "src.modal", "policyengine_simulation_executor", "policyengine_simulation_observability", "policyengine_simulation_contract", diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py index 99ca0bc39..908da9459 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py @@ -47,7 +47,9 @@ def test_the_sweep_image_installs_the_locked_group_and_the_runtime_sources( assert calls["uv_sync"][1]["frozen"] is True assert calls["uv_sync"][1]["extra_options"] == "--only-group modal-simulation-image" + # The app module imports src.modal at module level, in the container too. assert calls["add_local_python_source"][0] == ( + "src.modal", "policyengine_simulation_executor", "policyengine_simulation_observability", "policyengine_simulation_contract", From 290039ecc604d33f369454164559047f774a1ea7 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:03:59 +0400 Subject: [PATCH 06/17] Judge a recorded call by its own inputs, and a child after its report Modal returns the whole call graph a call belongs to, and nests a call spawned inside another call's input under that input, so the sweep now reads only the inputs whose call ID is the recorded one, wherever they are, and treats a call missing from its graph as unknown. Before, a worker's simulation run was judged by its coordinator's status. A simulation run is now listed only while no report run that combines it is running: that report run's coordinator records its children's outcomes, and after a pre-emption it cancels them and starts them again under new calls, so a sweep in that gap could fail a run that would have finished. The sweep handles report runs first, so a dead coordinator's children are reached in the same sweep. Listing has no row limit, so runs that stay unknown can't hide newer ones. Co-Authored-By: Claude Opus 5.5 --- .../report_run_store.py | 62 +++++++------ .../tests/test_report_run_sweep_store.py | 31 ++++--- .../stage12_runtime/run_sweep.py | 85 ++++++++++++++---- .../tests/test_stage12_run_sweep.py | 86 +++++++++++++++++-- 4 files changed, 202 insertions(+), 62 deletions(-) diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py index e5bfcd640..4a8f25a39 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py @@ -14,7 +14,7 @@ from enum import StrEnum from typing import TYPE_CHECKING, Any -from sqlalchemy import func, insert, or_, select, update +from sqlalchemy import exists, func, insert, or_, select, update from sqlalchemy.dialects.postgresql import insert as postgresql_insert from policyengine_stage12_persistence.report_run_tables import ( @@ -318,34 +318,44 @@ def fail_simulation_run( ) def list_running_runs( - self, *, started_before: datetime, limit: int = 500 + self, kind: RunKind, *, started_before: datetime ) -> tuple[RunningRun, ...]: - """Return up to ``limit`` runs of each kind running since before a time. - - Only runs with a recorded Modal call are returned, oldest first; a run - without a recorded start counts from its last update. + """Return the runs of ``kind`` running under a recorded call since a time. + + Runs are oldest first; a run without a recorded start counts from its + last update. A simulation run is listed only while no running report + run names it as its baseline or reform: that report run's coordinator + records its children's outcomes, and stops and starts them again when + Modal restarts it, so their calls are its to judge until it has + finished. """ - running: list[RunningRun] = [] - with self._transaction("list_running_runs") as connection: - for kind, table in RUN_TABLES.items(): - started = func.coalesce(table.c.started_at, table.c.updated_at) - statement = ( - select(table.c.id, table.c.modal_call_id) - .where( - table.c.status == RunStatus.RUNNING.value, - table.c.modal_call_id.is_not(None), - started < started_before, - ) - .order_by(started, table.c.id) - .limit(limit) + table = RUN_TABLES[kind] + started = func.coalesce(table.c.started_at, table.c.updated_at) + conditions = [ + table.c.status == RunStatus.RUNNING.value, + table.c.modal_call_id.is_not(None), + started < started_before, + ] + if kind is RunKind.SIMULATION_RUN: + conditions.append( + ~exists().where( + or_( + report_runs.c.baseline_simulation_run_id == table.c.id, + report_runs.c.reform_simulation_run_id == table.c.id, + ), + report_runs.c.status == RunStatus.RUNNING.value, ) - running.extend( - RunningRun( - kind=kind, run_id=row.id, modal_call_id=row.modal_call_id - ) - for row in connection.execute(statement) - ) - return tuple(running) + ) + statement = ( + select(table.c.id, table.c.modal_call_id) + .where(*conditions) + .order_by(started, table.c.id) + ) + with self._transaction("list_running_runs") as connection: + return tuple( + RunningRun(kind=kind, run_id=row.id, modal_call_id=row.modal_call_id) + for row in connection.execute(statement) + ) def fail_run_of_ended_call( self, diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py index c00929e87..2b7dcfbf4 100644 --- a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -72,23 +72,36 @@ def _running(seeder, seeded_run: SeededReportRun): ) +def _listed(store, cutoff): + return ( + *store.list_running_runs(RunKind.REPORT_RUN, started_before=cutoff), + *store.list_running_runs(RunKind.SIMULATION_RUN, started_before=cutoff), + ) + + def _listed_for(store, seeded_run): ids = { seeded_run.run_ids.report_run_id, seeded_run.run_ids.baseline_simulation_run_id, seeded_run.run_ids.reform_simulation_run_id, } - return { - run - for run in store.list_running_runs(started_before=CUTOFF) - if run.run_id in ids - } + return {run for run in _listed(store, CUTOFF) if run.run_id in ids} def test_the_sweep_lists_runs_running_under_a_call_since_before_the_cutoff( store, seeder, seeded_run: SeededReportRun ) -> None: report_run, baseline_run = _running(seeder, seeded_run) + # Only once its report run is no longer running is a simulation run's + # call the sweep's to check. + assert _listed_for(store, seeded_run) == {report_run} + _set_run( + seeder, + "report_runs", + seeded_run.run_ids.report_run_id, + status="failed", + completed_at=CUTOFF, + ) # The reform run is running, but its call has no ID recorded yet. _set_run( seeder, @@ -98,7 +111,7 @@ def test_the_sweep_lists_runs_running_under_a_call_since_before_the_cutoff( started_at=NOW, ) - assert _listed_for(store, seeded_run) == {report_run, baseline_run} + assert _listed_for(store, seeded_run) == {baseline_run} assert _listed_for_cutoff(store, seeded_run, NOW) == set() @@ -107,11 +120,7 @@ def _listed_for_cutoff(store, seeded_run, cutoff): seeded_run.run_ids.report_run_id, seeded_run.run_ids.baseline_simulation_run_id, } - return { - run - for run in store.list_running_runs(started_before=cutoff) - if run.run_id in ids - } + return {run for run in _listed(store, cutoff) if run.run_id in ids} def test_the_sweep_fails_a_run_once_while_its_ended_call_still_holds_it( diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py index c4b58d5b5..cf1c0f951 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py @@ -4,8 +4,11 @@ ends without recording the run's outcome, for example because its container ran out of memory, the run stays running. The sweep asks Modal for the status of each recorded call and marks the run failed only once the call has ended. -Modal restarts a pre-empted call under the same call ID, and reports it -pending meanwhile, so a pre-empted run is never failed. + +Modal restarts a pre-empted call under the same call ID and reports it +pending meanwhile, so a pre-empted coordinator's report run is never failed. +Its children are cancelled and started again under new calls, so the sweep +judges a simulation run only once no report run combining it is running. """ from __future__ import annotations @@ -17,6 +20,8 @@ from importlib import import_module from typing import TYPE_CHECKING, Any, Protocol +from policyengine_stage12_persistence import RunKind + if TYPE_CHECKING: from collections.abc import Callable @@ -24,15 +29,17 @@ logger = logging.getLogger(__name__) -# A run must have been running at least this long before the sweep checks -# its call, so it never races the dispatch that records the call. +# A run must have been running at least this long, counted from its first +# claim, before the sweep checks its call. DEFAULT_MINIMUM_AGE = timedelta(minutes=10) ENDED_CALL_MESSAGE = ( "modal_call_ended: The Modal call recorded for this run ended without " "recording the run's outcome." ) -# Modal's input statuses (``modal.call_graph.InputStatus``) by name; only a -# pending input is still running or waiting to run again. +# Modal's input statuses (``modal.call_graph.InputStatus``) by name. A pending +# input is running or waiting to run again; Modal reports an input pre-empted +# and waiting to restart (internally INTERNAL_FAILURE) as pending. Any other +# name leaves the call's state unknown. RUNNING_INPUT_STATUSES = frozenset({"PENDING"}) ENDED_INPUT_STATUSES = frozenset( {"SUCCESS", "FAILURE", "TERMINATED", "TIMEOUT", "INIT_FAILURE"} @@ -50,8 +57,10 @@ class CallState(StrEnum): class StuckRunStore(Protocol): """The sweep's reads and writes; ``ReportRunStore`` implements them.""" - def list_running_runs(self, *, started_before: datetime) -> tuple[RunningRun, ...]: - """Return the runs running under a recorded call since before a time.""" + def list_running_runs( + self, kind: RunKind, *, started_before: datetime + ) -> tuple[RunningRun, ...]: + """Return the runs of ``kind`` running under a recorded call since a time.""" ... def fail_run_of_ended_call( @@ -74,14 +83,19 @@ class SweepSummary: def modal_call_state(modal_call_id: str, *, modal_module: Any = None) -> CallState: # noqa: ANN401 # the modal module, or a stand-in for it """Return whether Modal reports the call as running or ended. - A call whose inputs are all finished has ended; one with any pending input - is running. A call Modal cannot describe, or any error asking, is unknown, - so the sweep leaves its run alone. + Modal returns the whole call graph the call is part of, where a call + spawned inside another call's input is nested under it, so the call's own + inputs are found by their call ID anywhere in the graph. A call whose + inputs are all finished has ended; one with any pending input is running. + A call missing from its graph, or any error asking, is unknown, so the + sweep leaves its run alone. """ modal = modal_module if modal_module is not None else import_module("modal") try: - inputs = modal.FunctionCall.from_id(modal_call_id).get_call_graph() - statuses = {input_info.status.name for input_info in inputs} + graph = modal.FunctionCall.from_id(modal_call_id).get_call_graph() + statuses = { + input_info.status.name for input_info in _inputs_of(graph, modal_call_id) + } except Exception as error: # noqa: BLE001 # any failure to ask leaves the run as it is logger.warning( "Could not read the status of Modal call %s (%s)", @@ -96,6 +110,18 @@ def modal_call_state(modal_call_id: str, *, modal_module: Any = None) -> CallSta return CallState.UNKNOWN +def _inputs_of(graph: list[Any], modal_call_id: str) -> list[Any]: + """Return every input in ``graph`` that belongs to call ``modal_call_id``.""" + found: list[Any] = [] + pending = list(graph) + while pending: + input_info = pending.pop() + if input_info.function_call_id == modal_call_id: + found.append(input_info) + pending.extend(input_info.children) + return found + + def sweep_stuck_runs( store: StuckRunStore, *, @@ -105,13 +131,38 @@ def sweep_stuck_runs( ) -> SweepSummary: """Fail every listed run whose recorded Modal call has ended. - Each call is asked about once. A run whose call is running, or whose state - is unknown, is left as it is; so is one that finished or moved to another - call since it was listed, which makes two sweeps at once safe. + Report runs are swept first, so a dead coordinator's simulation runs are + listed in the same sweep. Each call is asked about once. A run whose call + is running, or whose state is unknown, is left as it is; so is one that + finished or moved to another call since it was listed, which makes two + sweeps at once safe. """ now = now or datetime.now(UTC) - runs = store.list_running_runs(started_before=now - minimum_age) states: dict[str, CallState] = {} + checked = failed = running = unknown = 0 + for kind in (RunKind.REPORT_RUN, RunKind.SIMULATION_RUN): + runs = store.list_running_runs(kind, started_before=now - minimum_age) + checked += len(runs) + outcome = _sweep_runs( + store, runs, states=states, call_state=call_state, now=now + ) + failed += outcome.failed + running += outcome.running + unknown += outcome.unknown + return SweepSummary( + checked=checked, failed=failed, running=running, unknown=unknown + ) + + +def _sweep_runs( + store: StuckRunStore, + runs: tuple[RunningRun, ...], + *, + states: dict[str, CallState], + call_state: Callable[[str], CallState], + now: datetime, +) -> SweepSummary: + """Fail each of ``runs`` whose call has ended, asking about each call once.""" failed = running = unknown = 0 for run in runs: if run.modal_call_id not in states: diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py index 7a00dbc96..8eae345c5 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py @@ -35,13 +35,16 @@ def __init__(self, runs): self.runs = list(runs) self.cutoffs = [] self.failed = [] + self.events = [] - def list_running_runs(self, *, started_before): + def list_running_runs(self, kind, *, started_before): self.cutoffs.append(started_before) - return tuple(self.runs) + self.events.append(f"list:{kind.value}") + return tuple(run for run in self.runs if run.kind is kind) def fail_run_of_ended_call(self, run, *, error_message, failed_at): self.failed.append((run, error_message, failed_at)) + self.events.append(f"fail:{run.kind.value}") return RunWrite(changed=True, status=RunStatus.FAILED) @@ -79,7 +82,25 @@ def test_the_sweep_lists_runs_older_than_its_minimum_age() -> None: minimum_age=timedelta(minutes=15), ) - assert store.cutoffs == [NOW - timedelta(minutes=15)] + assert store.cutoffs == [NOW - timedelta(minutes=15)] * 2 + + +def test_report_runs_are_swept_before_simulation_runs_are_listed() -> None: + # A simulation run is listed only once no report run combining it is + # running, so failing a dead coordinator's report run first lets the same + # sweep reach its children. + report_run = _run("fc-coordinator", RunKind.REPORT_RUN) + simulation_run = _run("fc-worker") + store = FakeSweepStore([simulation_run, report_run]) + + sweep_stuck_runs(store, call_state=lambda _: CallState.ENDED, now=NOW) + + assert store.events == [ + "list:report_run", + "fail:report_run", + "list:simulation_run", + "fail:simulation_run", + ] def test_a_call_running_several_runs_is_asked_once() -> None: @@ -129,16 +150,29 @@ def fail_run_of_ended_call(self, run, *, error_message, failed_at): assert summary.failed == 0 -def _fake_modal(statuses=None, error=None): +def _node(call_id, status, children=()): + return SimpleNamespace( + function_call_id=call_id, status=status, children=list(children) + ) + + +def _fake_modal(statuses=None, error=None, *, graph=None): + """Return a modal stand-in whose call graph holds call fc-1's inputs. + + By default fc-1 is a top-level call; ``graph`` gives the whole graph a + spawned call is part of instead. + """ + def from_id(call_id): if error is not None: raise error assert call_id == "fc-1" - return SimpleNamespace( - get_call_graph=lambda: [ - SimpleNamespace(status=status) for status in (statuses or []) - ] + roots = ( + graph + if graph is not None + else [_node("fc-1", status) for status in (statuses or [])] ) + return SimpleNamespace(get_call_graph=lambda: roots) return SimpleNamespace(FunctionCall=SimpleNamespace(from_id=from_id)) @@ -166,3 +200,39 @@ def test_modal_call_state_is_unknown_when_modal_cannot_answer() -> None: ) assert state is CallState.UNKNOWN + + +@pytest.mark.parametrize( + ("coordinator_status", "worker_status", "expected"), + [ + # A coordinator that ran out of memory left its worker running. + (FakeInputStatus.FAILURE, FakeInputStatus.PENDING, CallState.RUNNING), + (FakeInputStatus.PENDING, FakeInputStatus.TERMINATED, CallState.ENDED), + ], +) +def test_modal_call_state_reads_the_recorded_calls_own_inputs( + coordinator_status, worker_status, expected +) -> None: + # A worker is spawned inside its coordinator's input, so Modal nests its + # inputs under the coordinator's in the call graph. + graph = [ + _node( + "fc-coordinator", + coordinator_status, + children=[ + _node("fc-1", worker_status, children=[_node("fc-segment", "x")]), + _node("fc-other-worker", FakeInputStatus.SUCCESS), + ], + ) + ] + + assert modal_call_state("fc-1", modal_module=_fake_modal(graph=graph)) is expected + + +def test_a_call_missing_from_its_call_graph_is_unknown() -> None: + graph = [_node("fc-coordinator", FakeInputStatus.SUCCESS)] + + assert ( + modal_call_state("fc-1", modal_module=_fake_modal(graph=graph)) + is CallState.UNKNOWN + ) From f6b47187fca89bf09f0ac805d42b8d8e2f5490ba Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:28:19 +0400 Subject: [PATCH 07/17] Fail runs still running a day after their first claim Modal reports a call it won't retry after an internal failure as pending, stops describing old calls, and a run can be marked running with no call recorded; none of these could be decided, so such a run, and the simulation runs it names, stayed running for good. The sweep now also fails every run still running 24 hours after its first claim, whatever Modal reports and with or without a recorded call, a time no attempt of the v2 functions can approach. A run listed without a call is failed only while it still has none. The summary now also counts runs left because they finished or moved to another call, and runs failed for their age, and the listing of simulation runs is tested with pending, several and finished report runs naming them. Co-Authored-By: Claude Opus 5.5 --- .../report_run_store.py | 43 +++-- .../tests/test_report_run_sweep_store.py | 70 ++++++++ .../stage12_runtime/run_sweep.py | 157 +++++++++++------- .../tests/test_stage12_run_sweep.py | 59 ++++++- .../tests/test_stage12_run_sweep_app.py | 6 +- 5 files changed, 255 insertions(+), 80 deletions(-) diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py index 4a8f25a39..409bbffb1 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py @@ -63,11 +63,14 @@ class RunKind(StrEnum): @dataclass(frozen=True) class RunningRun: - """A run recorded as running, and the Modal call recorded as running it.""" + """A run recorded as running, and the Modal call recorded as running it. + + ``modal_call_id`` is None for a run marked running with no call recorded. + """ kind: RunKind run_id: UUID - modal_call_id: str + modal_call_id: str | None @dataclass(frozen=True) @@ -318,24 +321,30 @@ def fail_simulation_run( ) def list_running_runs( - self, kind: RunKind, *, started_before: datetime + self, + kind: RunKind, + *, + started_before: datetime, + include_runs_without_call: bool = False, ) -> tuple[RunningRun, ...]: - """Return the runs of ``kind`` running under a recorded call since a time. - - Runs are oldest first; a run without a recorded start counts from its - last update. A simulation run is listed only while no running report - run names it as its baseline or reform: that report run's coordinator - records its children's outcomes, and stops and starts them again when - Modal restarts it, so their calls are its to judge until it has - finished. + """Return the runs of ``kind`` running since before a time. + + Only runs with a recorded Modal call are returned, unless + ``include_runs_without_call``. Runs are oldest first; a run without a + recorded start counts from its last update. A simulation run is + listed only while no running report run names it as its baseline or + reform: that report run's coordinator records its children's + outcomes, and stops and starts them again when Modal restarts it, so + their calls are its to judge until it has finished. """ table = RUN_TABLES[kind] started = func.coalesce(table.c.started_at, table.c.updated_at) conditions = [ table.c.status == RunStatus.RUNNING.value, - table.c.modal_call_id.is_not(None), started < started_before, ] + if not include_runs_without_call: + conditions.append(table.c.modal_call_id.is_not(None)) if kind is RunKind.SIMULATION_RUN: conditions.append( ~exists().where( @@ -367,15 +376,21 @@ def fail_run_of_ended_call( """Mark ``run`` failed, only while it is running under ``run.modal_call_id``. A run that finished, or that moved to another call since it was - listed, is left as it is, so concurrent sweeps change it once. + listed, is left as it is, so concurrent sweeps change it once. A run + listed with no call is failed only while it still has none. """ table = RUN_TABLES[run.kind] + same_call = ( + table.c.modal_call_id.is_(None) + if run.modal_call_id is None + else table.c.modal_call_id == run.modal_call_id + ) statement = ( update(table) .where( table.c.id == run.run_id, table.c.status == RunStatus.RUNNING.value, - table.c.modal_call_id == run.modal_call_id, + same_call, ) .values( status=RunStatus.FAILED.value, diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py index 2b7dcfbf4..c67c6481d 100644 --- a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -9,6 +9,7 @@ from concurrent.futures import ThreadPoolExecutor from datetime import timedelta from threading import Barrier +from uuid import uuid4 import pytest from policyengine_stage12_persistence import ( @@ -19,6 +20,7 @@ RunWrite, ) from report_run_database import NOW, SeededReportRun, worker_database_url +from sqlalchemy import insert CUTOFF = NOW + timedelta(minutes=30) ENDED_MESSAGE = "modal_call_ended: the run's Modal call ended without recording it" @@ -204,3 +206,71 @@ def sweep(run_store): assert seeder.row("report_runs", seeded_run.run_ids.report_run_id)["status"] == ( "failed" ) + + +def _second_report_run(seeder, seeded_run: SeededReportRun, status): + """Insert another report run of the same report, naming the same two runs.""" + first = seeder.row("report_runs", seeded_run.run_ids.report_run_id) + second_id = uuid4() + with seeder.engine.begin() as connection: + connection.execute( + insert(seeder.table("report_runs")).values( + id=second_id, + report_id=first["report_id"], + baseline_simulation_run_id=first["baseline_simulation_run_id"], + reform_simulation_run_id=first["reform_simulation_run_id"], + status=status, + trigger="manual", + country_package_version=first["country_package_version"], + policyengine_version=first["policyengine_version"], + **({"completed_at": CUTOFF} if status == "succeeded" else {}), + ) + ) + return second_id + + +def _baseline_listed(store, seeded_run): + baseline_id = seeded_run.run_ids.baseline_simulation_run_id + return any( + run.run_id == baseline_id + for run in store.list_running_runs( + RunKind.SIMULATION_RUN, started_before=CUTOFF + ) + ) + + +def test_a_simulation_run_is_listed_unless_a_report_run_naming_it_is_running( + store, seeder, seeded_run: SeededReportRun +) -> None: + _running(seeder, seeded_run) + # The first report run is running, so its coordinator judges the run. + assert not _baseline_listed(store, seeded_run) + + _set_run(seeder, "report_runs", seeded_run.run_ids.report_run_id, status="pending") + assert _baseline_listed(store, seeded_run) + + second = _second_report_run(seeder, seeded_run, "running") + assert not _baseline_listed(store, seeded_run) + + _set_run(seeder, "report_runs", second, status="succeeded", completed_at=CUTOFF) + assert _baseline_listed(store, seeded_run) + + +def test_runs_past_the_maximum_age_are_listed_and_failed_without_a_call( + store, seeder, seeded_run: SeededReportRun +) -> None: + report_run_id = seeded_run.run_ids.report_run_id + _set_run(seeder, "report_runs", report_run_id, status="running", started_at=NOW) + + with_calls = store.list_running_runs(RunKind.REPORT_RUN, started_before=CUTOFF) + every_run = store.list_running_runs( + RunKind.REPORT_RUN, started_before=CUTOFF, include_runs_without_call=True + ) + + assert report_run_id not in {run.run_id for run in with_calls} + (unrecorded,) = [run for run in every_run if run.run_id == report_run_id] + assert unrecorded.modal_call_id is None + failed = store.fail_run_of_ended_call( + unrecorded, error_message="run_exceeded_maximum_age: too old", failed_at=CUTOFF + ) + assert failed == RunWrite(changed=True, status=RunStatus.FAILED) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py index cf1c0f951..7bd96be54 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py @@ -36,6 +36,17 @@ "modal_call_ended: The Modal call recorded for this run ended without " "recording the run's outcome." ) +# A run still running this long after its first claim is failed whatever +# Modal reports, or with no call recorded at all. No attempt of the v2 +# functions can run anywhere near this long (the coordinator's own time +# limit is 90 minutes), however often Modal pre-empts and restarts it; it +# stops Modal's pending status for an internal failure it won't retry, or +# a call Modal no longer describes, from keeping a run running for good. +DEFAULT_MAXIMUM_AGE = timedelta(hours=24) +EXCEEDED_AGE_MESSAGE = ( + "run_exceeded_maximum_age: The run was still running 24 hours after it " + "was first claimed, longer than any Modal call of it can run." +) # Modal's input statuses (``modal.call_graph.InputStatus``) by name. A pending # input is running or waiting to run again; Modal reports an input pre-empted # and waiting to restart (internally INTERNAL_FAILURE) as pending. Any other @@ -58,9 +69,13 @@ class StuckRunStore(Protocol): """The sweep's reads and writes; ``ReportRunStore`` implements them.""" def list_running_runs( - self, kind: RunKind, *, started_before: datetime + self, + kind: RunKind, + *, + started_before: datetime, + include_runs_without_call: bool = False, ) -> tuple[RunningRun, ...]: - """Return the runs of ``kind`` running under a recorded call since a time.""" + """Return the runs of ``kind`` running since before a time.""" ... def fail_run_of_ended_call( @@ -72,12 +87,20 @@ def fail_run_of_ended_call( @dataclass(frozen=True) class SweepSummary: - """How many listed runs the sweep checked, failed, and left for each reason.""" + """What the sweep did with the runs it listed. + + ``checked`` runs had their call asked about: ``failed`` because it ended, + left because it is ``running`` or its state is ``unknown``, or ``left`` + because the run finished or moved to another call after it was listed. + ``failed_for_age`` runs were failed for exceeding the maximum age. + """ checked: int failed: int running: int unknown: int + left: int = 0 + failed_for_age: int = 0 def modal_call_state(modal_call_id: str, *, modal_module: Any = None) -> CallState: # noqa: ANN401 # the modal module, or a stand-in for it @@ -87,8 +110,8 @@ def modal_call_state(modal_call_id: str, *, modal_module: Any = None) -> CallSta spawned inside another call's input is nested under it, so the call's own inputs are found by their call ID anywhere in the graph. A call whose inputs are all finished has ended; one with any pending input is running. - A call missing from its graph, or any error asking, is unknown, so the - sweep leaves its run alone. + A call missing from its graph, a status this code doesn't know, or any + error asking, is unknown, so the sweep leaves its run alone. """ modal = modal_module if modal_module is not None else import_module("modal") try: @@ -97,11 +120,7 @@ def modal_call_state(modal_call_id: str, *, modal_module: Any = None) -> CallSta input_info.status.name for input_info in _inputs_of(graph, modal_call_id) } except Exception as error: # noqa: BLE001 # any failure to ask leaves the run as it is - logger.warning( - "Could not read the status of Modal call %s (%s)", - modal_call_id, - type(error).__name__, - ) + _log_unreadable_call(modal_call_id, error) return CallState.UNKNOWN if statuses & RUNNING_INPUT_STATUSES: return CallState.RUNNING @@ -122,74 +141,90 @@ def _inputs_of(graph: list[Any], modal_call_id: str) -> list[Any]: return found -def sweep_stuck_runs( +@dataclass +class _Tally: + """Running counts of one sweep.""" + + checked: int = 0 + failed: int = 0 + running: int = 0 + unknown: int = 0 + left: int = 0 + failed_for_age: int = 0 + + +def sweep_stuck_runs( # the store, then keyword-only seams and limits store: StuckRunStore, *, call_state: Callable[[str], CallState] = modal_call_state, now: datetime | None = None, minimum_age: timedelta = DEFAULT_MINIMUM_AGE, + maximum_age: timedelta = DEFAULT_MAXIMUM_AGE, ) -> SweepSummary: """Fail every listed run whose recorded Modal call has ended. Report runs are swept first, so a dead coordinator's simulation runs are listed in the same sweep. Each call is asked about once. A run whose call - is running, or whose state is unknown, is left as it is; so is one that - finished or moved to another call since it was listed, which makes two - sweeps at once safe. + is running, or whose state is unknown, is left as it is, unless it has + been running longer than ``maximum_age``; so is one that finished or + moved to another call since it was listed, which makes two sweeps at + once safe. """ now = now or datetime.now(UTC) states: dict[str, CallState] = {} - checked = failed = running = unknown = 0 + tally = _Tally() for kind in (RunKind.REPORT_RUN, RunKind.SIMULATION_RUN): - runs = store.list_running_runs(kind, started_before=now - minimum_age) - checked += len(runs) - outcome = _sweep_runs( - store, runs, states=states, call_state=call_state, now=now - ) - failed += outcome.failed - running += outcome.running - unknown += outcome.unknown - return SweepSummary( - checked=checked, failed=failed, running=running, unknown=unknown - ) - - -def _sweep_runs( + for run in store.list_running_runs(kind, started_before=now - minimum_age): + _sweep_run( + store, run, states=states, call_state=call_state, now=now, tally=tally + ) + for run in store.list_running_runs( + kind, started_before=now - maximum_age, include_runs_without_call=True + ): + if store.fail_run_of_ended_call( + run, error_message=EXCEEDED_AGE_MESSAGE, failed_at=now + ).changed: + tally.failed_for_age += 1 + return SweepSummary(**vars(tally)) + + +def _sweep_run( # noqa: PLR0913 # one run, the store, and the sweep's shared state store: StuckRunStore, - runs: tuple[RunningRun, ...], + run: RunningRun, *, states: dict[str, CallState], call_state: Callable[[str], CallState], now: datetime, -) -> SweepSummary: - """Fail each of ``runs`` whose call has ended, asking about each call once.""" - failed = running = unknown = 0 - for run in runs: - if run.modal_call_id not in states: - states[run.modal_call_id] = _safe_call_state(call_state, run.modal_call_id) - state = states[run.modal_call_id] - if state is CallState.RUNNING: - running += 1 - elif state is CallState.UNKNOWN: - unknown += 1 - elif store.fail_run_of_ended_call( - run, error_message=ENDED_CALL_MESSAGE, failed_at=now - ).changed: - failed += 1 - return SweepSummary( - checked=len(runs), failed=failed, running=running, unknown=unknown + tally: _Tally, +) -> None: + """Fail ``run`` if its call has ended, asking about each call once.""" + tally.checked += 1 + modal_call_id = run.modal_call_id + if modal_call_id is None: + tally.unknown += 1 + return + if modal_call_id not in states: + try: + states[modal_call_id] = call_state(modal_call_id) + except Exception as error: # noqa: BLE001 # a failed lookup must never fail the run + _log_unreadable_call(modal_call_id, error) + states[modal_call_id] = CallState.UNKNOWN + state = states[modal_call_id] + if state is CallState.RUNNING: + tally.running += 1 + elif state is CallState.UNKNOWN: + tally.unknown += 1 + elif store.fail_run_of_ended_call( + run, error_message=ENDED_CALL_MESSAGE, failed_at=now + ).changed: + tally.failed += 1 + else: + tally.left += 1 + + +def _log_unreadable_call(modal_call_id: str, error: Exception) -> None: + logger.warning( + "Could not read the status of Modal call %s (%s)", + modal_call_id, + type(error).__name__, ) - - -def _safe_call_state( - call_state: Callable[[str], CallState], modal_call_id: str -) -> CallState: - try: - return call_state(modal_call_id) - except Exception as error: # noqa: BLE001 # a failed lookup must never fail the run - logger.warning( - "Could not read the status of Modal call %s (%s)", - modal_call_id, - type(error).__name__, - ) - return CallState.UNKNOWN diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py index 8eae345c5..2976cf815 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py @@ -31,13 +31,20 @@ class FakeInputStatus(Enum): class FakeSweepStore: - def __init__(self, runs): + def __init__(self, runs, *, expired=()): self.runs = list(runs) + # Runs older than the maximum age, listed with or without a call. + self.expired = list(expired) self.cutoffs = [] self.failed = [] self.events = [] - def list_running_runs(self, kind, *, started_before): + def list_running_runs( + self, kind, *, started_before, include_runs_without_call=False + ): + if include_runs_without_call: + self.events.append(f"list-expired:{kind.value}") + return tuple(run for run in self.expired if run.kind is kind) self.cutoffs.append(started_before) self.events.append(f"list:{kind.value}") return tuple(run for run in self.runs if run.kind is kind) @@ -69,7 +76,9 @@ def test_only_runs_whose_call_ended_are_failed() -> None: (_, message, failed_at) = store.failed[0] assert message.startswith("modal_call_ended: ") assert failed_at == NOW - assert summary == SweepSummary(checked=3, failed=1, running=1, unknown=1) + assert summary == SweepSummary( + checked=3, failed=1, running=1, unknown=1, left=0, failed_for_age=0 + ) def test_the_sweep_lists_runs_older_than_its_minimum_age() -> None: @@ -98,8 +107,10 @@ def test_report_runs_are_swept_before_simulation_runs_are_listed() -> None: assert store.events == [ "list:report_run", "fail:report_run", + "list-expired:report_run", "list:simulation_run", "fail:simulation_run", + "list-expired:simulation_run", ] @@ -130,7 +141,9 @@ def call_state(_call_id): summary = sweep_stuck_runs(store, call_state=call_state, now=NOW) assert store.failed == [] - assert summary == SweepSummary(checked=1, failed=0, running=0, unknown=1) + assert summary == SweepSummary( + checked=1, failed=0, running=0, unknown=1, left=0, failed_for_age=0 + ) def test_a_run_another_sweep_failed_first_is_not_counted_again() -> None: @@ -148,6 +161,8 @@ def fail_run_of_ended_call(self, run, *, error_message, failed_at): ) assert summary.failed == 0 + # It finished, or moved to another call, after it was listed. + assert summary.left == 1 def _node(call_id, status, children=()): @@ -236,3 +251,39 @@ def test_a_call_missing_from_its_call_graph_is_unknown() -> None: modal_call_state("fc-1", modal_module=_fake_modal(graph=graph)) is CallState.UNKNOWN ) + + +def test_a_run_past_the_maximum_age_is_failed_whatever_modal_reports() -> None: + # Modal reports a call it won't retry after an internal failure as + # pending, and no longer describes old calls; neither may keep a run + # running for good, and nor may a run with no recorded call. + reported_running = _run("fc-reported-running", RunKind.REPORT_RUN) + without_call = RunningRun( + kind=RunKind.REPORT_RUN, run_id=uuid4(), modal_call_id=None + ) + store = FakeSweepStore([reported_running], expired=[reported_running, without_call]) + + summary = sweep_stuck_runs( + store, + call_state=lambda _: CallState.RUNNING, + now=NOW, + maximum_age=timedelta(hours=24), + ) + + assert [run for run, _, _ in store.failed] == [reported_running, without_call] + assert all( + message.startswith("run_exceeded_maximum_age: ") + for _, message, _ in store.failed + ) + assert summary.failed_for_age == 2 + assert summary.running == 1 + + +def test_an_unrecognised_input_status_leaves_the_call_unknown() -> None: + unrecognised = SimpleNamespace(name="SOMETHING_NEW") + graph = [_node("fc-1", unrecognised)] + + assert ( + modal_call_state("fc-1", modal_module=_fake_modal(graph=graph)) + is CallState.UNKNOWN + ) diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py index 908da9459..f3d47ef13 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py @@ -65,7 +65,9 @@ def test_the_scheduled_function_sweeps_with_the_report_run_store(monkeypatch) -> def recording_sweep(received_store): swept.append(received_store) - return run_sweep.SweepSummary(checked=3, failed=1, running=1, unknown=1) + return run_sweep.SweepSummary( + checked=3, failed=1, running=1, unknown=1, left=0, failed_for_age=2 + ) monkeypatch.setattr(run_sweep, "sweep_stuck_runs", recording_sweep) @@ -74,5 +76,7 @@ def recording_sweep(received_store): "failed": 1, "running": 1, "unknown": 1, + "left": 0, + "failed_for_age": 2, } assert swept == [store] From f151d0cfe69304d5789b54f4abf8d7a93bc5e121 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:43:01 +0400 Subject: [PATCH 08/17] Check the age limit first and keep a combined child out of the sweep The sweep now fails runs past the maximum age before it asks Modal about any call, so lookups that stall can't use up the scheduled function's time before the check that exists for undecidable runs. The update that fails a simulation run repeats the condition that no running report run combines it, so a report run that starts combining it after the listing keeps it. The failing write is renamed fail_stuck_run, the age message no longer names a fixed age, and the changelog fragment and docstrings say that a run past the maximum age is failed whatever Modal reports. Co-Authored-By: Claude Opus 5.5 --- changelog.d/stuck-run-sweep.added.md | 2 +- .../report_run_store.py | 45 ++++++++++++------- .../tests/test_report_run_sweep_store.py | 37 ++++++++++++--- .../src/modal/run_sweep_app.py | 6 ++- .../stage12_runtime/run_sweep.py | 34 +++++++------- .../tests/test_stage12_run_sweep.py | 35 ++++++++++++--- 6 files changed, 113 insertions(+), 46 deletions(-) diff --git a/changelog.d/stuck-run-sweep.added.md b/changelog.d/stuck-run-sweep.added.md index 286f915b7..6f5418b29 100644 --- a/changelog.d/stuck-run-sweep.added.md +++ b/changelog.d/stuck-run-sweep.added.md @@ -1 +1 @@ -A scheduled Modal function, in the new Modal application `policyengine-simulation-run-sweep`, marks the Public API's report runs and simulation runs failed when they are still running but the Modal call recorded for them has ended. It checks every ten minutes, never fails a run whose call is still running or whose state Modal cannot report, and is safe to run twice at once. An operator deploys it once per Modal environment. +A scheduled Modal function, in the new Modal application `policyengine-simulation-run-sweep`, marks the Public API's report runs and simulation runs failed when they are still running but the Modal call recorded for them has ended. It checks every ten minutes and is safe to run twice at once. It leaves a run alone while Modal reports its call running, or can't report it, until the run has been running for 24 hours, longer than any of its calls can run; then it fails the run whatever Modal reports. An operator deploys it once per Modal environment. diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py index 409bbffb1..42ac135b8 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py @@ -33,7 +33,7 @@ from uuid import UUID from policyengine_simulation_contract.stage12_execution import ReportRunIds - from sqlalchemy import Engine, Table, Update + from sqlalchemy import ColumnElement, Engine, Table, Update from sqlalchemy.engine import Connection from policyengine_stage12_persistence.report_run_results import ReportRunResult @@ -346,15 +346,7 @@ def list_running_runs( if not include_runs_without_call: conditions.append(table.c.modal_call_id.is_not(None)) if kind is RunKind.SIMULATION_RUN: - conditions.append( - ~exists().where( - or_( - report_runs.c.baseline_simulation_run_id == table.c.id, - report_runs.c.reform_simulation_run_id == table.c.id, - ), - report_runs.c.status == RunStatus.RUNNING.value, - ) - ) + conditions.append(_no_running_report_run_names(table)) statement = ( select(table.c.id, table.c.modal_call_id) .where(*conditions) @@ -366,18 +358,20 @@ def list_running_runs( for row in connection.execute(statement) ) - def fail_run_of_ended_call( + def fail_stuck_run( self, run: RunningRun, *, error_message: str, failed_at: datetime, ) -> RunWrite: - """Mark ``run`` failed, only while it is running under ``run.modal_call_id``. + """Mark a stuck ``run`` failed, while it is as it was listed. - A run that finished, or that moved to another call since it was - listed, is left as it is, so concurrent sweeps change it once. A run - listed with no call is failed only while it still has none. + It must still be running under ``run.modal_call_id``, or with no call + if it was listed with none, and a simulation run must still have no + running report run naming it. A run that finished, moved to another + call, or that a running report run has named since it was listed, is + left as it is, so concurrent sweeps change it once. """ table = RUN_TABLES[run.kind] same_call = ( @@ -391,6 +385,11 @@ def fail_run_of_ended_call( table.c.id == run.run_id, table.c.status == RunStatus.RUNNING.value, same_call, + *( + (_no_running_report_run_names(table),) + if run.kind is RunKind.SIMULATION_RUN + else () + ), ) .values( status=RunStatus.FAILED.value, @@ -399,7 +398,7 @@ def fail_run_of_ended_call( updated_at=failed_at, ) ) - return self._write(table, run.run_id, statement, "fail_run_of_ended_call") + return self._write(table, run.run_id, statement, "fail_stuck_run") def _write( self, @@ -454,3 +453,17 @@ def _uniform_rows(rows: tuple[dict[str, Any], ...]) -> list[dict[str, Any]]: """Give every row the same columns, unset ones null, for one batched insert.""" columns = {name for row in rows for name in row} return [dict.fromkeys(columns) | row for row in rows] + + +def _no_running_report_run_names(table: Table) -> ColumnElement[bool]: + """Return the condition that no running report run names the run. + + A report run names its simulation runs as its baseline and its reform. + """ + return ~exists().where( + or_( + report_runs.c.baseline_simulation_run_id == table.c.id, + report_runs.c.reform_simulation_run_id == table.c.id, + ), + report_runs.c.status == RunStatus.RUNNING.value, + ) diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py index c67c6481d..e1e23b4df 100644 --- a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -130,13 +130,13 @@ def test_the_sweep_fails_a_run_once_while_its_ended_call_still_holds_it( ) -> None: report_run, baseline_run = _running(seeder, seeded_run) - first = store.fail_run_of_ended_call( + first = store.fail_stuck_run( report_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF ) - again = store.fail_run_of_ended_call( + again = store.fail_stuck_run( report_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF ) - simulation = store.fail_run_of_ended_call( + simulation = store.fail_stuck_run( baseline_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF ) @@ -168,10 +168,10 @@ def test_the_sweep_leaves_a_run_that_moved_to_another_call_or_finished( completed_at=CUTOFF, ) - moved = store.fail_run_of_ended_call( + moved = store.fail_stuck_run( baseline_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF ) - finished = store.fail_run_of_ended_call( + finished = store.fail_stuck_run( report_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF ) @@ -189,7 +189,7 @@ def test_two_sweeps_at_once_fail_each_run_exactly_once( def sweep(run_store): start_together.wait() return [ - run_store.fail_run_of_ended_call( + run_store.fail_stuck_run( run, error_message=ENDED_MESSAGE, failed_at=CUTOFF ).changed for run in runs @@ -270,7 +270,30 @@ def test_runs_past_the_maximum_age_are_listed_and_failed_without_a_call( assert report_run_id not in {run.run_id for run in with_calls} (unrecorded,) = [run for run in every_run if run.run_id == report_run_id] assert unrecorded.modal_call_id is None - failed = store.fail_run_of_ended_call( + failed = store.fail_stuck_run( unrecorded, error_message="run_exceeded_maximum_age: too old", failed_at=CUTOFF ) assert failed == RunWrite(changed=True, status=RunStatus.FAILED) + + +def test_a_simulation_run_a_report_run_is_now_running_is_not_failed( + store, seeder, seeded_run: SeededReportRun +) -> None: + _, baseline_run = _running(seeder, seeded_run) + _set_run( + seeder, + "report_runs", + seeded_run.run_ids.report_run_id, + status="failed", + completed_at=CUTOFF, + ) + assert _baseline_listed(store, seeded_run) + # After the listing, another report run naming it starts running; its + # coordinator now judges the simulation run. + _second_report_run(seeder, seeded_run, "running") + + write = store.fail_stuck_run( + baseline_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ) + + assert write == RunWrite(changed=False, status=RunStatus.RUNNING) diff --git a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py index c32655659..793714002 100644 --- a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py +++ b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py @@ -3,8 +3,10 @@ Every ten minutes it lists the Public API's report runs and simulation runs that have been running under a recorded Modal call for at least ten minutes, asks Modal for each call's status, and marks a run failed once its call has -ended. It connects as the Modal worker role, from the same Modal secret the -v2 coordinator and workers use. This application is not versioned with the +ended. It also fails any run still running 24 hours after it started, longer +than any Modal call of it can run, whatever Modal reports. It connects as +the Modal worker role, from the same Modal secret the v2 coordinator and +workers use. This application is not versioned with the .py bundle: one copy runs per Modal environment. Deploy it, once per Modal environment, from this project's directory: diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py index 7bd96be54..fc6351195 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py @@ -3,7 +3,10 @@ A coordinator or worker records the Modal call running each run. If that call ends without recording the run's outcome, for example because its container ran out of memory, the run stays running. The sweep asks Modal for the status -of each recorded call and marks the run failed only once the call has ended. +of each recorded call and marks the run failed once the call has ended. It +also fails a run still running long after it started, longer than any Modal +call of it can run, whatever Modal reports and with or without a recorded +call, so a run Modal can't decide doesn't stay running for good. Modal restarts a pre-empted call under the same call ID and reports it pending meanwhile, so a pre-empted coordinator's report run is never failed. @@ -44,8 +47,8 @@ # a call Modal no longer describes, from keeping a run running for good. DEFAULT_MAXIMUM_AGE = timedelta(hours=24) EXCEEDED_AGE_MESSAGE = ( - "run_exceeded_maximum_age: The run was still running 24 hours after it " - "was first claimed, longer than any Modal call of it can run." + "run_exceeded_maximum_age: The run was still running long after it " + "started, longer than any Modal call of it can run." ) # Modal's input statuses (``modal.call_graph.InputStatus``) by name. A pending # input is running or waiting to run again; Modal reports an input pre-empted @@ -78,7 +81,7 @@ def list_running_runs( """Return the runs of ``kind`` running since before a time.""" ... - def fail_run_of_ended_call( + def fail_stuck_run( self, run: RunningRun, *, error_message: str, failed_at: datetime ) -> RunWrite: """Mark ``run`` failed while it is still running under its listed call.""" @@ -164,27 +167,28 @@ def sweep_stuck_runs( # the store, then keyword-only seams and limits """Fail every listed run whose recorded Modal call has ended. Report runs are swept first, so a dead coordinator's simulation runs are - listed in the same sweep. Each call is asked about once. A run whose call - is running, or whose state is unknown, is left as it is, unless it has - been running longer than ``maximum_age``; so is one that finished or - moved to another call since it was listed, which makes two sweeps at - once safe. + listed in the same sweep. For each kind, runs older than ``maximum_age`` + are failed first, before any Modal lookup can use up the scheduled + function's time. Each call is then asked about once. A run whose call is + running, or whose state is unknown, is left as it is; so is one that + finished or moved to another call since it was listed, which makes two + sweeps at once safe. """ now = now or datetime.now(UTC) states: dict[str, CallState] = {} tally = _Tally() for kind in (RunKind.REPORT_RUN, RunKind.SIMULATION_RUN): - for run in store.list_running_runs(kind, started_before=now - minimum_age): - _sweep_run( - store, run, states=states, call_state=call_state, now=now, tally=tally - ) for run in store.list_running_runs( kind, started_before=now - maximum_age, include_runs_without_call=True ): - if store.fail_run_of_ended_call( + if store.fail_stuck_run( run, error_message=EXCEEDED_AGE_MESSAGE, failed_at=now ).changed: tally.failed_for_age += 1 + for run in store.list_running_runs(kind, started_before=now - minimum_age): + _sweep_run( + store, run, states=states, call_state=call_state, now=now, tally=tally + ) return SweepSummary(**vars(tally)) @@ -214,7 +218,7 @@ def _sweep_run( # noqa: PLR0913 # one run, the store, and the sweep's shared s tally.running += 1 elif state is CallState.UNKNOWN: tally.unknown += 1 - elif store.fail_run_of_ended_call( + elif store.fail_stuck_run( run, error_message=ENDED_CALL_MESSAGE, failed_at=now ).changed: tally.failed += 1 diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py index 2976cf815..1b4ddcc59 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py @@ -36,6 +36,7 @@ def __init__(self, runs, *, expired=()): # Runs older than the maximum age, listed with or without a call. self.expired = list(expired) self.cutoffs = [] + self.expired_cutoffs = [] self.failed = [] self.events = [] @@ -43,13 +44,14 @@ def list_running_runs( self, kind, *, started_before, include_runs_without_call=False ): if include_runs_without_call: + self.expired_cutoffs.append(started_before) self.events.append(f"list-expired:{kind.value}") return tuple(run for run in self.expired if run.kind is kind) self.cutoffs.append(started_before) self.events.append(f"list:{kind.value}") return tuple(run for run in self.runs if run.kind is kind) - def fail_run_of_ended_call(self, run, *, error_message, failed_at): + def fail_stuck_run(self, run, *, error_message, failed_at): self.failed.append((run, error_message, failed_at)) self.events.append(f"fail:{run.kind.value}") return RunWrite(changed=True, status=RunStatus.FAILED) @@ -105,12 +107,12 @@ def test_report_runs_are_swept_before_simulation_runs_are_listed() -> None: sweep_stuck_runs(store, call_state=lambda _: CallState.ENDED, now=NOW) assert store.events == [ + "list-expired:report_run", "list:report_run", "fail:report_run", - "list-expired:report_run", + "list-expired:simulation_run", "list:simulation_run", "fail:simulation_run", - "list-expired:simulation_run", ] @@ -150,8 +152,8 @@ def test_a_run_another_sweep_failed_first_is_not_counted_again() -> None: run = _run("fc-ended") class AlreadyFailedStore(FakeSweepStore): - def fail_run_of_ended_call(self, run, *, error_message, failed_at): - super().fail_run_of_ended_call( + def fail_stuck_run(self, run, *, error_message, failed_at): + super().fail_stuck_run( run, error_message=error_message, failed_at=failed_at ) return RunWrite(changed=False, status=RunStatus.FAILED) @@ -287,3 +289,26 @@ def test_an_unrecognised_input_status_leaves_the_call_unknown() -> None: modal_call_state("fc-1", modal_module=_fake_modal(graph=graph)) is CallState.UNKNOWN ) + + +def test_the_age_limit_is_checked_first_with_its_own_cutoff() -> None: + # Before any Modal lookup, so lookups that stall can't keep it from + # running within the scheduled function's time limit. + store = FakeSweepStore([_run("fc-1", RunKind.REPORT_RUN)]) + asked = [] + + def call_state(call_id): + asked.append((call_id, list(store.events))) + return CallState.RUNNING + + sweep_stuck_runs( + store, + call_state=call_state, + now=NOW, + minimum_age=timedelta(minutes=10), + maximum_age=timedelta(hours=24), + ) + + assert store.expired_cutoffs == [NOW - timedelta(hours=24)] * 2 + assert store.cutoffs == [NOW - timedelta(minutes=10)] * 2 + assert asked == [("fc-1", ["list-expired:report_run", "list:report_run"])] From 98d519359607d92423a20e432f93f50c7d61ead1 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:52:49 +0400 Subject: [PATCH 09/17] Fail both kinds' overage runs before the sweep asks Modal anything The sweep checked simulation runs' age only after every report run's Modal lookup, so lookups that stalled could still keep simulation runs past the maximum age from being failed. It now fails report runs, then simulation runs, past the maximum age before any lookup, then asks about each kind's calls, report runs first. A refused failure in the age check counts as left, and the docstrings say a write is refused also when a running report run started combining the simulation run. Co-Authored-By: Claude Opus 5.5 --- .../stage12_runtime/run_sweep.py | 32 ++++++++++++------- .../tests/test_stage12_run_sweep.py | 29 +++++++++++++---- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py index fc6351195..d5f386d7a 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py @@ -84,7 +84,12 @@ def list_running_runs( def fail_stuck_run( self, run: RunningRun, *, error_message: str, failed_at: datetime ) -> RunWrite: - """Mark ``run`` failed while it is still running under its listed call.""" + """Mark ``run`` failed while it is as it was listed. + + It must still be running under its listed call, or with no call if it + was listed with none; a simulation run must still have no running + report run combining it. + """ ... @@ -93,9 +98,11 @@ class SweepSummary: """What the sweep did with the runs it listed. ``checked`` runs had their call asked about: ``failed`` because it ended, - left because it is ``running`` or its state is ``unknown``, or ``left`` - because the run finished or moved to another call after it was listed. - ``failed_for_age`` runs were failed for exceeding the maximum age. + or left because it is ``running`` or its state is ``unknown``. + ``failed_for_age`` runs were failed for exceeding the maximum age. ``left`` + counts listed runs whose conditional failure changed nothing, because + they finished, moved to another call, or a running report run started + combining them after they were listed. """ checked: int @@ -166,13 +173,13 @@ def sweep_stuck_runs( # the store, then keyword-only seams and limits ) -> SweepSummary: """Fail every listed run whose recorded Modal call has ended. - Report runs are swept first, so a dead coordinator's simulation runs are - listed in the same sweep. For each kind, runs older than ``maximum_age`` - are failed first, before any Modal lookup can use up the scheduled - function's time. Each call is then asked about once. A run whose call is - running, or whose state is unknown, is left as it is; so is one that - finished or moved to another call since it was listed, which makes two - sweeps at once safe. + Runs older than ``maximum_age`` are failed first, report runs then + simulation runs, before any Modal lookup can use up the scheduled + function's time. Then each recorded call is asked about once, report + runs first, so a dead coordinator's simulation runs are listed in the + same sweep. A run whose call is running, or whose state is unknown, is + left as it is; so is one whose conditional failure changes nothing, which + makes two sweeps at once safe. """ now = now or datetime.now(UTC) states: dict[str, CallState] = {} @@ -185,6 +192,9 @@ def sweep_stuck_runs( # the store, then keyword-only seams and limits run, error_message=EXCEEDED_AGE_MESSAGE, failed_at=now ).changed: tally.failed_for_age += 1 + else: + tally.left += 1 + for kind in (RunKind.REPORT_RUN, RunKind.SIMULATION_RUN): for run in store.list_running_runs(kind, started_before=now - minimum_age): _sweep_run( store, run, states=states, call_state=call_state, now=now, tally=tally diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py index 1b4ddcc59..a574a187c 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py @@ -43,13 +43,17 @@ def __init__(self, runs, *, expired=()): def list_running_runs( self, kind, *, started_before, include_runs_without_call=False ): + # As the real store, a run this fake failed is no longer listed. + failed = {failed_run for failed_run, _, _ in self.failed} if include_runs_without_call: self.expired_cutoffs.append(started_before) self.events.append(f"list-expired:{kind.value}") - return tuple(run for run in self.expired if run.kind is kind) - self.cutoffs.append(started_before) - self.events.append(f"list:{kind.value}") - return tuple(run for run in self.runs if run.kind is kind) + listed = self.expired + else: + self.cutoffs.append(started_before) + self.events.append(f"list:{kind.value}") + listed = self.runs + return tuple(run for run in listed if run.kind is kind and run not in failed) def fail_stuck_run(self, run, *, error_message, failed_at): self.failed.append((run, error_message, failed_at)) @@ -108,9 +112,9 @@ def test_report_runs_are_swept_before_simulation_runs_are_listed() -> None: assert store.events == [ "list-expired:report_run", + "list-expired:simulation_run", "list:report_run", "fail:report_run", - "list-expired:simulation_run", "list:simulation_run", "fail:simulation_run", ] @@ -278,7 +282,8 @@ def test_a_run_past_the_maximum_age_is_failed_whatever_modal_reports() -> None: for _, message, _ in store.failed ) assert summary.failed_for_age == 2 - assert summary.running == 1 + # Failed for its age first, it is no longer listed for a lookup. + assert summary.running == 0 def test_an_unrecognised_input_status_leaves_the_call_unknown() -> None: @@ -311,4 +316,14 @@ def call_state(call_id): assert store.expired_cutoffs == [NOW - timedelta(hours=24)] * 2 assert store.cutoffs == [NOW - timedelta(minutes=10)] * 2 - assert asked == [("fc-1", ["list-expired:report_run", "list:report_run"])] + # Both kinds' age checks come before any lookup. + assert asked == [ + ( + "fc-1", + [ + "list-expired:report_run", + "list-expired:simulation_run", + "list:report_run", + ], + ) + ] From e07f35be53393d766612ab97806a908d7a43fbe4 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:48:40 +0400 Subject: [PATCH 10/17] List pending runs for the stuck-run sweep, aged from their creation The simulation service now records the Modal call it starts on a run while the run is still pending, and a run whose call was never started stays pending. The store therefore lists pending runs as well as running ones, with their status: a running run counts from its first claim and a pending one from its creation, so the 24-hour limit also covers pending runs. A pending report run now keeps the simulation runs it combines out of the listing, as a running one does: its coordinator, queued in Modal, will start their calls. Failing a listed run requires its listed status as well as its listed call, so a run that was claimed, or that the simulation service recorded a call on, after the listing is left as it is. Co-Authored-By: Claude Opus 5.5 --- .../__init__.py | 4 +- .../report_run_store.py | 79 +++++---- .../report_run_tables.py | 2 + .../tests/test_report_run_sweep_store.py | 167 +++++++++++++----- .../stage12_runtime/run_sweep.py | 30 ++-- .../tests/test_stage12_run_sweep.py | 26 +-- 6 files changed, 201 insertions(+), 107 deletions(-) diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/__init__.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/__init__.py index aec46b6c8..5f86c8e0b 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/__init__.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/__init__.py @@ -6,9 +6,9 @@ report_run_result, ) from policyengine_stage12_persistence.report_run_store import ( + ActiveRun, ReportRunStore, RunKind, - RunningRun, RunStatus, RunWrite, ) @@ -24,6 +24,7 @@ ) __all__ = [ + "ActiveRun", "ConnectionOpener", "DistrictCodes", "ReportRunResult", @@ -31,7 +32,6 @@ "RunKind", "RunStatus", "RunWrite", - "RunningRun", "Stage12PersistenceStore", "Stage12RuntimeAccessError", "create_stage12_engine", diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py index 42ac135b8..139cf83fb 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py @@ -62,14 +62,15 @@ class RunKind(StrEnum): @dataclass(frozen=True) -class RunningRun: - """A run recorded as running, and the Modal call recorded as running it. +class ActiveRun: + """A pending or running run, and the Modal call recorded on it. - ``modal_call_id`` is None for a run marked running with no call recorded. + ``modal_call_id`` is None for a run with no call recorded. """ kind: RunKind run_id: UUID + status: RunStatus modal_call_id: str | None @@ -320,58 +321,62 @@ def fail_simulation_run( simulation_runs, simulation_run_id, statement, "fail_simulation_run" ) - def list_running_runs( + def list_active_runs( self, kind: RunKind, *, - started_before: datetime, - include_runs_without_call: bool = False, - ) -> tuple[RunningRun, ...]: - """Return the runs of ``kind`` running since before a time. - - Only runs with a recorded Modal call are returned, unless - ``include_runs_without_call``. Runs are oldest first; a run without a - recorded start counts from its last update. A simulation run is - listed only while no running report run names it as its baseline or - reform: that report run's coordinator records its children's - outcomes, and stops and starts them again when Modal restarts it, so - their calls are its to judge until it has finished. + active_since_before: datetime, + ) -> tuple[ActiveRun, ...]: + """Return the pending and running runs of ``kind`` active since before a time. + + A run counts from its first claim, or from its creation while it is + pending. Runs are oldest first, with or without a recorded Modal + call. A simulation run is listed only while no pending or running + report run names it as its baseline or reform: that report run's + coordinator starts its children's calls, records their outcomes, and + stops and starts them again when Modal restarts it, so they are its + to judge until it has finished. """ table = RUN_TABLES[kind] - started = func.coalesce(table.c.started_at, table.c.updated_at) + active_since = func.coalesce(table.c.started_at, table.c.created_at) conditions = [ - table.c.status == RunStatus.RUNNING.value, - started < started_before, + table.c.status.in_(ACTIVE_STATUSES), + active_since < active_since_before, ] - if not include_runs_without_call: - conditions.append(table.c.modal_call_id.is_not(None)) if kind is RunKind.SIMULATION_RUN: - conditions.append(_no_running_report_run_names(table)) + conditions.append(_no_active_report_run_names(table)) statement = ( - select(table.c.id, table.c.modal_call_id) + select(table.c.id, table.c.status, table.c.modal_call_id) .where(*conditions) - .order_by(started, table.c.id) + .order_by(active_since, table.c.id) ) - with self._transaction("list_running_runs") as connection: + with self._transaction("list_active_runs") as connection: return tuple( - RunningRun(kind=kind, run_id=row.id, modal_call_id=row.modal_call_id) + ActiveRun( + kind=kind, + run_id=row.id, + status=RunStatus(row.status), + modal_call_id=row.modal_call_id, + ) for row in connection.execute(statement) ) def fail_stuck_run( self, - run: RunningRun, + run: ActiveRun, *, error_message: str, failed_at: datetime, ) -> RunWrite: """Mark a stuck ``run`` failed, while it is as it was listed. - It must still be running under ``run.modal_call_id``, or with no call - if it was listed with none, and a simulation run must still have no - running report run naming it. A run that finished, moved to another - call, or that a running report run has named since it was listed, is - left as it is, so concurrent sweeps change it once. + It must still have its listed status and ``run.modal_call_id``, or no + call if it was listed with none, and a simulation run must still have + no pending or running report run naming it. A run that was claimed, + finished, or moved to another call, that the simulation service + recorded a call on, or that a pending or running report run has named + since it was listed, is left as it is, so concurrent sweeps change it + once. """ table = RUN_TABLES[run.kind] same_call = ( @@ -383,10 +388,10 @@ def fail_stuck_run( update(table) .where( table.c.id == run.run_id, - table.c.status == RunStatus.RUNNING.value, + table.c.status == run.status.value, same_call, *( - (_no_running_report_run_names(table),) + (_no_active_report_run_names(table),) if run.kind is RunKind.SIMULATION_RUN else () ), @@ -455,8 +460,8 @@ def _uniform_rows(rows: tuple[dict[str, Any], ...]) -> list[dict[str, Any]]: return [dict.fromkeys(columns) | row for row in rows] -def _no_running_report_run_names(table: Table) -> ColumnElement[bool]: - """Return the condition that no running report run names the run. +def _no_active_report_run_names(table: Table) -> ColumnElement[bool]: + """Return the condition that no pending or running report run names the run. A report run names its simulation runs as its baseline and its reform. """ @@ -465,5 +470,5 @@ def _no_running_report_run_names(table: Table) -> ColumnElement[bool]: report_runs.c.baseline_simulation_run_id == table.c.id, report_runs.c.reform_simulation_run_id == table.c.id, ), - report_runs.c.status == RunStatus.RUNNING.value, + report_runs.c.status.in_(ACTIVE_STATUSES), ) diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_tables.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_tables.py index 91728e95c..ab44a6f3d 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_tables.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_tables.py @@ -33,6 +33,7 @@ "report_runs", metadata, Column("id", UUID(as_uuid=True), primary_key=True), + Column("created_at", DateTime(timezone=True), nullable=False), Column("baseline_simulation_run_id", UUID(as_uuid=True), nullable=False), Column("reform_simulation_run_id", UUID(as_uuid=True)), Column("status", report_run_status_type, nullable=False), @@ -53,6 +54,7 @@ "simulation_runs", metadata, Column("id", UUID(as_uuid=True), primary_key=True), + Column("created_at", DateTime(timezone=True), nullable=False), Column("status", simulation_run_status_type, nullable=False), Column("modal_call_id", String(255)), Column("error_message", String), diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py index e1e23b4df..face0f150 100644 --- a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -13,9 +13,9 @@ import pytest from policyengine_stage12_persistence import ( + ActiveRun, ReportRunStore, RunKind, - RunningRun, RunStatus, RunWrite, ) @@ -24,6 +24,7 @@ CUTOFF = NOW + timedelta(minutes=30) ENDED_MESSAGE = "modal_call_ended: the run's Modal call ended without recording it" +NOT_STARTED_MESSAGE = "run_not_started: no Modal call was recorded for the run" @pytest.fixture @@ -61,14 +62,16 @@ def _running(seeder, seeded_run: SeededReportRun): started_at=NOW, ) return ( - RunningRun( + ActiveRun( kind=RunKind.REPORT_RUN, run_id=run_ids.report_run_id, + status=RunStatus.RUNNING, modal_call_id="fc-coordinator", ), - RunningRun( + ActiveRun( kind=RunKind.SIMULATION_RUN, run_id=run_ids.baseline_simulation_run_id, + status=RunStatus.RUNNING, modal_call_id="fc-baseline", ), ) @@ -76,21 +79,21 @@ def _running(seeder, seeded_run: SeededReportRun): def _listed(store, cutoff): return ( - *store.list_running_runs(RunKind.REPORT_RUN, started_before=cutoff), - *store.list_running_runs(RunKind.SIMULATION_RUN, started_before=cutoff), + *store.list_active_runs(RunKind.REPORT_RUN, active_since_before=cutoff), + *store.list_active_runs(RunKind.SIMULATION_RUN, active_since_before=cutoff), ) -def _listed_for(store, seeded_run): +def _listed_for(store, seeded_run, cutoff=CUTOFF): ids = { seeded_run.run_ids.report_run_id, seeded_run.run_ids.baseline_simulation_run_id, seeded_run.run_ids.reform_simulation_run_id, } - return {run for run in _listed(store, CUTOFF) if run.run_id in ids} + return {run for run in _listed(store, cutoff) if run.run_id in ids} -def test_the_sweep_lists_runs_running_under_a_call_since_before_the_cutoff( +def test_the_sweep_lists_runs_running_since_before_the_cutoff( store, seeder, seeded_run: SeededReportRun ) -> None: report_run, baseline_run = _running(seeder, seeded_run) @@ -105,24 +108,53 @@ def test_the_sweep_lists_runs_running_under_a_call_since_before_the_cutoff( completed_at=CUTOFF, ) # The reform run is running, but its call has no ID recorded yet. - _set_run( - seeder, - "simulation_runs", - seeded_run.run_ids.reform_simulation_run_id, - status="running", - started_at=NOW, - ) + reform_id = seeded_run.run_ids.reform_simulation_run_id + _set_run(seeder, "simulation_runs", reform_id, status="running", started_at=NOW) - assert _listed_for(store, seeded_run) == {baseline_run} - assert _listed_for_cutoff(store, seeded_run, NOW) == set() + assert _listed_for(store, seeded_run) == { + baseline_run, + ActiveRun( + kind=RunKind.SIMULATION_RUN, + run_id=reform_id, + status=RunStatus.RUNNING, + modal_call_id=None, + ), + } + assert _listed_for(store, seeded_run, NOW) == set() -def _listed_for_cutoff(store, seeded_run, cutoff): - ids = { - seeded_run.run_ids.report_run_id, +def test_a_pending_run_is_listed_from_its_creation( + store, seeder, seeded_run: SeededReportRun +) -> None: + report_run_id = seeded_run.run_ids.report_run_id + _set_run(seeder, "report_runs", report_run_id, created_at=NOW) + unstarted = ActiveRun( + kind=RunKind.REPORT_RUN, + run_id=report_run_id, + status=RunStatus.PENDING, + modal_call_id=None, + ) + # The simulation runs are pending too, but the pending report run's + # coordinator will start them. + for simulation_run_id in ( seeded_run.run_ids.baseline_simulation_run_id, + seeded_run.run_ids.reform_simulation_run_id, + ): + _set_run(seeder, "simulation_runs", simulation_run_id, created_at=NOW) + + assert _listed_for(store, seeded_run) == {unstarted} + assert _listed_for(store, seeded_run, NOW) == set() + + # The simulation service recorded the call it started. + _set_run(seeder, "report_runs", report_run_id, modal_call_id="fc-coordinator") + assert _listed_for(store, seeded_run) == { + ActiveRun( + kind=RunKind.REPORT_RUN, + run_id=report_run_id, + status=RunStatus.PENDING, + modal_call_id="fc-coordinator", + ) } - return {run for run in _listed(store, cutoff) if run.run_id in ids} def test_the_sweep_fails_a_run_once_while_its_ended_call_still_holds_it( @@ -179,10 +211,51 @@ def test_the_sweep_leaves_a_run_that_moved_to_another_call_or_finished( assert finished == RunWrite(changed=False, status=RunStatus.SUCCEEDED) +def test_a_pending_run_is_failed_only_while_pending_under_its_listed_call( + store, seeder, seeded_run: SeededReportRun +) -> None: + report_run_id = seeded_run.run_ids.report_run_id + _set_run(seeder, "report_runs", report_run_id, created_at=NOW) + (listed_without_call,) = _listed_for(store, seeded_run) + # After the listing, the simulation service recorded the call it started. + _set_run(seeder, "report_runs", report_run_id, modal_call_id="fc-coordinator") + late_call = store.fail_stuck_run( + listed_without_call, error_message=NOT_STARTED_MESSAGE, failed_at=CUTOFF + ) + (listed_with_call,) = _listed_for(store, seeded_run) + # After the listing, the call claimed the run. + _set_run( + seeder, + "report_runs", + report_run_id, + status="running", + started_at=CUTOFF, + ) + claimed = store.fail_stuck_run( + listed_with_call, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ) + + assert listed_without_call.modal_call_id is None + assert late_call == RunWrite(changed=False, status=RunStatus.PENDING) + assert listed_with_call.modal_call_id == "fc-coordinator" + assert claimed == RunWrite(changed=False, status=RunStatus.RUNNING) + assert seeder.row("report_runs", report_run_id)["error_message"] is None + + def test_two_sweeps_at_once_fail_each_run_exactly_once( seeder, seeded_run: SeededReportRun ) -> None: - runs = _running(seeder, seeded_run) + reform_id = seeded_run.run_ids.reform_simulation_run_id + _set_run(seeder, "simulation_runs", reform_id, created_at=NOW) + runs = ( + *_running(seeder, seeded_run), + ActiveRun( + kind=RunKind.SIMULATION_RUN, + run_id=reform_id, + status=RunStatus.PENDING, + modal_call_id=None, + ), + ) stores = [ReportRunStore(worker_database_url()) for _ in range(2)] start_together = Barrier(len(stores)) @@ -202,10 +275,11 @@ def sweep(run_store): for run_store in stores: run_store.engine.dispose() - assert [sum(column) for column in zip(*changes, strict=True)] == [1, 1] + assert [sum(column) for column in zip(*changes, strict=True)] == [1, 1, 1] assert seeder.row("report_runs", seeded_run.run_ids.report_run_id)["status"] == ( "failed" ) + assert seeder.row("simulation_runs", reform_id)["status"] == "failed" def _second_report_run(seeder, seeded_run: SeededReportRun, status): @@ -233,20 +307,30 @@ def _baseline_listed(store, seeded_run): baseline_id = seeded_run.run_ids.baseline_simulation_run_id return any( run.run_id == baseline_id - for run in store.list_running_runs( - RunKind.SIMULATION_RUN, started_before=CUTOFF + for run in store.list_active_runs( + RunKind.SIMULATION_RUN, active_since_before=CUTOFF ) ) -def test_a_simulation_run_is_listed_unless_a_report_run_naming_it_is_running( +def test_a_simulation_run_is_listed_unless_a_pending_or_running_report_run_names_it( store, seeder, seeded_run: SeededReportRun ) -> None: _running(seeder, seeded_run) # The first report run is running, so its coordinator judges the run. assert not _baseline_listed(store, seeded_run) + # A pending report run's coordinator is still to start it. _set_run(seeder, "report_runs", seeded_run.run_ids.report_run_id, status="pending") + assert not _baseline_listed(store, seeded_run) + + _set_run( + seeder, + "report_runs", + seeded_run.run_ids.report_run_id, + status="failed", + completed_at=CUTOFF, + ) assert _baseline_listed(store, seeded_run) second = _second_report_run(seeder, seeded_run, "running") @@ -256,28 +340,31 @@ def test_a_simulation_run_is_listed_unless_a_report_run_naming_it_is_running( assert _baseline_listed(store, seeded_run) -def test_runs_past_the_maximum_age_are_listed_and_failed_without_a_call( +def test_a_run_with_no_call_is_listed_and_failed_while_it_still_has_none( store, seeder, seeded_run: SeededReportRun ) -> None: report_run_id = seeded_run.run_ids.report_run_id _set_run(seeder, "report_runs", report_run_id, status="running", started_at=NOW) - with_calls = store.list_running_runs(RunKind.REPORT_RUN, started_before=CUTOFF) - every_run = store.list_running_runs( - RunKind.REPORT_RUN, started_before=CUTOFF, include_runs_without_call=True - ) - - assert report_run_id not in {run.run_id for run in with_calls} - (unrecorded,) = [run for run in every_run if run.run_id == report_run_id] - assert unrecorded.modal_call_id is None + (unrecorded,) = [ + run + for run in store.list_active_runs( + RunKind.REPORT_RUN, active_since_before=CUTOFF + ) + if run.run_id == report_run_id + ] failed = store.fail_stuck_run( unrecorded, error_message="run_exceeded_maximum_age: too old", failed_at=CUTOFF ) + + assert unrecorded.modal_call_id is None + assert unrecorded.status is RunStatus.RUNNING assert failed == RunWrite(changed=True, status=RunStatus.FAILED) -def test_a_simulation_run_a_report_run_is_now_running_is_not_failed( - store, seeder, seeded_run: SeededReportRun +@pytest.mark.parametrize("second_status", ["pending", "running"]) +def test_a_simulation_run_a_report_run_now_names_is_not_failed( + store, seeder, seeded_run: SeededReportRun, second_status ) -> None: _, baseline_run = _running(seeder, seeded_run) _set_run( @@ -288,9 +375,9 @@ def test_a_simulation_run_a_report_run_is_now_running_is_not_failed( completed_at=CUTOFF, ) assert _baseline_listed(store, seeded_run) - # After the listing, another report run naming it starts running; its - # coordinator now judges the simulation run. - _second_report_run(seeder, seeded_run, "running") + # After the listing, another report run naming it is created or starts + # running; its coordinator now judges the simulation run. + _second_report_run(seeder, seeded_run, second_status) write = store.fail_stuck_run( baseline_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py index d5f386d7a..004cfd710 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py @@ -28,7 +28,7 @@ if TYPE_CHECKING: from collections.abc import Callable - from policyengine_stage12_persistence import RunningRun, RunWrite + from policyengine_stage12_persistence import ActiveRun, RunWrite logger = logging.getLogger(__name__) @@ -71,24 +71,20 @@ class CallState(StrEnum): class StuckRunStore(Protocol): """The sweep's reads and writes; ``ReportRunStore`` implements them.""" - def list_running_runs( - self, - kind: RunKind, - *, - started_before: datetime, - include_runs_without_call: bool = False, - ) -> tuple[RunningRun, ...]: - """Return the runs of ``kind`` running since before a time.""" + def list_active_runs( + self, kind: RunKind, *, active_since_before: datetime + ) -> tuple[ActiveRun, ...]: + """Return the pending and running runs of ``kind`` active since a time.""" ... def fail_stuck_run( - self, run: RunningRun, *, error_message: str, failed_at: datetime + self, run: ActiveRun, *, error_message: str, failed_at: datetime ) -> RunWrite: """Mark ``run`` failed while it is as it was listed. - It must still be running under its listed call, or with no call if it - was listed with none; a simulation run must still have no running - report run combining it. + It must still have its listed status and call, or no call if it was + listed with none; a simulation run must still have no pending or + running report run combining it. """ ... @@ -185,9 +181,7 @@ def sweep_stuck_runs( # the store, then keyword-only seams and limits states: dict[str, CallState] = {} tally = _Tally() for kind in (RunKind.REPORT_RUN, RunKind.SIMULATION_RUN): - for run in store.list_running_runs( - kind, started_before=now - maximum_age, include_runs_without_call=True - ): + for run in store.list_active_runs(kind, active_since_before=now - maximum_age): if store.fail_stuck_run( run, error_message=EXCEEDED_AGE_MESSAGE, failed_at=now ).changed: @@ -195,7 +189,7 @@ def sweep_stuck_runs( # the store, then keyword-only seams and limits else: tally.left += 1 for kind in (RunKind.REPORT_RUN, RunKind.SIMULATION_RUN): - for run in store.list_running_runs(kind, started_before=now - minimum_age): + for run in store.list_active_runs(kind, active_since_before=now - minimum_age): _sweep_run( store, run, states=states, call_state=call_state, now=now, tally=tally ) @@ -204,7 +198,7 @@ def sweep_stuck_runs( # the store, then keyword-only seams and limits def _sweep_run( # noqa: PLR0913 # one run, the store, and the sweep's shared state store: StuckRunStore, - run: RunningRun, + run: ActiveRun, *, states: dict[str, CallState], call_state: Callable[[str], CallState], diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py index a574a187c..a87755d4d 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py @@ -14,9 +14,12 @@ modal_call_state, sweep_stuck_runs, ) -from policyengine_stage12_persistence import RunKind, RunningRun, RunStatus, RunWrite +from policyengine_stage12_persistence import ActiveRun, RunKind, RunStatus, RunWrite NOW = datetime(2026, 11, 4, 12, tzinfo=UTC) +# The sweep lists runs older than its maximum age first, then runs older +# than its minimum age; a cutoff before this one is the first listing's. +MAXIMUM_AGE_CUTOFFS_BEFORE = NOW - timedelta(hours=1) class FakeInputStatus(Enum): @@ -40,17 +43,15 @@ def __init__(self, runs, *, expired=()): self.failed = [] self.events = [] - def list_running_runs( - self, kind, *, started_before, include_runs_without_call=False - ): + def list_active_runs(self, kind, *, active_since_before): # As the real store, a run this fake failed is no longer listed. failed = {failed_run for failed_run, _, _ in self.failed} - if include_runs_without_call: - self.expired_cutoffs.append(started_before) + if active_since_before < MAXIMUM_AGE_CUTOFFS_BEFORE: + self.expired_cutoffs.append(active_since_before) self.events.append(f"list-expired:{kind.value}") listed = self.expired else: - self.cutoffs.append(started_before) + self.cutoffs.append(active_since_before) self.events.append(f"list:{kind.value}") listed = self.runs return tuple(run for run in listed if run.kind is kind and run not in failed) @@ -62,7 +63,9 @@ def fail_stuck_run(self, run, *, error_message, failed_at): def _run(call_id, kind=RunKind.SIMULATION_RUN): - return RunningRun(kind=kind, run_id=uuid4(), modal_call_id=call_id) + return ActiveRun( + kind=kind, run_id=uuid4(), status=RunStatus.RUNNING, modal_call_id=call_id + ) def test_only_runs_whose_call_ended_are_failed() -> None: @@ -264,8 +267,11 @@ def test_a_run_past_the_maximum_age_is_failed_whatever_modal_reports() -> None: # pending, and no longer describes old calls; neither may keep a run # running for good, and nor may a run with no recorded call. reported_running = _run("fc-reported-running", RunKind.REPORT_RUN) - without_call = RunningRun( - kind=RunKind.REPORT_RUN, run_id=uuid4(), modal_call_id=None + without_call = ActiveRun( + kind=RunKind.REPORT_RUN, + run_id=uuid4(), + status=RunStatus.RUNNING, + modal_call_id=None, ) store = FakeSweepStore([reported_running], expired=[reported_running, without_call]) From bc2bb432eb389f067b7542da2bdc0f81b7d91516 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:50:16 +0400 Subject: [PATCH 11/17] Fail a pending run the simulation service never started a call for A pending run with no Modal call recorded ten minutes after its creation is failed with `run_not_started`, without asking Modal: the simulation service stopped between inserting the run and starting its call, so nothing will run it. A pending run with a recorded call is judged by its call, exactly as a running one: Modal may still be queueing it, or it may have ended without claiming the run. A running run with no call recorded is still left to the 24-hour limit, which now counts a never-claimed run from its creation. Co-Authored-By: Claude Opus 5.5 --- changelog.d/stuck-run-sweep.added.md | 2 +- .../src/modal/run_sweep_app.py | 16 ++-- .../stage12_runtime/run_sweep.py | 87 ++++++++++++------- .../tests/test_stage12_run_sweep.py | 46 +++++++++- 4 files changed, 111 insertions(+), 40 deletions(-) diff --git a/changelog.d/stuck-run-sweep.added.md b/changelog.d/stuck-run-sweep.added.md index 6f5418b29..e52c19325 100644 --- a/changelog.d/stuck-run-sweep.added.md +++ b/changelog.d/stuck-run-sweep.added.md @@ -1 +1 @@ -A scheduled Modal function, in the new Modal application `policyengine-simulation-run-sweep`, marks the Public API's report runs and simulation runs failed when they are still running but the Modal call recorded for them has ended. It checks every ten minutes and is safe to run twice at once. It leaves a run alone while Modal reports its call running, or can't report it, until the run has been running for 24 hours, longer than any of its calls can run; then it fails the run whatever Modal reports. An operator deploys it once per Modal environment. +A scheduled Modal function, in the new Modal application `policyengine-simulation-run-sweep`, marks the Public API's report runs and simulation runs failed when nothing will finish them: a pending or running run whose recorded Modal call has ended, or a pending run that still has no Modal call recorded ten minutes after its creation. It checks every ten minutes and is safe to run twice at once. It leaves a run alone while Modal reports its call running, or can't report it, until the run is 24 hours old, counted from its first claim or, if it was never claimed, from its creation, longer than any of its calls can run; then it fails the run whatever Modal reports. An operator deploys it once per Modal environment. diff --git a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py index 793714002..623ca2d8c 100644 --- a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py +++ b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py @@ -1,12 +1,14 @@ -"""A scheduled Modal function that fails runs left running after their call ended. +"""A scheduled Modal function that fails runs nothing will finish. Every ten minutes it lists the Public API's report runs and simulation runs -that have been running under a recorded Modal call for at least ten minutes, -asks Modal for each call's status, and marks a run failed once its call has -ended. It also fails any run still running 24 hours after it started, longer -than any Modal call of it can run, whatever Modal reports. It connects as -the Modal worker role, from the same Modal secret the v2 coordinator and -workers use. This application is not versioned with the +that have been pending or running for at least ten minutes, counted from +their first claim or, while pending, from their creation. It asks Modal for +the status of each recorded call and marks a run failed once its call has +ended, and fails a pending run that has no call recorded, because nothing +will run it. It also fails any run still unfinished 24 hours after it was +created or started, longer than any Modal call of it can run, whatever Modal +reports. It connects as the Modal worker role, from the same Modal secret the +v2 coordinator and workers use. This application is not versioned with the .py bundle: one copy runs per Modal environment. Deploy it, once per Modal environment, from this project's directory: diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py index 004cfd710..a4b1db7d6 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py @@ -1,17 +1,22 @@ -"""The sweep that fails report runs and simulation runs whose Modal call ended. - -A coordinator or worker records the Modal call running each run. If that call -ends without recording the run's outcome, for example because its container -ran out of memory, the run stays running. The sweep asks Modal for the status -of each recorded call and marks the run failed once the call has ended. It -also fails a run still running long after it started, longer than any Modal +"""The sweep that fails report runs and simulation runs nothing will finish. + +The simulation service records the Modal call it starts on a pending report +run, and a coordinator or worker records the call running each run it claims. +If that call ends without recording the run's outcome, for example because +its container ran out of memory, the run stays pending or running. The sweep +asks Modal for the status of each recorded call and marks the run failed once +the call has ended. A pending run with no call recorded soon after its +creation is failed without asking Modal: the simulation service stopped +before starting its call, so nothing will run it. The sweep also fails a run +still unfinished long after it was created or started, longer than any Modal call of it can run, whatever Modal reports and with or without a recorded -call, so a run Modal can't decide doesn't stay running for good. +call, so a run Modal can't decide doesn't stay unfinished for good. Modal restarts a pre-empted call under the same call ID and reports it pending meanwhile, so a pre-empted coordinator's report run is never failed. -Its children are cancelled and started again under new calls, so the sweep -judges a simulation run only once no report run combining it is running. +Its children are cancelled and started again under new calls, and it starts +them only once Modal runs it, so the sweep judges a simulation run only once +no pending or running report run combines it. """ from __future__ import annotations @@ -23,7 +28,7 @@ from importlib import import_module from typing import TYPE_CHECKING, Any, Protocol -from policyengine_stage12_persistence import RunKind +from policyengine_stage12_persistence import RunKind, RunStatus if TYPE_CHECKING: from collections.abc import Callable @@ -32,23 +37,29 @@ logger = logging.getLogger(__name__) -# A run must have been running at least this long, counted from its first -# claim, before the sweep checks its call. +# A run must have been unfinished at least this long, counted from its first +# claim or, while it is pending, from its creation, before the sweep judges +# it; a pending run with no call recorded by then is failed. DEFAULT_MINIMUM_AGE = timedelta(minutes=10) ENDED_CALL_MESSAGE = ( "modal_call_ended: The Modal call recorded for this run ended without " "recording the run's outcome." ) -# A run still running this long after its first claim is failed whatever -# Modal reports, or with no call recorded at all. No attempt of the v2 +NOT_STARTED_MESSAGE = ( + "run_not_started: The run stayed pending with no Modal call recorded, so " + "nothing was going to run it." +) +# A run still unfinished this long after its first claim, or its creation if +# it was never claimed, is failed whatever Modal reports, or with no call +# recorded at all. No attempt of the v2 # functions can run anywhere near this long (the coordinator's own time # limit is 90 minutes), however often Modal pre-empts and restarts it; it # stops Modal's pending status for an internal failure it won't retry, or # a call Modal no longer describes, from keeping a run running for good. DEFAULT_MAXIMUM_AGE = timedelta(hours=24) EXCEEDED_AGE_MESSAGE = ( - "run_exceeded_maximum_age: The run was still running long after it " - "started, longer than any Modal call of it can run." + "run_exceeded_maximum_age: The run was still unfinished long after it " + "was created or started, longer than any Modal call of it can run." ) # Modal's input statuses (``modal.call_graph.InputStatus``) by name. A pending # input is running or waiting to run again; Modal reports an input pre-empted @@ -93,12 +104,15 @@ def fail_stuck_run( class SweepSummary: """What the sweep did with the runs it listed. - ``checked`` runs had their call asked about: ``failed`` because it ended, - or left because it is ``running`` or its state is ``unknown``. + ``checked`` runs were judged after the minimum age: ``failed`` because + their call ended, ``failed_without_call`` because they stayed pending with + no call recorded, or left because their call is ``running`` or its state + is ``unknown``, as is a running run with no call recorded. ``failed_for_age`` runs were failed for exceeding the maximum age. ``left`` counts listed runs whose conditional failure changed nothing, because - they finished, moved to another call, or a running report run started - combining them after they were listed. + they were claimed, finished, moved to another call, had a call recorded, + or a pending or running report run started combining them after they + were listed. """ checked: int @@ -107,6 +121,7 @@ class SweepSummary: unknown: int left: int = 0 failed_for_age: int = 0 + failed_without_call: int = 0 def modal_call_state(modal_call_id: str, *, modal_module: Any = None) -> CallState: # noqa: ANN401 # the modal module, or a stand-in for it @@ -157,6 +172,7 @@ class _Tally: unknown: int = 0 left: int = 0 failed_for_age: int = 0 + failed_without_call: int = 0 def sweep_stuck_runs( # the store, then keyword-only seams and limits @@ -167,15 +183,17 @@ def sweep_stuck_runs( # the store, then keyword-only seams and limits minimum_age: timedelta = DEFAULT_MINIMUM_AGE, maximum_age: timedelta = DEFAULT_MAXIMUM_AGE, ) -> SweepSummary: - """Fail every listed run whose recorded Modal call has ended. + """Fail every listed run whose recorded Modal call has ended or was never started. Runs older than ``maximum_age`` are failed first, report runs then simulation runs, before any Modal lookup can use up the scheduled - function's time. Then each recorded call is asked about once, report - runs first, so a dead coordinator's simulation runs are listed in the - same sweep. A run whose call is running, or whose state is unknown, is - left as it is; so is one whose conditional failure changes nothing, which - makes two sweeps at once safe. + function's time. Then, report runs first, so a dead coordinator's + simulation runs are listed in the same sweep, each run older than + ``minimum_age`` is judged: a pending run with no call recorded is failed, + and each recorded call is asked about once. A run whose call is running, + or whose state is unknown, is left as it is, as is a running run with no + call recorded, until the maximum age; so is one whose conditional failure + changes nothing, which makes two sweeps at once safe. """ now = now or datetime.now(UTC) states: dict[str, CallState] = {} @@ -205,11 +223,22 @@ def _sweep_run( # noqa: PLR0913 # one run, the store, and the sweep's shared s now: datetime, tally: _Tally, ) -> None: - """Fail ``run`` if its call has ended, asking about each call once.""" + """Fail ``run`` if its call has ended or was never started. + + Each call is asked about once. + """ tally.checked += 1 modal_call_id = run.modal_call_id if modal_call_id is None: - tally.unknown += 1 + if run.status is not RunStatus.PENDING: + # Nothing to ask Modal about; the maximum age decides it. + tally.unknown += 1 + elif store.fail_stuck_run( + run, error_message=NOT_STARTED_MESSAGE, failed_at=now + ).changed: + tally.failed_without_call += 1 + else: + tally.left += 1 return if modal_call_id not in states: try: diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py index a87755d4d..dabab5295 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py @@ -62,12 +62,52 @@ def fail_stuck_run(self, run, *, error_message, failed_at): return RunWrite(changed=True, status=RunStatus.FAILED) -def _run(call_id, kind=RunKind.SIMULATION_RUN): - return ActiveRun( - kind=kind, run_id=uuid4(), status=RunStatus.RUNNING, modal_call_id=call_id +def _run(call_id, kind=RunKind.SIMULATION_RUN, status=RunStatus.RUNNING): + return ActiveRun(kind=kind, run_id=uuid4(), status=status, modal_call_id=call_id) + + +def test_a_pending_run_with_no_call_is_failed_without_asking_modal() -> None: + # The simulation service stopped between inserting the run and starting + # its call, so nothing will run it. + never_started = _run(None, RunKind.REPORT_RUN, RunStatus.PENDING) + # A running run with no call recorded is left to the maximum age. + running_without_call = _run(None) + store = FakeSweepStore([never_started, running_without_call]) + asked = [] + + summary = sweep_stuck_runs(store, call_state=asked.append, now=NOW) + + assert asked == [] + assert [run for run, _, _ in store.failed] == [never_started] + (_, message, failed_at) = store.failed[0] + assert message.startswith("run_not_started: ") + assert failed_at == NOW + assert summary == SweepSummary( + checked=2, + failed=0, + running=0, + unknown=1, + left=0, + failed_for_age=0, + failed_without_call=1, ) +def test_a_pending_run_with_a_call_is_judged_by_its_call() -> None: + # The simulation service recorded the call it started; Modal may still + # be queueing it, or it may have ended without claiming the run. + queued = _run("fc-queued", RunKind.REPORT_RUN, RunStatus.PENDING) + ended = _run("fc-ended", RunKind.REPORT_RUN, RunStatus.PENDING) + store = FakeSweepStore([queued, ended]) + states = {"fc-queued": CallState.RUNNING, "fc-ended": CallState.ENDED} + + summary = sweep_stuck_runs(store, call_state=states.__getitem__, now=NOW) + + assert [run for run, _, _ in store.failed] == [ended] + assert store.failed[0][1].startswith("modal_call_ended: ") + assert (summary.checked, summary.failed, summary.running) == (2, 1, 1) + + def test_only_runs_whose_call_ended_are_failed() -> None: ended = _run("fc-ended") alive = _run("fc-alive") From d23f18dfa28d2dffa1ad09ab3d6b72f6be93e473 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:51:51 +0400 Subject: [PATCH 12/17] Deploy the stuck-run sweep from the executor's deployment job The `deploy_executor` job of the reusable deployment workflow now runs `modal deploy` on `src/modal/run_sweep_app.py` after the versioned executor, with only the Modal tokens in the step's environment, as the executor's own deploy step has them. The sweep reads its database URL at run time from the Modal secret `policyengine-modal-worker-database`, so this step fails until an operator has created that secret in the Modal environment; the secret must exist before this merges. Co-Authored-By: Claude Opus 5.5 --- .../workflows/simulation-deploy.reusable.yml | 11 ++++++ changelog.d/stuck-run-sweep.added.md | 2 +- .../src/modal/run_sweep_app.py | 7 ++-- .../tests/test_stage12_run_sweep_app.py | 35 +++++++++++++++++++ 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/.github/workflows/simulation-deploy.reusable.yml b/.github/workflows/simulation-deploy.reusable.yml index 667b222db..cd7d8e162 100644 --- a/.github/workflows/simulation-deploy.reusable.yml +++ b/.github/workflows/simulation-deploy.reusable.yml @@ -377,6 +377,17 @@ jobs: POLICYENGINE_OTEL_GOOGLE_AUDIENCE: ${{ vars.OBSERVABILITY_OTLP_GOOGLE_AUDIENCE }} run: uv run --frozen modal deploy --env="${{ inputs.modal_environment }}" src/modal/app.py + # The scheduled sweep that fails report runs and simulation runs nothing + # will finish. Its database URL comes from the Modal secret + # policyengine-modal-worker-database, so this step fails until an + # operator has created that secret in the Modal environment. + - name: Deploy the stuck-run sweep + working-directory: projects/policyengine-simulation-executor + env: + MODAL_TOKEN_ID: ${{ secrets.MODAL_TOKEN_ID }} + MODAL_TOKEN_SECRET: ${{ secrets.MODAL_TOKEN_SECRET }} + run: uv run --frozen modal deploy --env="${{ inputs.modal_environment }}" src/modal/run_sweep_app.py + - name: Run executor unit tests working-directory: projects/policyengine-simulation-executor run: uv run --frozen pytest tests/ -v diff --git a/changelog.d/stuck-run-sweep.added.md b/changelog.d/stuck-run-sweep.added.md index e52c19325..063aa164f 100644 --- a/changelog.d/stuck-run-sweep.added.md +++ b/changelog.d/stuck-run-sweep.added.md @@ -1 +1 @@ -A scheduled Modal function, in the new Modal application `policyengine-simulation-run-sweep`, marks the Public API's report runs and simulation runs failed when nothing will finish them: a pending or running run whose recorded Modal call has ended, or a pending run that still has no Modal call recorded ten minutes after its creation. It checks every ten minutes and is safe to run twice at once. It leaves a run alone while Modal reports its call running, or can't report it, until the run is 24 hours old, counted from its first claim or, if it was never claimed, from its creation, longer than any of its calls can run; then it fails the run whatever Modal reports. An operator deploys it once per Modal environment. +A scheduled Modal function, in the new Modal application `policyengine-simulation-run-sweep`, marks the Public API's report runs and simulation runs failed when nothing will finish them: a pending or running run whose recorded Modal call has ended, or a pending run that still has no Modal call recorded ten minutes after its creation. It checks every ten minutes and is safe to run twice at once. It leaves a run alone while Modal reports its call running, or can't report it, until the run is 24 hours old, counted from its first claim or, if it was never claimed, from its creation, longer than any of its calls can run; then it fails the run whatever Modal reports. The deployment workflow deploys it with the executor, and fails until the Modal secret `policyengine-modal-worker-database` exists in the Modal environment. diff --git a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py index 623ca2d8c..970ee85c5 100644 --- a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py +++ b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py @@ -11,10 +11,9 @@ v2 coordinator and workers use. This application is not versioned with the .py bundle: one copy runs per Modal environment. -Deploy it, once per Modal environment, from this project's directory: - - uv run --frozen modal deploy --env=staging src/modal/run_sweep_app.py - uv run --frozen modal deploy --env=main src/modal/run_sweep_app.py +The deployment workflow's executor job deploys it to each Modal environment, +after the versioned executor (``simulation-deploy.reusable.yml``). Modal +refuses the deployment until the Modal secret exists in that environment. """ from __future__ import annotations diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py index f3d47ef13..91a04fa96 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep_app.py @@ -4,13 +4,22 @@ import importlib import sys +from pathlib import Path +import yaml from fixtures.fake_modal import install_fake_modal from policyengine_simulation_executor.stage12_runtime import ( report_run_access as report_run_access_module, ) from policyengine_simulation_executor.stage12_runtime import run_sweep +REUSABLE_DEPLOY_WORKFLOW = ( + Path(__file__).resolve().parents[3] + / ".github" + / "workflows" + / "simulation-deploy.reusable.yml" +) + def _load(monkeypatch): install_fake_modal(monkeypatch) @@ -18,6 +27,32 @@ def _load(monkeypatch): return importlib.import_module("src.modal.run_sweep_app") +def test_the_executor_deployment_deploys_the_sweep_with_the_modal_tokens() -> None: + workflow = yaml.safe_load(REUSABLE_DEPLOY_WORKFLOW.read_text(encoding="utf-8")) + steps = workflow["jobs"]["deploy_executor"]["steps"] + step_names = [step.get("name") for step in steps] + (sweep_step,) = [ + step for step in steps if "src/modal/run_sweep_app.py" in step.get("run", "") + ] + + assert sweep_step["run"] == ( + 'uv run --frozen modal deploy --env="${{ inputs.modal_environment }}" ' + "src/modal/run_sweep_app.py" + ) + assert ( + sweep_step["working-directory"] == "projects/policyengine-simulation-executor" + ) + # Only Modal's tokens: the database URL reaches the scheduled function from + # its Modal secret, which must exist before this step can deploy it. + assert sweep_step["env"] == { + "MODAL_TOKEN_ID": "${{ secrets.MODAL_TOKEN_ID }}", + "MODAL_TOKEN_SECRET": "${{ secrets.MODAL_TOKEN_SECRET }}", + } + assert step_names.index(sweep_step["name"]) > step_names.index( + "Deploy versioned Modal executor" + ) + + def test_the_sweep_runs_every_ten_minutes_with_the_worker_database_secret( monkeypatch, ) -> None: From b2321536df2b4cbfe92d3bd1845111e3961c7ed5 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 6 Nov 2026 00:04:40 +0400 Subject: [PATCH 13/17] Describe the sweep's never-started runs exactly, and fill two test gaps The sweep's module docstring and `run_not_started` message gave one cause for a pending run with no call; it also follows a call the simulation service started but failed to record, whose claim is then refused. The scheduled function and the store protocol described their old scope. The sweep tests' stand-in store now tells the maximum-age listing by the exact cutoff the sweep asks for, instead of by a fixed hour. New cases: a pending run is failed for age like a running one, and a dead coordinator's never-started simulation runs are listed once its report run has failed. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_report_run_sweep_store.py | 33 +++++++++++++++++++ .../src/modal/run_sweep_app.py | 2 +- .../stage12_runtime/run_sweep.py | 23 ++++++------- .../tests/test_stage12_run_sweep.py | 26 ++++++++++----- 4 files changed, 63 insertions(+), 21 deletions(-) diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py index face0f150..ca5eecf69 100644 --- a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -157,6 +157,39 @@ def test_a_pending_run_is_listed_from_its_creation( } +def test_never_started_simulation_runs_are_listed_once_their_report_run_finished( + store, seeder, seeded_run: SeededReportRun +) -> None: + # A coordinator that died before starting its children leaves them + # pending with no call; once its report run has failed, they are listed. + run_ids = seeded_run.run_ids + for simulation_run_id in ( + run_ids.baseline_simulation_run_id, + run_ids.reform_simulation_run_id, + ): + _set_run(seeder, "simulation_runs", simulation_run_id, created_at=NOW) + _set_run( + seeder, + "report_runs", + run_ids.report_run_id, + status="failed", + completed_at=CUTOFF, + ) + + assert _listed_for(store, seeded_run) == { + ActiveRun( + kind=RunKind.SIMULATION_RUN, + run_id=simulation_run_id, + status=RunStatus.PENDING, + modal_call_id=None, + ) + for simulation_run_id in ( + run_ids.baseline_simulation_run_id, + run_ids.reform_simulation_run_id, + ) + } + + def test_the_sweep_fails_a_run_once_while_its_ended_call_still_holds_it( store, seeder, seeded_run: SeededReportRun ) -> None: diff --git a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py index 970ee85c5..98dd27d45 100644 --- a/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py +++ b/projects/policyengine-simulation-executor/src/modal/run_sweep_app.py @@ -73,7 +73,7 @@ secrets=[modal_worker_database_secret], ) def sweep_stuck_runs() -> dict: - """Fail the runs whose Modal call ended; return how many were checked and why.""" + """Fail the runs nothing will finish; return how many were judged and why.""" from policyengine_simulation_executor.stage12_runtime import ( # noqa: PLC0415 # Modal function body: loaded in the container, not at deploy time report_run_access, run_sweep, diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py index a4b1db7d6..435fc6461 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py @@ -1,13 +1,15 @@ """The sweep that fails report runs and simulation runs nothing will finish. -The simulation service records the Modal call it starts on a pending report -run, and a coordinator or worker records the call running each run it claims. -If that call ends without recording the run's outcome, for example because -its container ran out of memory, the run stays pending or running. The sweep -asks Modal for the status of each recorded call and marks the run failed once -the call has ended. A pending run with no call recorded soon after its -creation is failed without asking Modal: the simulation service stopped -before starting its call, so nothing will run it. The sweep also fails a run +A coordinator or worker records the Modal call running each run it claims, +and the simulation service is to record the call it starts on a pending +report run. If that call ends without recording the run's outcome, for +example because its container ran out of memory, the run stays pending or +running. The sweep asks Modal for the status of each recorded call and marks +the run failed once the call has ended. A pending run with no call recorded +soon after its creation is failed without asking Modal: usually the +simulation service stopped before starting its call. If it started the call +but failed to record it, that call's claim is refused once the run has +failed, so it finishes nothing. The sweep also fails a run still unfinished long after it was created or started, longer than any Modal call of it can run, whatever Modal reports and with or without a recorded call, so a run Modal can't decide doesn't stay unfinished for good. @@ -46,8 +48,7 @@ "recording the run's outcome." ) NOT_STARTED_MESSAGE = ( - "run_not_started: The run stayed pending with no Modal call recorded, so " - "nothing was going to run it." + "run_not_started: The run stayed pending with no Modal call recorded for it." ) # A run still unfinished this long after its first claim, or its creation if # it was never claimed, is failed whatever Modal reports, or with no call @@ -85,7 +86,7 @@ class StuckRunStore(Protocol): def list_active_runs( self, kind: RunKind, *, active_since_before: datetime ) -> tuple[ActiveRun, ...]: - """Return the pending and running runs of ``kind`` active since a time.""" + """Return the pending and running runs of ``kind`` older than a time.""" ... def fail_stuck_run( diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py index dabab5295..1c2fe8e0e 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py @@ -9,6 +9,7 @@ import pytest from policyengine_simulation_executor.stage12_runtime.run_sweep import ( + DEFAULT_MAXIMUM_AGE, CallState, SweepSummary, modal_call_state, @@ -17,9 +18,6 @@ from policyengine_stage12_persistence import ActiveRun, RunKind, RunStatus, RunWrite NOW = datetime(2026, 11, 4, 12, tzinfo=UTC) -# The sweep lists runs older than its maximum age first, then runs older -# than its minimum age; a cutoff before this one is the first listing's. -MAXIMUM_AGE_CUTOFFS_BEFORE = NOW - timedelta(hours=1) class FakeInputStatus(Enum): @@ -34,10 +32,12 @@ class FakeInputStatus(Enum): class FakeSweepStore: - def __init__(self, runs, *, expired=()): + def __init__(self, runs, *, expired=(), maximum_age=DEFAULT_MAXIMUM_AGE): self.runs = list(runs) - # Runs older than the maximum age, listed with or without a call. + # Runs older than the maximum age, which the sweep lists first, at + # exactly this cutoff; every other listing is for the minimum age. self.expired = list(expired) + self.maximum_age_cutoff = NOW - maximum_age self.cutoffs = [] self.expired_cutoffs = [] self.failed = [] @@ -46,7 +46,7 @@ def __init__(self, runs, *, expired=()): def list_active_runs(self, kind, *, active_since_before): # As the real store, a run this fake failed is no longer listed. failed = {failed_run for failed_run, _, _ in self.failed} - if active_since_before < MAXIMUM_AGE_CUTOFFS_BEFORE: + if active_since_before == self.maximum_age_cutoff: self.expired_cutoffs.append(active_since_before) self.events.append(f"list-expired:{kind.value}") listed = self.expired @@ -307,13 +307,17 @@ def test_a_run_past_the_maximum_age_is_failed_whatever_modal_reports() -> None: # pending, and no longer describes old calls; neither may keep a run # running for good, and nor may a run with no recorded call. reported_running = _run("fc-reported-running", RunKind.REPORT_RUN) + # A pending run counts from its creation; its call may still be queued. + still_queued = _run("fc-queued", RunKind.REPORT_RUN, RunStatus.PENDING) without_call = ActiveRun( kind=RunKind.REPORT_RUN, run_id=uuid4(), status=RunStatus.RUNNING, modal_call_id=None, ) - store = FakeSweepStore([reported_running], expired=[reported_running, without_call]) + store = FakeSweepStore( + [reported_running], expired=[reported_running, still_queued, without_call] + ) summary = sweep_stuck_runs( store, @@ -322,12 +326,16 @@ def test_a_run_past_the_maximum_age_is_failed_whatever_modal_reports() -> None: maximum_age=timedelta(hours=24), ) - assert [run for run, _, _ in store.failed] == [reported_running, without_call] + assert [run for run, _, _ in store.failed] == [ + reported_running, + still_queued, + without_call, + ] assert all( message.startswith("run_exceeded_maximum_age: ") for _, message, _ in store.failed ) - assert summary.failed_for_age == 2 + assert summary.failed_for_age == 3 # Failed for its age first, it is no longer listed for a lookup. assert summary.running == 0 From 49a7f9d3d5b9ab8518d50dd0c9f03f8343ac1a07 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 6 Nov 2026 21:46:30 +0400 Subject: [PATCH 14/17] Say which report runs keep a simulation run out of the sweep A report run names its baseline and reform simulation runs in its own columns, so the sweep's docstrings now say a pending or running report run names the simulation runs it keeps out, not that it combines them. The store tests also check that a report run keeps its reform run, not only its baseline, out of the sweep until it finishes. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_report_run_sweep_store.py | 38 +++++++++++++++++-- .../stage12_runtime/run_sweep.py | 7 ++-- .../tests/test_stage12_run_sweep.py | 2 +- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py index ca5eecf69..68f65f1e8 100644 --- a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -336,16 +336,19 @@ def _second_report_run(seeder, seeded_run: SeededReportRun, status): return second_id -def _baseline_listed(store, seeded_run): - baseline_id = seeded_run.run_ids.baseline_simulation_run_id +def _simulation_run_listed(store, simulation_run_id): return any( - run.run_id == baseline_id + run.run_id == simulation_run_id for run in store.list_active_runs( RunKind.SIMULATION_RUN, active_since_before=CUTOFF ) ) +def _baseline_listed(store, seeded_run): + return _simulation_run_listed(store, seeded_run.run_ids.baseline_simulation_run_id) + + def test_a_simulation_run_is_listed_unless_a_pending_or_running_report_run_names_it( store, seeder, seeded_run: SeededReportRun ) -> None: @@ -373,6 +376,35 @@ def test_a_simulation_run_is_listed_unless_a_pending_or_running_report_run_names assert _baseline_listed(store, seeded_run) +def test_a_report_run_keeps_its_reform_run_out_of_the_sweep_until_it_finishes( + store, seeder, seeded_run: SeededReportRun +) -> None: + report_run_id = seeded_run.run_ids.report_run_id + reform_id = seeded_run.run_ids.reform_simulation_run_id + _set_run( + seeder, + "report_runs", + report_run_id, + status="running", + modal_call_id="fc-coordinator", + started_at=NOW, + ) + _set_run( + seeder, + "simulation_runs", + reform_id, + status="running", + modal_call_id="fc-reform", + started_at=NOW, + ) + # The running report run names it as its reform, so its coordinator + # judges the run. + assert not _simulation_run_listed(store, reform_id) + + _set_run(seeder, "report_runs", report_run_id, status="failed", completed_at=CUTOFF) + assert _simulation_run_listed(store, reform_id) + + def test_a_run_with_no_call_is_listed_and_failed_while_it_still_has_none( store, seeder, seeded_run: SeededReportRun ) -> None: diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py index 435fc6461..a4e1ada45 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/stage12_runtime/run_sweep.py @@ -18,7 +18,7 @@ pending meanwhile, so a pre-empted coordinator's report run is never failed. Its children are cancelled and started again under new calls, and it starts them only once Modal runs it, so the sweep judges a simulation run only once -no pending or running report run combines it. +no pending or running report run names it as its baseline or reform. """ from __future__ import annotations @@ -96,7 +96,7 @@ def fail_stuck_run( It must still have its listed status and call, or no call if it was listed with none; a simulation run must still have no pending or - running report run combining it. + running report run naming it. """ ... @@ -112,8 +112,7 @@ class SweepSummary: ``failed_for_age`` runs were failed for exceeding the maximum age. ``left`` counts listed runs whose conditional failure changed nothing, because they were claimed, finished, moved to another call, had a call recorded, - or a pending or running report run started combining them after they - were listed. + or a pending or running report run named them after they were listed. """ checked: int diff --git a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py index 1c2fe8e0e..4c03fabdd 100644 --- a/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py +++ b/projects/policyengine-simulation-executor/tests/test_stage12_run_sweep.py @@ -144,7 +144,7 @@ def test_the_sweep_lists_runs_older_than_its_minimum_age() -> None: def test_report_runs_are_swept_before_simulation_runs_are_listed() -> None: - # A simulation run is listed only once no report run combining it is + # A simulation run is listed only once no report run naming it is # running, so failing a dead coordinator's report run first lets the same # sweep reach its children. report_run = _run("fc-coordinator", RunKind.REPORT_RUN) From 046e317f06cac6d9345667d68d26d2dede69207a Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 6 Nov 2026 21:43:24 +0400 Subject: [PATCH 15/17] Test a report run that names a simulation run only as its reform The sweep's store tests now also check that a simulation run a pending or running report run names only as its reform is not failed, and that a report run with no reform run keeps only its baseline out of the sweep. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_report_run_sweep_store.py | 124 +++++++++++++++++- 1 file changed, 120 insertions(+), 4 deletions(-) diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py index 68f65f1e8..66f49d053 100644 --- a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -315,8 +315,22 @@ def sweep(run_store): assert seeder.row("simulation_runs", reform_id)["status"] == "failed" -def _second_report_run(seeder, seeded_run: SeededReportRun, status): - """Insert another report run of the same report, naming the same two runs.""" +_SAME_RUN = object() + + +def _second_report_run( + seeder, + seeded_run: SeededReportRun, + status, + *, + baseline_simulation_run_id=_SAME_RUN, + reform_simulation_run_id=_SAME_RUN, +): + """Insert another report run of the same report. + + It names the first report run's simulation runs unless given others; a + null reform run is a report with only a baseline. + """ first = seeder.row("report_runs", seeded_run.run_ids.report_run_id) second_id = uuid4() with seeder.engine.begin() as connection: @@ -324,8 +338,16 @@ def _second_report_run(seeder, seeded_run: SeededReportRun, status): insert(seeder.table("report_runs")).values( id=second_id, report_id=first["report_id"], - baseline_simulation_run_id=first["baseline_simulation_run_id"], - reform_simulation_run_id=first["reform_simulation_run_id"], + baseline_simulation_run_id=( + first["baseline_simulation_run_id"] + if baseline_simulation_run_id is _SAME_RUN + else baseline_simulation_run_id + ), + reform_simulation_run_id=( + first["reform_simulation_run_id"] + if reform_simulation_run_id is _SAME_RUN + else reform_simulation_run_id + ), status=status, trigger="manual", country_package_version=first["country_package_version"], @@ -336,6 +358,32 @@ def _second_report_run(seeder, seeded_run: SeededReportRun, status): return second_id +def _finished_simulation_run(seeder, seeded_run: SeededReportRun): + """Insert a succeeded simulation run of the seeded baseline's simulation.""" + baseline = seeder.row( + "simulation_runs", seeded_run.run_ids.baseline_simulation_run_id + ) + run_id = uuid4() + copied = { + name: value + for name, value in baseline.items() + if name not in {"id", "created_at", "updated_at"} + } + with seeder.engine.begin() as connection: + connection.execute( + insert(seeder.table("simulation_runs")).values( + { + **copied, + "id": run_id, + "status": "succeeded", + "started_at": NOW, + "completed_at": NOW, + } + ) + ) + return run_id + + def _simulation_run_listed(store, simulation_run_id): return any( run.run_id == simulation_run_id @@ -449,3 +497,71 @@ def test_a_simulation_run_a_report_run_now_names_is_not_failed( ) assert write == RunWrite(changed=False, status=RunStatus.RUNNING) + + +@pytest.mark.parametrize("second_status", ["pending", "running"]) +def test_a_reform_run_a_report_run_now_names_is_not_failed( + store, seeder, seeded_run: SeededReportRun, second_status +) -> None: + run_ids = seeded_run.run_ids + _set_run( + seeder, + "simulation_runs", + run_ids.reform_simulation_run_id, + status="running", + modal_call_id="fc-reform", + started_at=NOW, + ) + _set_run( + seeder, + "report_runs", + run_ids.report_run_id, + status="failed", + completed_at=CUTOFF, + ) + (reform_run,) = [ + run + for run in store.list_active_runs( + RunKind.SIMULATION_RUN, active_since_before=CUTOFF + ) + if run.run_id == run_ids.reform_simulation_run_id + ] + # After the listing, another report run names it, as its reform only. + _second_report_run( + seeder, + seeded_run, + second_status, + baseline_simulation_run_id=_finished_simulation_run(seeder, seeded_run), + ) + + write = store.fail_stuck_run( + reform_run, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ) + + assert write == RunWrite(changed=False, status=RunStatus.RUNNING) + + +def test_a_report_run_with_only_a_baseline_keeps_only_its_baseline_out( + store, seeder, seeded_run: SeededReportRun +) -> None: + run_ids = seeded_run.run_ids + _running(seeder, seeded_run) + _set_run( + seeder, + "simulation_runs", + run_ids.reform_simulation_run_id, + status="running", + modal_call_id="fc-reform", + started_at=NOW, + ) + _set_run( + seeder, + "report_runs", + run_ids.report_run_id, + status="failed", + completed_at=CUTOFF, + ) + _second_report_run(seeder, seeded_run, "running", reform_simulation_run_id=None) + + assert not _baseline_listed(store, seeded_run) + assert _simulation_run_listed(store, run_ids.reform_simulation_run_id) From 8d2663fbb378d7cd0a0c4150a8d0687b0436d588 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 6 Nov 2026 22:00:36 +0400 Subject: [PATCH 16/17] Test a simulation run no report run names, and say what the sweep can't see - A running simulation run that no report run names, as one the simulation service starts on its own would be, is listed and failed by the sweep. The rebuilt history lost that case when the member rows it deleted went away. - `fail_stuck_run` says a report run naming the simulation run counts once its insert has committed before the update starts: PostgreSQL makes neither statement wait for the other. - The tests' extra simulation run copies only the columns that say what it is, and a second report run that is failed gets its completion time, as the table's check requires. Co-Authored-By: Claude Opus 5.5 --- .../report_run_store.py | 4 +- .../tests/test_report_run_sweep_store.py | 71 +++++++++++++++---- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py index 139cf83fb..9e5edeb1c 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_store.py @@ -376,7 +376,9 @@ def fail_stuck_run( finished, or moved to another call, that the simulation service recorded a call on, or that a pending or running report run has named since it was listed, is left as it is, so concurrent sweeps change it - once. + once. A report run naming it counts once its insert has committed, + before this update starts: the database doesn't make an insert in + progress wait for this update, or this update for it. """ table = RUN_TABLES[run.kind] same_call = ( diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py index 66f49d053..b735804f2 100644 --- a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -352,38 +352,51 @@ def _second_report_run( trigger="manual", country_package_version=first["country_package_version"], policyengine_version=first["policyengine_version"], - **({"completed_at": CUTOFF} if status == "succeeded" else {}), + **( + {"completed_at": CUTOFF} + if status in {"succeeded", "failed"} + else {} + ), ) ) return second_id -def _finished_simulation_run(seeder, seeded_run: SeededReportRun): - """Insert a succeeded simulation run of the seeded baseline's simulation.""" +# What another simulation run of the seeded baseline's simulation copies. +SIMULATION_RUN_DEFINITION = ( + "simulation_id", + "year", + "tax_benefit_model_version_id", + "dataset_id", + "country_package_version", + "policyengine_version", +) + + +def _another_simulation_run(seeder, seeded_run: SeededReportRun, **state): + """Insert another simulation run of the seeded baseline's simulation.""" baseline = seeder.row( "simulation_runs", seeded_run.run_ids.baseline_simulation_run_id ) run_id = uuid4() - copied = { - name: value - for name, value in baseline.items() - if name not in {"id", "created_at", "updated_at"} - } with seeder.engine.begin() as connection: connection.execute( insert(seeder.table("simulation_runs")).values( - { - **copied, - "id": run_id, - "status": "succeeded", - "started_at": NOW, - "completed_at": NOW, - } + id=run_id, + **{name: baseline[name] for name in SIMULATION_RUN_DEFINITION}, + **state, ) ) return run_id +def _finished_simulation_run(seeder, seeded_run: SeededReportRun): + """Insert a succeeded simulation run of the seeded baseline's simulation.""" + return _another_simulation_run( + seeder, seeded_run, status="succeeded", started_at=NOW, completed_at=NOW + ) + + def _simulation_run_listed(store, simulation_run_id): return any( run.run_id == simulation_run_id @@ -565,3 +578,31 @@ def test_a_report_run_with_only_a_baseline_keeps_only_its_baseline_out( assert not _baseline_listed(store, seeded_run) assert _simulation_run_listed(store, run_ids.reform_simulation_run_id) + + +def test_a_simulation_run_no_report_run_names_is_listed_and_failed( + store, seeder, seeded_run: SeededReportRun +) -> None: + # A simulation run the simulation service starts on its own, with no + # report run naming it, is the sweep's to judge from the start. + unnamed_id = _another_simulation_run( + seeder, + seeded_run, + status="running", + modal_call_id="fc-unnamed", + started_at=NOW, + ) + unnamed = ActiveRun( + kind=RunKind.SIMULATION_RUN, + run_id=unnamed_id, + status=RunStatus.RUNNING, + modal_call_id="fc-unnamed", + ) + + listed = _simulation_run_listed(store, unnamed_id) + failed = store.fail_stuck_run( + unnamed, error_message=ENDED_MESSAGE, failed_at=CUTOFF + ) + + assert listed + assert failed == RunWrite(changed=True, status=RunStatus.FAILED) From c07edccbed630f8884c3923f435b64c382fbb717 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 6 Nov 2026 22:10:39 +0400 Subject: [PATCH 17/17] Fail the run as listed in the unnamed-run test, and name the sweep in docs The test of a simulation run no report run names now fails the run the sweep listed, so it also checks the listing's status and call. The persistence package's README and the run tables' module docstring name the stuck-run sweep among the Modal worker role's readers and writers. Co-Authored-By: Claude Opus 5.5 --- .../README.md | 7 ++-- .../report_run_tables.py | 6 ++-- .../tests/test_report_run_sweep_store.py | 32 ++++++++++++------- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/libs/policyengine-stage12-persistence/README.md b/libs/policyengine-stage12-persistence/README.md index 40cb5aee3..a0bf80d21 100644 --- a/libs/policyengine-stage12-persistence/README.md +++ b/libs/policyengine-stage12-persistence/README.md @@ -11,9 +11,10 @@ revisions and live schema. This package never changes a schema. SPM receipts and data version, `simulation_runs`, and the result tables, the two labour supply tables included. The v2 Modal coordinator and workers write them as the Modal worker role (`policyengine_v2_modal_worker`), for a - call that names a report run. The mappings declare only the columns the - coordinator's and workers' statements read or write, and each `UPDATE` - sets only columns the role may update. Every status write is one `UPDATE` + call that names a report run, and the scheduled stuck-run sweep fails the + runs nothing will finish, as the same role. The mappings declare only the + columns those statements read or write, and each `UPDATE` sets only + columns the role may update. Every status write is one `UPDATE` conditional on the run's status; a report run's completion sets its own result columns and inserts its result rows in the same transaction. diff --git a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_tables.py b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_tables.py index ab44a6f3d..99cb71418 100644 --- a/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_tables.py +++ b/libs/policyengine-stage12-persistence/src/policyengine_stage12_persistence/report_run_tables.py @@ -1,9 +1,9 @@ """SQLAlchemy mappings of the Public API's run tables that report calls update. ``policyengine-api`` owns these tables and every schema migration. The -mappings declare only the columns the coordinator and workers read or write -as the Modal worker role, so they can issue typed DML; runtime code never -emits DDL. +mappings declare only the columns the coordinator, the workers and the +stuck-run sweep read or write as the Modal worker role, so they can issue +typed DML; runtime code never emits DDL. """ from __future__ import annotations diff --git a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py index b735804f2..492090f74 100644 --- a/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py +++ b/libs/policyengine-stage12-persistence/tests/test_report_run_sweep_store.py @@ -592,17 +592,25 @@ def test_a_simulation_run_no_report_run_names_is_listed_and_failed( modal_call_id="fc-unnamed", started_at=NOW, ) - unnamed = ActiveRun( - kind=RunKind.SIMULATION_RUN, - run_id=unnamed_id, - status=RunStatus.RUNNING, - modal_call_id="fc-unnamed", - ) - listed = _simulation_run_listed(store, unnamed_id) - failed = store.fail_stuck_run( - unnamed, error_message=ENDED_MESSAGE, failed_at=CUTOFF - ) + listed = [ + run + for run in store.list_active_runs( + RunKind.SIMULATION_RUN, active_since_before=CUTOFF + ) + if run.run_id == unnamed_id + ] + failed = [ + store.fail_stuck_run(run, error_message=ENDED_MESSAGE, failed_at=CUTOFF) + for run in listed + ] - assert listed - assert failed == RunWrite(changed=True, status=RunStatus.FAILED) + assert listed == [ + ActiveRun( + kind=RunKind.SIMULATION_RUN, + run_id=unnamed_id, + status=RunStatus.RUNNING, + modal_call_id="fc-unnamed", + ) + ] + assert failed == [RunWrite(changed=True, status=RunStatus.FAILED)]