Skip to content

drpcstream: add per-stream send-window credit primitive#70

Merged
suj-krishnan merged 2 commits into
mainfrom
sujatha/flow-control-send-window
Jul 16, 2026
Merged

drpcstream: add per-stream send-window credit primitive#70
suj-krishnan merged 2 commits into
mainfrom
sujatha/flow-control-send-window

Conversation

@suj-krishnan

@suj-krishnan suj-krishnan commented Jul 1, 2026

Copy link
Copy Markdown

Adds sendWindow, the per-stream flow-control credit balance on the sender:

  • acquire(ctx, n) — spends credit, blocking until enough is available; interruptible by both context cancellation and close, consuming no credit when it bails.
  • grant(n) — strictly additive (no-revoke); wakes a parked acquirer.
  • close(err) — terminates the window, waking all waiters with the error.

A grant only ever adds credit; the receiver never takes back credit it has already issued. The balance is a signed int64: the flow control enablement design (pre-armed accounting, see the design doc) has the sender debit bytes it sends before flow control begins enforcing, which can leave the balance negative — a stream then repays that deficit from incoming grants before its next message. acquire blocks whenever available credit is below what the next frame needs, negative balances included.

Scope: the primitive only. It is not yet wired into the stream write path.

Tests: immediate acquire, grants accumulate, blocks-until-grant, context-cancel wake, close wake, acquire-after-close. All pass under -race.

@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-window-update branch from 513c137 to 14e3978 Compare July 13, 2026 07:00
@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-send-window branch 2 times, most recently from a4ad9e8 to 6ff7baf Compare July 13, 2026 07:06
@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-window-update branch from 14e3978 to 4273044 Compare July 13, 2026 08:41
@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-send-window branch 3 times, most recently from 9c3bc23 to efc77b4 Compare July 13, 2026 08:55
@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-window-update branch from 4273044 to 9c0b23b Compare July 13, 2026 08:59
@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-send-window branch from efc77b4 to d88576b Compare July 13, 2026 08:59
Define the KindWindowUpdate control frame that carries a flow-control
credit grant: a varint byte delta in the body, for the stream named by
the frame's stream id. Add WindowUpdateFrame/ParseWindowUpdate helpers
to build and read it. Stream id 0 is reserved for a possible future
connection-level window and is unused in v1.

The control bit marks the frame as out-of-band signaling, so it is
emitted without blocking on data backpressure. It is not relied on for
backward compatibility: an unknown control frame is only dropped after
packet assembly, so it is not safely ignorable on an active stream.
Instead, grants are only ever sent once flow control is active
cluster-wide, so a peer that predates flow control never receives one.

ParseWindowUpdate enforces the whole v1 wire contract in one place, since
these frames are intercepted before packet assembly and its message-id
checks: it accepts only a self-contained control frame (Control and Done
set) naming a real stream (id != 0) with a positive delta and no trailing
bytes. A non-conforming frame yields ok=false and is dropped by the
caller, so a reserved stream-0 update from a future peer is ignored
rather than mishandled.

This is dormant scaffolding for the flow-control work: nothing emits or
acts on the frame yet.

Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-window-update branch from 9c0b23b to 5c858e3 Compare July 13, 2026 09:17
@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-send-window branch 7 times, most recently from b1f5883 to 4d08d0b Compare July 13, 2026 12:29
@suj-krishnan
suj-krishnan marked this pull request as ready for review July 13, 2026 12:50
Base automatically changed from sujatha/flow-control-window-update to main July 14, 2026 12:35

@shubhamdhama shubhamdhama left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM.

Comment thread drpcstream/send_window.go Outdated
// now. acquire spends credit (blocking until enough is available), grant adds
// credit, and close terminates the window.
//
// avail is a signed int64;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant

Comment thread drpcstream/send_window.go Outdated
Comment on lines +131 to +132
close(w.notify)
w.notify = make(chan struct{})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Employ something like

// unblockWritesLocked wakes every producer parked on backpressure by closing
// the current drain channel (a broadcast) and installing a fresh one. The
// blocked guard keeps this allocation-free on the common path where nobody is
// waiting. It must be called with mu held.
func (mw *MuxWriter) unblockWritesLocked() {
if mw.blocked > 0 {
close(mw.drain)
mw.drain = make(chan struct{})
}
}

@suj-krishnan suj-krishnan Jul 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean, add the check so we don't allocate when there are no waiters? I didn't keep it because grants are not as frequent (we coalesce and grant only for every 1 MB or so) - keeping a blocked counter and updating in every case didn't seem worth it. (The mux_writer logic is clearly in the hot path and needs it).
I will use a nil check on the channel instead to save on the allocation.

@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-send-window branch from 4d08d0b to deefd7b Compare July 15, 2026 11:03
@shubhamdhama
shubhamdhama self-requested a review July 15, 2026 14:44
Add sendWindow, the per-stream flow-control credit balance on the sender.
acquire spends credit, blocking until enough is available; grant adds
credit; close terminates the window. acquire returns early on close (with
the close error) or context cancellation (with ctx.Err()), consuming no
credit in either case; both are checked before any debit, so a canceled
context never consumes credit or lets a frame proceed even when credit is
available.

Grants are no-revoke: grant takes an unsigned delta (the wire type), so a
grant can only ever raise the balance, and the addition saturates at
math.MaxInt64 so a large or malicious delta can never wrap it negative.
The balance is a signed int64; acquire never drives it below zero, but the
enablement layer may debit credit for bytes sent before flow control
begins enforcing, leaving it negative, in which case acquire blocks until
grants restore it.

This is the primitive only -- it is not yet wired into the stream write
path (that gate comes in a later commit).

Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
@suj-krishnan
suj-krishnan force-pushed the sujatha/flow-control-send-window branch from deefd7b to af4331b Compare July 16, 2026 09:08
@suj-krishnan

Copy link
Copy Markdown
Author

TFTR @shubhamdhama!
Merging this PR - the code in it is never exercised and therefore safe across drpc version bumps.

@suj-krishnan
suj-krishnan merged commit b8b4dcb into main Jul 16, 2026
3 checks passed
@suj-krishnan
suj-krishnan deleted the sujatha/flow-control-send-window branch July 16, 2026 09:10
@suj-krishnan
suj-krishnan restored the sujatha/flow-control-send-window branch July 16, 2026 09:29
@suj-krishnan
suj-krishnan deleted the sujatha/flow-control-send-window branch July 16, 2026 09:30
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.

3 participants