Repository navigation
Make worker liveness heartbeat-based so executions stop being falsely finalized - #14
Merged
Merged
Conversation
Liveness for RUNNING executions is now heartbeat-based instead of pid-based. Workers refresh last_heartbeat_at every ADMIN_RUNNER_HEARTBEAT_INTERVAL (default 30 s) on a daemon thread, and the stale-run sweeper finalizes RUNNING rows whose heartbeat is older than ADMIN_RUNNER_HEARTBEAT_STALE_AFTER (default 180 s). Rationale: a pid is only meaningful inside its own PID namespace. In container deployments (web pods serving the admin, a separate task cluster pod) the old sweep checked pids from the wrong namespace, so *every* watched long-running command was falsely finalized as FAILED 'worker lost' minutes into a healthy run; conversely, a recycled pid in the cluster pod kept dead rows RUNNING forever. The heartbeat flows through the shared database and stays meaningful across pods. Rows recorded before this change keep the legacy pid rules, now with a PID-reuse guard (a pid occupied by a process older than the execution counts as dead, not alive). Also: - stop/force_stop signals are only sent when the worker ran on the local host (worker_host recorded at claim); the cooperative stop flag remains the cross-host mechanism. Previously a Force Stop from a web pod silently no-oped - or could signal an unrelated local process on pid collision. - finalize_stale looks up the django-q2 Task by id (pk) instead of name: task_id stores the UUID, Task.name holds a humanhash, so timeout attribution never matched. - django-q2 tasks are enqueued with ack_failure=True so failed tasks are not redelivered after the broker retry window; the per-task retry=-1 option was dropped (django-q2 silently ignores unknown q_options keys) and tasks get a readable 'admin-runner:<pk>' name. - sys.stdin is redirected to an EOF, non-tty StringIO while a command runs: interactive prompts (input()/click) now fail fast with EOFError instead of hanging the worker forever (commands ran with a TTY- reporting stdout, so prompts believed a terminal was attached). Adds ADMIN_RUNNER_HEARTBEAT_INTERVAL and ADMIN_RUNNER_HEARTBEAT_STALE_AFTER settings (docs updated).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Liveness for RUNNING executions was pid-based, but a pid is only meaningful inside its own PID namespace. In container deployments (web pods serving the admin, a separate task-cluster pod) the stale sweep checked pids from the wrong namespace: every watched long-running command was falsely finalized as FAILED "worker lost" minutes into a healthy run, while recycled pids kept dead rows RUNNING forever. Liveness is now heartbeat-based (database-backed), which stays meaningful across pods.
Changes
CommandExecution: newworker_host,worker_started_at,last_heartbeat_atfields (migration included); worker refreshes the heartbeat everyADMIN_RUNNER_HEARTBEAT_INTERVAL(default 30 s) on a daemon threadADMIN_RUNNER_HEARTBEAT_STALE_AFTER(default 180 s); pre-heartbeat rows keep the legacy pid rules, now with a PID-reuse guard (a pid occupied by an older process counts as dead, not alive)stop()/force_stop()only send signals when the worker ran on the local host — previously a Force Stop from a web pod silently no-oped, or could signal an unrelated local process on pid collision; the cooperative stop flag remains the cross-host mechanismfinalize_stalelooks up the django-q2 Task by id (pk) instead of name:task_idstores the UUID,Task.nameholds a humanhash, so timeout attribution never matchedack_failure=True(no broker redelivery of failed tasks); the per-taskretry=-1option was dropped (django-q2 silently drops unknown q_options keys); tasks get a readableadmin-runner:<pk>namesys.stdinis redirected to an EOF, non-tty stream while a command runs: interactive prompts (input()/click) now fail fast withEOFErrorinstead of hanging the worker foreverTesting
uv run --group test pytest tests/→ 386 passed (newtests/test_heartbeat_liveness.pycovers heartbeat sweep verdicts, pid-reuse guard, host-gated signals, q2 attribution by id, stdin EOF guard;tests/test_runners.py/tests/test_cancellation.pyupdated to the new enqueue/signal semantics).ruff check+ruff format+pyrightclean.Risks / follow-ups
ADMIN_RUNNER_SWEEP_INTERVALand setADMIN_RUNNER_STALE_AFTERexplicitly.