Skip to content

Add regression test for the GET /api/v1/runs endpoint - #540

Draft
LudovicoRighi wants to merge 3 commits into
mainfrom
lrighi/regression-test
Draft

Add regression test for the GET /api/v1/runs endpoint#540
LudovicoRighi wants to merge 3 commits into
mainfrom
lrighi/regression-test

Conversation

@LudovicoRighi

@LudovicoRighi LudovicoRighi commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

#539 moved /runs serialization off the event loop into a background thread (asyncio.to_thread) to stop it blocking the loop on large, unfiltered queries. This fixes the blocking, but introduces a data race.

Why #539 is broken

Before #539, runs_to_jso ran synchronously on the event loop. It contains no await, so it was atomic with respect to every other coroutine — nothing could mutate a run mid-serialization. Slow, but safe.

Under to_thread, serialization runs concurrently with the event loop, which keeps driving the run state machine (_process_updatesApsis._transition) over the same Run objects being serialized. run_to_summary_jso iterates run.times.items() while Run._transition mutates that dict in place (self.times[state.name] = ...). The result:

RuntimeError: dictionary changed size during iteration

The likelihood scales with exactly the case #539 targets — many runs → long serialization window → more transitions land inside it — so it's not a corner case. (run.meta is aliased into the JSO and also mutated in place by _transition, so it's exposed the same way.)

The test

test/unit/test_runs_endpoint_race.py drives the real api.runs handler in process (mock DB — no server, network, or real Apsis) with a background task transitioning runs, and asserts the handler never crashes.

  • Fails on current main with the RuntimeError above.
  • Passes if 80a547a is reverted.

The race is probabilistic per call, so the test serves a large run set and repeats the call several times; at these sizes a single buggy call crashes with probability well above 1/2, making a false pass negligible while a correct (atomic) implementation passes every call. It reproduces at Python's default thread-switch interval — no scheduler tuning.

Scope

This PR adds only the failing test to pin down the regression. The fix is deliberately left to follow-up discussion.

cc @gusostow

@LudovicoRighi

Copy link
Copy Markdown
Collaborator Author

The test fails on latest main (which includes 80a547a)

FAILED test/unit/test_runs_endpoint_race.py::test_runs_endpoint_survives_concurrent_transitions - AssertionError: /runs crashed under concurrent transitions: RuntimeError('dictionary changed size during iteration')
assert RuntimeError('dictionary changed size during iteration') is None

@LudovicoRighi

LudovicoRighi commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

And passes after reverting 80a547a

@LudovicoRighi
LudovicoRighi force-pushed the lrighi/regression-test branch from 00664e2 to 1c4b452 Compare July 10, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants