Skip to content

fix(cron): malformed job records no longer freeze the scheduler (class fix) - #61723

Merged
teknium1 merged 4 commits into
mainfrom
salvage/cron-freeze-cluster
Jul 10, 2026
Merged

fix(cron): malformed job records no longer freeze the scheduler (class fix)#61723
teknium1 merged 4 commits into
mainfrom
salvage/cron-freeze-cluster

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

A single malformed record in cron/jobs.json can no longer freeze the whole scheduler. The due scan trusted persisted job shape; one bad record (missing id, non-dict schedule, unparseable next_run_at/last_run_at) raised mid-loop before save_jobs(), discarding every sibling's recovered state — the scheduler re-crashed identically every tick, forever.

Consolidates three contributor fixes (cherry-picked, authorship preserved) and adds the structural guard none of them had:

Changes

  • cron/jobs.py: three shape normalizations at the top of _get_due_jobs_locked + per-job containment guard
  • tests/cron/test_jobs.py: three contributor regression tests + containment test; restored test_repeated_concurrent_runs_accumulate_completed_count to its original class (accidentally re-parented by the fix(cron): malformed next_run_at no longer freezes the scheduler #61581 diff)
  • scripts/release.py: AUTHOR_MAP for hydracoco7

Validation

Before After
One malformed job whole tick aborts, scheduler frozen job skipped/repaired, siblings run
E2E (all 5 malformed shapes + healthy sibling in one store) freeze single tick contains all five, repairs persisted, tick 2 stable
Tests 670 cron tests green

Infographic

infographic

bassis ho and others added 4 commits July 9, 2026 17:09
A cron record authored by a direct jobs.json edit that bypassed
add_job() can lack an "id" key (older writers used "job_id"). Every
site in _get_due_jobs_locked indexes job["id"] eagerly — both the
logging helpers (job.get("name", job["id"]) evaluates the default
argument unconditionally) and the 'for rj in raw_jobs: if rj["id"] ==
job["id"]' persistence loops. A single malformed record therefore
raised KeyError mid-tick, aborting the entire scan before save_jobs()
ran. Result: healthy jobs' fast-forwarded next_run_at was computed in
memory then discarded on the exception unwind, freezing the whole
profile's scheduler in a per-minute loop (observed dormant for weeks).

Fix: normalize id-less records at the top of _get_due_jobs_locked before
anything keys off job["id"] — recover the id from a drifted "job_id"
key when present, else synthesize one via uuid4, and persist. This
repairs the whole bug class at the source rather than guarding each of
the ~12 downstream index sites.

Adds a regression test that fails with KeyError on the current code and
passes with the fix, asserting a healthy sibling job is still returned
when an id-less record shares the store.
A job record in jobs.json can have a non-dict 'schedule' value (null, string,
etc.) from direct edit or old writers.

In _get_due_jobs_locked:
  schedule = job.get('schedule', {})
  kind = schedule.get('kind')

This (and direct schedule['kind'] in compute_next_run etc.) raises and
aborts the entire due-jobs scan before save_jobs() or advancing next_run_at
for healthy jobs. Exactly the same failure mode as the id-less job P1.

Fix: normalize non-dict schedules to {} early (before any use), matching the
defense added for id-less records. Also added defensive guards in compute
functions.

Added regression test that a bad schedule does not crash and healthy sibling
is still returned.

Refs similar pattern in #61382.
One bad next_run_at value in jobs.json aborts the due-jobs scan with
ValueError from fromisoformat, before any save_jobs, so siblings lose
progress (fast-forwards etc).

Early normalization in _get_due_jobs_locked + defensive parses in
compute_next_run / _recoverable_oneshot_run_at.

Added test_bad_next_run_at_does_not_crash_or_block_sibling_jobs.
… class

Structural completion of the malformed-job freeze fixes (#61382 id-less,
#61525 non-dict schedule, #61581 bad next_run_at): wrap the per-job body
of _get_due_jobs_locked in try/except so any FUTURE malformed-field
variant degrades to skipping that one job for the tick instead of
aborting the scan before save_jobs() and freezing the whole profile's
scheduler.

Also: restore test_repeated_concurrent_runs_accumulate_completed_count
to TestMarkJobRunConcurrency (accidentally re-parented by the #61581
diff), add a containment regression test, and AUTHOR_MAP for hydracoco7.

E2E: one jobs.json carrying all five malformed shapes (drifted job_id,
missing id, null schedule, garbage next_run_at, non-string last_run_at)
plus a healthy sibling — single tick contains all five, sibling fires,
repairs persist, second tick stable. 670 cron tests green.
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cron Cron scheduler and job management labels Jul 10, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #61382 (id-less job repair, @hydracoco7), #61525 (non-dict schedule, @necoweb3), #61581 (malformed next_run_at/last_run_at, @necoweb3) — this PR consolidates all three (authorship preserved) and adds the structural per-job try/except containment guard none of them had. Also related to the closed superset #51267 (per-job quarantine) and the narrowest open #50377 (guards the exact fromisoformat parse). Same silent-whole-cron-scheduler-freeze family; this is the canonical class-fix. Maintainer to pick between this consolidation and the individual source PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P1 High — major feature broken, no workaround type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants