Conversation
…loops Every service's read loop treated all non-EOF read errors the same way: log and `continue`. That is right for a bad frame (the scanner advances past it) but wrong for a terminal transport error such as an over-long frame that bufio.Scanner cannot resync past — the loop then spins on the same error forever instead of ending the pump. Classify by error type instead. A DecodeError is recoverable and keeps the loop alive; any other non-EOF error is terminal. The broker's read loop returns errTerminalRead so Serve tears down cleanly, and the producer goroutine stops feeding the channel once it hits a terminal error rather than blocking forever. Signed-off-by: woodsonl <65194841+woodsonl@users.noreply.github.com>
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
Every service read loop treated all non-EOF read errors the same way: log the
error and
continue. That is correct for a bad frame, which the scanner hasalready advanced past. It is wrong for a terminal transport error such as an
over-long frame that
bufio.Scannercannot resync past. There the nextReadreturns the same error, so the loop spins on it forever.
This classifies read errors by type. A
DecodeError(bad JSON, wrong version)stays recoverable and keeps the loop alive. Any other non-EOF error is
terminal: the broker's read loop returns
errTerminalReadsoServetearsdown cleanly, and the producer goroutine stops feeding the channel once it hits
a terminal error rather than blocking forever.
Release intent
Changelog title
Services stop cleanly on a terminal stream read error
Changelog body
A service that hit a non-recoverable read error on its JSON-RPC stream, such
as an over-long frame the scanner cannot resync past, used to log the error
and retry forever. Read errors are now split into recoverable frame errors,
which are skipped, and terminal transport errors, which end the read pump.
Bumps
Scope
Included: the
readLooperror classification in every service that readsJSON-RPC from a stream, the
DecodeErrorcodec alias the classification needs,and the broker's
recoverableDecode/errTerminalReadhelpers with a test.Excluded: nothing else. This is one behavior change applied uniformly.
Validation
go build ./...andgo test ./...in every touched module(
nvpair-cluster-manager,nvpair-errors,nvpair-job-scheduler,nvpair-manual-nodes,nvpair-node-scanner,nvpair-node-settings,nvpair-proxy,nvpair-tui,nvpair-ui-broker,nvpair-workload-manager)services/nvpair-ui-broker/terminal_read_test.gocovers the three cases:a non-EOF transport error ends the loop with
errTerminalRead, EOF exitscleanly, and a malformed frame keeps the loop alive.
Risk
Low. The recoverable path is unchanged; only the terminal path now stops
instead of looping. An engine or peer that sends an over-long frame will now
end that connection rather than pin a CPU, which is the intended behavior.
Checklist
git commit -s), certifying the Developer Certificate of Origin.services/versions.jsonis written by automation — do not edit it by hand.