Keep long-running tasks from going stale - #58
Merged
Merged
Conversation
Eager tasks carry the task ID on the request, not in the message headers, so `exit_session` bailed out and leaked the client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A single daemon thread pings all in-flight tasks so that tasks with a `stale_timeout` don't go stale while they are still working. Adds the `heartbeat_interval` and `stale_timeout` options to the system integrations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Starts on `task_prerun` and stops on `task_postrun`. The interval is resolved where the task is created so that it matches the task's `stale_timeout`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers sync and async tasks; `track` accepts `heartbeat_interval` and `stale_timeout`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Checks that a long running task's `updated` time advances while it runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`run_worker` blocks the test, so the task samples its own `updated` time either side of the sleep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only close the session that the signal handlers opened; an eager task inside a caller's `Session()` was closing it out from under them. Also read per-call heartbeat options back out of the headers, which is where they stay when `before_task_publish` doesn't run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A `@track`ed task kept `_taskbadger_system = None` when the integration was constructed after it was registered, so it never picked up the system's options. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keep the beat loop alive across unexpected errors, and round the derived stale timeout up so a sub-second interval can't produce 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Tasks with a
stale_timeoutgo stale unless something updates them, so today a long-running Celery or Procrastinate task has to ping itself from the task body.heartbeat_intervalmakes the worker do it — settable on the system integration, on the task, or per call.Design decisions the diff doesn't show:
task_prerun, so publishers never get one, and a forked prefork child builds its own (an inherited thread reportsis_alive() == False)._maybe_create_taskfor eager/canvas tasks — so it always matches thestale_timeoutit's protecting, instead of a worker inventing an interval for a timeout that was never set.stale_timeoutdefaults to2 × interval, the same ratiotaskbadger runalready uses.Worth a closer look:
exit_sessionownership (a23ada4).f8ea1cffixed a session leak for eager tasks but made them close a session they never opened — an eager task inside a caller'swith taskbadger.Session():popped the caller's frame, so the caller's__exit__raisedIndexError. The handlers now mark the request when they open the session. Those two commits are worth squashing if you'd rather the series bisect clean._taskbadger_systemtiming (4f4496c). It's now set before the idempotency return, so a@tracked task registered beforeProcrastinateSystemIntegration(...)inherits the system's options. That also restores therecord_task_argsinheritancetrack()'s docstring already promises — a behaviour change to existing functionality, deliberate.Both integration tests were verified to fail with
Heartbeat.startstubbed to a no-op, so they aren't vacuous passes. They ran against the real API with local Redis and Postgres (7 passed).Unrelated, noticed while testing: the Celery suite uses Redis DB 0, so messages from any other project on the same box can make the worker thread snapshot an unconfigured
Badgerbefore_bind_settingsruns and fail a handful of pre-existing tests. Pinningcelery_configto a dedicated DB would fix it; left alone here.