Skip to content

Commit 6649f34

Browse files
drpcstream: gate data-frame sends on the per-stream send window
Wire the sendWindow credit gate into rawWriteLocked: a KindMessage frame acquires len(frame) bytes of per-stream send credit before it is handed to the writer. Control frames (invoke/metadata) bypass the gate. 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. SendCancel and Cancel already terminate before taking the write lock, so they wake a credit-parked writer. Per-stream only. NOTE: Close/SendError/CloseSend still take the write lock while holding s.mu, so a send parked on credit can deadlock them; the lock-ordering rework that makes those paths preempt a parked writer is handled separately. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
1 parent eb12ac3 commit 6649f34

2 files changed

Lines changed: 140 additions & 0 deletions

File tree

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// Copyright (C) 2026 Cockroach Labs.
2+
// See LICENSE for copying information.
3+
4+
package drpcstream
5+
6+
import (
7+
"context"
8+
"errors"
9+
"io"
10+
"testing"
11+
"time"
12+
13+
"github.com/zeebo/assert"
14+
"github.com/zeebo/errs"
15+
16+
"storj.io/drpc/drpcwire"
17+
)
18+
19+
// newGateStream builds a stream writing to io.Discard with an explicit
20+
// SplitSize so small payloads are a single frame.
21+
func newGateStream(t *testing.T) *Stream {
22+
mw := testMuxWriter(t)
23+
return NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
24+
}
25+
26+
// By default no send window is installed, so data writes are ungated
27+
// (unlimited) and behavior is unchanged.
28+
func TestStream_SendWindowDefaultUngated(t *testing.T) {
29+
st := newGateStream(t)
30+
assert.That(t, st.sendw == nil)
31+
assert.NoError(t, st.RawWrite(drpcwire.KindMessage, []byte("hello")))
32+
}
33+
34+
// With a finite send window, a data write blocks until enough credit is
35+
// granted.
36+
func TestStream_SendWindowGatesDataWrite(t *testing.T) {
37+
st := newGateStream(t)
38+
st.sendw = newSendWindow(4) // 4 bytes of credit
39+
40+
done := make(chan error, 1)
41+
go func() { done <- st.RawWrite(drpcwire.KindMessage, []byte("hello")) }() // 5 bytes > 4
42+
43+
select {
44+
case <-done:
45+
t.Fatal("data write returned before sufficient credit")
46+
case <-time.After(blockShort):
47+
}
48+
49+
st.sendw.grant(1) // 4 + 1 = 5 >= 5
50+
51+
select {
52+
case err := <-done:
53+
assert.NoError(t, err)
54+
case <-time.After(time.Second):
55+
t.Fatal("data write did not complete after grant")
56+
}
57+
}
58+
59+
// Control kinds (here, invoke) are not flow-controlled: they proceed even with
60+
// zero send credit.
61+
func TestStream_SendWindowControlKindsBypassGate(t *testing.T) {
62+
st := newGateStream(t)
63+
st.sendw = newSendWindow(0) // no credit at all
64+
65+
assert.NoError(t, st.WriteInvoke("service.Method", nil))
66+
}
67+
68+
// SendCancel preempts a send parked on credit: it terminates (closing the
69+
// window) before taking the write lock, so the parked write wakes, releases the
70+
// lock, and the cancel frame goes out.
71+
func TestStream_SendWindowSendCancelPreemptsParkedWrite(t *testing.T) {
72+
st := newGateStream(t)
73+
st.sendw = newSendWindow(0) // send will park immediately
74+
75+
done := make(chan error, 1)
76+
go func() { done <- st.RawWrite(drpcwire.KindMessage, []byte("data")) }()
77+
78+
select {
79+
case <-done:
80+
t.Fatal("data write returned before cancel")
81+
case <-time.After(blockShort):
82+
}
83+
84+
assert.NoError(t, st.SendCancel(context.Canceled))
85+
86+
select {
87+
case err := <-done:
88+
// Same error as a send parked in WriteFrame or a later send would see.
89+
assert.That(t, errors.Is(err, io.EOF))
90+
case <-time.After(time.Second):
91+
t.Fatal("parked data write was not preempted by SendCancel")
92+
}
93+
94+
// A subsequent send observes the same error as the parked one.
95+
assert.That(t, errors.Is(st.RawWrite(drpcwire.KindMessage, []byte("more")), io.EOF))
96+
}
97+
98+
// Terminating the stream wakes a send parked on credit.
99+
func TestStream_SendWindowTerminateWakesParkedWrite(t *testing.T) {
100+
st := newGateStream(t)
101+
st.sendw = newSendWindow(0) // send will park immediately
102+
103+
done := make(chan error, 1)
104+
go func() { done <- st.RawWrite(drpcwire.KindMessage, []byte("data")) }()
105+
106+
select {
107+
case <-done:
108+
t.Fatal("data write returned before termination")
109+
case <-time.After(blockShort):
110+
}
111+
112+
st.Cancel(errs.New("boom"))
113+
114+
select {
115+
case err := <-done:
116+
// Cancel pre-sets sigs.send to io.EOF; the window closes with it.
117+
assert.That(t, errors.Is(err, io.EOF))
118+
case <-time.After(time.Second):
119+
t.Fatal("parked data write did not wake on termination")
120+
}
121+
}

drpcstream/stream.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ type Stream struct {
6161
cbuf []byte // compression scratch buffer
6262
dbuf []byte // decompression scratch buffer
6363

64+
// sendw is the per-stream send-side flow-control window. It is nil when
65+
// flow control is not enabled, in which case data writes are ungated.
66+
sendw *sendWindow
67+
6468
mu sync.Mutex // protects state transitions
6569
sigs struct {
6670
send drpcsignal.Signal // set when done sending messages
@@ -361,6 +365,12 @@ func (s *Stream) terminate(err error) {
361365
s.sigs.recv.Set(err)
362366
s.sigs.term.Set(err)
363367
s.recvQueue.Close(err)
368+
if s.sendw != nil {
369+
// Close with the send-side error: sigs.send is first-wins, so when a
370+
// caller pre-set it (io.EOF for cancel/error), a send parked on credit
371+
// returns the same error as one parked in WriteFrame or a later send.
372+
s.sendw.close(s.sigs.send.Err())
373+
}
364374
s.checkFinished()
365375
}
366376

@@ -417,6 +427,15 @@ func (s *Stream) rawWriteLocked(kind drpcwire.Kind, data []byte) (err error) {
417427
fr.Data, data = drpcwire.SplitData(data, n)
418428
fr.Done = len(data) == 0
419429

430+
// Only data frames consume send credit; a nil window (flow control
431+
// disabled) leaves sends ungated. acquire parks until credit arrives,
432+
// the ctx is canceled, or the window closes (stream termination).
433+
if kind == drpcwire.KindMessage && s.sendw != nil {
434+
if err := s.sendw.acquire(s.Context(), int64(len(fr.Data))); err != nil {
435+
return err
436+
}
437+
}
438+
420439
drpcopts.GetStreamStats(&s.opts.Internal).AddWritten(uint64(len(fr.Data)))
421440
s.log("SEND", fr.String)
422441

0 commit comments

Comments
 (0)