Skip to content

fix(broker): dispatch requests on a bounded pool and coalesce relay deliveries - #104

Closed
woodsonl wants to merge 1 commit into
NVIDIA:developfrom
woodsonl:fix/broker-dispatch-and-relay-backpressure
Closed

woodsonl wants to merge 1 commit into
NVIDIA:developfrom
woodsonl:fix/broker-dispatch-and-relay-backpressure

Conversation

@woodsonl

Copy link
Copy Markdown

Description

The broker handled every inbound message inline on the read loop. Each
handleMessage can run a synchronous worker relay bounded by
rpcWorkerCallTimeout (5s), so one slow proxy, cluster, or settings call
blocked every other client request behind it.

This dispatches through a small fixed pool. Cross-request ordering is preserved
where it matters by the per-handler mutexes (workloadEmitMu for workload
apply → fan → emit, per-state mutexes for subscription bookkeeping). JSON-RPC
does not promise cross-request response ordering: each response carries its own
id, and the codec's write mutex keeps concurrent responses from interleaving.

The relay had a matching problem. It delivered to each subscriber with a
blocking send, so a subscriber that stopped reading could stall the pump for
everyone. Deliver now coalesces under the subscriber's own lock and drops the
oldest pending message when a subscriber falls behind, so one stuck client
cannot hold up the shared stream.

The read loop also classifies its read errors, matching the change in the
service read loops: a DecodeError is recoverable and keeps the loop alive,
any other non-EOF error is terminal.

Release intent

Changelog title

Broker no longer lets one slow worker stall the control plane

Changelog body

The broker now handles client requests on a small worker pool instead of
inline on the read loop, so a slow worker call no longer blocks unrelated
requests behind it. Relay deliveries to a subscriber that has stopped reading
coalesce instead of blocking the stream for every other subscriber.

Bumps

  • services: patch
  • nvpair-cluster-manager: none
  • nvpair-engine-manager: none
  • nvpair-errors: none
  • nvpair-job-scheduler: none
  • 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: patch
  • nvpair-workload-manager: none

Scope

Included: the broker dispatch pool, the relay's coalescing delivery, and the
broker read loop's error classification with its test.

Excluded: the same read-error classification in the individual service read
loops, which is proposed separately.

Validation

  • go build ./... and go test ./... in services/nvpair-ui-broker
  • services/nvpair-ui-broker/relay/relay_test.go covers the coalescing
    delivery path, and terminal_read_test.go covers the read-loop cases.

Risk

Low. Handler ordering is unchanged for the channels that depend on it because
the serialization lives in per-handler mutexes, not the read loop. A subscriber
that falls behind now drops its oldest pending message rather than stalling,
which is the behavior the relay's design already implied.

Checklist

  • I have read the Contributing Guidelines.
  • Every commit is signed off (git commit -s), certifying the Developer Certificate of Origin.
  • New or existing tests cover the change.
  • Relevant documentation is updated.
  • I checked the diff, changed filenames, and commit messages for credentials, private data, internal URLs, internal issue identifiers, and generated artifacts.
  • I recorded the validation commands and results above.
  • I declared version bumps in the release-intent block above. services/versions.json is written by automation — do not edit it by hand.

…liveries

The broker handled every inbound message inline on the read loop. Each
handleMessage can run a synchronous worker relay bounded by
rpcWorkerCallTimeout (5s), so one slow proxy/cluster/settings call blocked
every other client request behind it.

Dispatch through a small fixed pool instead. Cross-request ordering is
preserved where it matters by the per-handler mutexes (workloadEmitMu for
workload apply→fan→emit, per-state mutexes for subscription bookkeeping),
and JSON-RPC does not promise cross-request response ordering — each
response carries its own id, and the codec's write mutex keeps concurrent
responses from interleaving.

The relay also delivered to each subscriber with a blocking send, so a
subscriber that stopped reading could stall the pump for everyone. Deliver
now coalesces under the subscriber's own lock and drops the oldest pending
message when a subscriber falls behind, so one stuck client cannot
back-pressure the shared stream. The pump re-checks done before every
Send, so a trigger that races Unsubscribe cannot deliver to a consumer
that's gone; Unsubscribe itself does not wait for the pump to exit.

The read loop also classifies read errors: a DecodeError is recoverable
and keeps the loop alive, any other non-EOF error is terminal and ends the
pump (errTerminalRead) instead of spinning.

Signed-off-by: woodsonl <65194841+woodsonl@users.noreply.github.com>
@woodsonl woodsonl closed this Sep 24, 2026
@woodsonl
woodsonl deleted the fix/broker-dispatch-and-relay-backpressure branch September 24, 2026 05:25
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.

1 participant