Skip to content

A run stuck at 'pending' can be neither resumed nor retired #734

Description

@Masterplanner25

Summary

pending is in neither REHYDRATABLE_RUN_STATUSES nor TERMINAL_RUN_STATUSES. So a run record left in that state is invisible to both halves of the machinery that would deal with it:

  • rehydration only adopts waiting / running / retry_scheduled, so nothing picks it up
  • workflow cleanup and the store's record-cap eviction both filter on TERMINAL_RUN_STATUSES (store.py:460, :619, :641), so nothing retires it

It leaks, permanently, and LocalWorkflowStore.list_runs() is linear in accumulated records (#380), so leaked ones keep costing.

Reproduction

store = create_workflow_store(backend="local", root=root)
rec = store.create_run(run_id="p1", graph_id="p1", workflow_name="w", execution_kind="workflow")

rec.status                                   -> 'pending'
store.list_rehydratable_runs()                -> []
rec.status in TERMINAL_RUN_STATUSES           -> False

Reachability

run_workflow calls create_run, then claim_run, then sets the record running and saves. A process that dies inside that window leaves the record at pending. The window is small, which is why this is severity:low rather than higher — but nothing narrows it and nothing recovers from it.

How it surfaced

models.py says of the status vocabulary: "Partitioned deliberately: every status is terminal or not, and the two sets below must stay disjoint. tests/test_run_status_vocabulary.py drives off this tuple, so a ninth status fails the suite until it is classified."

Three things there were untrue, and they held each other up:

  1. tests/test_run_status_vocabulary.py does not exist. The real file is tests/test_run_cancellation.py.
  2. Its partition test could not fail. test_every_status_is_terminal_or_not_and_never_both iterated RUN_STATUSES asserting status in set(RUN_STATUSES) — true by construction. The disjointness half was real; the coverage half was a tautology.
  3. So "a ninth status fails the suite until it is classified" was false. Verified: adding a ninth status classified into neither set fails only the count assertion beside it, which a maintainer resolves by bumping the count — and the suite is then green with the status still unclassified.

Fixing the tautology is what surfaced pending, which has been violating the documented partition all along.

Fix direction

Decide which half pending belongs to, rather than adding a third category:

  • terminal — cleanup retires an abandoned pending run on its normal retention schedule. Simple, and matches the fact that nothing can continue a run that never started.
  • rehydratable — adoption picks it up and starts it. Better if a created-but-unstarted run should still run, but adoption currently means "register the graph so someone can carry it on", and there is no graph state for a run that never began.

Terminal looks right, and is the smaller change.

The partition test is corrected in the #733 branch and currently records pending as a known exception pointing here, so the guard is honest in the meantime and a tenth status cannot hide behind it.

Affected versions

v5.9.0 (current), and every version since the vocabulary was consolidated.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions