Stop the stale-job reaper from revoking jobs that are created but not yet started - #1355
Conversation
Jobs can be created without starting them immediately — for example, to pre-configure several jobs and launch them later. Such a job sits in the CREATED state until a user enqueues it. The periodic stale-job reaper (check_stale_jobs) selected jobs by JobState.running_states(), which included CREATED. A pre-configured job older than the staleness cutoff was therefore revoked before it ever ran. A CREATED job has no Celery task and no async (NATS/Redis) resources, so it has nothing to reap or reconcile. enqueue() flips it to PENDING the moment it is dispatched, and a job stuck mid-dispatch is already PENDING (still reaped). Removing CREATED from running_states() also keeps the lost-images reconciler and the async snapshot check from touching jobs that have not started. Closes #1354 Co-Authored-By: Claude <noreply@anthropic.com>
✅ Deploy Preview for antenna-preview canceled.
|
✅ Deploy Preview for antenna-ssec canceled.
|
|
Warning Review limit reached
More reviews will be available in 33 minutes and 52 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6b1df09 to
df5af6f
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the job “stale/hung” cleanup logic so that jobs left in CREATED (pre-configured but not enqueued) are not considered “running” and therefore won’t be revoked by periodic reaping tasks. It also adds a regression test to lock in this behavior, but includes an additional migration that appears unrelated to the stated PR scope.
Changes:
- Remove
CREATEDfromJobState.running_states()so reapers/reconcilers don’t select created-but-unstarted jobs. - Add a regression test asserting a stale
CREATEDjob is left untouched bycheck_stale_jobs. - Add a migration that globally enables
async_pipeline_workersfor all existing projects (appears out of scope vs. PR description/title).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ami/jobs/models.py | Updates the definition of “running” job states so CREATED jobs are excluded from reaping/reconciliation selection. |
| ami/jobs/tests/test_update_stale_jobs.py | Adds regression coverage ensuring CREATED-but-unstarted jobs are not reaped. |
| ami/main/migrations/0094_enable_async_pipeline_workers.py | Introduces a global feature-flag rollout migration for async pipeline workers (scope discrepancy with PR metadata). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ning_states Shorten the running_states() note and rewrite the finalizable_states() comment so it no longer describes itself as "running_states() minus ..." — that framing broke once CREATED left running_states() while finalizable_states() still includes it. Describe each set directly. Co-Authored-By: Claude <noreply@anthropic.com>
|
Claude says: Retracted — this comment was wrong. Please disregard it. I originally argued that a production incident on 2026-07-29 was a counter-case to this PR's rationale, on the grounds that jobs sitting in
I believe the error came from conflating a stage status of So this PR's rationale for stopping at The point I was reaching for — that a no-progress timer cannot distinguish "this job is stuck" from "this job is fine and something downstream of it has failed" — is real, but it belongs to the reaper's predicate generally and not to this change. It is being tracked in #1383. |
Summary
Jobs can be created without starting them immediately — for example, to pre-configure several jobs and launch them at a later time. A job created this way waits in the
CREATEDstate until someone starts it. A periodic background task revokes jobs that look stale or hung, and it was also revoking these created-but-unstarted jobs, so a pre-configured job would be cancelled before it ever ran. This change leavesCREATEDjobs alone, so they wait to be started instead of being reaped.Closes #1354
List of Changes
CREATEDfromJobState.running_states(), the set the reaper and reconcilers select jobs from.CREATEDjob past the staleness cutoff is left untouched.test_skips_created_but_unstarted_jobsinami/jobs/tests/test_update_stale_jobs.py.Detailed Description
The stale-job reaper (
check_stale_jobsinami/jobs/tasks.py) finds jobs that are still in a running state and whoseupdated_atis older thanJob.STALLED_JOBS_MAX_MINUTES(10 minutes), then revokes them. It selected candidates viaJobState.running_states(), which includedCREATED. A job that was pre-configured and left for later therefore qualified once it was older than the cutoff, and was revoked.A
CREATEDjob has no Celery task and no async (NATS/Redis) resources, so there is nothing to reap or reconcile.Job.enqueue()flips a job toPENDINGthe moment it is dispatched, and a job that gets stuck mid-dispatch is alreadyPENDING— so genuinely stuck jobs are still caught. RemovingCREATEDfromrunning_states()also keeps the lost-images reconciler and the async snapshot check (the other callers ofrunning_states()) from touching jobs that have not started.finalizable_states()is intentionally left unchanged — it keepsCREATEDbecause a synchronous job can legitimately transitionCREATED → SUCCESSthrough the normal completion path; that is separate from time-based reaping.The full stale-job test suite passes (7/7).
Out-of-scope observations (left for follow-up)
While sweeping for code that depends on
running_states(), I found a few pre-existing places in the frontend that classify theCREATEDstate independently of the backend. They are not consumers ofrunning_states()(which is Python-only), so this change does not affect them, and they are not modified here. Noting them for a later pass:ui/src/data-services/models/capture-set.tsandui/src/data-services/models/capture-details.ts—hasJobInProgresscounts aCREATEDjob as "in progress". A created-but-unstarted job has not been dispatched yet, so this may overstate activity. It could also be intentional (signalling that a job is already set up for the capture set, to avoid duplicates) — worth a deliberate decision rather than a silent change.ui/src/data-services/models/job.ts—canCancelonly allowsSTARTED/PENDING, but the backend'sfinalizable_states()permits cancelling aCREATEDjob too. The result is a harmless asymmetry: the cancel button is hidden forCREATEDjobs even though the backend would accept the request.These are flagged only; no behavior change is intended in this PR.