Skip to content

fix(scheduler): make stuck-job timeout env-overridable and never re-queue stuck prints - #163

Open
qjkme wants to merge 1 commit into
codeofaxel:mainfrom
qjkme:fix/scheduler-stuck-requeue
Open

qjkme wants to merge 1 commit into
codeofaxel:mainfrom
qjkme:fix/scheduler-stuck-requeue

Conversation

@qjkme

@qjkme qjkme commented Sep 14, 2026

Copy link
Copy Markdown

fix(scheduler): make stuck-job timeout env-overridable and never re-queue stuck prints

Summary

A PRINTING job over the hard-coded 2 h stuck-timeout was handed to
_requeue_or_fail, which reset it to QUEUED and re-dispatched the same
file
— without checking whether the printer was still physically printing
it. This PR makes the timeout env-overridable (default 12 h) and makes the
stuck branch permanently fail the job instead of re-queuing it.

Real-world damage (why "re-queue on timeout" is wrong)

2026-09-13, Snapmaker U1: a 2h05m print was judged stuck at the 2 h cap →
_requeue_or_fail reset the job to QUEUED and re-dispatched the same file →
the same object was physically printed 3 times, and the second plate dropped
onto the first plate's finished part.

The stuck-timeout is a queue-side guess ("printer may be disconnected or
hung"), not a machine verdict. The machine's own words are in
error_msg; the queue cannot know whether the printer is still mid-print.
Re-dispatching a job that may still be printing is the duplicate-print
accident. A genuinely hung machine needs a human to recover it; the queue's
job is to stop guessing on its behalf.

The dispatch-failure retry path (_requeue_or_fail for start_print failures,
printer errors, unregistered printers) is untouched — those re-queue decisions
are made on evidence the print never started, which is a different situation.

Changes

  • kiln/src/kiln/scheduler.py
    • _STUCK_JOB_TIMEOUT_SECONDS: 7200 (hard-coded) → env
      KILN_STUCK_JOB_TIMEOUT_SECONDS, default 43200 (12 h), read via the
      existing kiln.parse_float_env helper (same pattern as queue.py's
      KILN_STUCK_JOB_TIMEOUT_MINUTES).
    • Stuck branch: no _requeue_or_fail. Mirrors the existing
      permanent-failure path: pop retry bookkeeping, discard seen-printing,
      mark_failed, publish JOB_FAILED, record the resolution locally via
      _auto_record_outcome with contribute=False (the stuck-timeout says
      nothing about the model, so it never federates to the community pool),
      and append to tick()'s failed report so callers see the verdict.
    • Class docstring updated to match the new behavior.
  • kiln/tests/test_scheduler.py
    • New TestStuckJobTimeout:
      • stuck PRINTING job → status FAILED, tick() reports it in failed,
        and there is no second dispatch (same tick or any later tick);
      • exactly one JOB_FAILED event and no JOB_SUBMITTED (the requeue
        path's fingerprint);
      • env override: KILN_STUCK_JOB_TIMEOUT_SECONDS=1800 declares a
        30-min-old PRINTING job stuck (it is under the legacy 2 h cap);
      • garbage env value falls back to the 12 h default (no import crash);
      • valid env value is read at module load;
      • dispatch failure with retries remaining still re-queues (retry path
        preserved).
    • test_stuck_timeout_guess_contributes_nothing fixture updated
      (7300 s → 43300 s): with the new 12 h default, a 2h05m print is no
      longer over the timeout.

Related dead code (not changed in this PR)

queue.py already has a KILN_STUCK_JOB_TIMEOUT_MINUTES env knob feeding
check_stuck_jobs(), but nothing calls check_stuck_jobs() — the knob is
currently dead. This PR intentionally does not touch it (different module,
different timeout, and consolidating it deserves its own change); flagged
here for a follow-up: either wire check_stuck_jobs() into a poller or
remove the unused knob.

Test plan

  • pytest kiln/tests/test_scheduler.py — 85 passed (79 pre-existing +
    6 new; 1 pre-existing fixture updated for the new default)
  • ruff check kiln/src/kiln/scheduler.py kiln/tests/test_scheduler.py — clean
  • Full suite pytest kiln/tests/ -q -n auto — see CI run

Verification on a live install

The identical semantics change (env knob + never-requeue stuck branch) has
been running on a production kiln3d 1.4.1.1 install since 2026-09-14
(applied via a local patch; this PR is the upstreamable form of that fix).

…ueue stuck prints

## Problem

`_STUCK_JOB_TIMEOUT_SECONDS` was hard-coded to 7200 s (2 h), and a PRINTING
job over that cap was treated as hung and handed to `_requeue_or_fail`, which
reset it to QUEUED and re-dispatched **the same file** — without checking
whether the printer was still physically printing it.

Real-world damage (2026-09-13, Snapmaker U1): a 2h05m print was judged stuck
at the 2h cap, re-queued, and re-dispatched; the same object was physically
printed three times and the second plate dropped onto the first one's
finished part.

A stuck-timeout is a queue-side guess ("printer may be disconnected or
hung"), not a machine verdict. Re-dispatching a job that may still be
printing is the duplicate-print accident itself. A genuinely hung machine
needs a human; the dispatch-failure retry path (`_requeue_or_fail`) is
unchanged and still handles transient start failures.

## Changes

- `kiln/src/kiln/scheduler.py`
  - `_STUCK_JOB_TIMEOUT_SECONDS` reads env `KILN_STUCK_JOB_TIMEOUT_SECONDS`
    (default 43200 = 12 h) via the existing `kiln.parse_float_env` helper,
    matching `queue.py`'s `KILN_STUCK_JOB_TIMEOUT_MINUTES` pattern (that
    knob's `check_stuck_jobs()` currently has no caller — dead code — noted
    for follow-up rather than silently removed in this patch).
  - The stuck branch no longer calls `_requeue_or_fail`. It now mirrors the
    existing permanent-failure path exactly: pop retry bookkeeping, discard
    the seen-printing mark, `mark_failed`, publish `JOB_FAILED`, record the
    resolution locally via `_auto_record_outcome` (contribute=False — the
    stuck-timeout says nothing about the model, so it never federates),
    and append to `tick()`'s `failed` report so callers see the verdict.
- `kiln/tests/test_scheduler.py`
  - `TestStuckJobTimeout`: a PRINTING job over the timeout is FAILED with no
    second dispatch (then or on a later tick); a JOB_FAILED event is
    published and no JOB_SUBMITTED/re-queue; env override (1800 s) declares a
    30-min-old job stuck; a garbage env value falls back to the 12 h default;
    a valid env value is read at module load; a dispatch failure with retries
    remaining still goes through the retry path.
  - Updated `test_stuck_timeout_guess_contributes_nothing`'s fixture
    (7300 s → 43300 s) because the new 12 h default no longer triggers at 2h05m.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codeofaxel

Copy link
Copy Markdown
Owner

Thanks for this, and sorry about the U1 prints. I read the branch on main and your diagnosis is exactly right: the stuck check only fires while the printer is reporting PRINTING, then hands the job to the retry path, which resets it to QUEUED and re-dispatches the same file the moment the machine goes idle. Any queued print over 2 h hit it. Want this in.

Three things before merge:

  1. CLA — the bot needs your signature (link in the check above).

  2. CI — I've approved the run. Our main had an unrelated ruff failure in kiln/printers/ from a push yesterday; that's fixed on our side, so if the lint step goes red on files you didn't touch, it's ours.

  3. Two small changes:

    • The two headline tests (test_stuck_job_fails_and_is_never_redispatched, test_stuck_job_publishes_job_failed_event) pass on unfixed main. The shared scheduler fixture builds with max_retries=0, so the old code already fails permanently there and the re-queue path is never reached. I ran your test class against main's scheduler: the three env tests fail as expected, those two pass. Construct the scheduler in those two with max_retries=2 (the production default) so they demonstrate the bug before the fix.
    • The permanent-fail block in the stuck branch duplicates the bottom half of _requeue_or_fail. A retry=False (or permanent=True) flag on that helper, or a _fail_permanently extracted from it, keeps one copy of the JOB_FAILED / outcome / failed bookkeeping.

    Optional: reading the env in __init__ (a stuck_timeout_seconds kwarg defaulting from the env) would let the three env tests drop importlib.reload, which rebinds the module's classes for every other test in the process.

Agreed on the rest: the wall-clock timeout is still a guess (a 13 h print would now be marked failed while still running), so the real fix is a no-progress-for-N-minutes detector, and queue.check_stuck_jobs() with its unused env knob is dead. Both are ours as follow-ups; no need to widen this PR.

@qjkme
qjkme force-pushed the fix/scheduler-stuck-requeue branch from f366b94 to 65b3bed Compare September 15, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants