Skip to content

fix: gate background loops behind enable_background_workers - #298

Open
nishant-uxs wants to merge 2 commits into
OpenHands:mainfrom
nishant-uxs:fix/enable-background-workers-gate-286
Open

fix: gate background loops behind enable_background_workers#298
nishant-uxs wants to merge 2 commits into
OpenHands:mainfrom
nishant-uxs:fix/enable-background-workers-gate-286

Conversation

@nishant-uxs

Copy link
Copy Markdown

Problem

lifespan starts the scheduler, dispatcher, and watchdog unconditionally. Uvicorn runs lifespan once per worker process, so multi-worker / multi-replica deployments run replicas × --workers copies of each loop against the same DB. Watchdog copies fan out sandbox-API verifies (contributing to HTTP 429 → false FAILED, see #285) and put background CPU on webhook-serving processes.

Root cause

Ownership of background work was tied to process startup, not to a deployment-level singleton. There was no way for a request-serving worker to skip the loops.

Solution

Implement the preferred fix from #286:

  • New setting AUTOMATION_ENABLE_BACKGROUND_WORKERS (default true)
  • When false, this process starts zero of scheduler / dispatcher / watchdog
  • When true, starts exactly one of each (prior single-process behavior)

Extracted start_background_worker_tasks() so lifespan shutdown only awaits tasks that were actually started.

Operator model

Process role Setting
Local / single-process default true (no change)
Dedicated background owner true
Request-serving replicas false

This is an explicit single-owner gate, not leader election — enabling the flag on every worker still fans out (covered by a regression test so the invariant stays documented).

Why this approach

  • Matches the issue author's preferred fix (vs. claim-before-verify, which only softens watchdog fan-out)
  • No public API change; default preserves local/dev behavior
  • Minimal diff — no Temporal rewrite, no distributed lock redesign

Risks

  • Multi-worker production deploys must set the env var correctly on request servers; leaving the default on every worker retains today's duplication
  • Helm/compose charts are out of scope for this PR (config knob only)

Testing

  • uv run pytest tests/test_background_workers.py tests/test_config.py tests/test_health.py57 passed
  • Confirmed fail-before-fix: with the gate forced off, test_disabled_* and test_multi_worker_simulation_single_owner fail (3 tasks started when disabled)
  • ruff check / ruff format clean on touched files
  • pyright — 0 errors on app.py / config.py

Regression coverage

  • Disabled → no scheduler/dispatcher/watchdog
  • Enabled → exactly one of each
  • Multi-worker simulation (2 request + 1 owner) → one of each across the deployment
  • All workers enabled → still 3×3 (documents operator invariant)
  • Default true; env false respected

Fixes #286

Uvicorn runs lifespan once per worker, so multi-worker deployments were
starting N schedulers/dispatchers/watchdogs against the same DB. Add
AUTOMATION_ENABLE_BACKGROUND_WORKERS (default true) so request-serving
replicas can leave the loops to a single dedicated process.

Fixes OpenHands#286
@github-actions github-actions Bot added the type: fix A bug fix label Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Background scheduler, dispatcher and watchdog start once per uvicorn worker instead of once per deployment

1 participant