drpcstream: gate data-frame sends on the per-stream send window by suj-krishnan · Pull Request #71 · cockroachdb/drpc

suj-krishnan · 2026-07-01T14:54:50Z

Two commits:

1. drpcstream: take the write lock without holding s.mu — a standalone locking change: Close/SendError/CloseSend release s.mu, wait for s.write, then re-lock and re-check state. Holding s.mu while waiting deadlocks every path that needs it (Cancel, terminate, the connection reader) behind a slow writer, and a mixed order is an ABBA deadlock. Uniform rule: s.write before s.mu; never wait on s.write holding s.mu. Regression test uses transport backpressure (no flow control involved).

2. drpcstream: gate data-frame sends on the per-stream send window — wires the sendWindow credit gate into the stream write path:

In rawWriteLocked, a KindMessage frame acquires len(frame) bytes of per-stream send credit before it is handed to the writer.
Control kinds (invoke/metadata/etc.) bypass the gate — flow control applies to data frames only.
terminate closes the window with the send-side error (sigs.send.Err() — first-wins, so io.EOF when a cancel/error path pre-set it): a send parked on credit returns the same error as one parked in MuxWriter.WriteFrame or a later send. SendError's pre-close uses io.EOF and Close's uses termClosed for the same parked/unparked agreement.
Close and SendError close the send window before touching the write lock (signal-first, like SendCancel), so a credit-parked send wakes and releases the lock instead of stalling them on a grant that may never come.

Preemption matrix for the four callers that take the write lock:

Caller	Terminates?	Behind a parked send
`SendCancel`	yes	preempts (already signal-first)
`Close`	yes	preempts (fixed here)
`SendError`	yes	preempts (fixed here)
`CloseSend`	no — half-close	waits (write-then-mu order); parked send completes on grant, and termination can always break the wait

…or/CloseSend Close, SendError, and CloseSend used to hold s.mu while waiting for the write lock. The writer holding that lock can be blocked for a long time (transport backpressure today; send credit once flow control lands), and holding s.mu while waiting wedges every path that needs it -- Cancel, terminate, and the connection reader delivering inbound terminal frames. A mixed order is also an ABBA deadlock waiting to happen between any two of these paths. All three now release s.mu, wait for the write lock, then re-lock and re-check stream state before proceeding (returning nil if another path terminated meanwhile, matching their existing no-op contracts). The stream's lock order is uniform: s.write before s.mu; never wait on s.write while holding s.mu. The regression test parks a send on a full write buffer, queues CloseSend behind it, and verifies Cancel still unwedges everything -- with the old ordering, Cancel blocks on s.mu held by CloseSend and the parked send is never freed. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>

Wire the sendWindow credit gate into rawWriteLocked: a KindMessage frame now acquires len(frame) bytes of per-stream send credit before it is handed to the writer. Control frames (invoke/metadata) bypass the gate, and write stats are recorded only after credit is acquired, next to the WriteFrame they describe. The window is opt-in: a stream has no send window by default, so data writes stay ungated (unlimited) and behavior is unchanged until one is installed. terminate closes the window with the send-side error (sigs.send is first-wins, holding io.EOF when a cancel/error path pre-set it), so a send parked on credit returns the same error as one parked in WriteFrame or a later send. A send parked on credit is freed only by terminate (grants, ctx cancel, or window close), so Close and SendError close the send window before touching the write lock, mirroring SendCancel's signal-first ordering; otherwise they would stall behind the parked send waiting on a grant a slow consumer may never send. CloseSend deliberately does not preempt: it is a graceful half-close, waiting (without s.mu, per the previous commit) for a parked send to complete once credit arrives. Per-stream only; the connection-level window and the enablement path that installs the window come in later commits. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>

shubhamdhama

These are the comments for the first commit.

shubhamdhama · 2026-07-20T15:36:02Z

 	defer s.checkFinished()

-	// Wait for the write lock without holding s.mu (see Close).
+	// Wait for the write lock without holding s.mu, matching CloseSend's


This comment is related to the other PR.

shubhamdhama · 2026-07-21T07:21:53Z

 	s.mu.Lock()
 	if s.sigs.term.IsSet() {
 		s.mu.Unlock()
 		return nil
 	}

+	// Close the send window before taking the write lock so a send parked on
+	// credit wakes and releases the lock (same ordering as Close). io.EOF to
+	// match the sigs.send error set below, so parked and later sends agree.
+	if s.sendw != nil {
+		s.sendw.close(io.EOF)
+	}
+	s.mu.Unlock()


This doesn't need to be wrapped in s.mu locks. Both are concurrency safe.

shubhamdhama · 2026-07-21T07:22:14Z

 	s.mu.Lock()
 	if s.sigs.term.IsSet() {
 		s.mu.Unlock()
 		return nil
 	}

+	// Close the send window before taking the write lock so a send parked on
+	// credit wakes and releases the lock, instead of stalling the close on a
+	// grant that may never come (mirrors SendCancel's signal-first ordering).
+	if s.sendw != nil {
+		s.sendw.close(termClosed)
+	}
+	s.mu.Unlock()


Same here: This doesn't need to be wrapped in s.mu locks. Both are concurrency safe.

shubhamdhama · 2026-07-21T07:22:38Z

 		s.mu.Unlock()
 		return nil
 	}
+	s.mu.Unlock()


This one too: This doesn't need to be wrapped in s.mu locks. This is concurrency safe.

suj-krishnan mentioned this pull request Jul 1, 2026

drpcstream: per-stream receive window and read-path grant wiring #72

Open

suj-krishnan force-pushed the sujatha/flow-control-send-window branch from 50c697f to a4ad9e8 Compare July 13, 2026 07:00

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch from 58a5fed to 30ffb52 Compare July 13, 2026 07:00

suj-krishnan force-pushed the sujatha/flow-control-send-window branch from a4ad9e8 to 6ff7baf Compare July 13, 2026 07:06

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch from 30ffb52 to 9b6d87b Compare July 13, 2026 07:06

suj-krishnan force-pushed the sujatha/flow-control-send-window branch from 6ff7baf to 7fb0331 Compare July 13, 2026 08:41

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch from 9b6d87b to 2512021 Compare July 13, 2026 08:41

suj-krishnan force-pushed the sujatha/flow-control-send-window branch from 7fb0331 to 9c3bc23 Compare July 13, 2026 08:47

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch from 2512021 to c086e23 Compare July 13, 2026 08:47

suj-krishnan force-pushed the sujatha/flow-control-send-window branch from 9c3bc23 to efc77b4 Compare July 13, 2026 08:55

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch from c086e23 to b14cb1f Compare July 13, 2026 08:55

suj-krishnan force-pushed the sujatha/flow-control-send-window branch from efc77b4 to d88576b Compare July 13, 2026 08:59

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch from b14cb1f to 6594c14 Compare July 13, 2026 08:59

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 force-pushed the sujatha/flow-control-send-gate branch 4 times, most recently from fe9d197 to 228866a Compare July 14, 2026 08:46

suj-krishnan marked this pull request as ready for review July 14, 2026 09:51

suj-krishnan requested a review from shubhamdhama July 14, 2026 09:54

suj-krishnan self-assigned this Jul 14, 2026

suj-krishnan requested review from Nukitt and cthumuluru-crdb July 14, 2026 09:54

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch from 228866a to de7b9dc Compare July 14, 2026 10:00

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch 2 times, most recently from fcfa2e4 to 10b863f Compare July 14, 2026 10:25

suj-krishnan force-pushed the sujatha/flow-control-send-window branch from 4d08d0b to deefd7b Compare July 15, 2026 11:03

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch 2 times, most recently from 7d03098 to 336c61c Compare July 16, 2026 09:08

suj-krishnan force-pushed the sujatha/flow-control-send-window branch from deefd7b to af4331b Compare July 16, 2026 09:08

Base automatically changed from sujatha/flow-control-send-window to main July 16, 2026 09:10

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch from 336c61c to ef342ce Compare July 16, 2026 10:16

suj-krishnan and others added 2 commits July 17, 2026 11:07

suj-krishnan force-pushed the sujatha/flow-control-send-gate branch from ef342ce to 0d5cf31 Compare July 17, 2026 05:38

shubhamdhama reviewed Jul 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

drpcstream: gate data-frame sends on the per-stream send window#71

drpcstream: gate data-frame sends on the per-stream send window#71
suj-krishnan wants to merge 2 commits into
mainfrom
sujatha/flow-control-send-gate

suj-krishnan commented Jul 1, 2026 •

edited

Loading

Uh oh!

shubhamdhama left a comment

Uh oh!

shubhamdhama Jul 20, 2026

Uh oh!

shubhamdhama Jul 21, 2026

Uh oh!

shubhamdhama Jul 21, 2026

Uh oh!

shubhamdhama Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

suj-krishnan commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shubhamdhama left a comment

Choose a reason for hiding this comment

Uh oh!

shubhamdhama Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

shubhamdhama Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

shubhamdhama Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

shubhamdhama Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

suj-krishnan commented Jul 1, 2026 •

edited

Loading