Skip to content

fix(schedule): bound per-node re-dispatch and surface a loud CapErr (#828) - #859

Draft
buroa wants to merge 1 commit into
mainfrom
fix/828-scheduler-redispatch-cap
Draft

fix(schedule): bound per-node re-dispatch and surface a loud CapErr (#828)#859
buroa wants to merge 1 commit into
mainfrom
fix/828-scheduler-redispatch-cap

Conversation

@buroa

@buroa buroa commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

This is the Exit-B safety net for #828 - it converts a silent hang into a
loud, attributable failure, and is not a fix
for the underlying determinism
defect. #828's core determinism requirement stays open as an approved follow-up
(see Scope below).

A schedulable node whose emitted content is non-deterministic across concurrent
re-runs feeds an unbounded scheduler re-dispatch storm, so Scheduler.Run never
reaches its inFlight == 0 fixpoint and hangs silently.

What this changes

  • Per-node re-dispatch cap (pkg/schedule/schedule.go): a cumulative
    dispatchCount enforced at the single Run dispatch chokepoint. Past
    capLimit = 32 + 4*len(nodes) the node is force-terminalized and latched
    capped, so no path (OnArrival re-admission, the fixpoint rerun sweep) can
    reopen the storm. The scheduler gains no store handle and Run's signature is
    unchanged.
  • Loud, attributable surfacing (CapErr() + pkg/orchestrator/dagrun.go):
    an additive Scheduler.CapErr() accessor reports the capped node ids and
    counts after Run returns; runDAG folds it into its returned error via
    errors.Join, so a recurrence surfaces as an errored run naming the offending
    node instead of a hang.
  • Tests (pkg/schedule/schedule_test.go):
    TestSchedulerCapFiresOnPathologicalNode drives the real OnArrival
    re-dispatch seam past the cap (a gated keepalive holds the pool non-idle so
    the assertion cannot pass vacuously) and asserts Run returns with CapErr
    naming the node; TestSchedulerCapNoFalsePositiveOnConvergingDAG proves a
    converging large DAG does not trip the cap.

Net diff: 3 files, +247/-5 (pkg/schedule/schedule.go,
pkg/orchestrator/dagrun.go, pkg/schedule/schedule_test.go).

Scope: what this does NOT do

This does not close #828. It ships only the load-bearing p1 safety net
(Change A). Every-run convergence and byte-identical parallel output depend on
eliminating the still-unidentified non-deterministic schedulable emit (Change
B), which - together with its -race red->green reproduction - is blocked on a
cgo + fetched-chart -race environment and is tracked as an approved follow-up.

Requirement Status
R1 - Parallel reconcile converges every run; dispatch workers stay bounded deferred (Change B follow-up)
R2 - Parallel output byte-identical to --concurrency 1; non-deterministic emit eliminated deferred (Change B follow-up)
R3 - Serial / already-converging topologies unchanged (no regression) covered
R4 - A test reproduces the concurrency-gated non-determinism and guards the regression deferred (Change B follow-up)

Limitations

  • -race was not run (no cgo toolchain in the build environment). The
    offline suite is race-clean by construction (CapErr() and the test fakes'
    concurrent signals are mutex-guarded); a -race run on pkg/schedule remains
    the backstop on a cgo-capable runner.
  • The tests guard the safety net, not the defect. They exercise the cap
    firing through the real re-dispatch seam, not the underlying non-determinism,
    which the follow-up still owes.

Refs #828. Please keep #828 open - this PR does not deliver its determinism
guarantee.


Draft opened by an autonomous Gas City github-issue-fix run
(workflow root fl-oku; triage: reproduced, p1). This is a draft: it
is the Exit-B Change A safety net, not a complete fix. Review the framing above
before promoting or merging.

…828)

A schedulable node whose emitted content is non-deterministic across
concurrent re-runs feeds an unbounded scheduler re-dispatch storm, so
Scheduler.Run never reaches its inFlight==0 fixpoint and hangs silently.

Add a per-node cumulative dispatch counter enforced at the single Run
dispatch chokepoint: past capLimit = 32 + 4*len(nodes) the node is
force-terminalized instead of dispatched, latched capped so no path
(OnArrival re-admission, the fixpoint rerun sweep) can reopen the storm.
An additive Scheduler.CapErr() accessor reports the capped node ids and
counts after Run returns; runDAG folds it into its returned error, so a
recurrence surfaces as a loud, attributable errored run instead of a
hang. The scheduler gains no store handle and Run's signature is
unchanged.

This is the Exit-B safety net: it converts the silent livelock into a
loud failure and does NOT close #828's determinism requirement, which
remains a follow-up blocked on a cgo + fetched-chart -race environment.

Tests: TestSchedulerCapFiresOnPathologicalNode drives the real OnArrival
re-dispatch seam past the cap (a gated keepalive holds the pool non-idle
so it cannot pass vacuously) and asserts Run returns with CapErr naming
the node; TestSchedulerCapNoFalsePositiveOnConvergingDAG proves a
converging large DAG does not trip the cap.

Signed-off-by: Steven Kreitzer <skre@skre.me>
@onedr0p
onedr0p force-pushed the fix/828-scheduler-redispatch-cap branch from eb77002 to 7967137 Compare July 26, 2026 00:16
dark-vex added a commit to dark-vex/infra-cd that referenced this pull request Aug 7, 2026
…elock

Root-caused the recurring "flate + kubeconform (kubenuc)" hang (3
occurrences, 2026-08-06): a confirmed, still-open upstream bug in flate
itself (home-operations/flate#828), not anything specific to this repo.
Under flate's default concurrency (runtime.NumCPU()*4), a race in the
DAG scheduler can make a HelmRelease's rendered output non-deterministic
across re-runs, which re-arms the scheduler's dispatch loop and never
converges - a livelock. kubenuc is this repo's largest/most complex
cluster tree (26 chart sources, 29 apps), making it statistically most
likely to hit a non-deterministic, DAG-size-scaling race - not something
unique to its content.

The issue reporter's documented workaround (--concurrency 1) got 8/8
clean runs vs hangs on ~half of parallel runs, byte-identical output to
the good parallel runs. Add it to both flate test and flate build. This
prevents the race rather than just bounding its blast radius like the
timeout-minutes added in #1794, which stays as defense-in-depth in case
this doesn't fully eliminate it.

The upstream fix in flight (home-operations/flate#859) only adds a
re-dispatch cap that converts a hang into a loud failure - it's not
merged, and even merged it wouldn't restore correctness, just fail
faster. Revisit --concurrency 1 once a flate release actually fixes the
underlying render non-determinism.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Daniele De Lorenzi <2905124+dark-vex@users.noreply.github.com>
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.

Non-deterministic hang under parallel reconcile — DAG scheduler never converges; --concurrency 1 avoids it

1 participant