Skip to content

Retry failed scheduler priority notifications - #122

Open
sylvesterkaczmarek wants to merge 1 commit into
NVIDIA:developfrom
sylvesterkaczmarek:sylvesterkaczmarek/fix-scheduler-delivery-state
Open

sylvesterkaczmarek wants to merge 1 commit into
NVIDIA:developfrom
sylvesterkaczmarek:sylvesterkaczmarek/fix-scheduler-delivery-state

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Description

Closes #121.

Record a priority snapshot as emitted only after its JSON-RPC notification write succeeds. Previously a failed write updated the delivery cache first, so ordinary reconciliation suppressed retries and scheduler status incorrectly showed the failed update as delivered.

The change keeps notification I/O outside the state mutex and retains the existing recompute serialization. Each engine remains independent: a successful delivery is not repeated because another engine failed. Ranking policy, wire fields, retry cadence and receiver behavior are unchanged.

Release intent

Changelog title

Retry failed scheduler priority notifications

Changelog body

The scheduler retries changed priorities after a failed local notification write and keeps its status aligned with the last successful delivery.

Bumps

  • services: patch
  • nvpair-cluster-manager: none
  • nvpair-engine-manager: none
  • nvpair-errors: none
  • nvpair-job-scheduler: patch
  • nvpair-manual-nodes: none
  • nvpair-node-info: none
  • nvpair-node-scanner: none
  • nvpair-node-settings: none
  • nvpair-proxy: none
  • nvpair-tui: none
  • nvpair-ui-broker: none
  • nvpair-workload-manager: none

Scope

Only the scheduler delivery bookkeeping, its regression tests and its README are changed. The scheduler binary and services aggregate receive patch intent. No version files are edited by hand.

The broker, shared wire types, proxies, desktop and terminal interface need no contract changes. Existing cross-process scheduler tests verify the emitted frames and ranking behavior. The open engine-addition PRs modify the engine list in the same source file, not this delivery path.

Validation

  • Five new regression scenarios fail before the fix and pass afterward. They cover initial write failure, error and zero-byte retry paths, retention of an older successful snapshot, and failed forced-refresh timestamps.
  • The complete scheduler package passes with race detection on macOS and Linux arm64: 36 top-level tests and 11 subtests, no failures or skips.
  • Four existing cross-process scheduler tests pass, driving real built binaries through stdin/stdout. They cover workload updates, telemetry, restart replay and a 50-job synthetic burst. The harness builds the service binaries but the selected tests start only the scheduler.
  • All 16 shared JSON-RPC tests pass with race detection on macOS.
  • go vet, the scheduler build, gofmt, repository SPDX-header checks and git diff --check pass. The release-intent description validates locally.
  • Linux tests run in an isolated container with networking disabled. App-data paths are isolated under temporary directories.
cd services/nvpair-job-scheduler
go test -race -count=1 ./...
go vet ./...
go build .
# From services/tests:
go test -race -count=1 -run '^TestScheduler' ./...
# From services/shared:
go test -race -count=1 ./jsonrpc
# From the repository root:
node scripts/spdx-headers.mjs

Tested with Go 1.27.1. The desktop gates, whole cross-process suite, live engines and multi-node integration were not run. No test or runtime guard was disabled. These tests cover failures before any frame bytes are delivered; they do not claim recovery from partial-frame corruption.

Risk

Low, localized state-ordering correction. No new dependency, network listener, persistent data, protocol field or service boundary. The previous snapshot remains authoritative when a new write fails. The issue was reported on 25 September 2026 and rechecked for assignments and competing fixes before submission.

Checklist

  • I have read the contributing guidelines.
  • Every commit is signed off.
  • Regression and existing tests cover the change.
  • Relevant documentation is updated.
  • The diff and commit contents contain no credentials, private data or generated artifacts.
  • Validation commands and limitations are recorded above.
  • Version bumps are declared in the release-intent block; automation-owned version files are unchanged.

Fixes NVIDIA#121. Add regression coverage and preserve existing successful behavior.

Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@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.

Scheduler suppresses retry after a failed priority notification

1 participant