Skip to content

test(execution): stop processor cancel tests timing out behind a full GC per item - #9949

Merged
Pfannkuchensack merged 1 commit into
invoke-ai:mainfrom
Pfannkuchensack:fix/session-cancel-race
Oct 19, 2026
Merged

Pfannkuchensack merged 1 commit into
invoke-ai:mainfrom
Pfannkuchensack:fix/session-cancel-race

Conversation

@Pfannkuchensack

@Pfannkuchensack Pfannkuchensack commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Summary

The flaky session-processor cancel/retry tests in #9948 never lost an event; the processor reached the item too late for the test's wait. DefaultSessionProcessor._process runs a full gc.collect() before every item it claims, and late in an xdist session that walks a heap of 3–5 million objects. Every failure cited in the issue was a 5 s wait. The If failures came from #9910's branch, which predated the 30 s ceiling from #9940, and the flat For test still waited 5 s on main.

  • The session-processor test directory now skips the processor's per-item collection. The fixture rebinds the module's gc, so the global gc.collect is untouched. None of these tests check memory.
  • test_for_loop_processor_sqlite.py gets one 30 s hang ceiling, _PROCESSOR_TIMEOUT, like test_if_processor_sqlite.py. It is the wait_for_status default and the shutdown join timeout, and it replaces the per-test test_timeout locals and the remaining 5 s waits.

A cancel issued from on_after_run_node runs on the runner thread, and the guarded status transition always emits the canceled event, so no production change is needed.

Related Issues / Discussions

Closes #9948

QA Instructions

  • Reproduce the CI failure: make the processor module's gc.collect sleep 6 s and run pytest tests/app/services/session_processor/test_for_loop_processor_sqlite.py -k flat_for_cancel. Before this PR it fails at wait_for_status(1, 'canceled') (line 1696), the same as CI. After this PR, with every claim attempt delayed 6 s instead, the flat For test (12 s) and the If early-cancellation test (6 s) pass.
  • Measured cost of the per-item collection in a local full run (pytest tests -n 4 -m "not slow"): 2.96 s mean and 3.87 s max per call in the For/If SQLite tests, against 0.66 s per item in a fresh single-test process. A loaded two-worker Windows runner ends up above 5 s.
  • pytest tests/app/services/session_processor: 119 passed. The run took 43 s before and 11 s after; the flat For test dropped from about 1.8 s to under 0.21 s. Each changed file also passes on its own.
  • ruff check and ruff format --check (0.11.2) pass on the changed files.
  • Not verified locally: behavior under the CI xdist matrix. The local full run's failures were all outside this directory and environmental (LFS pointer files in a fresh worktree, a private HF mirror refused by the SSRF guard).

Review

No material findings. Each processor item still pays a full gc.collect() in production too. That is left for a separate, measured change.

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Meaningful regression coverage added / updated where needed; obsolete tests/code removed
  • Persisted-state and API changes include required migrations / compatibility validation
  • Relevant performance/efficiency opportunities considered; material claims have evidence
  • Material review findings resolved and relevant checks rerun
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

… GC per item

Skip the worker's per-item gc.collect() in the session-processor tests; late in an xdist session it took seconds, past the 5 s waits that failed.
Give the For SQLite tests one 30 s hang ceiling, like the If tests.
@github-actions github-actions Bot added the python-tests PRs that change python tests label Oct 19, 2026
@Pfannkuchensack
Pfannkuchensack merged commit cc3502d into invoke-ai:main Oct 19, 2026
17 checks passed
@Pfannkuchensack
Pfannkuchensack deleted the fix/session-cancel-race branch October 19, 2026 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python-tests PRs that change python tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: session-processor cancel/retry tests intermittently never see the canceled status on CI

2 participants