drpcstream: per-stream receive window and read-path grant wiring#72
drpcstream: per-stream receive window and read-path grant wiring#72suj-krishnan wants to merge 1 commit into
Conversation
58a5fed to
30ffb52
Compare
f6bc0d6 to
e72ebc5
Compare
30ffb52 to
9b6d87b
Compare
e72ebc5 to
4953332
Compare
9b6d87b to
2512021
Compare
4953332 to
25af70a
Compare
2512021 to
c086e23
Compare
25af70a to
85794ad
Compare
c086e23 to
b14cb1f
Compare
85794ad to
e6a5378
Compare
b14cb1f to
6594c14
Compare
e6a5378 to
74ad6c3
Compare
fcfa2e4 to
10b863f
Compare
2b5a3d4 to
c07fef5
Compare
10b863f to
7d03098
Compare
1e6988a to
baf780b
Compare
7d03098 to
336c61c
Compare
be3886d to
1f8204f
Compare
336c61c to
ef342ce
Compare
1f8204f to
848a5a5
Compare
ef342ce to
0d5cf31
Compare
848a5a5 to
92ebb38
Compare
1520087 to
6649f34
Compare
9c97b8e to
f493958
Compare
6649f34 to
48cda98
Compare
5297544 to
c492906
Compare
|
Quick comment: the three commits in this PR aren't independent. The last two commits should definitely be squashed and to make sense of how receive_window is going to be used that commits needs to be with the above squashed commits. I would suggest squash all three commits. |
| // Grants are out-of-band signaling: intercept before the assembler so one | ||
| // arriving mid-message cannot disturb reassembly. Without a send window | ||
| // (flow control disabled) the grant is ignored. |
There was a problem hiding this comment.
I think this much of the comment would be enough.
// Intercept before the assembler so one
// arriving mid-message cannot disturb reassembly.
Add the per-stream receive side of flow control and wire it into the read
path.
recvWindow tracks buffered bytes (in-progress reassembly plus completed,
not-yet-consumed data) and decides when to return credit to the sender:
grants are withheld while buffered is at or above the high-water mark, and
accrued returnable credit is otherwise released once it reaches the
threshold, coalescing many frames into a single grant. Consuming reopens a
gate the high-water mark had closed. dispatched/consumed return the credit
delta to emit as a KindWindowUpdate.
Read-path wiring:
- HandleFrame intercepts an incoming KindWindowUpdate before the
assembler and applies it to the send window. Intercepting early keeps a
grant that interleaves with an in-progress message (grants are emitted
off the write lock) from disturbing reassembly.
- A dispatched KindMessage frame accrues credit; RawRecv/MsgRecv return
it on consume, which can reopen a gate the high-water mark closed. The
dispatch and release deltas are summed and emitted as a single window
update per frame.
Grants are control frames, appended without blocking, so emitting them
from the reader goroutine is safe. The window is opt-in: with no recvw
installed no grants are emitted, and an incoming grant with no send window
is ignored, so default behavior is unchanged. 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 invite more doomed
data.
The packet assembler can silently drop an unfinished message when a higher
message id supersedes it (a legitimate sequence: a send preempted
mid-message, then a cancel). Those KindMessage bytes were counted into the
receive window at dispatch and would inflate buffered forever, wedging the
high-water gate shut and deadlocking the sender once its credit ran out.
The assembler now records the drop (DiscardedBytes, tracked for KindMessage
only since only message payloads are byte-accounted) and HandleFrame
releases the bytes as consumed. The release runs after the replacement
frame is counted, so the gate sees the final buffered state rather than a
transient dip, and for any frame kind so a non-message supersede (e.g.
CloseSend) still frees its bytes; a terminal supersede emits one benign
grant, which is acceptable.
Stream-level only; the connection-level receive budget and the enablement
path that installs the windows come in later commits.
Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
c492906 to
080622f
Compare
shubhamdhama
left a comment
There was a problem hiding this comment.
My current understanding of this design is that pending tracks credit that has not yet been returned, while buffered tracks bytes held in partial reassembly or completed messages. HandleFrame adds to both and may emit a grant immediately while buffered is below highWater. MsgRecv and RawRecv reduce buffered and may flush grants that were withheld after reaching highWater.
Is that understanding correct? If so, why do we need to emit grants from HandleFrame? Why not emit them only when a message leaves receiver-owned storage in MsgRecv or RawRecv, along with the error and discard paths that release storage?
I can see that the current approach returns credit earlier instead of waiting for the application to consume the message. That may improve throughput by keeping the sender moving, but it also makes the model harder to reason about. Unless we have benchmarks showing that application-level grants are too slow, this feels like a premature optimization. We can add a frame-level fast path later if we find that we need it.
Another possible reason for the fast path is a message larger than the sender's available window. The sender could exhaust its credit halfway through the message, while the receiver cannot return credit through MsgRecv because the message is incomplete. However, the current policy only helps up to highWater. For example:
initial window: 1 KiB
highWater: 8 KiB
message size: 16 KiB
The receiver can return credit as frames arrive until approximately 8 KiB is buffered. It then stops granting credit to apply backpressure. The sender eventually exhausts its remaining credit, the message remains incomplete, and MsgRecv still cannot consume it. The policy moves the stall point from the initial window to roughly highWater plus any remaining credit, but it does not solve the general problem.
I think flow control should operate at packet boundaries instead. Once a packet is admitted, let the sender finish it even if its credit becomes negative. Then block the sender from starting another packet until the receiver consumes the first one and returns its credit. This keeps the receive-side model simple and avoids stopping in the middle of something that DRPC can only consume as a complete packet.
This allows one packet to exceed the window, so we would still need a maximum packet size to bound memory usage. Even with that trade-off, I think we should start with the simpler model and add frame-level grant behavior only if we can show that it is needed.
1. Per-stream receive-window grant policy (
recvWindow).Tracks buffered bytes (in-progress reassembly + completed, not-yet-consumed) and decides when to return credit: withholds grants while buffered is at/above the high-water mark, otherwise releases accrued returnable credit once it reaches the threshold.
dispatched/consumedreturn the credit delta to emit; consuming reopens a gate the high-water mark had closed.2. Read-path wiring.
HandleFrameintercepts an incomingKindWindowUpdatebefore the assembler and applies it to the send window. Intercepting early keeps a grant that interleaves with an in-progress message (grants are emitted off the write lock) from disturbing reassembly.bufferedforever, causing the high-water gate to shut. So, the assembler now reports the drop (TakeDiscarded) andHandleFramereleases the bytes as consumed, so their accrued credit flows back.