Repository navigation
Broadcast workload frames in order and keep same-numbered jobs apart - #29
Merged
Merged
Conversation
Two bugs could leave peers with phantom load: - Every inter-node frame was broadcast in its own goroutine, so a workloads:remove could overtake the upsert it follows and resurrect a ghost workload on a peer. A single ordered worker now sends frames in the order they were produced. A full queue drops the new frame with a warning; the heartbeat re-syncs lifecycle state. - The receiver recorded a dedup key before emitting to its broker, so a failed emit's retry was swallowed as a duplicate. The key is now recorded only after a successful emit, and concurrent emits for one key are serialized. This fork's live progress frames (workloads:progress) share the ordered queue, so they cannot overtake a lifecycle frame either. Ported from NVIDIA/Personal-AI-Router@691372c (NVIDIA#80). The contract checker keeps this fork's notifyWithShutdown patterns and gains upstream's exclusion of Broadcast(ctx, frame). (cherry picked from commit 691372c) Co-authored-by: Mallikh Kaula <mallikh@users.noreply.github.com> Co-authored-by: Kaylee Lubick <kjlubick@users.noreply.github.com> Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
Every proxy numbers its jobs from 1, and again from 1 when it restarts, but the desktop keyed a job by (originatedFrom, id) only. An Ollama job "3" and an oMLX job "3" on one node shared one card, the later one overwriting the earlier, and so did a job and the one a restarted proxy gave the same number. A recovery eviction then removed both. Key a job by the backend's own identity, (originatedFrom, engine, runId, id), and parse the runId the broker already sends. A broker workloads:remove names only (originatedFrom, id), so it drops every matching job, as the broker's store does; a removal the desktop raises itself names the engine and run and drops exactly one. Removals seen during a recovery fetch also keep the older snapshot from bringing a matching job back. No wire contract changes. The desktop half of NVIDIA#142's identity fix, reimplemented for this fork. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
The terminal interface keyed a job by (originatedFrom, id), so an Ollama job "3" and an oMLX job "3" on one node, or a job and the one a restarted proxy gave the same number, shared one row. Key a row by the backend's identity, (originatedFrom, engine, runId, id). A workloads:remove names only (originatedFrom, id) and drops every matching row, as the broker's store does. The recovery journal keeps keying workload events by (originatedFrom, id), so an upsert and a later removal of a job still collapse to the latest. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
…d TUI recovery Review follow-ups: - Job cards carried only origin and id as their DOM anchor, so the connection line of one same-numbered job attached to another's card. Cards now carry the full job key. - A removal the desktop raises for a job with no run sent runId: null, which matched any run. Only an absent field is now a wildcard. - The terminal interface's recovery journal kept one workload event per (origin, id), in no particular order, so same-numbered jobs on two engines collapsed and a removal could undo a later upsert. Workload events are now journaled and replayed in arrival order. Signed-off-by: Jared Ping <13906915+JaredP94@users.noreply.github.com>
JaredP94
force-pushed
the
fix/workload-ordering
branch
from
November 2, 2026 18:46
88764f6 to
2f9c34d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Three fixes to how jobs are tracked:
nvpair-workload-manager). Each inter-node frame was sent in its own goroutine, so aworkloads:removecould overtake its upsert and resurrect a ghost job on a peer. One ordered worker now sends frames in production order (live progress frames included). The receiver records a dedup key only after a successful broker emit, so a failed emit's retry is no longer swallowed. Ported from NVIDIA/Personal-AI-Router@691372c.(originatedFrom, id), so an Ollama job 3 and an oMLX job 3 on one node shared a card. Jobs are now keyed by(originatedFrom, engine, runId, id)— the broker's own key;runIdis now parsed. A broker removal (which names only origin+id) drops every match, as the broker's store does; removals during a recovery fetch can't be undone by the older snapshot. Desktop half of Keep same-numbered jobs apart and apply each job's events in order NVIDIA/Personal-AI-Router#142, reimplemented.nvpair-tui; its recovery journal now records workload events in arrival order, so same-numbered jobs and a removal followed by an upsert replay correctly.Review follow-ups also key the job cards' DOM anchors (used by the connection lines) by the full identity, and make
runId: nullin an exact removal mean "no run" rather than "any run".Scope
No wire contract changes.
verify-service-contracts.tskeeps the fork'snotifyWithShutdownpatterns and adds upstream'sBroadcast(ctx, frame)exclusion.Validation
go test -race ./...innvpair-workload-manager;go test ./...innvpair-tui.typecheck,lint,test:unit(729 passed at the time),dead-code:check,service-contracts:check. New tests cover same-numbered jobs and removal semantics.Risk
A full broadcast queue (1024) drops the new frame. The heartbeat re-asserts tracked lifecycle state, but a dropped removal is not replayed and may leave a stale row on a peer until it ages out. workload-manager 1.3.1, tui 1.2.1.
Checklist
git commit -s), certifying the Developer Certificate of Origin.services/versions.json, and described user-visible changes above so they reach the release notes.Part of the upstream issue/PR review. Each branch is independent off
main; they all bumpservices/versions.json, so the second one merged will need a trivial conflict resolution there.