drpcstream: grant-on-consume receive-side flow control - #88
Conversation
d5a4e41 to
a0d41c6
Compare
shubhamdhama
left a comment
shubhamdhama
left a comment
There was a problem hiding this comment.
I also think sending grants for ever single consumption would be very chatty. We should send at fixed thresholds. I liked what Yamux have done here: https://github.com/hashicorp/yamux/blob/76348a0c5ab413fe3c5c564077ea26e21a8769a3/stream.go#L278
Agreed - wanted to take it up as a follow-up but it is a fairly simple change (the previous PR already had similar accounting). |
8b5880b to
9726a8d
Compare
The PR is updated. The 'threshold' will be a fraction of the stream window (either 1/4 or 1/2). |
| // It holds no lock: consumed is called only from the stream read path | ||
| // (RawRecv/MsgRecv), which the read lock already serializes. |
There was a problem hiding this comment.
We don't need to say "It holds no lock...". Agents tend to over-comment.
There was a problem hiding this comment.
Please merge these tests within the recv_window_test.go. It's an unnecessary addition of files.
| // recvw is the per-stream receive-side flow-control window. The read path | ||
| // returns credit to the sender through it as messages are consumed, | ||
| // coalesced at its threshold. It is nil when flow control is not enabled, | ||
| // in which case no grants are emitted. | ||
| recvw *recvWindow |
There was a problem hiding this comment.
nit: over-commenting. It's leaking the implementation details here. This commenting is already done in the definition of recvWindow.
There was a problem hiding this comment.
Yeah, it keeps re-adding comments during subsequent commits even after I remove them 😢 So I change the comments just once before I push.
7f8e018 to
12ae0a8
Compare
Add the receive side of per-stream flow control: return credit to the
sender as the application consumes received bytes (grant-on-consume),
coalesced into one KindWindowUpdate per threshold bytes.
Chosen over a grant-on-dispatch design (a high-water gate over buffered
bytes, prototyped in an earlier revision of this work) for its simplicity
and its tight memory bound: in-flight plus buffered bytes never exceed
the window. The receive side needs almost no machinery:
- recvWindow is a consumed-byte counter and a threshold; consumed
returns a coalesced grant once the accrued amount crosses the
threshold. No mutex -- consumed is called only from the read path,
which the read lock already serializes -- and no buffered/dispatched
tracking, since grant-on-consume never grants on receipt.
- RawRecv/MsgRecv return credit for the consumed wire bytes inline,
before decompression reassigns the buffer and before any decode error
can return, so credit is released on every path without a defer.
- HandleFrame gains only the KindWindowUpdate intercept: an incoming
grant is applied to the send window before the assembler, so a grant
interleaved with an in-progress message cannot disturb reassembly.
- No discarded-partial accounting. A partial superseded by a higher
message id only happens when the sender aborted mid-message, and
every such path terminates the send side, so the unreturned credit
belongs to a stream that is already done; there is no receiver-side
counter for it to corrupt. Revisit if a connection-level receive
budget aggregates credit across streams.
emitGrant is a no-op once the stream has terminated: the ring drains
queued messages after close, and returning credit behind the terminal
frame would only invite more doomed data.
Trade-offs, both deliberate: throughput is capped at window/RTT (no BDP
estimation), and coalescing withholds at most threshold bytes of credit,
which the enablement path keeps small relative to the window. A larger
threshold cuts control-frame chatter at the cost of more withheld credit.
Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
12ae0a8 to
5a99ec2
Compare
2 participants
Adds the receive side of per-stream flow control: credit is returned to the sender as the application consumes received bytes (grant-on-consume), coalesced into one window update per threshold bytes.
This approach was picked over a grant-on-dispatch design (credit returned on receipt, gated by a high-water mark over buffered bytes — see #72) for its simplicity and tight memory bound: in-flight plus buffered bytes never exceed
the window, and the receive side needs almost no machinery.
What's here
recvWindowis a consumed-bytes counter and a threshold:RawRecv/MsgRecvaccrue the dequeued wire bytes on the way out (deferred, so a decode failure still releases them) and emit aKindWindowUpdateonce the accrued credit reaches the threshold, bounding update frequency to one per threshold bytes. Withheld credit is at most threshold bytes and flushes with the next grant — never lost.HandleFramegains only theKindWindowUpdateintercept: an incoming grant is applied to the send window before the assembler, so a grant interleaved with an in-progress message cannot disturb reassembly.Deliberate trade-offs