Repository navigation
Conversation
meling
force-pushed
the
fix/325/refused-stream-retry
branch
from
October 11, 2026 20:40
1978abc to
66dc711
Compare
meling
added this pull request to stack #362
October 11, 2026 20:40
| return | ||
| } | ||
| s, err := c.open() | ||
| refusal = nil |
meling
force-pushed
the
fix/325/refused-stream-retry
branch
from
October 12, 2026 20:10
66dc711 to
6b82aa3
Compare
A server that refuses a stream, for example a peer verifier that rejects the dialer's identity, ends it with a status before it sends any frame. The outbound channel treated that as a lost stream: it requeued the pending calls and opened a new stream at once, because a request was waiting. The calls failed only when their contexts ended, never with the refusal's code, and both servers spun at thousands of streams per second. A stream now counts as refused when it ends with Unauthenticated, PermissionDenied, InvalidArgument, or Unimplemented before it delivered a frame. Only the receive loop learns the stream's status, so a send that fails with io.EOF before the first frame only cancels the stream and leaves the pending calls to the receive loop. The session's verdict channel tells the run loop whether the stream was refused. Calls on a refused stream fail with the refusal, streaming calls included. The channel then backs off (50 ms, doubling to 2 s) before it opens the next stream, with or without eager reconnect, and fails each request at once with the refusal until the backoff ends. A failed dial-side verification backs off the same way. Lost streams keep the old behavior: requeue and reconnect.
A send completes locally before the server's verdict on a new stream arrives. So after a refusal, each new stream's first send cleared LastErr, and LastErr flipped between the refusal and nil for about one round trip per attempt. Callers that sort nodes with ByLastError then saw a refusing node as healthy. A session opened after a refusal is now suspect until a stream proves viable. A send on a suspect session clears LastErr only once its stream has delivered a frame or lived 2 s; a received frame clears it as before.
A new stream counts as up as soon as it opens, before the server's verdict arrives. So each stream that a server refused again counted as up for about one round trip: the refused node's ConnectedPeers and WithPeerChange flapped once per backoff period, and under stream dedup its WaitForAll could return although the peer refused every stream. A session opened after a refusal now counts as up only once its stream delivers a frame or lives 2 s. The send loop marks it up, on the channel's goroutine, as OnStreamChange requires. The first stream still counts as up until its refusal arrives, and after the server accepts again, an idle stream counts as up 2 s after it opened. Exact up states on both sides need a protocol change.
meling
force-pushed
the
fix/325/refused-stream-retry
branch
from
October 12, 2026 20:22
6b82aa3 to
0acb890
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.
Stacked on #359. Merge #359 first; this PR's base will then switch to
master.Problem
When a server refused a dialed stream at accept time (for example, a peer that fails
WithPeerVerifier's check), the dialer requeued the pending call and opened a new stream at once.One pending call caused about 15,000 refused streams per second; the call failed only at its deadline, with the context's error instead of the refusal's code.
Each attempt also counted as "up" for one round trip, so the refused side's
ConnectedPeersandWithPeerChangeflapped, and underWithStreamDedupitsWaitForAllcould return although every stream was refused.Change
A dialed stream is refused when it ends with
Unauthenticated,PermissionDenied,InvalidArgument, orUnimplementedbefore the server sent anything. Then:stream: fail and back off on a stream the server refuses— pending calls (streaming calls included) fail at once with the refusal's status, and the channel waits before the next stream: 50 ms, doubling to 2 s, with or without eager reconnect. Calls made during the wait fail at once with the refusal. A small handoff between the send and receive loops makes sure the receive loop, which alone learns the status, disposes of the pending calls exactly once. A failed dial-side check is paced the same way.stream: keep a stream refusal in LastErr until a stream succeeds—Node.LastErrkeeps the refusal until a stream proves viable.stream: count a stream opened after a refusal as up once viable— after a refusal, a new stream counts as up only after the server sends a frame or the stream lives 2 s, so a refused peer no longer flaps inConnectedPeersor completesWaitForAll. The first stream still counts as up for one round trip; an exact up state needs a protocol change, which is left for later.Transient failures (a broken connection, a server restart, GOAWAY,
Unknown, or any status after the server has sent a frame) keep today's behavior: requeue and reconnect.Effect
Testing
LastErr, recovery, and that transient failures still retry.internal/tests/tlsand root tests now expectcodes.Unauthenticated(andInvalidArgumentfor a self-ID claim) instead of a deadline.make testandmake integrationtestpass; the new tests pass under-race;make modernizechanges nothing andmake goplscheckis clean.Docs:
WithPeerVerifier, the user guide's "Authenticating Peers", and the developer guide's stream layer describe the new behavior.