From 8d3dafc7d00c733ecbd61d08728bfbb2b4d58797 Mon Sep 17 00:00:00 2001 From: Sujatha Sivaramakrishnan Date: Wed, 1 Jul 2026 20:35:47 +0530 Subject: [PATCH 1/5] drpcstream: add per-stream receive-window grant policy Add recvWindow, the per-stream receive side of flow control. It 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 the accrued returnable credit is otherwise released once it reaches the threshold, coalescing many frames into a single grant. Consuming reopens a gate that the high-water mark had closed. dispatched/consumed return the credit delta the caller should emit as a KindWindowUpdate. Emitting the frame, wiring into the read path (HandleFrame/MsgRecv), and the connection-level receive budget come in later commits. Co-Authored-By: roachdev-claude --- drpcstream/recv_window.go | 62 ++++++++++++++++++++++++++++++++++ drpcstream/recv_window_test.go | 62 ++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 drpcstream/recv_window.go create mode 100644 drpcstream/recv_window_test.go diff --git a/drpcstream/recv_window.go b/drpcstream/recv_window.go new file mode 100644 index 00000000..c916de77 --- /dev/null +++ b/drpcstream/recv_window.go @@ -0,0 +1,62 @@ +// Copyright (C) 2026 Cockroach Labs. +// See LICENSE for copying information. + +package drpcstream + +import "sync" + +// recvWindow decides when the receive side returns credit to the sender: it +// withholds grants while buffered bytes are at or above the high-water mark +// (credit keeps accruing), and otherwise releases accrued credit once it +// reaches the threshold, coalescing many frames into one grant. dispatched +// (reader goroutine) and consumed (application goroutine) return the credit +// delta to send as a KindWindowUpdate and may run concurrently. +type recvWindow struct { + mu sync.Mutex + buffered int64 // in-progress reassembly + completed, not-yet-consumed bytes + pending int64 // returnable credit accrued but not yet granted + highWater int64 // withhold grants while buffered is at or above this + threshold int64 // release accrued credit once it reaches this +} + +// newRecvWindow returns a recvWindow seeded with highWater and threshold. +func newRecvWindow(highWater, threshold int64) *recvWindow { + return &recvWindow{highWater: highWater, threshold: threshold} +} + +// dispatched records that n bytes were dispatched off the wire into the +// stream's buffers and returns the credit delta to grant (0 if none). +func (w *recvWindow) dispatched(n int64) int64 { + w.mu.Lock() + defer w.mu.Unlock() + w.buffered += n + w.pending += n + return w.maybeGrantLocked() +} + +// consumed records that n bytes were consumed by the application and returns +// the credit delta to grant (0 if none). +func (w *recvWindow) consumed(n int64) int64 { + w.mu.Lock() + defer w.mu.Unlock() + w.buffered -= n + return w.maybeGrantLocked() +} + +// bufferedBytes returns the current buffered byte count. +func (w *recvWindow) bufferedBytes() int64 { + w.mu.Lock() + defer w.mu.Unlock() + return w.buffered +} + +// maybeGrantLocked releases accrued credit when the gate is open (buffered +// below high-water) and pending has reached the threshold. Requires w.mu. +func (w *recvWindow) maybeGrantLocked() int64 { + if w.buffered < w.highWater && w.pending >= w.threshold { + g := w.pending + w.pending = 0 + return g + } + return 0 +} diff --git a/drpcstream/recv_window_test.go b/drpcstream/recv_window_test.go new file mode 100644 index 00000000..3331c1cf --- /dev/null +++ b/drpcstream/recv_window_test.go @@ -0,0 +1,62 @@ +// Copyright (C) 2026 Cockroach Labs. +// See LICENSE for copying information. + +package drpcstream + +import ( + "testing" + + "github.com/zeebo/assert" +) + +// Below the high-water mark, accrued returnable credit is released as a grant +// once it reaches the threshold, and withheld (0) below it. +func TestRecvWindowGrantsAtThreshold(t *testing.T) { + w := newRecvWindow(1000, 100) + assert.Equal(t, w.dispatched(60), int64(0)) // pending 60 < 100 + assert.Equal(t, w.dispatched(60), int64(120)) // pending 120 >= 100, buffered 120 < 1000 + assert.Equal(t, w.bufferedBytes(), int64(120)) + assert.Equal(t, w.dispatched(50), int64(0)) // pending reset by the grant; 50 < 100 +} + +// At or above the high-water mark, grants are withheld even when the accrued +// credit is past the threshold. +func TestRecvWindowWithholdsAtOrAboveHighWater(t *testing.T) { + w := newRecvWindow(100, 50) + assert.Equal(t, w.dispatched(120), int64(0)) // buffered 120 >= 100 -> withhold + assert.Equal(t, w.bufferedBytes(), int64(120)) +} + +// Consuming buffered data drops below the high-water mark and flushes the +// credit accrued while the gate was closed (resume-on-consume). +func TestRecvWindowResumesOnConsume(t *testing.T) { + w := newRecvWindow(100, 50) + assert.Equal(t, w.dispatched(120), int64(0)) // withheld; pending 120 + assert.Equal(t, w.consumed(30), int64(120)) // buffered 90 < 100 -> flush pending + assert.Equal(t, w.bufferedBytes(), int64(90)) +} + +// Consuming while the accrued credit is below the threshold emits no grant. +func TestRecvWindowConsumeBelowThresholdNoGrant(t *testing.T) { + w := newRecvWindow(1000, 100) + assert.Equal(t, w.dispatched(50), int64(0)) // pending 50 < 100 + assert.Equal(t, w.consumed(20), int64(0)) // buffered 30, pending 50 < 100 + assert.Equal(t, w.bufferedBytes(), int64(30)) +} + +// Many small dispatches coalesce into few grants, each carrying the accrued +// amount. +func TestRecvWindowCoalescesGrants(t *testing.T) { + w := newRecvWindow(1_000_000, 100) + grants, granted := 0, int64(0) + for i := 0; i < 10; i++ { + if g := w.dispatched(30); g > 0 { + grants++ + granted += g + } + } + // 10 dispatches of 30 (300 bytes) with threshold 100 coalesce into 2 grants + // of 120 each; the remaining 60 stays accrued. + assert.Equal(t, grants, 2) + assert.Equal(t, granted, int64(240)) +} From 3b297032336af5639ff9017b85e74582a4f74285 Mon Sep 17 00:00:00 2001 From: Sujatha Sivaramakrishnan Date: Wed, 1 Jul 2026 20:45:38 +0530 Subject: [PATCH 2/5] drpcstream,drpcwire: wire the receive-window grant policy into the read path Install a per-stream recvWindow and drive it from the read/recv path: - 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 returns credit via recvWindow, emitting a KindWindowUpdate once the accrued amount crosses the threshold and the high-water gate is open. - RawRecv/MsgRecv call recvWindow after dequeuing; consuming can reopen a gate the high-water mark had closed and flush the accrued grant. 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. Stream-level only; the connection-level receive budget and the enablement path that installs the windows come in later commits. 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 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 (TakeDiscarded) and HandleFrame releases the bytes as consumed, letting their credit flow back. The release runs last (deferred past the replacement frame's accounting and handlePacket), so the grant decision sees the final buffered state rather than the transient dip, and a terminal packet that supersedes a partial suppresses the grant via termination. emitGrant is also a no-op once the stream has terminated: the ring drains queued messages after close, and returning credit behind the terminal frame would invite the peer to send more doomed data. Co-Authored-By: roachdev-claude --- drpcstream/recv_window_wiring_test.go | 317 ++++++++++++++++++++++++++ drpcstream/stream.go | 66 +++++- drpcwire/packet_assembler.go | 23 +- drpcwire/packet_assembler_test.go | 63 +++++ 4 files changed, 464 insertions(+), 5 deletions(-) create mode 100644 drpcstream/recv_window_wiring_test.go diff --git a/drpcstream/recv_window_wiring_test.go b/drpcstream/recv_window_wiring_test.go new file mode 100644 index 00000000..7858eef0 --- /dev/null +++ b/drpcstream/recv_window_wiring_test.go @@ -0,0 +1,317 @@ +// Copyright (C) 2026 Cockroach Labs. +// See LICENSE for copying information. + +package drpcstream + +import ( + "context" + "io" + "testing" + "time" + + "github.com/zeebo/assert" + "github.com/zeebo/errs" + + "storj.io/drpc" + "storj.io/drpc/drpcwire" +) + +// captureWriter returns a MuxWriter whose output frames are decoded and +// delivered on the returned channel, so tests can observe emitted grants. +func captureWriter(t *testing.T) (*drpcwire.MuxWriter, <-chan drpcwire.Frame) { + t.Helper() + pr, pw := io.Pipe() + mw := drpcwire.NewMuxWriter(pw, func(error) {}) + frames := make(chan drpcwire.Frame, 64) + rd := drpcwire.NewReader(pr) + go func() { + for { + fr, err := rd.ReadFrame() + if err != nil { + return + } + frames <- fr + } + }() + t.Cleanup(func() { + mw.Stop(nil) + <-mw.Done() + _ = pw.Close() + _ = pr.Close() + }) + return mw, frames +} + +func waitFrame(t *testing.T, frames <-chan drpcwire.Frame) drpcwire.Frame { + t.Helper() + select { + case fr := <-frames: + return fr + case <-time.After(time.Second): + t.Fatal("expected a frame to be written, got none") + return drpcwire.Frame{} + } +} + +func assertNoFrame(t *testing.T, frames <-chan drpcwire.Frame) { + t.Helper() + select { + case fr := <-frames: + t.Fatalf("unexpected frame written: %v", fr) + case <-time.After(blockShort): + } +} + +func msgFrame(sid, mid uint64, data []byte, done bool) drpcwire.Frame { + return drpcwire.Frame{ + ID: drpcwire.ID{Stream: sid, Message: mid}, + Kind: drpcwire.KindMessage, + Data: data, + Done: done, + } +} + +// An incoming KindWindowUpdate is intercepted and applied to the send window. +func TestStream_IncomingGrantAppliesToSendWindow(t *testing.T) { + st := newGateStream(t) + st.sendw = newSendWindow(0) + + assert.NoError(t, st.HandleFrame(drpcwire.WindowUpdateFrame(st.ID(), 500))) + assert.Equal(t, st.sendw.available(), int64(500)) +} + +// A grant interleaved with an in-progress message is intercepted before the +// assembler, so reassembly is undisturbed. +func TestStream_GrantDoesNotDisturbReassembly(t *testing.T) { + st := newGateStream(t) + sid := st.ID() + + assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, []byte("foo"), false))) + assert.NoError(t, st.HandleFrame(drpcwire.WindowUpdateFrame(sid, 100))) // interleaved + assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, []byte("bar"), true))) + + got, err := st.RawRecv() + assert.NoError(t, err) + assert.Equal(t, string(got), "foobar") +} + +// Dispatching a data frame returns credit once the threshold is met. +func TestStream_DispatchEmitsGrant(t *testing.T) { + mw, frames := captureWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10}) + st.recvw = newRecvWindow(1<<20, 100) + + assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 150), true))) + + fr := waitFrame(t, frames) + sid, delta, ok := drpcwire.ParseWindowUpdate(fr) + assert.That(t, ok) + assert.Equal(t, sid, st.ID()) + assert.Equal(t, delta, uint64(150)) +} + +// Above the high-water mark, dispatch withholds; consuming resumes and flushes +// the accrued credit. +func TestStream_ConsumeEmitsGrantOnResume(t *testing.T) { + mw, frames := captureWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10}) + st.recvw = newRecvWindow(100, 50) + + // 120 bytes buffered >= high-water 100 -> withheld. + assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 120), true))) + assertNoFrame(t, frames) + + // Consume -> buffered drops below high-water -> flush accrued 120. + _, err := st.RawRecv() + assert.NoError(t, err) + + fr := waitFrame(t, frames) + _, delta, ok := drpcwire.ParseWindowUpdate(fr) + assert.That(t, ok) + assert.Equal(t, delta, uint64(120)) +} + +// With no receive window installed, no grants are emitted and an incoming grant +// with no send window is harmlessly ignored. +func TestStream_NoWindowsNoGrants(t *testing.T) { + mw, frames := captureWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10}) + + assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 150), true))) + assertNoFrame(t, frames) + + assert.NoError(t, st.HandleFrame(drpcwire.WindowUpdateFrame(st.ID(), 100))) +} + +// Nonconforming grant frames are dropped by the intercept without crediting. +func TestStream_MalformedGrantIgnored(t *testing.T) { + st := newGateStream(t) + st.sendw = newSendWindow(0) + + missing := drpcwire.WindowUpdateFrame(st.ID(), 1) + missing.Data = nil // no delta payload + assert.NoError(t, st.HandleFrame(missing)) + assert.Equal(t, st.sendw.available(), int64(0)) + + zero := drpcwire.WindowUpdateFrame(st.ID(), 1) + zero.Data = drpcwire.AppendVarint(nil, 0) // zero delta + assert.NoError(t, st.HandleFrame(zero)) + assert.Equal(t, st.sendw.available(), int64(0)) +} + +// A grant arriving after termination is dropped. +func TestStream_GrantAfterTerminationDropped(t *testing.T) { + st := newGateStream(t) + st.sendw = newSendWindow(0) + + st.Cancel(errs.New("boom")) + assert.NoError(t, st.HandleFrame(drpcwire.WindowUpdateFrame(st.ID(), 100))) + assert.Equal(t, st.sendw.available(), int64(0)) +} + +// An unfinished message superseded by a higher message id must release its +// buffered bytes, or the high-water gate sticks shut and the withheld grant +// is never emitted (sender deadlock once its credit is exhausted). +func TestStream_DiscardedPartialMessageReleasesCredit(t *testing.T) { + mw, frames := captureWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10}) + st.recvw = newRecvWindow(100, 50) + sid := st.ID() + + // Partial message 1 reaches the high-water mark: grants withheld. + assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, make([]byte, 120), false))) + assertNoFrame(t, frames) + + // Message 2 supersedes it; the discarded 120 bytes leave the buffer after + // the replacement is counted, the gate reopens, and all accrued credit + // (120 discarded + 10 replacement) flushes. + assert.NoError(t, st.HandleFrame(msgFrame(sid, 2, make([]byte, 10), true))) + + _, delta, ok := drpcwire.ParseWindowUpdate(waitFrame(t, frames)) + assert.That(t, ok) + assert.Equal(t, delta, uint64(130)) + + // The completed message is intact and its own credit accounting holds. + got, err := st.RawRecv() + assert.NoError(t, err) + assert.Equal(t, len(got), 10) + assert.Equal(t, st.recvw.bufferedBytes(), int64(0)) +} + +// The discard release must not decide on the transient dip before the +// replacement frame is counted: a large replacement that lands above +// high-water withholds everything, including the discarded bytes' credit. +func TestStream_DiscardReleaseSeesReplacementFrame(t *testing.T) { + mw, frames := captureWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10}) + st.recvw = newRecvWindow(100, 50) + sid := st.ID() + + // Partial message 1 pins buffered at the high-water mark. + assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, make([]byte, 120), false))) + assertNoFrame(t, frames) + + // Message 2 (1000 bytes) supersedes it. Final buffered = 1000 >= 100, so + // no grant may be emitted -- releasing the 120 on the dip would be wrong. + assert.NoError(t, st.HandleFrame(msgFrame(sid, 2, make([]byte, 1000), true))) + assertNoFrame(t, frames) + + // Consuming the completed message reopens the gate and flushes everything. + _, err := st.RawRecv() + assert.NoError(t, err) + _, delta, ok := drpcwire.ParseWindowUpdate(waitFrame(t, frames)) + assert.That(t, ok) + assert.Equal(t, delta, uint64(1120)) +} + +// A terminal frame that supersedes a partial message must not flush the +// discarded bytes' credit: termination is applied first, and emitGrant +// no-ops on a terminated stream. +func TestStream_TerminalSupersedeEmitsNoGrant(t *testing.T) { + mw, frames := captureWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10}) + st.recvw = newRecvWindow(100, 50) + sid := st.ID() + + // Partial message 1: grants withheld above high-water. + assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, make([]byte, 120), false))) + assertNoFrame(t, frames) + + // A remote cancel with a higher message id discards the partial and + // terminates the stream; the withheld credit must stay unemitted. + assert.NoError(t, st.HandleFrame(drpcwire.Frame{ + ID: drpcwire.ID{Stream: sid, Message: 2}, Kind: drpcwire.KindCancel, Done: true, + })) + assert.That(t, st.IsTerminated()) + assertNoFrame(t, frames) +} + +// Draining queued messages after termination must not emit grants: credit +// returned behind the terminal frame would invite more doomed data. +func TestStream_NoGrantsAfterTermination(t *testing.T) { + mw, frames := captureWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10}) + st.recvw = newRecvWindow(100, 50) + + // 120 bytes buffered >= high-water 100: grant withheld while live. + assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 120), true))) + assertNoFrame(t, frames) + + st.Cancel(errs.New("boom")) + + // The queued message still drains (ring close keeps buffered data + // readable), which on a live stream would flush the accrued 120 bytes of + // credit; after termination it must stay unemitted. + got, err := st.RawRecv() + assert.NoError(t, err) + assert.Equal(t, len(got), 120) + assertNoFrame(t, frames) +} + +// rawEnc round-trips raw byte slices for MsgRecv tests. +type rawEnc struct{} + +func (rawEnc) Marshal(msg drpc.Message) ([]byte, error) { return *(msg.(*[]byte)), nil } +func (rawEnc) Unmarshal(buf []byte, msg drpc.Message) error { + *(msg.(*[]byte)) = append([]byte(nil), buf...) + return nil +} + +// The MsgRecv consume path also returns credit, like RawRecv. +func TestStream_MsgRecvEmitsGrant(t *testing.T) { + mw, frames := captureWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10}) + st.recvw = newRecvWindow(100, 50) + + // 120 bytes buffered >= high-water 100 -> withheld on dispatch. + assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 120), true))) + assertNoFrame(t, frames) + + var msg []byte + assert.NoError(t, st.MsgRecv(&msg, rawEnc{})) + assert.Equal(t, len(msg), 120) + + _, delta, ok := drpcwire.ParseWindowUpdate(waitFrame(t, frames)) + assert.That(t, ok) + assert.Equal(t, delta, uint64(120)) +} + +// A message that fails to decompress still releases its bytes to the receive +// window. The bytes have left the queue, so buffered must not stay inflated -- +// otherwise the high-water gate could wedge credit shut after a bad frame. +func TestStream_DecompressErrorReleasesCredit(t *testing.T) { + mw := testMuxWriter(t) + opts := Options{SplitSize: 64 << 10, Compression: drpc.CompressionSnappy} + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), opts) + st.recvw = newRecvWindow(1<<20, 1<<20) // high threshold: no grant to mask the release + + // A KindMessage payload that is not valid snappy. + bad := []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff} + assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, bad, true))) + assert.Equal(t, st.recvw.bufferedBytes(), int64(len(bad))) // dispatched, not yet consumed + + // RawRecv fails to decompress, but the bytes must still be released. + _, err := st.RawRecv() + assert.Error(t, err) + assert.Equal(t, st.recvw.bufferedBytes(), int64(0)) +} diff --git a/drpcstream/stream.go b/drpcstream/stream.go index ce9c73aa..84cc235f 100644 --- a/drpcstream/stream.go +++ b/drpcstream/stream.go @@ -65,6 +65,10 @@ type Stream struct { // flow control is not enabled, in which case data writes are ungated. sendw *sendWindow + // recvw is the per-stream receive-side flow-control policy. It is nil when + // flow control is not enabled, in which case no grants are emitted. + recvw *recvWindow + mu sync.Mutex // protects state transitions sigs struct { send drpcsignal.Signal // set when done sending messages @@ -220,10 +224,38 @@ func (s *Stream) HandleFrame(fr drpcwire.Frame) (err error) { return nil } + // 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. + if fr.Kind == drpcwire.KindWindowUpdate { + if s.sendw != nil { + if _, delta, ok := drpcwire.ParseWindowUpdate(fr); ok { + s.sendw.grant(delta) + } + } + return nil + } + packet, packetReady, err := s.pa.AppendFrame(fr) if err != nil { return err } + + // Release bytes the assembler discarded (an unfinished message superseded + // by a higher id). Deferred so the gate decision sees the final buffered + // state and a terminal packet suppresses the grant (emitGrant checks term). + defer func() { + if kind, n := s.pa.TakeDiscarded(); n > 0 && kind == drpcwire.KindMessage && s.recvw != nil { + s.emitGrant(s.recvw.consumed(int64(n))) + } + }() + + // Accrue credit for a dispatched data frame; granted now or when the + // high-water gate reopens. + if fr.Kind == drpcwire.KindMessage && s.recvw != nil { + s.emitGrant(s.recvw.dispatched(int64(len(fr.Data)))) + } + if !packetReady { return nil } @@ -289,6 +321,17 @@ func (s *Stream) handlePacket(pkt drpcwire.Packet) (err error) { } } +// emitGrant sends a credit grant of delta bytes; no-op unless delta is +// positive and the stream is live (no credit behind a terminal frame). +// Control frames append without blocking, so the reader may call this. +// Best-effort: a write error means the stream is going away. +func (s *Stream) emitGrant(delta int64) { + if delta <= 0 || s.sigs.term.IsSet() { + return + } + _ = s.wr.WriteFrame(drpcwire.WindowUpdateFrame(s.id.Stream, uint64(delta)), nil) +} + // // helpers // @@ -463,6 +506,15 @@ func (s *Stream) RawRecv() (data []byte, err error) { return nil, err } defer s.recvQueue.Done() + n := len(b) // wire bytes, pre-decompression: matches dispatch accounting + + // The bytes have left the queue; release them to the receive window on every + // return path (including a decode failure below), or buffered stays inflated + // and the high-water gate can wedge credit shut. Consuming may also reopen + // the gate, releasing credit accrued at dispatch. + if s.recvw != nil { + defer func() { s.emitGrant(s.recvw.consumed(int64(n))) }() + } if s.opts.Compression != drpc.CompressionNone { s.dbuf, err = drpcwire.Decompress(s.opts.Compression, s.dbuf[:0], b) @@ -472,7 +524,6 @@ func (s *Stream) RawRecv() (data []byte, err error) { b = s.dbuf } data = append([]byte(nil), b...) - return data, nil } @@ -515,6 +566,15 @@ func (s *Stream) MsgRecv(msg drpc.Message, enc drpc.Encoding) (err error) { return err } defer s.recvQueue.Done() + n := len(b) // wire bytes, pre-decompression: matches dispatch accounting + + // The bytes have left the queue; release them to the receive window on every + // return path (including a decode failure below), or buffered stays inflated + // and the high-water gate can wedge credit shut. Consuming may also reopen + // the gate, releasing credit accrued at dispatch. + if s.recvw != nil { + defer func() { s.emitGrant(s.recvw.consumed(int64(n))) }() + } if s.opts.Compression != drpc.CompressionNone { s.dbuf, err = drpcwire.Decompress(s.opts.Compression, s.dbuf[:0], b) @@ -523,9 +583,7 @@ func (s *Stream) MsgRecv(msg drpc.Message, enc drpc.Encoding) (err error) { } b = s.dbuf } - err = enc.Unmarshal(b, msg) - - return err + return enc.Unmarshal(b, msg) } // diff --git a/drpcwire/packet_assembler.go b/drpcwire/packet_assembler.go index 2cf7fae3..f319eb93 100644 --- a/drpcwire/packet_assembler.go +++ b/drpcwire/packet_assembler.go @@ -16,6 +16,9 @@ type PacketAssembler struct { pk Packet assembling bool streamInitialized bool + + discardedKind Kind + discardedLen int } // NewPacketAssembler returns a new PacketAssembler ready to assemble frames. @@ -41,12 +44,26 @@ func (pa *PacketAssembler) Reset() { } pa.assembling = false pa.streamInitialized = false + pa.discardedKind, pa.discardedLen = 0, 0 +} + +// TakeDiscarded returns the kind and payload byte count of an unfinished +// message dropped by the most recent AppendFrame (a higher message id +// superseded it), and clears the record. n is 0 when nothing was dropped. +func (pa *PacketAssembler) TakeDiscarded() (kind Kind, n int) { + kind, n = pa.discardedKind, pa.discardedLen + pa.discardedKind, pa.discardedLen = 0, 0 + return kind, n } // AppendFrame adds a frame to the in-progress packet. It returns the completed // packet and true when a frame with Done=true is received. It returns false // when more frames are needed to complete the packet. func (pa *PacketAssembler) AppendFrame(fr Frame) (packet Packet, packetReady bool, err error) { + // A discard record is scoped to the most recent AppendFrame (see + // TakeDiscarded); clear any stale one before possibly setting a fresh one. + pa.discardedKind, pa.discardedLen = 0, 0 + // Enforce stream ID consistency: infer from first frame or reject mismatches. if !pa.streamInitialized { pa.pk.ID.Stream = fr.ID.Stream @@ -60,7 +77,11 @@ func (pa *PacketAssembler) AppendFrame(fr Frame) (packet Packet, packetReady boo return Packet{}, false, drpc.ProtocolError.New( "message id monotonicity violation: got %v, expected >= %v", fr.ID.Message, pa.pk.ID.Message) } else if fr.ID.Message > pa.pk.ID.Message || !pa.assembling { - // New message: reset the buffer and start assembling. + // New message: reset and start assembling. Record any dropped + // unfinished bytes so byte-accounting callers can release them. + if pa.assembling && len(pa.pk.Data) > 0 { + pa.discardedKind, pa.discardedLen = pa.pk.Kind, len(pa.pk.Data) + } pa.pk.Data = pa.pk.Data[:0] pa.assembling = true pa.pk.ID.Message = fr.ID.Message diff --git a/drpcwire/packet_assembler_test.go b/drpcwire/packet_assembler_test.go index 41cf70da..8c19667a 100644 --- a/drpcwire/packet_assembler_test.go +++ b/drpcwire/packet_assembler_test.go @@ -87,6 +87,69 @@ func TestPacketAssembler_HigherMsgDiscardsInProgress(t *testing.T) { assert.NoError(t, err) assert.That(t, ready) assert.DeepEqual(t, pkt.Data, []byte("kept")) + + // The dropped bytes are reported once so byte accounting can release them. + kind, n := pa.TakeDiscarded() + assert.Equal(t, kind, KindMessage) + assert.Equal(t, n, len("discard")) + _, n = pa.TakeDiscarded() + assert.Equal(t, n, 0) +} + +// A completed message followed by the next message id is not a discard. +func TestPacketAssembler_CompletedMessageNotReportedDiscarded(t *testing.T) { + pa := NewPacketAssembler() + pa.SetStreamID(1) + + _, ready, err := pa.AppendFrame(Frame{ + ID: ID{Stream: 1, Message: 1}, Kind: KindMessage, Data: []byte("done"), Done: true, + }) + assert.NoError(t, err) + assert.That(t, ready) + + _, ready, err = pa.AppendFrame(Frame{ + ID: ID{Stream: 1, Message: 2}, Kind: KindMessage, Data: []byte("next"), Done: true, + }) + assert.NoError(t, err) + assert.That(t, ready) + + _, n := pa.TakeDiscarded() + assert.Equal(t, n, 0) +} + +// Discard state is scoped to the most recent AppendFrame: a later +// non-discarding append clears an undrained record, and Reset drops it too. +func TestPacketAssembler_DiscardStateNotStale(t *testing.T) { + pa := NewPacketAssembler() + pa.SetStreamID(1) + + _, _, err := pa.AppendFrame(Frame{ + ID: ID{Stream: 1, Message: 1}, Kind: KindMessage, Data: []byte("old"), Done: false, + }) + assert.NoError(t, err) + _, _, err = pa.AppendFrame(Frame{ // supersedes m1: discard recorded + ID: ID{Stream: 1, Message: 2}, Kind: KindMessage, Data: []byte("a"), Done: false, + }) + assert.NoError(t, err) + _, _, err = pa.AppendFrame(Frame{ // non-discarding: undrained record cleared + ID: ID{Stream: 1, Message: 2}, Kind: KindMessage, Data: []byte("b"), Done: true, + }) + assert.NoError(t, err) + _, n := pa.TakeDiscarded() + assert.Equal(t, n, 0) + + // Reset clears a pending record as well. + _, _, err = pa.AppendFrame(Frame{ + ID: ID{Stream: 1, Message: 3}, Kind: KindMessage, Data: []byte("old"), Done: false, + }) + assert.NoError(t, err) + _, _, err = pa.AppendFrame(Frame{ + ID: ID{Stream: 1, Message: 4}, Kind: KindMessage, Data: []byte("new"), Done: true, + }) + assert.NoError(t, err) + pa.Reset() + _, n = pa.TakeDiscarded() + assert.Equal(t, n, 0) } // Continuation frames (same message ID, mid-assembly) must carry the same From 0a9b83d9ab446b1e0400661622a9c674a3012fae Mon Sep 17 00:00:00 2001 From: Sujatha Sivaramakrishnan Date: Thu, 16 Jul 2026 11:46:56 +0530 Subject: [PATCH 3/5] drpcstream: install per-stream windows from an internal flow-control option Streams install their send and receive windows from a FlowControl option carried in the internal (drpcopts) stream options. Keeping enablement internal-only makes accidental activation impossible for module consumers: bumping the drpc dependency mid-implementation cannot turn flow control on, because no public API reaches it. A later change promotes the option to the public Options once the version-gated enablement in CockroachDB is ready. Construction validates the configuration and panics on one that cannot make progress (silently disabling would drop the protection instead): all sizes positive, SplitSize bounded (negative means unsplit frames, so no bound holds; zero uses SplitData's 64 KiB default), and GrantThreshold + frame size <= StreamWindow -- up to GrantThreshold-1 bytes stay withheld at quiescence, and the remaining window must still cover the sender's next frame. Co-Authored-By: roachdev-claude --- drpcstream/flow_control_option_test.go | 119 +++++++++++++++++++++++++ drpcstream/stream.go | 34 ++++++- internal/drpcopts/stream.go | 26 ++++++ 3 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 drpcstream/flow_control_option_test.go diff --git a/drpcstream/flow_control_option_test.go b/drpcstream/flow_control_option_test.go new file mode 100644 index 00000000..702f4854 --- /dev/null +++ b/drpcstream/flow_control_option_test.go @@ -0,0 +1,119 @@ +// Copyright (C) 2026 Cockroach Labs. +// See LICENSE for copying information. + +package drpcstream + +import ( + "context" + "math" + "testing" + "time" + + "github.com/zeebo/assert" + + "storj.io/drpc/drpcwire" + "storj.io/drpc/internal/drpcopts" +) + +// fcOptions returns stream Options with flow control enabled via the internal +// option, the only way to enable it until it is promoted to a public option. +func fcOptions(window, highWater, threshold int64) Options { + opts := Options{SplitSize: 64 << 10} + drpcopts.SetStreamFlowControl(&opts.Internal, drpcopts.FlowControl{ + Enabled: true, + StreamWindow: window, + HighWater: highWater, + GrantThreshold: threshold, + }) + return opts +} + +// Enabling flow control installs the send and receive windows, seeding the +// send window with the configured stream window as initial credit. +func TestStream_FlowControlEnabledInstallsWindows(t *testing.T) { + mw := testMuxWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), fcOptions(256<<10, 4<<20, 128<<10)) + assert.That(t, st.sendw != nil) + assert.That(t, st.recvw != nil) + assert.Equal(t, st.sendw.available(), int64(256<<10)) +} + +// Without flow control enabled, no windows are installed and behavior is +// unchanged (ungated). +func TestStream_FlowControlDisabledNoWindows(t *testing.T) { + mw := testMuxWriter(t) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10}) + assert.That(t, st.sendw == nil) + assert.That(t, st.recvw == nil) +} + +// An invalid flow-control configuration panics at construction rather than +// deadlocking later or silently disabling the protection. +func TestStream_FlowControlValidation(t *testing.T) { + mustPanic := func(name string, opts Options) { + t.Helper() + defer func() { + if recover() == nil { + t.Fatalf("%s: expected construction to panic", name) + } + }() + NewWithOptions(context.Background(), 1, testMuxWriter(t), NewBufferPool(), opts) + } + + mustPanic("zero window", fcOptions(0, 4<<20, 128<<10)) + mustPanic("zero high water", fcOptions(256<<10, 0, 128<<10)) + mustPanic("zero threshold", fcOptions(256<<10, 4<<20, 0)) + mustPanic("negative window", fcOptions(-1, 4<<20, 128<<10)) + + // Threshold + frame must fit in the window, or a quiescent receiver can + // strand the sender below the next frame's cost. + mustPanic("threshold+frame exceeds window", fcOptions(128<<10, 4<<20, 128<<10)) + + // A near-max threshold must not sneak past via int64 overflow of the + // threshold+frame sum. + mustPanic("near-max threshold overflow", fcOptions(256<<10, 4<<20, math.MaxInt64)) + + // SplitSize < 0 means unsplit (unbounded) frames: no bound holds. + unbounded := fcOptions(256<<10, 4<<20, 64<<10) + unbounded.SplitSize = -1 + mustPanic("unbounded SplitSize", unbounded) + + // SplitSize 0 uses SplitData's 64 KiB default as the frame size. + def := fcOptions(128<<10, 4<<20, 64<<10) + def.SplitSize = 0 + NewWithOptions(context.Background(), 1, testMuxWriter(t), NewBufferPool(), def) // 64K+64K <= 128K: ok + def2 := fcOptions(96<<10, 4<<20, 64<<10) + def2.SplitSize = 0 + mustPanic("default frame exceeds window headroom", def2) +} + +// End to end via the option: an option-installed window gates a data write +// that exceeds the initial credit, and an incoming grant resumes it. SplitSize +// 2 with a 4-byte window means "hello" (frames 2+2+1) parks on its last frame. +func TestStream_FlowControlOptionGatesAndResumes(t *testing.T) { + mw := testMuxWriter(t) + opts := Options{SplitSize: 2} + drpcopts.SetStreamFlowControl(&opts.Internal, drpcopts.FlowControl{ + Enabled: true, StreamWindow: 4, HighWater: 1 << 20, GrantThreshold: 2, + }) + st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), opts) + + done := make(chan error, 1) + go func() { done <- st.RawWrite(drpcwire.KindMessage, []byte("hello")) }() // 5 bytes > 4 + + select { + case <-done: + t.Fatal("write returned before sufficient credit") + case <-time.After(blockShort): + } + + // A grant from the peer tops up the send window and resumes the write. + assert.NoError(t, st.HandleFrame(drpcwire.WindowUpdateFrame(st.ID(), 1))) + + select { + case err := <-done: + assert.NoError(t, err) + case <-time.After(time.Second): + t.Fatal("write did not resume after grant") + } +} diff --git a/drpcstream/stream.go b/drpcstream/stream.go index 84cc235f..e87ba7ee 100644 --- a/drpcstream/stream.go +++ b/drpcstream/stream.go @@ -34,10 +34,33 @@ type Options struct { // All KindMessage data is compressed on send and decompressed on receive. Compression drpc.Compression - // Internal contains options that are for internal use only. + // Internal contains options that are for internal use only. Per-stream + // flow control is configured here (drpcopts.SetStreamFlowControl) and is + // internal-only until the version-gated enablement work is ready. Internal drpcopts.Stream } +// validateFlowControl panics on a flow-control configuration that cannot make +// progress: an invalid window is a programmer error, and disabling silently +// would silently drop the protection. The frame size is SplitSize, or +// SplitData's 64 KiB default when SplitSize is 0. +func validateFlowControl(fc drpcopts.FlowControl, splitSize int) { + frame := int64(splitSize) + if splitSize == 0 { + frame = 64 << 10 + } + switch { + case splitSize < 0: + panic("drpcstream: flow control requires a bounded SplitSize") + case fc.StreamWindow <= 0 || fc.HighWater <= 0 || fc.GrantThreshold <= 0: + panic(fmt.Sprintf("drpcstream: flow-control sizes must be positive: %+v", fc)) + case frame > fc.StreamWindow || fc.GrantThreshold > fc.StreamWindow-frame: + panic(fmt.Sprintf( + "drpcstream: GrantThreshold (%d) + frame size (%d) exceeds StreamWindow (%d)", + fc.GrantThreshold, frame, fc.StreamWindow)) + } +} + // Stream represents an rpc actively happening on a transport. type Stream struct { ctx streamCtx @@ -132,6 +155,15 @@ func NewWithOptions( onDequeue: drpcopts.GetStreamOnReceiveQueueDequeue(&opts.Internal), }) + // When flow control is enabled, install the per-stream windows. The send + // window starts with StreamWindow credit (statically agreed with the peer); + // the receive window drives grant emission. + if fc := drpcopts.GetStreamFlowControl(&opts.Internal); fc.Enabled { + validateFlowControl(fc, opts.SplitSize) + s.sendw = newSendWindow(fc.StreamWindow) + s.recvw = newRecvWindow(fc.HighWater, fc.GrantThreshold) + } + return s } diff --git a/internal/drpcopts/stream.go b/internal/drpcopts/stream.go index d0eb29da..be7b2f7e 100644 --- a/internal/drpcopts/stream.go +++ b/internal/drpcopts/stream.go @@ -17,8 +17,34 @@ type Stream struct { stats *drpcstats.Stats onReceiveQueueEnqueue func(int64) onReceiveQueueDequeue func(int64) + flowControl FlowControl } +// FlowControl configures per-stream flow control. Internal-only until the +// CockroachDB version-gated enablement is ready; a later change promotes it to +// a public option. The installation site validates it (see drpcstream). +type FlowControl struct { + // Enabled turns per-stream flow control on. + Enabled bool + + // StreamWindow is the sender's initial and nominal per-stream credit. + StreamWindow int64 + + // HighWater is the receive-side buffered-byte mark at or above which + // grants are withheld. + HighWater int64 + + // GrantThreshold is the credit that must accrue before a grant is emitted, + // coalescing many frames into one KindWindowUpdate. + GrantThreshold int64 +} + +// GetStreamFlowControl returns the FlowControl stored in the options. +func GetStreamFlowControl(opts *Stream) FlowControl { return opts.flowControl } + +// SetStreamFlowControl sets the FlowControl stored in the options. +func SetStreamFlowControl(opts *Stream, fc FlowControl) { opts.flowControl = fc } + // GetStreamTransport returns the drpc.Transport stored in the options. func GetStreamTransport(opts *Stream) drpc.Transport { return opts.transport } From 86774e22b91771a99602c723c1239266de7c2c90 Mon Sep 17 00:00:00 2001 From: Sujatha Sivaramakrishnan Date: Thu, 16 Jul 2026 11:47:30 +0530 Subject: [PATCH 4/5] drpcmanager: end-to-end test for flow-control enablement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manager already threads drpcstream.Options through newStream, so enabling FlowControl in Options.Stream installs windows on every stream the manager creates (client and server) with no additional wiring. Add an end-to-end test over a real net.Pipe connection between two flow-control-enabled managers: a message larger than the send window completes only if credit grants flow back across the connection, exercising the full stream-level credit loop (gate -> grant-on-dispatch -> apply-grant) through the real read/write paths. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: roachdev-claude drpcstream: install per-stream windows from a flow-control option Add Options.FlowControl to enable per-stream flow control. When Enabled, NewWithOptions installs the send window (seeded with StreamWindow as initial credit) and the receive window (HighWater + GrantThreshold), so data writes become credit-gated and grants are emitted -- all driven by configuration rather than injected by tests. This is static, symmetric enablement: both peers must enable it with compatible sizes. Advertise-on-enable (safe under mixed enablement), the manager wiring that turns it on for a whole connection, and the connection-level window come in later commits. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: roachdev-claude drpcstream,drpcwire: wire the receive-window grant policy into the read path Install a per-stream recvWindow and drive it from the read/recv path: - 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 returns credit via recvWindow, emitting a KindWindowUpdate once the accrued amount crosses the threshold and the high-water gate is open. - RawRecv/MsgRecv call recvWindow after dequeuing; consuming can reopen a gate the high-water mark had closed and flush the accrued grant. 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. Stream-level only; the connection-level receive budget and the enablement path that installs the windows come in later commits. 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 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 (TakeDiscarded) and HandleFrame releases the bytes as consumed, letting their credit flow back. The release runs last (deferred past the replacement frame's accounting and handlePacket), so the grant decision sees the final buffered state rather than the transient dip, and a terminal packet that supersedes a partial suppresses the grant via termination. emitGrant is also a no-op once the stream has terminated: the ring drains queued messages after close, and returning credit behind the terminal frame would invite the peer to send more doomed data. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: roachdev-claude drpcstream: add per-stream receive-window grant policy Add recvWindow, the per-stream receive side of flow control. It 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 the accrued returnable credit is otherwise released once it reaches the threshold, coalescing many frames into a single grant. Consuming reopens a gate that the high-water mark had closed. dispatched/consumed return the credit delta the caller should emit as a KindWindowUpdate. Emitting the frame, wiring into the read path (HandleFrame/MsgRecv), and the connection-level receive budget come in later commits. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: roachdev-claude drpcstream: gate data-frame sends on the per-stream send window 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. Close and SendError close the send window before touching the write lock, so a send parked on credit wakes and releases the lock instead of stalling them on a grant that may never come. SendError pre-closes with io.EOF to match the send signal it sets moments later; Close pre-closes with termClosed for the same reason. CloseSend is a graceful half-close, so it waits for a parked send to complete rather than aborting it. All three (Close, SendError, CloseSend) wait for the write lock without holding s.mu and re-check stream state after acquiring it: the parked writer is only freed by terminate, whose paths (Cancel/Close/SendError/inbound terminal frames) all need s.mu -- and a uniform write-then-mu order is also what keeps CloseSend and Close/SendError from ABBA-deadlocking against each other. Per-stream only; the connection-level window and the enablement path that installs the window come in later commits. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: roachdev-claude drpcstream: add per-stream send-window credit primitive 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). Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: roachdev-claude drpcwire: add KindWindowUpdate flow-control frame 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. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: roachdev-claude drpcwire,drpcstream: return the real error from a preempted send A send blocked on backpressure used to return an opaque "interrupted" sentinel from WriteFrame, which the data path then remapped through CheckCancelError. WriteFrame lives in drpcwire and cannot know why a send was stopped, so it could only return a generic error and leave the stream to recover the cause. Pass the stream's send signal to WriteFrame directly and have it return that signal's error when the signal fires. The send signal is the write side's own "stop sending" signal, and terminate() always sets it, so a parked send is woken on every termination and returns send.Err(), which is io.EOF in the cancel and error cases. That is the same error the write loop's guard already returns when send is set before a frame is sent, so the parked and unparked paths now return the same thing. This is why the CheckCancelError remap on the write path is removed. It existed to turn the sentinel into the cancellation cause, but the write path now returns io.EOF, which is what gRPC reports on send; the real cause reaches the caller through the receive side, not the send side. Every cancel sets the send signal under the same lock, so the loop guard returns io.EOF before WriteFrame is even reached, and a send woken while parked returns it too. The only WriteFrame errors left are genuine transport failures, which happen only when the stream was not terminated, so there is no cancel cause to substitute and the raw error is correct. Passing a signal instead of a channel also lets WriteFrame resolve its channel lazily, only when a producer actually parks, so a stream that never hits backpressure no longer allocates that channel on every send. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: Claude Opus 4.8 drpcstream: make SendCancel abortive The previous commit stops terminal frames from being dropped, but SendCancel can still get stuck behind a backpressured send. SendCancel took the write lock before setting the termination signal, so when an in-flight MsgSend/RawWrite was parked on a full buffer while holding the write lock, SendCancel blocked on that lock and the cancel could not take effect until the buffer drained on its own. Local context cancellation should not have to wait on a congested connection. Make SendCancel abortive, matching gRPC where context cancellation does not guarantee that already-buffered messages are flushed first: - Set the termination signal before taking the write lock. A backpressured send watches term in WriteFrame, so setting it first interrupts that send and releases the lock, letting the cancel preempt it instead of waiting for the buffer to drain. - Let a control-bit frame bypass the buffer cap in WriteFrame. KindCancel carries the control bit, so the cancel frame is appended immediately even past the high-water mark. This overshoots the cap by at most one small control frame per terminating stream, which is bounded. SendError, Close, and CloseSend stay ordered: they hold the write lock across the send so their frame stays behind any in-flight data. Document the abortive-vs-ordered contract on each method. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: Claude Opus 4.8 drpcstream: don't drop terminal frames under backpressure Bounding the connection write buffer made WriteFrame block a producer when the buffer is at its high-water mark; it takes a cancel channel so the producer can bail out. The stream passed s.sigs.term.Signal() as that cancel channel for terminal frames too. That drops terminal frames under congestion. Close, SendError, and SendCancel all call terminate() first, which sets term, and only then send their frame through sendPacketLocked. Since term is already set, a full buffer makes WriteFrame take the cancel branch immediately and the frame is never enqueued. The remote is left without a clean Close/Error/Cancel exactly when the connection is backed up. Before the buffer was bounded this could not happen because WriteFrame never blocked. Pass nil as the cancel channel for terminal frames so they block until the buffer drains and always reach the wire. The ordered terminal paths (SendError/Close/CloseSend) hold the write lock across the send, so their frame stays behind any in-flight data. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: Claude Opus 4.8 drpcwire: make the write buffer limit configurable The high-water mark for the mux writer was hardcoded, so callers had no way to raise it for high-throughput connections or lower it under tight memory budgets. Expose it as WriterOptions.MaximumBufferSize and plumb it through drpcmanager.Options.Writer, mirroring how Reader and Stream options already flow through the manager. The default stays at 1 MiB when the option is left unset, so existing behavior is unchanged. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: Claude Opus 4.8 drpcwire: bound the connection write buffer The receiver side already bounds memory through the stream ring buffer, but the connection-level MuxWriter had no such limit. A producer that serializes frames faster than the network can drain them would let the pending buffer grow without end, so a fast sender over a slow connection could OOM the process. MuxWriter now enforces a high-water mark on the pending buffer. Once it is reached, WriteFrame blocks the calling stream until run drains the buffer onto the wire. This is plain backpressure rather than a full credit-based flow control scheme, which is enough to cap sender memory. The trade-off is that peak memory is up to ~2x the mark, since the in-flight buffer being written coexists with the pending one. Blocking on a full buffer means a write can no longer ignore stream termination, which matters now that multiplexing has many streams sharing one MuxWriter: a single canceled stream must not wait on the shared buffer draining. WriteFrame therefore takes a cancel channel, and the stream passes its term signal so cancel, close, and remote errors all unblock a parked write. The error is mapped back through CheckCancelError like the existing termination checks. Producers parked on backpressure are woken by closing and replacing a drain channel after each flush, which broadcasts to every waiter at once. Stop wakes them too, so shutdown does not depend on the buffer draining even when run is stuck in a slow underlying write. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: Claude Opus 4.8 drpcstream: introduce shared BufferPool for ring buffer Add a BufferPool backed by sync.Pool that is shared across all streams within a Manager. The ring buffer now obtains a pooled buffer on Enqueue and copies the message into it, instead of growing a fixed slice per slot. Dequeue hands the buffer's data to the consumer and advances the tail immediately, and Done releases the buffer back to the pool once the consumer is finished with it. Keeping the Dequeue/Done contract means the consumer works with a plain []byte and never has to know whether the queue is backed by a pool or by fixed buffers. Because Dequeue advances the tail right away rather than waiting for Done, Close no longer has to block until in-flight buffers are released. The pool is a required parameter in the Stream constructor, created once per Manager and passed to all streams it creates. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: Claude Opus 4.6 drpcpool: close connections returned to a closed pool Pool.Close only sees the connections currently idle in the cache. Connections that are checked out serving an in-flight Invoke or NewStream live outside the cache and are returned later via Put. Because the pool had no closed state, such a late Put re-inserted the connection and armed a fresh expiration timer, resurrecting the pool and leaking the connection (and its manager goroutines) until the timer fired. This is reachable on shutdown: long-lived streams (e.g. rangefeeds) hold their connection checked out, so Close cannot reach them. When the stream context is later canceled, the connection is handed back via Put and lingers well past the point the pool was closed. Mark the pool closed under the lock in Close, and have Put close the connection immediately instead of caching it once the pool is closed. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: Claude Opus 4.8 drpc: enable stream multiplexing Squashed result of the upstream stream-multiplexing branch. See the merged PRs for granular history: - #39 drpcmanager: fix race between manageReader and stream creation - #42 *: move frame assembly from reader to stream - #43 *: extract PacketAssembler for frame-to-packet assembly - #44 drpcmanager: replace manageStreams loop with per-stream goroutines - #45 *: use per-stream Finished signal instead of shared sfin channel - #46 drpcmanager: use atomic counter for client stream ID generation - #47 drpcmanager: replace streamBuffer with a streams registry - #51 drpc: enable stream multiplexing A connection now runs multiple concurrent client and server streams over a single transport. Frames carry stream IDs and are interleaved on the wire by a shared MuxWriter. Each stream owns its own packet ring buffer, Finished signal, and goroutine, and the manager tracks live streams in an activeStreams registry. New: drpcwire.MuxWriter, drpcwire.PacketAssembler, drpcstream.ringBuffer, drpcmanager.activeStreams. Removed: the drpccache package, drpcwire/writer (now MuxWriter), drpcstream/pktbuf (now ringBuffer), drpcmanager/streambuf (now activeStreams), drpcmanager.Options.InactivityTimeout, and the drpcconn shared write buffer plus stats infrastructure (CollectStats, Stats, drpcstats wiring). protoc-gen-go-drpc: generate HTTP gateway routes from proto annotations Read `google.api.http` annotations from proto method options and emit `DRPCGatewayRoutes` functions in the generated `*_drpc.pb.go` files. Each function returns a `[]drpc.HTTPRoute` containing the HTTP method, path, and a reference to the corresponding RPC client method. This lets consumers (like CockroachDB's DRPC gateway) register HTTP routes from generated data instead of maintaining manual route tables that drift out of sync with proto definitions. Streaming RPCs and methods without HTTP annotations are skipped. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: Claude Opus 4.6 ci: add merge guard workflow for fixup commits and do-not-merge label This guard will add a layer of protection against accidental merge of fixup commits. fixup! drpcpool: add support for ShouldRecord Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> fixup! drpcpool: add support for ShouldRecord fixup! drpcpool: add support for ShouldRecord fixup! drpcpool: add support for ShouldRecord fixup! drpcpool: add support for ShouldRecord drpcpool: add support for ShouldRecord *: plug in no-op metric implementations for nil fields This is a follow-up to PR #33 with the following fixes: - Initialize nil metric fields with no-op implementations at construction time (server, client, pool) so that call sites don't need nil guards. - Unexport MeteredTransport behind a NewMeteredTransport constructor that handles nil counters. *: add drpc client and server metrics Add metrics collection for drpc client and server connections, and connection pools. - Introduce Counter and Gauge metric interfaces in the drpc package, allowing us to plug in the metrics implementation from cockroach - Client metrics: bytes sent/received - Server metrics: bytes sent/received, TLS handshake errors - Pool metrics: pool size, connection hits/misses drpcserver: add TLSConfig and cipher suite restriction options Add TLSConfig and TLSCipherRestrict server options. When TLSConfig is set, Serve() wraps the listener with tls.NewListener and ServeOne() performs an explicit TLS handshake. The TLSCipherRestrict callback, if provided, runs after a successful handshake and rejects connections using a cipher that is not allowed for server use. *: migrate golangci-lint config to v2 golangci-lint v2 changed the config schema: `linters-settings` moved under `linters.settings`, formatters (gofmt, gci, goimports) are no longer linters, and several settings were removed (golint, unused. check-exported, unparam.algo, govet.use-installed-packages). Linters absorbed into staticcheck (gosimple, stylecheck) and removed upstream (rowserrcheck) are dropped from the disable list. The noctx linter now flags tls.Conn.Handshake — switched to HandshakeContext(ctx) so cancellation propagates to the TLS handshake. Also adds a default-configuration tripwire: both directions of a default-options connection are sniffed at the byte level, and the test fails if any KindWindowUpdate reaches the wire -- so no future change in the stack can silently enable flow control by default. Co-Authored-By: roachdev-claude *: map conn close to Unavailable (client) and Canceled (server) Introduce drpc.StreamKind (StreamKindClient/StreamKindServer) and add `Kind()` to the drpc.Stream interface so that remote conn close errors are mapped correctly, matching gRPC semantics: - Client streams return `ClosedError` (mapped to `Unavailable` status code). - Server streams return `context.Canceled`. internal/integration: align tests with gRPC status behavior Update integration tests to match the current gRPC-status-based error model. - Replace drpcmetadata.Get with drpcmetadata.GetFromIncomingContext. - Switch error assertions from drpcerr.Code and direct equality to status.FromError plus codes.Code checks. - Update cancel/transport tests to handle wrapped status errors (Canceled and Unavailable). - Adjust stats test byte-count expectations for the new error encoding. - Bump internal/integration to Go 1.25 and update golang.org/x/exp (with corresponding go.sum updates) for trace compatibility. drpchttp: remove drpchttp/ We have DRPC gateway in cockroach, so this package can be removed. Fixes: https://github.com/cockroachdb/cockroach/issues/164479 internal: remove twirpcompat examples: remove examples *: overhaul Makefile and add GitHub action This commit cleans up our fork to focus on our needs by removing storj specific CI and build infrastructure that we don't use. The original Makefile relied heavily on custom wrapper scripts in scripts/ that orchestrated testing across multiple modules and build configurations specific to storj's environment. 1. Deleted Jenkinsfile (Jenkins CI), flake.nix and flake.lock (Nix development environment), and the entire scripts/ directory which contained run.sh, docs.sh, cloc.sh, and other tooling we don't need. 2. Makefile: Rewrote it with direct go commands instead of wrapper scripts. The PKG variable excludes drpchttp/, internal/ (separate test modules), and cmd/. I plan to remove drpchttp/ package and add the cmd/ package back later as I was seeing some errors with it. The build target now compiles all relevant packages for verification. 3. Created .github/workflows/ci.yaml with actions checkout@v4, setup-go@v5, golangci-lint-action@v4 and two parallel jobs: build-and-test runs build/test/vet, while lint runs staticcheck and golangci-lint. The workflow triggers on pushes and PRs to main, using Go 1.25. 4. Fixed and suppressed some lint errors. 5. Disabled failing linters temporarily: Moved errcheck, errorlint, godot, and err113 to the disabled list with TODO comments so they can be re-enabled incrementally. Also cleaned up deprecated linters like exportloopref (replaced by copyloopvar in Go 1.22+), exhaustivestruct, gomnd, ifshort, and interfacer, and fixed golangci-lint config deprecation warnings. The repository is now significantly cleaner with 11 files deleted (including a 9.5MB committed binary that was accidentally tracked) and a straightforward build process. Fixes: cockroachdb/cockroach#163908 Merge pull request #28 from cthumuluru-crdb/drpc-errors *: convert errors to status errors to stay backward compatible with gRPC *: convert errors to status errors to stay backward compatible with gRPC gRPC treats connection errors specially. It uses `ConnectionError` interface to represent failures at the transport level, which are mapped to the `Unavailable` status code. `ConnectionError` can occur in the following situations: - During server transport creation: - [Failure during the server handshake](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_server.go#L149-L165). - [Errors while writing the settings frame](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_server.go#L209-L211). - [Errors while writing the window update frame](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_server.go#L213-L217). - [Failure to set the TCP timeout](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_server.go#L236-L240). - [Errors while reading the client preface or settings frame](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_server.go#L310-L337). - During client connection setup (dialing): - [Failure to dial the connection](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_client.go#L222-L228). - [Failure to set the TCP timeout](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_client.go#L269-L275). - [Errors during the client handshake](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_client.go#L291-L307). - [Errors while writing the settings frame](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_client.go#L422-L430). - [Errors while writing the window update frame](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_client.go#L445-L456). - [Errors while reading frames from the server](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_client.go#L1666-L1691). - [Errors while reading the server preface or settings frames](https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/internal/transport/http2_client.go#L1625-L1636). All errors are ultimately translated into gRPC status codes, which callers use to determine the appropriate next steps. To be compatible with gRPC, this change introduces ConnectionError class and converts it to `Unavailable` status code. drpcmetadata: split incoming and outgoing metadata keys (#23) - Add separate keys and methods for incoming and outgoing metadata. - Renamed methods to be similar to their grpc counterparts - Removed Add and AddPairs (for incoming context) as they don't have grpc counterparts and no existing usage Merge pull request #25 from cthumuluru-crdb/issue-159372 drpcclient: classify connection errors as `Unavailable` drpcmanager: Add GrpcMetadataCompatMode (#26) drpcclient: classify connection errors as Unavailable gRPC classifies connection failures as `Unavailable`. Connection errors include failures during TCP dialing as well as the TLS handshake. For backward compatibility, we mirror gRPC's behavior and return the same status codes to clients. Merge pull request #24 from cthumuluru-crdb/issue-158270 drpcclient: disable soft cancel dial option by default drpcclient: disable soft cancel dial option by default Soft cancel support in drpc allows the underlying transport to remain open when a stream's context is canceled. Soft cancel is enaled by default to reduce connection churn seen with stream pooling. However, after enabling soft cancel, we observed a hang where the server-side reader blocks waiting for the stream buffer to close, an event that only occurs when the manager (also the transport) itself is closed. While we work on a fix for this issue, we are disabling soft cancel usage by default for dialing DRPC connections. ----- goroutine trace blocked waiting for the stream buffer to close: ``` goroutine 3335 [sync.Cond.Wait]: sync.runtime_notifyListWait(0x140060fdae8, 0x0) GOROOT/src/runtime/sema.go:606 +0x140 sync.(*Cond).Wait(0x140060fdad8) GOROOT/src/sync/cond.go:71 +0xa4 storj.io/drpc/drpcmanager.(*streamBuffer).Wait(0x140060fdad0, 0x3) external/io_storj_drpc/drpcmanager/streambuf.go:53 +0xac storj.io/drpc/drpcmanager.(*Manager).manageReader(0x140060fda00) external/io_storj_drpc/drpcmanager/manager.go:299 +0x508 created by storj.io/drpc/drpcmanager.NewWithOptions in goroutine 3332 external/io_storj_drpc/drpcmanager/manager.go:137 +0x460 ``` Merge pull request #22 from suj-krishnan/handle_grpc_metadata drpcconn, drpcmanager: add support to send and receive grpc metadata drpcconn, drpcmanager: add support to send and receive grpc metadata This fix ensures that if the outgoing context has any grpc metadata, it is processed and sent as drpc metadata in the Invoke (unary) and NewStream (stream) flow. On the receiving end, the fix ensures that the server stream that processes an incoming packet duplicates any drpc metadata into the grpc metadata in the incoming context. This will enable us in the short-term to send and receive drpc metadata even while the cockroach code base continues to use the grpc metadata package to populate and receive metadata, when DRPC is enabled. Merge pull request #19 from cthumuluru-crdb/issue-149600 drpcclient: add additional dial options to dial a DRPC conn drpcclient: add additional dial options to dial a drpc conn `drpcclient.ClientConn` is a client connection with interceptor support. It takes a connection so that the interceptor chain can be applied to both direct and pooled connections. This change adds new dialer options `WithContextDialer` and `WithTLSConfig` to simplify the consuming code paths. This change also introduces the `WithContextDialer` and `WithTLSConfig` options to simplify connection dialing logic in the application (aka client) code. drpcwire: replace drpcerr with gRPC status protobuf for error marshaling (#16) In CockroachDB, we rely heavily on gRPC status for returning error messages with codes. This change ensures that our existing error handling continues to work with DRPC without requiring any modifications to the CockroachDB codebase. This is a pragmatic, temporary solution. In the future, when we eliminate gRPC, we can replace this dependency by improving the drpcerr package to achieve feature parity. This change replaces the custom binary error encoding (a simple 8-byte code + message) with the standard protobuf format from gRPC's google.rpc.Status. Additionally, we introduce an ErrorVersion system to future-proof the wire format for potential changes to error serialization. Key changes: - We're now using google.golang.org/grpc/status for error conversion instead of drpcerr. - Errors are marshaled and unmarshaled as protobuf-encoded google.rpc.Status messages. - The change handles both gRPC status errors and standard Go context errors (Canceled, DeadlineExceeded). - Added ErrorVersion enum with version byte prefix to support future serialization format changes. - Wire format is now: [1 byte: version][N bytes: protobuf data]. drpcmetadata: ensure metadata immutability by returning copy (#17) This prevents mutation of metadata maps stored in context by ensuring Get() returns a copy and Add()/AddPairs() work with copies rather than modifying the original map in place. drpc: add TLS certificate handling and metadata infra for auth interceptors (#11) This commit adds infrastructure needed for authentication interceptors: 1. New drpcctx/tlscert.go: Functions to store/retrieve TLS peer certificates in context 2. Server-side TLS certificate extraction in drpcserver 3. Improved metadata API with ClearContext, ClearContextExcept, and GetValue functions 4. Client-side per-RPC metadata support via WithPerRPCMetadata option drpcmux: add tests for server interceptors (#14) This commit adds tests for the server interceptors and RPC handling: - Tests for unary and streaming server interceptors - Validation of correct interceptor chaining and execution order - Error handling and propagation through interceptor chains - Edge cases with empty interceptor chains drpcmux: fix server interceptor selection logic (#13) This commit fixes a bug in the interceptor selection logic in HandleRPC. The issue was that we were passing a stream to the receiver for the case where the input is unitary and the output is a stream. The fix is to receive the message from the stream within the final receiver after going through the stream interceptor pipeline. This also means we no longer receive the message outside the switch statement. drpcmux: remove redundant context parameter from stream handlers (#12) This change simplifies the API by removing the explicit context parameter from StreamHandler and StreamServerInterceptor interfaces since it can be retrieved from the stream itself via stream.Context(). makefile: add gen-bazel target (#15) This change adds a `gen-bazel` target which generates a `WORKSPACE` file and runs gazelle to generate `BUILD.bazel` files. Generating these files allows a local clone of the repository to be used directly when building cockroach using the `override_repository` flag, for example: ``` ./dev build short -- --override_repository=io_storj_drpc=${CURDIR} ``` The new files are added to `.gitignore` so that generating them doesn't interfere with commits. This also adds a `clean-bazel` target that cleans up these files. Inspired by: https://github.com/cockroachdb/pebble/commit/cab9f08de0db22259b8c58ebc0980aec7b7eaae0 drpcmux: add support for server interceptors (#10) This change introduces support for server-side interceptors in DRPC. For a deeper understanding of interceptors, refer to the gRPC guide: https://grpc.io/docs/guides/interceptors lthough this functionality logically belongs in the `drpcserver` package, implementing it within `drpcmux` offers a more practical approach, avoiding a substantial refactor. create `drpc.ClientConn` wrapping a `drpc.Conn` drpcclient: create a clientconn wrapping drpc.Conn Here, we are updating constructor of clientConn to take in a `drpc.Conn` type object instead of a supplier of `drpc.Conn`. This simplifies the usage/construction of clientConn, and moves the responsibility of invoking a supplier/dialer to the caller of constructor. This way, closing of drpcpool would be a clear duty of the caller of this constructor. codegen: create RPC service interface with gRPC & DRPC adapters (#4) Define a shared `RPCClient` interface to wrap both gRPC's `Client` and DRPC's `DRPCClient`. Streaming methods will also return the common `RPC_Client`. Adds two simple adapters: - `NewGRPCClientAdapter` - `NewDRPCClientAdapter` They implement `RPCClient`, so we can swap clients at runtime via the cluster setting. codegen: generate common interfaces for stream server and client (#2) These interfaces contain the shared methods between DRPC and gRPC stream servers and clients as used in cockroach. codegen: update `RecvMsg` for gRPC compatibility (#3) It's already here in the implementation of the StreamServer implementations but not available through interface, so essentially it's hidden. In this change we update its parameters to become compatible with gRPC and added that definition in the interface. drpcclient: add client interceptor support in drpc (#6) Interceptors are not natively supported by drpc. Here, we add support for interceptors in drpc. The changes include definition of client interceptor interface types, logic to chain multiple unary/stream interceptors. This change also introduces a new struct `drpcclient.ClientConn` which satisfies the drpc.Conn interface, and decorates underlying drpc Connection object with interceptors. The struct drpcclient.ClientConn is defined such that it can be initialized with a dialer function which supplies underlying connection either concretely or from a pool. Functional options pattern is employed to provide flexibility with addition of unary and chain interceptors on ClientConn struct. Epic: CRDB-50378 Fixes: #146086 codegen: use cockroachdb/errors package The `cockroachdb` linters don't allow using the standard `errors` package, even in generated `*_drpc.pb.go` files. This change replaces its usage in `UnimplementedServer` with `cockroachdb/errors` to comply with linting requirements. drpcstream: runtime/trace support Change-Id: I91ebe74d3058a90ef8cc327accc9ac307fb323ab protoc-gen-go-drpc: generate GetStream method for client streams Change-Id: I0c87f4e8865e7eb153d28f045e402bbe5afbef8b drpcpool: fix bug with unset context field Change-Id: I2a58043a6901e21dda44afff2eae031a58e6b60b readme: update grpc version and benchmark results Change-Id: I6f26f75806e66d357a0c9faf977a335c98912401 drpcconn: expose read/written byte stats Change-Id: I852f8bebd20a4ff41bda112abf684f065e9154e1 drpcstats: add stats for bytes read/written by rpc Change-Id: I449a3447d9b320aef0d1296e46d083ffb22130b4 drpcwire: handle bytes even with an error from Reader adds a helper method that ensures the result from the read call is either valid data + nil error or no data + an error. if the reader spins returning no data and no error long enough, the helper returns a no progress error. Change-Id: If80f279f0597362f37de5182c99d3216683f62b6 all: some version bumps also removes the check-errs lint because it wasn't providing much value and was starting to become flaky with weird compiler like errors (missing symbol resolution, etc). a quick look at the source code of the package showed that it was not much more than a wrapper around go analysis tools so it's unclear what the problem is. Change-Id: I34056cdca57b39aef41b2b311906e48cbd96afcd drpcwire: add details to errors This additional information can help diagnose what might be going wrong. Change-Id: I7a1f4f2d8e7a50438874e170477ba418ebe9e296 README: update domain for go.elara.ws/drpc go.mod: bump language version to 1.19 Change-Id: I7996a08193efafcddda6bb1874f93dc085af9e88 drpcstream: SendCancel is busy when writing at all SendCancel would return that it was not busy if the stream was terminated, but there are ways for the stream to be terminated while it is still writing. Instead, check if any writes are happening before the check for termination. Change-Id: Ib3424d351f66741961cb7fe32795c0c0dfa99db2 all: bump flake and regen Change-Id: I44b395121aaad72c8ef6f8e5d3d7891e8f5a7518 drpcpool: make generic Change-Id: I1c643adfa0b5d4adb8f6a477697d869015d6da19 drpc{manager,stream}: allow passing error to SendCancel this way the stream will return the error from the context that was canceled instead of always context.Canceled. Change-Id: I970ef27d492a0918894af4ab6d3d292891ebb069 drpcmanager: check syscall pkg for reset error message Change-Id: I31dbf6aaf5233ea7a7da891b27e2c6fc080a04fd drpcmanager: catch more connection resets Change-Id: I1895723143cc727df7879c241510b89cfe743779 drpcmanager: treat connection reset as a type of ClosedError Change-Id: Icc49f3104f373de2fcf0b94ef060c0d2ea5d3ca5 drpcstream: SetManualFlush SetManualFlush is a new feature that allows for clients who want to inject messages into the write buffer without flushing right away. Example use case: flusher := stream.(interface{ GetStream() drpc.Stream }).GetStream().(interface{ SetManualFlush(bool) }) flusher.SetManualFlush(true) err = stream.Send(&pb.Message{Request: "hello, "}) flusher.SetManualFlush(false) if err != nil { return err } // the next send will send both messages in the same write // to the underlying connection. err = stream.Send(&pb.Message{Request: "world!"}) if err != nil { return err } Change-Id: I32928495e96e6f7504cc0be288a66811b637d698 drpc{stream,manager}: stop managing stream on finished Change-Id: I75f813ea02bded14fcc740a55be351656c402861 drpcstream: fix data race in packet buffer handling the packet buffer would go into a finished state when closed even if someone is still holding and using the data slice from get, allowing put to unblock and causing a data race. this adds a held boolean to the packet buffer state so that close will wait for any handing of the data buffer before going into the finished state. fixes #48 Change-Id: I131fc7addb26153e31b68015e58a2e400c1ce8f5 all: tiny fixes and corking fix Change-Id: I545415aca93dc443e3d071e64f95e1b722bfd1f7 flake.nix: update staticcheck Change-Id: Ie3e0898bf6157e761bc19cab8192736c54bcfba0 drpcserver/util.go: remove unused nolint directive Change-Id: Ifa62bbac9ac230ce9d28a2f561c61c8dd2738731 go.mod: bump lang to 1.18 Change-Id: I312b8a79c23f2fbdbe9f92ce417730bdceb17e5e drpcpool: add debug logging and small poolConn optimization Change-Id: Ie2c4b300fa0f88f6647e838742f7bf171753e851 drpcmanager: drop buffers after runs of small packets this avoids large heap usage when a single packet is large. after enough "small" packets have been sent where small is defined as less than 1/4th of the capacity of the buffer, then we drop the buffer for the next read. Change-Id: Ibc55d267764daff9e8fb9bb2c1e9630dbc7d6f74 drpcpool: use linked list for eviction the typical answer for linked lists is "dont", but a benchmark here shows a 10x reduction in cpu usage when there's a large number of connections (~10000) in the pool. Change-Id: Id3a3e8682e743ad553b91fc339218901750124dd drpc{conn,manager,stream,pool}: soft cancel support this commit adds "soft cancel" support where if a stream is canceled, it attempts to send a small cancel message instead of hard terminating the transport. the manager will block creation of new streams until the stream has sent this soft cancel message successfully, and terminate if it can't send it right away. the manager exposes a channel that is used by the pool to check if it is still in a state where it is soft canceling so that the pool can skip over connections in that state. one thing to consider is that with connection pooling, a soft cancel on a connection that has a bunch of data in the send queue could still proceed and consider the connection "unblocked" even though there will be some head of line blocking for the next rpc. perhaps some future code can either look at the send queue directly or wait for acknowledgement from the remote side. the latter option is not backwards compatible, though, so some care would have to be taken to figure out how to make that work. this adds more tests to the pool and adds some fixes found because of those tests. almost all of the tests from the storj rpcpool cache have been ported except for the fuzz test which is difficult to port due to this design having the cache and pool combined. Change-Id: I04a00b91e6a40fb3f1144a9a4f85bd2a9867d5f4 integration: add test around pooled cancel Change-Id: If8160587ed2a42d54483798a3a7dbd3c8528610d drpc{conn,stream}: skip flush until first read this allows 0-rtt rpcs to be made Change-Id: I14925261df11c0fe763478aa300d0cabd46ee280 README: add concurrent rpc 3rd party package Change-Id: Id0c4359a735c4cce717b164fa686d0af5eee403c drpcwire: regenerate README Change-Id: Ia0e346b9562528adddfb3af84c871f9aaf0c15ac README: Replace "bindings" by "support" (#47) The README used the word "bindings" for referring DRCP implementation in other programming language. Because other languages have to implement DRPC entirely without relying on the Go implementation. Fix links to examples (#45) Co-authored-by: Jeff Wendling drpctest: add new package for testing focused stuff Change-Id: I0e67886e35e0ab59e4ad21a7c4a8fe26efa6b0da drpc{wire,stream}: pass through Control frames rather than just drop frames with the control bit set we can assemble them into packets like usual with the packet control bit set. upper layers are then responsible for ignoring any unknown control packets for forwards compatibility. Change-Id: Icaf43d4f72c158f740701071e2c5b7ba4215a1ba all: bump flake/go versions and regenerate Change-Id: I9a084aec7eff9fef365775b2511fcc77a427be14 drpcwire: add back KindCancel for soft cancel support Change-Id: I285f9bd31b7fb1d3d20b7f4421e822d482afa421 drpcpool: add simple connection pool this is a pool designed around the techniques used in the connection pool in storj.io/common/rpc/rpcpool, but somewhat simplified and generic in the key type. it adds about 10% lines of code to the package, but this is a common issue multiple users have hit and either given up or invented themselves. Change-Id: Ie86f523fb36283ac63b5757f6652a39971b1aca2 grpccompat: bump libraries, apply fixes, update README this reruns the benchmarks, updates the table and lines of code counters at the bottom, and adds a blurb about why lines of code is important. Change-Id: Ia2f5fefb2c7cd7eebb7ad2f0b3d264855ef8fb3b drpcserver: fix lint issue Change-Id: I6b9c38e7f4634f82869925e28f7ecc0f981c6e63 examples/opentelemetry: add example with opentelemetry tracing Change-Id: I5ad8fffd922fa7f43bbd94e7e19a187c98034c29 all: replace deprecated ioutil Change-Id: Id5b4c9549aee21ba3742ef246c7fe84a923f0013 all: fix formatting Change-Id: I06fb50078d8369d09f11d0e63c87b058d05fe590 drpcmanager: reduce memory allocation in Reader (#40) Also, a `go fmt` while I'm here. examples: fix some small bugs - route before calling Run instead of concurrently - fix the http client parsing of json response Fixes #28 Change-Id: Iafe341cf67984abedd440eb1f13b72c12f081a97 drpcmigrate: avoid potential double close of default listener Change-Id: I700a43f306b000e1540558c0f302ca751b38ea05 drpcmigrate: close default listener properly Closing dprcmux can hang forever, if default Listener is used (.Accept). It should be closed similat to all other routers. Change-Id: I51dd56f5c319d18b09df5db218d14b7dd2b93697 drpcmanager: expose error from transport failures A cancel can sometimes only be performed by closing the underlying transport because that is the only way to interrupt a read/write syscall. But, rather than assume that every error from the transport failing is due to a cancel, only assume that an io.EOF means that. This can help with debugging when the remote side dies due things besides a "clean" close of the underlying transport. Closes #31 Change-Id: Ia655d07984f6abe0492e7f9300d2638a1c82b4d4 drpcwire: limit frame size based on packet size if packets are huge, then frames should be allowed to be too. similarly, if packets are small, then there is no reason for huge frames. Change-Id: I36b46abefbd7c49bfa807c62e00c739015fc3703 regen docs Change-Id: I04ac7c0c5bea05a95ee38cd0584ca624ce60f077 drpcwire: expose buffer size on Reader (#30) The maximum was hardcoded at 4MB. This allows it to be configurable for larger payloads. An option is exposed in `dprcmanager.Options`. drpcmanager/drpcstream: label streams with cli/srv kinds Change-Id: Ie324f958cadfa038d4107ea1459dc38a45c5a82f nix: add missing tools to flake.nix these were missed due to a non-hermetic environment being used to create it in the first place. Change-Id: I3e4db14f525890be6e067a2873dacb0a02e6ff1d drpchttp: fix linting issue Change-Id: I0d01dc856ba7bcd81229ebd9659e42b49535a657 drpcstream: return ClosedError for remote closes In logging it's helpful to distinguish between an actual stream error and a remote closing, which is to be expected. Updates https://github.com/storj/storj/issues/4609 Change-Id: I5e70413484820efb9d0d2c8792c9713de9f8658f drpc: add comment on interface Transport (#20) Add comment that drpc.Transport is compatible with net.conn drpcstream: set cancel state more often if a Close has started, and we Cancel, we should still set the cancelled state while Close is still executing. this is so that we don't surface the underying transport being closed errors. Change-Id: I2f3924f7b03c3952ab58b9952e4ae18ef1a5a88f drpcserver: log callback and temporary error sleeps It was possible before to spin a cpu core trying to accept from the tcp listener when it always returned a temporary error. Now, it will sleep for 500ms before trying again. Change-Id: I59d1503b4d14e9ded78d78c608ea4c416a4e10df drpcstream: control for max buffer size with many concurrent and potentially cached streams that send large messages, dropping large buffers can help with peak memory usage. Change-Id: Ia99f30ff78307bea909fde813c2ecb6667a4b1d8 Fixes "File is not `gofmt`-ed with `-s` (gofmt)" (#22) cmd/protoc-gen-go-drpc: proto3 optional support this additionally bumps the google.golang.org/protobuf dependency to v1.27.1, and the nix flake dependencies for protoc from v3.16.0 to v3.18.0. adds an optional field into the grpccompat tests rather than the integration tests because we generate using gogoprotobuf sometimes in the integration tests which does not support optional fields. Change-Id: Idb52705b2e57588120a7f6af7bc2db36ddde3d09 mod: bump semantic version to go 1.17 This enables module graph pruning and lazy module loading. Change-Id: I5a942d6f9bfedaa3ee682f08216cef04b7e5b633 drcpserver: fix windows build Change-Id: Ia4d2d138ece4a5b6d736e5e72405960191ac9ff1 drpcmanager: fix closing race condition in some situations, newStream could be blocked forever trying to send a stream into m.streams when the manageStreams goroutine has already exited due to manager terminating. this changes the send into a select that can be canceled by the manager terminating. also, there was a race in the tests for the muxer where rare failures could happen if the Route call was not scheduled before the connection with the given prefix was handled. Change-Id: If2033d8b8e3b2c3e3a50e8a72337bc263458321e drpcserver: add checks for temporary network errors when a temporary network error occurs, it should not crash the entire process. This PR also adds a method to check for specific windows errors that should be considered temporary. Change-Id: I258bb36103c028754c6829ec8618388cc1db4524 drpcmanager: remove allocations introduced in go1.17 launching a goroutine ends up allocating a closure object in go1.17 that it didn't before. this maybe has something to do with the register abi? in any case, because we only ever have 1 stream executing at a time, we can have a single goroutine that just runs the stream and pass it the information needed and launch that single goroutine at the start. this doesn't have an effect on any benchmarks, except for go1.17 benchmarks. Change-Id: Ifadce4a73423c7e7c31c296a2f22fd7c11b674b2 drpchttp: some renaming/comments Change-Id: I36e266178700edde3ffb3480d3efbc913cc7dcd2 drpchttp: add grpc-web support This refactors drpchttp to add expansion by defining the Protocol interface, and adding options for external consumers to define their own protocols. Additionally, it reverts the un-tagged option to control how Twirp codes are processed and adds reflect based code to grab the code from errors without having to import the Twirp module. It also reverts the breaking change introduced with the Twirp change to the New function by adding a NewWithOptions function much like how the drpc{conn,stream,manager,server} packages work. Change-Id: Ibaae7fa18d78640a6d2ee85d75826afc24f37803 ensure twirp compatibility Change-Id: I12299f3e4f130531271b577d9cbec0a65bda531d make tidy Change-Id: I281279c740e0fd7ce87e6fb1bb1405e5df971091 Add external packages, other languages, etc. Change-Id: Ieeb51296bcd54fa28967f5adbe23ed863467a6ba drpc: add nix flake support this allows two things: 1. running `nix shell github:storj/drpc` puts you into a shell that has `protoc-gen-go-drpc` installed, which is neat. 2. in the directory, running `nix develop` puts you into a shell that has all of the binaries required to run all the lints/generates/ci/etc. in fact, you can do `nix develop -c make` to run it all which is neat. both of those commands are fully hermetic and repoducible so they will always work forever on any system which is neat. no idea if i'm going to ever merge this, but nix flakes are cool. Change-Id: Ia2f1c53632da80b7cb2bfa832420aeeccc5f851c drpcwire: disallow repeating ids and more tests Change-Id: Ib53b66bb6fccd35476cc4371d931ad751b1b8b6d drpccache: add unit tests Change-Id: I819decfad4effc2737d5a01a7b7959b0ca0cbf99 drpc: remove Transport method from Conn Pooled connections or wrappers that dispatch to different underlying Conns will not have a canonical "transport" that they are writing to. The concrete drpcconn.Conn type can still export the method, but don't require it for all implementers. Change-Id: I8ee229a446bd9e0042fb4bf759c5ca2aa247b604 README: add loc estimate to keep me honest it helps to keep track of where most of the complexity is and makes the "few thousand lines of code" quantified. Change-Id: Iaa53ecc9a6656482bc49f700e963edf58946460e drpcmigrate: remove test dependency on x/sync Change-Id: I5240607abf0710ff42c64bb4ceae5cc41f684ede drpcwire: remove bufio.Scanner use it's a bit more code but there's some fairly big performance wins by managing our own buffers. for example, we can read into a large slice and remove frames until there are no more and only then move the tail of the slice to the front. that reduces copying from potentially quadratic to linear. name old/op new/op delta Unitary/Small 8.72µs 8.59µs -1.52% Unitary/Med 11.2µs 11.1µs -1.22% Unitary/Large 633µs 633µs ~ InputStream/Small 834ns 759ns -9.00% InputStream/Med 2.42µs 2.00µs -17.27% InputStream/Large 259µs 249µs -4.09% OutputStream/Small 846ns 757ns -10.48% OutputStream/Med 2.38µs 1.99µs -16.13% OutputStream/Large 241µs 239µs ~ BidirStream/Small 3.35µs 3.30µs -1.62% BidirStream/Med 5.01µs 4.94µs -1.38% BidirStream/Large 551µs 546µs ~ Change-Id: Ia21ec5097907b97c6059aac483927e1bc2f53336 lint: match github.com/storj/ci linting rules Change-Id: Ie776659984a2fca20548928e20bb3eed46714635 drpcdebug: more logging with more information Change-Id: Icde03daa1266f4cef8b045857dbb1fbe2942b957 drpcstream: remove three allocations this removes an allocation for a context to return the transport by passing it into the stream itself and having the owned stream context value return it directly. this removes another allocation by not having the manager look at the Terminated signal. it does this by passing in a channel that the stream sends once on. the manager ensures that only one stream is active at a time, so this is sufficient. both of the above were accomplished by extending the stream options with a set of internal options that can only be modified and read with a new internal drpcopts package. this also removes an allocation by avoiding callbacks by avoiding drpcwire.SplitN when writing. the compiler was unable to prove that the passed in data slice did not escape, so the string to []byte conversion when passing in the rpc name had to allocate. with the more inlined code, the compiler is now able to prove it does not escape and avoids making a copy. Change-Id: Id038f94eaeab32f31e6f88cbc06a502da63c9b05 fuzz: add some gofuzzbeta tests they don't do anything for now, but one day they will. Change-Id: Ie212126d65ed992a9662a948206d9a438e36da42 drpc{manager/stream}: add some randomized tests this also fixes a bug where the drpcstream packet buffer could panic under concurrent message send and cancellation. it also changes the mamnager to close any existing stream when a packet with a later id is received. additionally, benchmarks improve. this is prolly due ditching a channel for the packet buffer and reducing the memory allocations that it does. name old/op new/op delta Unitary/Small 9.71µs 9.36µs -3.68% Unitary/Med 12.3µs 12.0µs -2.47% Unitary/Large 643µs 593µs -7.75% InputStream/Small 835ns 809ns -3.16% InputStream/Med 2.58µs 2.44µs -5.53% InputStream/Large 259µs 247µs -4.72% OutputStream/Small 849ns 814ns -4.12% OutputStream/Med 2.53µs 2.43µs -3.96% OutputStream/Large 245µs 242µs -1.07% BidirStream/Small 3.68µs 3.43µs -6.97% BidirStream/Med 5.38µs 5.04µs -6.36% BidirStream/Large 559µs 555µs -0.69% name old allocs/op new allocs/op delta Unitary/Small 14.0 12.0 -14.29% Unitary/Med 16.0 14.0 -12.50% Unitary/Large 16.0 14.0 -12.50% Change-Id: I8a778a546201d0f0310d152816a1233a2d687a49 drpcerr: add Unimplemented code Change-Id: Idb800efbb31ebef7a3aa25f06da6149a7820a2dd add test that error messages are sent Change-Id: Iccdcdd2d3117aa777e76a0f5a568e961a16ef8d9 drpc{conn,manager,stream}: refactor and perf improvements this changes the manager to more consistently be reading from the underlying transport. it does this by always delivering messages to the active (or most recently active) stream as well as discarding any messages for older streams. in order to do this, it has to keep track of the active stream better. a secondary benefit of that is we don't have to send stuff over channels as often, which is a performance win. this adds a "ManualFlush" stream option that causes the stream flushing semantics to more closely match gRPC: it will only flush writes when the buffer is too large, a receive is called and there is data in the buffer, or when manually asked. this accounts for most of the performance difference on the stream benchmarks for small and medium sizes, and makes all of the benchmarks either as good as or better than grpc. the following is the performance delta for DRPC against itself with this change applied. note that for the speeds in particular, the measurements aren't very valid because some changes were made to the benchmarks themselves that changed the number of bytes being sent. tbh this change got away from me a bit. it should have been like 8 smaller changes. oh well. name old time/op new time/op delta Unitary/Small 15.6µs ± 2% 9.7µs ± 1% -37.56% Unitary/Med 19.7µs ± 2% 12.3µs ± 1% -37.58% Unitary/Large 654µs ± 4% 643µs ± 6% ~ InputStream/Small 2.59µs ± 1% 0.84µs ± 1% -67.72% InputStream/Med 3.59µs ± 5% 2.58µs ± 1% -27.98% InputStream/Large 256µs ± 4% 259µs ± 1% +1.43% OutputStream/Small 2.68µs ± 1% 0.85µs ± 2% -68.29% OutputStream/Med 3.60µs ± 2% 2.53µs ± 1% -29.69% OutputStream/Large 248µs ± 1% 245µs ± 1% -1.14% BidirStream/Small 5.50µs ± 2% 3.68µs ± 0% -33.02% BidirStream/Med 7.52µs ± 5% 5.38µs ± 1% -28.53% BidirStream/Large 586µs ± 7% 559µs ± 1% -4.56% name old speed new speed delta Unitary/Small 130kB/s ± 0% 208kB/s ± 4% +59.62% Unitary/Med 104MB/s ± 2% 167MB/s ± 1% +60.19% Unitary/Large 1.61GB/s ± 3% 1.63GB/s ± 5% ~ InputStream/Small 774kB/s ± 1% 2396kB/s ± 1% +209.41% InputStream/Med 573MB/s ± 5% 795MB/s ± 1% +38.73% InputStream/Large 4.10GB/s ± 4% 4.04GB/s ± 1% -1.44% OutputStream/Small 1.49MB/s ± 3% 2.36MB/s ± 2% +58.22% OutputStream/Med 572MB/s ± 2% 812MB/s ± 1% +42.07% OutputStream/Large 4.23GB/s ± 1% 4.28GB/s ± 1% +1.15% BidirStream/Small 363kB/s ± 2% 540kB/s ± 0% +48.82% BidirStream/Med 273MB/s ± 5% 380MB/s ± 3% +39.35% BidirStream/Large 1.79GB/s ± 6% 1.87GB/s ± 1% +4.65% name old alloc/op new alloc/op delta Unitary/Small 2.15kB ± 0% 1.54kB ± 0% -28.34% Unitary/Med 8.55kB ± 0% 7.94kB ± 0% -7.17% Unitary/Large 3.23MB ± 0% 3.16MB ± 0% -2.01% InputStream/Small 80.0B ± 0% 80.0B ± 0% ~ InputStream/Med 2.13kB ± 0% 2.13kB ± 0% ~ InputStream/Large 1.08MB ± 0% 1.05MB ± 0% -2.89% OutputStream/Small 160B ± 0% 80B ± 0% -50.00% OutputStream/Med 2.21kB ± 0% 2.13kB ± 0% -3.62% OutputStream/Large 1.06MB ± 0% 1.05MB ± 0% -1.02% BidirStream/Small 240B ± 0% 240B ± 0% ~ BidirStream/Med 4.34kB ± 0% 4.34kB ± 0% -0.09% BidirStream/Large 2.17MB ± 0% 2.10MB ± 0% -2.86% name old allocs/op new allocs/op delta Unitary/Small 21.0 ± 0% 14.0 ± 0% -33.33% Unitary/Med 23.0 ± 0% 16.0 ± 0% -30.43% Unitary/Large 23.2 ± 3% 16.0 ± 0% -31.18% InputStream/Small 1.00 ± 0% 1.00 ± 0% ~ InputStream/Med 2.00 ± 0% 2.00 ± 0% ~ InputStream/Large 2.00 ± 0% 2.00 ± 0% ~ OutputStream/Small 2.00 ± 0% 1.00 ± 0% -50.00% OutputStream/Med 3.00 ± 0% 2.00 ± 0% -33.33% OutputStream/Large 3.00 ± 0% 2.00 ± 0% -33.33% BidirStream/Small 3.00 ± 0% 3.00 ± 0% ~ BidirStream/Med 5.00 ± 0% 5.00 ± 0% ~ BidirStream/Large 5.25 ±14% 5.00 ± 0% ~ Change-Id: Ib135f7427c344c479ae237d7702c090f616b442b cmd/protoc-gen-go-drpc: recv into message allows callers to provide buffers to receive messages info for improved memory allocations and performance. Change-Id: I09b6fc41de932f8e2e9656ff3c154aaf25e056ac internal/integration: test for large buffers Change-Id: I2cc8396ff946d1ce306718290bed5612f1933bee update linting rules Change-Id: Iae1320d9e383ec512cac78ebc1c6de736ea93ee7 drpcconn: fully read unary RPCs (#16) * drpcmanager: expose termination signal * drpcconn: expose termination signal * drpcconn: fully read unary RPCs The previous behavior left a potential "CloseSend" frame pending on the manager's read queue. This delayed detection of a potential connection loss until the next RPC is started on the connection, which discards the old frame because of the stream ID mismatch. drpcstream: implement Context properly for streams the context package will panic if you use WithCancel on a context that closes the Done channel but does not return an error from the Err method. fix drpcstream so that the context it returns does not cause that panic. Change-Id: I4788e32869147558cc9b645546f843d19d0bad7b ci: fixes for errorlint Change-Id: Ib0956c8879030e83437c12a921e80f09dfba29c5 README: update benchmark comparisons Change-Id: Idfcc8d88b0928f83766731eaaaacf77a072a3a95 drpcwire: avoid godot lint failure Change-Id: I59204dc97bee9e79ab0f7a5aad94ad21b39bcd09 cmd/protoc-gen-go-drpc: support not generating json encoding Change-Id: I7c47c615abcb3c11a5c2014ed36b79dc4a86a41c drpcstream: add some basic functionality tests Change-Id: Id592b72dd60c3b4372ee7b72974a9a21feca9626 drpcstream: avoid select for handling packets name old time/op new time/op delta Unitary/Small-8 20.8µs ± 2% 17.1µs ± 2% -17.67% (p=0.000 n=8+8) Unitary/Med-8 25.2µs ± 2% 21.5µs ± 3% -14.51% (p=0.000 n=8+8) Unitary/Large-8 646µs ± 2% 648µs ± 4% ~ (p=0.955 n=7+8) InputStream/Small-8 2.70µs ± 2% 2.50µs ± 1% -7.26% (p=0.000 n=8+8) InputStream/Med-8 3.57µs ± 3% 3.37µs ± 3% -5.72% (p=0.000 n=8+8) InputStream/Large-8 266µs ± 2% 263µs ± 5% ~ (p=0.281 n=7+8) OutputStream/Small-8 2.95µs ± 6% 2.58µs ± 1% -12.48% (p=0.000 n=8+8) OutputStream/Med-8 4.32µs ±17% 3.49µs ± 3% -19.28% (p=0.000 n=8+8) OutputStream/Large-8 286µs ±11% 247µs ± 4% -13.62% (p=0.000 n=7+8) BidirStream/Small-8 6.40µs ±17% 5.27µs ± 2% -17.66% (p=0.000 n=8+7) BidirStream/Med-8 8.25µs ±16% 7.24µs ± 3% -12.16% (p=0.000 n=8+7) BidirStream/Large-8 649µs ±15% 609µs ±12% ~ (p=0.165 n=7+7) name old speed new speed delta Unitary/Small-8 100kB/s ± 0% 120kB/s ± 0% +20.00% (p=0.001 n=7+7) Unitary/Med-8 81.6MB/s ± 2% 95.4MB/s ± 3% +16.98% (p=0.000 n=8+8) Unitary/Large-8 1.62GB/s ± 2% 1.62GB/s ± 4% ~ (p=0.955 n=7+8) InputStream/Small-8 741kB/s ± 2% 800kB/s ± 1% +7.93% (p=0.000 n=8+8) InputStream/Med-8 575MB/s ± 3% 610MB/s ± 3% +6.04% (p=0.000 n=8+8) InputStream/Large-8 3.95GB/s ± 2% 3.99GB/s ± 5% ~ (p=0.281 n=7+8) OutputStream/Small-8 1.36MB/s ± 6% 1.55MB/s ± 1% +13.89% (p=0.000 n=8+8) OutputStream/Med-8 480MB/s ±15% 589MB/s ± 3% +22.65% (p=0.000 n=8+8) OutputStream/Large-8 3.57GB/s ±22% 4.24GB/s ± 4% +18.86% (p=0.000 n=8+8) BidirStream/Small-8 318kB/s ±15% 379kB/s ± 3% +19.24% (p=0.000 n=8+7) BidirStream/Med-8 250MB/s ±14% 284MB/s ± 3% +13.35% (p=0.000 n=8+7) BidirStream/Large-8 1.56GB/s ±30% 1.66GB/s ±30% ~ (p=0.234 n=8+8) name old alloc/op new alloc/op delta Unitary/Small-8 2.40kB ± 0% 2.05kB ± 0% -14.66% (p=0.000 n=8+8) Unitary/Med-8 8.81kB ± 0% 8.46kB ± 0% -4.01% (p=0.000 n=8+8) Unitary/Large-8 3.23MB ± 0% 3.22MB ± 0% ~ (p=0.121 n=7+8) InputStream/Small-8 80.0B ± 0% 80.0B ± 0% ~ (all equal) InputStream/Med-8 2.13kB ± 0% 2.13kB ± 0% ~ (all equal) InputStream/Large-8 1.08MB ± 0% 1.08MB ± 0% ~ (p=0.279 n=8+8) OutputStream/Small-8 160B ± 0% 160B ± 0% ~ (all equal) OutputStream/Med-8 2.21kB ± 0% 2.21kB ± 0% ~ (all equal) OutputStream/Large-8 1.06MB ± 0% 1.06MB ± 0% ~ (p=0.105 n=8+8) BidirStream/Small-8 240B ± 0% 240B ± 0% ~ (p=1.000 n=8+8) BidirStream/Med-8 4.34kB ± 0% 4.34kB ± 0% -0.02% (p=0.007 n=6+8) BidirStream/Large-8 2.18MB ± 0% 2.17MB ± 1% ~ (p=0.645 n=8+8) name old allocs/op new allocs/op delta Unitary/Small-8 24.0 ± 0% 20.0 ± 0% -16.67% (p=0.000 n=8+8) Unitary/Med-8 26.0 ± 0% 22.0 ± 0% -15.38% (p=0.000 n=8+8) Unitary/Large-8 26.4 ± 2% 22.6 ± 3% -14.22% (p=0.000 n=8+8) InputStream/Small-8 1.00 ± 0% 1.00 ± 0% ~ (all equal) InputStream/Med-8 2.00 ± 0% 2.00 ± 0% ~ (all equal) InputStream/Large-8 2.00 ± 0% 2.00 ± 0% ~ (all equal) OutputStream/Small-8 2.00 ± 0% 2.00 ± 0% ~ (all equal) OutputStream/Med-8 3.00 ± 0% 3.00 ± 0% ~ (all equal) OutputStream/Large-8 3.00 ± 0% 3.00 ± 0% ~ (all equal) BidirStream/Small-8 3.00 ± 0% 3.00 ± 0% ~ (all equal) BidirStream/Med-8 5.00 ± 0% 5.00 ± 0% ~ (all equal) BidirStream/Large-8 5.75 ±13% 5.62 ±11% ~ (p=1.000 n=8+8) Change-Id: Iac71d9b1d3e6386ea6a216c145fcfdda3fcae91e drpc{manager,stream}: reuse read buffers name old time/op new time/op delta Unitary/Small-8 20.6µs ± 6% 20.8µs ± 2% ~ (p=0.234 n=8+8) Unitary/Med-8 25.1µs ± 3% 25.2µs ± 2% ~ (p=1.000 n=7+8) Unitary/Large-8 1.49ms ± 3% 0.65ms ± 2% -56.51% (p=0.001 n=7+7) InputStream/Small-8 2.45µs ± 4% 2.70µs ± 2% +9.96% (p=0.000 n=8+8) InputStream/Med-8 3.81µs ± 3% 3.57µs ± 3% -6.30% (p=0.000 n=8+8) InputStream/Large-8 710µs ± 3% 266µs ± 2% -62.59% (p=0.001 n=7+7) OutputStream/Small-8 2.55µs ± 5% 2.95µs ± 6% +15.87% (p=0.000 n=8+8) OutputStream/Med-8 3.93µs ± 4% 4.32µs ±17% ~ (p=0.083 n=8+8) OutputStream/Large-8 672µs ± 2% 286µs ±11% -57.35% (p=0.001 n=7+7) BidirStream/Small-8 5.25µs ± 1% 6.40µs ±17% +21.95% (p=0.000 n=7+8) BidirStream/Med-8 8.11µs ± 9% 8.25µs ±16% ~ (p=0.959 n=8+8) BidirStream/Large-8 1.48ms ±15% 0.65ms ±15% -56.22% (p=0.000 n=8+7) name old speed new speed delta Unitary/Small-8 97.5kB/s ± 8% 100.0kB/s ± 0% ~ (p=0.533 n=8+7) Unitary/Med-8 81.7MB/s ± 3% 81.6MB/s ± 2% ~ (p=1.000 n=7+8) Unitary/Large-8 706MB/s ± 3% 1623MB/s ± 2% +129.90% (p=0.001 n=7+7) InputStream/Small-8 815kB/s ± 3% 741kB/s ± 2% -9.05% (p=0.000 n=8+8) InputStream/Med-8 539MB/s ± 3% 575MB/s ± 3% +6.74% (p=0.000 n=8+8) InputStream/Large-8 1.48GB/s ± 4% 3.95GB/s ± 2% +167.16% (p=0.001 n=7+7) OutputStream/Small-8 1.57MB/s ± 5% 1.36MB/s ± 6% -13.32% (p=0.000 n=8+8) OutputStream/Med-8 523MB/s ± 4% 480MB/s ±15% ~ (p=0.083 n=8+8) OutputStream/Large-8 1.56GB/s ± 2% 3.57GB/s ±22% +128.38% (p=0.000 n=7+8) BidirStream/Small-8 380kB/s ± 0% 318kB/s ±15% -16.45% (p=0.000 n=7+8) BidirStream/Med-8 254MB/s ± 8% 250MB/s ±14% ~ (p=0.959 n=8+8) BidirStream/Large-8 712MB/s ±14% 1559MB/s ±30% +118.99% (p=0.000 n=8+8) name old alloc/op new alloc/op delta Unitary/Small-8 2.25kB ± 0% 2.40kB ± 0% +6.68% (p=0.000 n=7+8) Unitary/Med-8 13.2kB ± 0% 8.8kB ± 0% -33.52% (p=0.000 n=7+8) Unitary/Large-8 14.0MB ± 0% 3.2MB ± 0% -77.02% (p=0.000 n=8+7) InputStream/Small-8 88.0B ± 0% 80.0B ± 0% -9.09% (p=0.000 n=8+8) InputStream/Med-8 4.43kB ± 0% 2.13kB ± 0% -51.99% (p=0.000 n=8+8) InputStream/Large-8 6.54MB ± 0% 1.08MB ± 0% -83.41% (p=0.000 n=8+8) OutputStream/Small-8 168B ± 0% 160B ± 0% -4.76% (p=0.000 n=8+8) OutputStream/Med-8 4.51kB ± 0% 2.21kB ± 0% -51.06% (p=0.000 n=8+8) OutputStream/Large-8 6.49MB ± 0% 1.06MB ± 0% -83.63% (p=0.000 n=8+8) BidirStream/Small-8 256B ± 0% 240B ± 0% -6.15% (p=0.000 n=8+8) BidirStream/Med-8 8.95kB ± 0% 4.34kB ± 0% -51.48% (p=0.000 n=7+6) BidirStream/Large-8 13.0MB ± 0% 2.2MB ± 0% -83.26% (p=0.000 n=8+8) name old allocs/op new allocs/op delta Unitary/Small-8 25.0 ± 0% 24.0 ± 0% -4.00% (p=0.000 n=8+8) Unitary/Med-8 27.0 ± 0% 26.0 ± 0% -3.70% (p=0.000 n=8+8) Unitary/Large-8 46.9 ± 4% 26.4 ± 2% -43.73% (p=0.000 n=8+8) InputStream/Small-8 2.00 ± 0% 1.00 ± 0% -50.00% (p=0.000 n=8+8) InputStream/Med-8 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=8+8) InputStream/Large-8 12.8 ± 6% 2.0 ± 0% -84.31% (p=0.000 n=8+8) OutputStream/Small-8 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=8+8) OutputStream/Med-8 4.00 ± 0% 3.00 ± 0% -25.00% (p=0.000 n=8+8) OutputStream/Large-8 13.6 ± 5% 3.0 ± 0% -77.98% (p=0.000 n=8+8) BidirStream/Small-8 5.00 ± 0% 3.00 ± 0% -40.00% (p=0.000 n=8+8) BidirStream/Med-8 7.00 ± 0% 5.00 ± 0% -28.57% (p=0.000 n=8+8) BidirStream/Large-8 25.8 ± 3% 5.8 ±13% -77.67% (p=0.000 n=8+8) Change-Id: Ibc0568bef7f97d366b2acdbb2f7cbc94d9ad0df0 drpc{enc,conn,stream}: reuse encode buffers if possible name old time/op new time/op delta Unitary/Small-8 21.5µs ± 2% 20.6µs ± 6% ~ (p=0.054 n=7+8) Unitary/Med-8 27.6µs ± 4% 25.1µs ± 3% -8.81% (p=0.000 n=8+7) Unitary/Large-8 1.86ms ±21% 1.49ms ± 3% -20.13% (p=0.000 n=8+7) InputStream/Small-8 2.79µs ±13% 2.45µs ± 4% -12.14% (p=0.000 n=8+8) InputStream/Med-8 4.78µs ± 3% 3.81µs ± 3% -20.23% (p=0.000 n=7+8) InputStream/Large-8 824µs ± 6% 710µs ± 3% -13.79% (p=0.001 n=7+7) OutputStream/Small-8 2.73µs ± 4% 2.55µs ± 5% -6.74% (p=0.000 n=7+8) OutputStream/Med-8 5.21µs ±13% 3.93µs ± 4% -24.57% (p=0.000 n=8+8) OutputStream/Large-8 804µs ± 3% 672µs ± 2% -16.43% (p=0.001 n=7+7) BidirStream/Small-8 5.71µs ± 5% 5.25µs ± 1% -7.99% (p=0.000 n=8+7) BidirStream/Med-8 11.5µs ±17% 8.1µs ± 9% -29.73% (p=0.000 n=8+8) BidirStream/Large-8 1.82ms ± 6% 1.48ms ±15% -18.35% (p=0.000 n=8+8) name old speed new speed delta Unitary/Small-8 90.0kB/s ± 0% 97.5kB/s ± 8% +8.33% (p=0.011 n=7+8) Unitary/Med-8 74.5MB/s ± 4% 81.7MB/s ± 3% +9.65% (p=0.000 n=8+7) Unitary/Large-8 569MB/s ±18% 706MB/s ± 3% +24.08% (p=0.000 n=8+7) InputStream/Small-8 719kB/s ±11% 815kB/s ± 3% +13.39% (p=0.000 n=8+8) InputStream/Med-8 430MB/s ± 3% 539MB/s ± 3% +25.32% (p=0.000 n=7+8) InputStream/Large-8 1.25GB/s ±13% 1.48GB/s ± 4% +18.16% (p=0.000 n=8+7) OutputStream/Small-8 1.46MB/s ± 4% 1.57MB/s ± 5% +7.15% (p=0.000 n=7+8) OutputStream/Med-8 396MB/s ±12% 523MB/s ± 4% +32.03% (p=0.000 n=8+8) OutputStream/Large-8 1.31GB/s ± 3% 1.56GB/s ± 2% +19.65% (p=0.001 n=7+7) BidirStream/Small-8 350kB/s ± 6% 380kB/s ± 0% +8.57% (p=0.000 n=8+7) BidirStream/Med-8 180MB/s ±16% 254MB/s ± 8% +40.59% (p=0.000 n=8+8) BidirStream/Large-8 578MB/s ± 5% 712MB/s ±14% +23.13% (p=0.000 n=8+8) name old alloc/op new alloc/op delta Unitary/Small-8 2.40kB ± 0% 2.25kB ± 0% -5.92% (p=0.001 n=7+7) Unitary/Med-8 15.7kB ± 0% 13.2kB ± 0% -15.52% (p=0.000 n=8+7) Unitary/Large-8 15.1MB ± 0% 14.0MB ± 0% -6.92% (p=0.000 n=8+8) InputStream/Small-8 96.0B ± 0% 88.0B ± 0% -8.33% (p=0.000 n=8+8) InputStream/Med-8 6.74kB ± 0% 4.43kB ± 0% -34.20% (p=0.002 n=7+8) InputStream/Large-8 7.59MB ± 0% 6.54MB ± 0% -13.89% (p=0.000 n=8+8) OutputStream/Small-8 176B ± 0% 168B ± 0% -4.55% (p=0.000 n=8+8) OutputStream/Med-8 6.82kB ± 0% 4.51kB ± 0% -33.80% (p=0.000 n=8+8) OutputStream/Large-8 7.55MB ± 0% 6.49MB ± 0% -13.94% (p=0.000 n=7+8) BidirStream/Small-8 272B ± 0% 256B ± 0% -5.88% (p=0.000 n=8+8) BidirStream/Med-8 13.6kB ± 0% 8.9kB ± 0% -34.00% (p=0.000 n=8+7) BidirStream/Large-8 15.1MB ± 0% 13.0MB ± 0% -13.85% (p=0.000 n=8+8) name old allocs/op new allocs/op delta Unitary/Small-8 32.0 ± 0% 25.0 ± 0% -21.88% (p=0.000 n=8+8) Unitary/Med-8 34.0 ± 0% 27.0 ± 0% -20.59% (p=0.000 n=8+8) Unitary/Large-8 53.9 ± 2% 46.9 ± 4% -12.99% (p=0.000 n=8+8) InputStream/Small-8 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=8+8) InputStream/Med-8 4.00 ± 0% 3.00 ± 0% -25.00% (p=0.000 n=8+8) InputStream/Large-8 13.6 ± 5% 12.8 ± 6% -6.42% (p=0.013 n=8+8) OutputStream/Small-8 4.00 ± 0% 3.00 ± 0% -25.00% (p=0.000 n=8+8) OutputStream/Med-8 5.00 ± 0% 4.00 ± 0% -20.00% (p=0.000 n=8+8) OutputStream/Large-8 14.6 ± 4% 13.6 ± 5% -6.84% (p=0.009 n=8+8) BidirStream/Small-8 7.00 ± 0% 5.00 ± 0% -28.57% (p=0.000 n=8+8) BidirStream/Med-8 9.00 ± 0% 7.00 ± 0% -22.22% (p=0.000 n=8+8) BidirStream/Large-8 28.4 ± 5% 25.8 ± 3% -9.25% (p=0.000 n=8+8) Change-Id: I2748e92dd0d756748924b3d0eea640cb2405f2fd drpcwire: change default split size name old time/op new time/op delta Unitary/Small-8 21.7µs ± 1% 21.5µs ± 2% -1.01% (p=0.038 n=7+7) Unitary/Med-8 32.7µs ± 2% 27.6µs ± 4% -15.62% (p=0.000 n=7+8) Unitary/Large-8 4.67ms ± 6% 1.86ms ±21% -60.20% (p=0.000 n=7+8) InputStream/Small-8 2.90µs ±12% 2.79µs ±13% ~ (p=0.328 n=8+8) InputStream/Med-8 7.43µs ± 6% 4.78µs ± 3% -35.72% (p=0.000 n=8+7) InputStream/Large-8 2.36ms ±15% 0.82ms ± 6% -65.12% (p=0.000 n=8+7) OutputStream/Small-8 2.84µs ± 8% 2.73µs ± 4% -3.75% (p=0.039 n=8+7) OutputStream/Med-8 7.50µs ± 4% 5.21µs ±13% -30.61% (p=0.000 n=7+8) OutputStream/Large-8 2.14ms ±13% 0.80ms ± 3% -62.45% (p=0.000 n=8+7) BidirStream/Small-8 5.69µs ±10% 5.71µs ± 5% ~ (p=0.798 n=8+8) BidirStream/Med-8 15.1µs ± 3% 11.5µs ±17% -23.49% (p=0.000 n=7+8) BidirStream/Large-8 4.62ms ± 9% 1.82ms ± 6% -60.72% (p=0.000 n=7+8) name old speed new speed delta Unitary/Small-8 90.0kB/s ± 0% 90.0kB/s ± 0% ~ (all equal) Unitary/Med-8 62.8MB/s ± 2% 74.5MB/s ± 4% +18.54% (p=0.000 n=7+8) Unitary/Large-8 225MB/s ± 6% 569MB/s ±18% +153.28% (p=0.000 n=7+8) InputStream/Small-8 695kB/s ±11% 719kB/s ±11% ~ (p=0.396 n=8+8) InputStream/Med-8 277MB/s ± 6% 430MB/s ± 3% +55.53% (p=0.000 n=8+7) InputStream/Large-8 446MB/s ±14% 1251MB/s ±13% +180.13% (p=0.000 n=8+8) OutputStream/Small-8 1.41MB/s ± 7% 1.46MB/s ± 4% ~ (p=0.057 n=8+7) OutputStream/Med-8 274MB/s ± 4% 396MB/s ±12% +44.79% (p=0.000 n=7+8) OutputStream/Large-8 493MB/s ±12% 1305MB/s ± 3% +164.71% (p=0.000 n=8+7) BidirStream/Small-8 352kB/s ± 9% 350kB/s ± 6% ~ (p=0.691 n=8+8) BidirStream/Med-8 135MB/s ± 6% 180MB/s ±16% +33.52% (p=0.000 n=8+8) BidirStream/Large-8 223MB/s ±13% 578MB/s ± 5% +159.40% (p=0.000 n=8+8) name old alloc/op new alloc/op delta Unitary/Small-8 2.40kB ± 0% 2.40kB ± 0% ~ (p=0.854 n=7+7) Unitary/Med-8 18.5kB ± 0% 15.7kB ± 0% -15.22% (p=0.000 n=8+8) Unitary/Large-8 16.0MB ± 0% 15.1MB ± 0% -5.52% (p=0.000 n=8+8) InputStream/Small-8 96.0B ± 0% 96.0B ± 0% ~ (all equal) InputStream/Med-8 8.14kB ± 0% 6.74kB ± 0% -17.30% (p=0.000 n=8+7) InputStream/Large-8 8.14MB ± 0% 7.59MB ± 0% -6.70% (p=0.000 n=7+8) OutputStream/Small-8 176B ± 0% 176B ± 0% ~ (all equal) OutputStream/Med-8 8.22kB ± 0% 6.82kB ± 0% -17.12% (p=0.000 n=8+8) OutputStream/Large-8 7.97MB ± 0% 7.55MB ± 0% -5.32% (p=0.000 n=8+7) BidirStream/Small-8 272B ± 0% 272B ± 0% ~ (all equal) BidirStream/Med-8 16.4kB ± 0% 13.6kB ± 0% -17.20% (p=0.000 n=7+8) BidirStream/Large-8 16.0MB ± 0% 15.1MB ± 0% -5.52% (p=0.000 n=8+8) name old allocs/op new allocs/op delta Unitary/Small-8 32.0 ± 0% 32.0 ± 0% ~ (all equal) Unitary/Med-8 36.0 ± 0% 34.0 ± 0% -5.56% (p=0.000 n=8+8) Unitary/Large-8 86.0 ± 1% 53.9 ± 2% -37.35% (p=0.000 n=8+8) InputStream/Small-8 3.00 ± 0% 3.00 ± 0% ~ (all equal) InputStream/Med-8 5.00 ± 0% 4.00 ± 0% -20.00% (p=0.000 n=8+8) InputStream/Large-8 30.5 ± 2% 13.6 ± 5% -55.33% (p=0.000 n=8+8) OutputStream/Small-8 4.00 ± 0% 4.00 ± 0% ~ (all equal) OutputStream/Med-8 6.00 ± 0% 5.00 ± 0% -16.67% (p=0.000 n=8+8) OutputStream/Large-8 30.5 ± 2% 14.6 ± 4% -52.05% (p=0.000 n=8+8) BidirStream/Small-8 7.00 ± 0% 7.00 ± 0% ~ (all equal) BidirStream/Med-8 11.0 ± 0% 9.0 ± 0% -18.18% (p=0.000 n=8+8) BidirStream/Large-8 61.5 ± 2% 28.4 ± 5% -53.86% (p=0.000 n=8+8) Change-Id: Ie52bf775e5effee272fb594a9ba04606748d6e73 drpcsignal: lazily allocate channels name old time/op new time/op delta Unitary/Small-8 21.4µs ± 1% 21.7µs ± 1% +1.62% (p=0.026 n=7+7) Unitary/Med-8 32.8µs ± 1% 32.7µs ± 2% ~ (p=0.805 n=7+7) Unitary/Large-8 4.68ms ±16% 4.67ms ± 6% ~ (p=0.383 n=7+7) InputStream/Small-8 2.65µs ± 1% 2.90µs ±12% +9.35% (p=0.004 n=7+8) InputStream/Med-8 7.44µs ± 4% 7.43µs ± 6% ~ (p=0.807 n=7+8) InputStream/Large-8 2.19ms ± 9% 2.36ms ±15% +7.82% (p=0.038 n=8+8) OutputStream/Small-8 2.75µs ± 1% 2.84µs ± 8% ~ (p=0.094 n=7+8) OutputStream/Med-8 7.78µs ± 7% 7.50µs ± 4% -3.60% (p=0.040 n=8+7) OutputStream/Large-8 2.09ms ± 4% 2.14ms ±13% ~ (p=0.613 n=7+8) BidirStream/Small-8 5.77µs ± 6% 5.69µs ±10% ~ (p=0.105 n=8+8) BidirStream/Med-8 15.6µs ± 6% 15.1µs ± 3% -3.50% (p=0.004 n=7+7) BidirStream/Large-8 4.67ms ± 2% 4.62ms ± 9% ~ (p=0.097 n=7+7) name old speed new speed delta Unitary/Small-8 90.0kB/s ± 0% 90.0kB/s ± 0% ~ (all equal) Unitary/Med-8 62.6MB/s ± 1% 62.8MB/s ± 2% ~ (p=0.805 n=7+7) Unitary/Large-8 218MB/s ±22% 225MB/s ± 6% ~ (p=0.694 n=8+7) InputStream/Small-8 756kB/s ± 2% 695kB/s ±11% -8.03% (p=0.003 n=7+8) InputStream/Med-8 276MB/s ± 4% 277MB/s ± 6% ~ (p=0.779 n=7+8) InputStream/Large-8 480MB/s ± 8% 446MB/s ±14% -6.91% (p=0.038 n=8+8) OutputStream/Small-8 1.46MB/s ± 1% 1.41MB/s ± 7% ~ (p=0.067 n=7+8) OutputStream/Med-8 264MB/s ± 7% 274MB/s ± 4% +3.62% (p=0.040 n=8+7) OutputStream/Large-8 497MB/s ± 7% 493MB/s ±12% ~ (p=0.505 n=8+8) BidirStream/Small-8 346kB/s ± 5% 352kB/s ± 9% ~ (p=0.142 n=8+8) BidirStream/Med-8 131MB/s ± 5% 135MB/s ± 6% +2.76% (p=0.029 n=7+8) BidirStream/Large-8 225MB/s ± 2% 223MB/s ±13% ~ (p=0.281 n=7+8) name old alloc/op new alloc/op delta Unitary/Small-8 2.78kB ± 0% 2.40kB ± 0% -13.85% (p=0.000 n=8+7) Unitary/Med-8 18.9kB ± 0% 18.5kB ± 0% -2.04% (p=0.000 n=8+8) Unitary/Large-8 16.0MB ± 0% 16.0MB ± 0% -0.00% (p=0.000 n=8+8) InputStream/Small-8 96.0B ± 0% 96.0B ± 0% ~ (all equal) InputStream/Med-8 8.14kB ± 0% 8.14kB ± 0% ~ (all equal) InputStream/Large-8 8.13MB ± 0% 8.14MB ± 0% ~ (p=0.267 n=8+7) OutputStream/Small-8 176B ± 0% 176B ± 0% ~ (all equal) OutputStream/Med-8 8.22kB ± 0% 8.22kB ± 0% ~ (p=0.608 n=8+8) OutputStream/Large-8 7.98MB ± 0% 7.97MB ± 0% ~ (p=0.130 n=8+8) BidirStream/Small-8 272B ± 0% 272B ± 0% ~ (all equal) BidirStream/Med-8 16.4kB ± 0% 16.4kB ± 0% -0.00% (p=0.014 n=8+7) BidirStream/Large-8 16.0MB ± 0% 16.0MB ± 0% ~ (p=0.367 n=8+8) name old allocs/op new allocs/op delta Unitary/Small-8 36.0 ± 0% 32.0 ± 0% -11.11% (p=0.000 n=8+8) Unitary/Med-8 40.0 ± 0% 36.0 ± 0% -10.00% (p=0.000 n=8+8) Unitary/Large-8 90.2 ± 1% 86.0 ± 1% -4.71% (p=0.000 n=8+8) InputStream/Small-8 3.00 ± 0% 3.00 ± 0% ~ (all equal) InputStream/Med-8 5.00 ± 0% 5.00 ± 0% ~ (all equal) InputStream/Large-8 30.6 ± 4% 30.5 ± 2% ~ (p=1.000 n=8+8) OutputStream/Small-8 4.00 ± 0% 4.00 ± 0% ~ (all equal) OutputStream/Med-8 6.00 ± 0% 6.00 ± 0% ~ (all equal) OutputStream/Large-8 31.0 ± 0% 30.5 ± 2% ~ (p=0.051 n=7+8) BidirStream/Small-8 7.00 ± 0% 7.00 ± 0% ~ (all equal) BidirStream/Med-8 11.0 ± 0% 11.0 ± 0% ~ (all equal) BidirStream/Large-8 61.9 ± 3% 61.5 ± 2% ~ (p=0.513 n=8+8) Change-Id: Iadbeffe25d3184e338352e981efe13bf398889bd better benchmarks old = grpc new = drpc name old time/op new time/op delta Unitary/Small-8 39.5µs ±50% 21.4µs ± 1% -45.99% (p=0.000 n=8+7) Unitary/Med-8 39.7µs ± 5% 32.8µs ± 1% -17.48% (p=0.001 n=7+7) Unitary/Large-8 1.35ms ± 4% 4.68ms ±16% +246.20% (p=0.001 n=7+7) InputStream/Small-8 856ns ± 9% 2650ns ± 1% +209.65% (p=0.001 n=7+7) InputStream/Med-8 2.95µs ± 2% 7.44µs ± 4% +152.10% (p=0.001 n=7+7) InputStream/Large-8 544µs ± 3% 2190µs ± 9% +302.21% (p=0.000 n=7+8) OutputStream/Small-8 953ns ± 4% 2745ns ± 1% +188.19% (p=0.001 n=7+7) OutputStream/Med-8 2.87µs ± 3% 7.78µs ± 7% +171.28% (p=0.000 n=8+8) OutputStream/Large-8 532µs ± 4% 2089µs ± 4% +292.71% (p=0.001 n=7+7) BidirectionalStream/Small-8 10.7µs ± 2% 5.8µs ± 6% -46.00% (p=0.000 n=7+8) BidirectionalStream/Med-8 15.9µs ± 2% 15.6µs ± 6% ~ (p=0.073 n=7+7) BidirectionalStream/Large-8 1.38ms ± 4% 4.67ms ± 2% +238.89% (p=0.001 n=7+7) name old speed new speed delta Unitary/Small-8 53.8kB/s ±44% 90.0kB/s ± 0% +67.44% (p=0.000 n=8+7) Unitary/Med-8 51.7MB/s ± 5% 62.6MB/s ± 1% +21.08% (p=0.001 n=7+7) Unitary/Large-8 775MB/s ± 4% 218MB/s ±22% -71.88% (p=0.000 n=7+8) InputStream/Small-8 2.28MB/s ±19% 0.76MB/s ± 2% -66.85% (p=0.000 n=8+7) InputStream/Med-8 696MB/s ± 2% 276MB/s ± 4% -60.32% (p=0.001 n=7+7) InputStream/Large-8 1.93GB/s ± 3% 0.48GB/s ± 8% -75.11% (p=0.000 n=7+8) OutputStream/Small-8 4.20MB/s ± 4% 1.46MB/s ± 1% -65.34% (p=0.001 n=7+7) OutputStream/Med-8 716MB/s ± 3% 264MB/s ± 7% -63.11% (p=0.000 n=8+8) OutputStream/Large-8 1.97GB/s ± 4% 0.50GB/s ± 7% -74.81% (p=0.000 n=7+8) BidirectionalStream/Small-8 185kB/s ± 8% 346kB/s ± 5% +87.16% (p=0.000 n=8+8) BidirectionalStream/Med-8 129MB/s ± 2% 131MB/s ± 5% ~ (p=0.073 n=7+7) BidirectionalStream/Large-8 761MB/s ± 4% 225MB/s ± 2% -70.50% (p=0.001 n=7+7) name old alloc/op new alloc/op delta Unitary/Small-8 8.43kB ± 0% 2.78kB ± 0% -67.01% (p=0.001 n=6+8) Unitary/Med-8 21.9kB ± 0% 18.9kB ± 0% -13.85% (p=0.000 n=8+8) Unitary/Large-8 6.51MB ± 0% 15.97MB ± 0% +145.36% (p=0.000 n=8+8) InputStream/Small-8 409B ± 2% 96B ± 0% -76.55% (p=0.000 n=8+8) InputStream/Med-8 7.09kB ± 0% 8.14kB ± 0% +14.86% (p=0.000 n=8+7) InputStream/Large-8 3.22MB ± 0% 8.13MB ± 0% +152.41% (p=0.000 n=8+8) OutputStream/Small-8 371B ± 1% 176B ± 0% -52.58% (p=0.000 n=7+8) OutputStream/Med-8 7.06kB ± 0% 8.22kB ± 0% +16.40% (p=0.000 n=7+8) OutputStream/Large-8 3.21MB ± 0% 7.98MB ± 0% +148.30% (p=0.000 n=8+8) BidirectionalStream/Small-8 1.02kB ± 0% 0.27kB ± 0% -73.31% (p=0.002 n=7+8) BidirectionalStream/Med-8 14.5kB ± 0% 16.4kB ± 0% +13.21% (p=0.000 n=8+8) BidirectionalStream/Large-8 6.52MB ± 0% 15.96MB ± 0% +144.89% (p=0.000 n=8+8) name old allocs/op new allocs/op delta Unitary/Small-8 169 ± 0% 36 ± 0% -78.70% (p=0.000 n=8+8) Unitary/Med-8 171 ± 0% 40 ± 0% -76.61% (p=0.000 n=8+8) Unitary/Large-8 422 ± 3% 90 ± 1% -78.63% (p=0.000 n=8+8) InputStream/Small-8 11.0 ± 0% 3.0 ± 0% -72.73% (p=0.000 n=8+8) InputStream/Med-8 12.0 ± 0% 5.0 ± 0% -58.33% (p=0.000 n=8+8) InputStream/Large-8 128 ± 5% 31 ± 4% -76.12% (p=0.000 n=8+8) OutputStream/Small-8 10.0 ± 0% 4.0 ± 0% -60.00% (p=0.000 n=8+8) OutputStream/Med-8 11.0 ± 0% 6.0 ± 0% -45.45% (p=0.000 n=8+8) OutputStream/Large-8 131 ± 1% 31 ± 0% -76.28% (p=0.000 n=6+7) BidirectionalStream/Small-8 41.0 ± 0% 7.0 ± 0% -82.93% (p=0.000 n=8+8) BidirectionalStream/Med-8 44.0 ± 0% 11.0 ± 0% -75.00% (p=0.000 n=8+8) BidirectionalStream/Large-8 291 ± 3% 62 ± 3% -78.72% (p=0.000 n=8+8) Change-Id: I7dde0698d2a2190177272f2e90a23056f88aecf2 drpcwire: fix Kind string conversion Change-Id: Id73790bad09c632486b6f5035eec44f766eb3e9a copy code of conduct from storj/storj Change-Id: If509873de41e3bf7647b1bd72843e80d5a56b8ad drpchttp: support application/protobuf this also makes it so that responses follow the way twirp does it, which means we can use twirp to generate clients for any unitary rpcs for other languages. Change-Id: Ia23d522694718abb30bad85989eb9d2783535f5d add battle tested Change-Id: I522f93a0a0f635bb87a059a8cb469c6dabca8a4e drpc: go back to Mux name 1. no breakage 2. drpc.Mux defined in drpcmux 3. only weirdness is that the method is named Register Change-Id: I8f45a3c7a6ac3cee7888524ece99f1bfd6fa76b1 drpchttp: move http serving out of drpcmux Change-Id: Ic5802c9edc5f07fd4216772d8775932771d6d25e change mux to registry and clean up some docs Change-Id: I33c310468af510fe13525c16673ba6fcf7a982bb examples link Change-Id: I3661975347c5b87927999d7fbabe8ca953d4bb18 readme updates Change-Id: I987667bf5a0f96a9bd1cbdebcce619a2d0422247 make loc target Change-Id: I0f655154db4536d6b9f147656f6092f98d4e422f add benchmarks and remove monitoring Change-Id: I294142a280ae27271fe51be801ad1c5985a63ddd bump google.golang.org/protobuf to v1.26.0 this required specifying the actual go_package line which requires then passing `paths=source_relative` to all the generation commands. annoying that it's not the default, tbh. Change-Id: I7dc748a9d33717f7a14ef14e28fe1713ef12e62b Update README.md drpcmanager: make inactivity timeout off by default the inactivity timeout breaks some expectations of users who are dropping drpc in for grpc. many users expect connections to keep working, and having the inactivity timeout on by default means users will often get unexpected errors. it's true that they should handle the closed connection case for robustness reasons, but perhaps we shouldn't force them to so frequently. we should let them enable it if they want to, though. i think we should consider making this on by default once we have a high quality conn pool implementation in this repo. Change-Id: I73691e6236d0d1fdeaeb6e378ef75dd60759c764 drpcmanager: return a more helpful error when the connection is closed Change-Id: I7319bc610e42c53315d0b95875dfe0aac958ddab stop using apis introduced in go1.16 Change-Id: If1050f482fd269d9e7d9dc34f0a2a3598be9f543 cmd: ignore files with no services Change-Id: I63083a11c3626369553d729e4394052281abd502 protoc-gen-go-drpc: include version info in generated code Change-Id: I08c361fb926b61dfed273ae86ee3acd175f3cbb0 rename to protoc-gen-go-drpc to match grpc Change-Id: Idf897cf600304759d1caff752c59c30bdc366087 drpcmigrate subpkg and examples This is the first step of moving two useful utilities out of storj/storj - dialing with a header, and the listenmux. We are interested in publishing a blog post with content about drpc, and one of the main angles in the blog post is how others, currently using grpc, can switch, so we want to make support for switching a first-class part of the drpc project. Change-Id: I5280841ba7861a7f94d5f9bdfe0bf32e24aac8d5 internal/backcompat: add backwards compatability tests this works by defining the following modules: ./backcompat ./backcompat/newservice ./backcompat/newservicedefs ./backcompat/oldservice ./backcompat/oldservicedefs ./backcompat/servicedefs the ./backcompat module depends on ./backcompat/servicedefs which just contains stub types/functions that match the generated drpc code. that module then creates a Main function that executes either as a server or a client, checking to see that the generated rpcs work. ./backcompat/newservicedefs are generated every time go generate runs and depend on the most recent drpc. ./backcompat/oldservicedefs are generated once with drpc version v0.0.17 and are expected to never change again. ./backcompat/newservice uses some module replace tricks to replace the ./backcompat module to the local directory, and the ./backcompat/servicedefs module to ./backcompat/newservicedefs. It then calls the Main function, creating a server/client binary that uses the ./backcompat/newservicedefs. ./backcompat/oldservice is the same way. The tests in ./backcompat then execute both ./backcompat/newservice and ./backcompat/oldservice in both client/server mode and point them at each other. Change-Id: I934e62fff3483d194e8a71d74cfbeb10a5a087b3 internal: silence some go generate protoc warnings Change-Id: I9a42d7f58fb60fe398bc467fa922410cbb11666a all: support different protobuf runtime libraries This adds an Encoding type that describes how to read and write the Message type. The generated code creates an Encoding based on the protobuf library that was used to generate the protobuf messages. The default Encoding generated uses google.golang.org/protobuf to marshal and unmarshal. This can be overridden with the protolib flag during an invocation to protoc-gen-drpc as follows: protoc --drpc_out=protolib=github.com/gogo/protobuf:. The argument is an import path. Both the google.golang.org/protobuf and github.com/gogo/protobuf import paths are recognized by the command to generate correct Encoding values. If an unrecognized import path is passed, it is assumed that the import defines functions that match the name and signature of the Encoding interface, and it will generate a type that matches it. Change-Id: I41e0c221e151ef2db8e8852d4ecb5ce9346f25af scripts: remove tools Change-Id: I3c17bfba54332109dd79e11748aa3ad0041c72da cmd/protoc-gen-drpc: use google.golang.org/protobuf to generate this changes the code to generate in a 2nd file much like the current grpc plugin. the api from google.golang.org/protobuf is a bit cleaner than gogo protobuf anyway. Change-Id: I2d383d0bb3035635d3e9750cb0c5041902d8453e drpcmetadata: remove protobuf dependency Change-Id: I4797faba25b5280e2539bcb314659c33f4dbeeb5 cmd/protoc-gen-drpc: generate UnimplementedServer XUnimplementedServer allows to embed it into servers such that when adding a method to service won't break compilation. Change-Id: I03d3bc8c8c707988382134d6707cbdfc31a46fd6 cmd/protoc-gen-drpc: bump gogo version to v1.3.2 Change-Id: I20d7b82d48f0bce40c7ed3a709161f2288eb8183 internal/integration: more test coverage around blocking transports Change-Id: I1418195c66436dbe2b70e927c85ebfb25135348d drpcmanager: add inactivity timeout for new server streams Change-Id: I4ac9658d2cc0b7992c84b161f174ba925918cb72 ci: use main branch Change-Id: Ide0d77e62df38c988ab2fc78fa4c9ce624682cd5 all: rework contexts to have better parents Change-Id: Id0502b1aebe315823d5fc92c9bcfcb250d13d7d1 drpcmanager: associate stream contexts with passed in context rather than have each manager have a background context, removing any association the stream has with the rpc that it is being used for, just pass the context through. this also lets us simplify some code as well. then we update the tests to ensure that simplified code doesn't break anything by making them check more than before. Change-Id: I0a9b5b74b2a2cc4c80e1e6e2c212eb638efcae6f ci: use storj/ci docker image and fix errors Change-Id: I3a18d2f78c9c2871ee7d649d3ec22e79cd13b89d ci: update staticcheck to 2020.2 Change-Id: Ic3006ae8ed550de118cad9e9f2c1657f4f0359a1 all: fix check-copyright Change-Id: Ie5fe7ad01522fc6b66869d3ff9c369e1b5794e86 Dockerfile: update to go v1.15.6 and golangci-ling v1.33.0 (#3) drpcmux: build on earlier go versions Change-Id: I6a15b2118174f22ba65a02215a13807a1da90745 drpcstream: fix missing doc generation Change-Id: Iee11f0b112a9be477d22d6b823b8f758553c177d drpcmux: fix import grouping Change-Id: I4f83fe8408262691eada7ff7d6de1e7a49e6fa5d drpcmanager: wait for previous stream to finish before creating new concurrent access to a drpc connection should be serialized by the semaphore in the drpcmanager, but that semaphore is released when the stream is terminated, not finished. that means that a previous stream can still use the underlying transport even after the manager allows a new stream to be created, causing issues. the manager now will wait for the previous stream to be finished before creating a new one. this doesn't cause any latency issues because the transport is assumed to be reliable and stream-oriented so if the close message is not being processed, then new messages from the new stream would not be either. Change-Id: Ie54b33023d9c5cb339fbd7ba9509abc6db581d8d drpcmux: add ServeHTTP method This allows one to serve unitary RPCs over an HTTP server using the same code. No gateway or other funny business. Change-Id: Ie0d88e13911fe8f03aaf5549eb5b4e8274f695a5 bump linters/go version Change-Id: Ic89263d9d6abac033c15eed3e40c06b3a0391c58 deps: bump monkit Change-Id: If4d318e44f0fcb459f71934b53db195d0da2bdda drpcerr: faster implementation Change-Id: I419b21c8387d4a0e1698257e1b2aa389c3eb9c1b drpcstream: add state machine diagram Change-Id: I20060c7bf237eb6ebf371cc25b69e323177da9c6 stream/manager: fix cancellation propagation in cases where a stream is not sending, like in a unitary rpc after the invoke or a stream rpc that just isn't writing, and a cancel happened, we would not close the transport even though we would require more reads and writes to complete the rpc because we considered the stream to be "finished." change "finished" to mean that there will never be any more reads or writes on the stream. this is accomplished by having a read and write mutex and polling them to see if we can acquire both after every read or write operation. if "terminated" is set and we can even temporarily acquire both mutexes, because terminated implies no new reads or writes will happen without an error, we know there are no active reads or writes and no traffic on the transport. we used to check for finished while we were holding the write mutex, but that caused problems when trying to acquire the read mutex because of mutex ordering possibly leading to deadlocks. we don't need to check all of the conditions atomically though, due to "terminated" as explained above. Change-Id: I0cd3237c33d4b1c999752e97b17c2bd5757d6fce internal/integration: test cancellation propagation Change-Id: I0d7f4e92a85762fd42eeaae2471628fce5747759 keep root README on "make docs" invocation Change-Id: I178c03d72813702bd13070e4ed80fb0301c23cb2 tests: add coverage around context errors Change-Id: I7534a2dfba86efcd7e6a99851162b53c2b24dbfc a more standard readme Change-Id: I2f6b096302e6a2a6ff1ebd6049173599778094a0 Licensed under MIT/expat Change-Id: If44c7e24b0e664833d35a3944fac74872dd84916 Relicense to BSD-2-Clause Plus Patent On the advice of external legal counsel, we should switch to this license. See https://writing.kemitchell.com/2019/11/07/BSD-Patents.html for similar information. https: //spdx.org/licenses/BSD-2-Clause-Patent.html https: //opensource.org/licenses/BSDplusPatent Change-Id: I496aea61ed736a28c4a822a7c8eaaaf66b6ec20c license: use MIT Change-Id: I4c4651dbd1eacb7781564c9f21459297edc13ca1 drpcmanager: really always ensure the stream is terminated and i mean it this time. really. Change-Id: I17c10f1c8b89509bf3882f4a17516b6374e1d651 add Closed method to drpc.Conn interface, clean up tests+regenerate Change-Id: I051bcb9b698c745c9c03050f85e5d23bc29cee2b drpccache: add per transport cache Some requests require checking whether some condition holds or is needed. Change-Id: Ia54689ccabb67980631b9edefe25d464008875c9 set minimum required to go1.13 Our minimum required for uplink is Go 1.13, so let's keep this at the same version until we are ready to bump. Change-Id: I835601b77e0b18c665413fcd77e94a6ecfe2b8c8 fix import statement grouping Change-Id: Ibca7b03e757ec7d44e1f54c4366d9f4533e2a15c drpcmetadata: encode and decode metadata (#2) add InvokeMetadata message and reserve control bit We need a way to send Metadata for an invoke such that older servers will be able to read newer clients, so they must ignore the metadata. Foruntately, when reading for a new Invoke, we ignore any packets that are not of kind Invoke. That means that newer clients can send whatever they want before the Invoke like metadata. It also reserves a bit of the control byte so that after everything is all rolled out, we can use it if necessary to extend the protocol. The current implementation will drop any frames that have the control bit set, so that we can send arbitrary data without future older clients breaking. Change-Id: I51f007f9ef71e5f4e3f5ec2a83d1f61c69528454 split a drpcmux.Mux out of drpcserver.Server All of the registration now happens on the Mux which is passed as a Handler to the Server. This lets one write wrappers around the Mux that also implement Handler so that you can do things like intercept the RPC to do before/after stuff. Change-Id: If28396aa1b82df3fe7f5b10826a202de65da28cc rename Handler to Receiver and consolidate Makefile scripts This allows us to add a Handler type ala net/http, and I just got tired of having a bunch of shell scripts repeating the same thing: move the logic into the makefile. Change-Id: I8bc3ff64d53c65f11b5c88a1dcfea97a1a5e3709 golangci-lint: Disable ifElseChain go-critic rule Disable the ifElseChain rule of the go-critic linter because it isn't of the taste of Storj. Change-Id: I83e82de22173154b6469d12f9c90b016a8820e48 golangci: Enable gosec linter Enable the gosec linter. Ticket: https://storjlabs.atlassian.net/browse/SM-431 Change-Id: Iaebe93d012662807dea3ffd9e1d077f390aed317 add comments to distinguish drpc generated code Change-Id: I938d4ebec515dafd0834a05532de51cc513c8c6c Use monkit/v3 Change-Id: I4d44f18d0c5b9918bd3056613edfc5f917b91110 golangci: Update config to the one used Update the golangci configuration the common configuration used across repositories. The changes in other files are due to make the linter happy. Change-Id: Iad89611996198e482816251bc283d71b7284912b use monkit/v2 for now and add overall request rates Change-Id: Ia1ab15b5d4aa466963591d53a42d9360104a5bee add monkit/v3 around some important functions Change-Id: I562d3b4ab4072cdc8e11d14601bd8e6846dbc8f9 add docs, staticcheck, and golangci-lint Change-Id: I01005a398742dea80595ea50dc600c32916d083e add options to control some buffer/split sizes Change-Id: I79026cd28a070e632caa781c7fb0db513516e726 protect against large packets Change-Id: Ieb98a061170929a9dffc886c6ae4e85e585e14b3 cancel streams that are unmanaged because clients terminate tcp connections if the stream is unfinished when it is canceled, remotes should put their stream into the cancel state when the tcp connection dies. fortunately, the unmanaged case is exactly when the transport dies as every other exit for those helper functions happens when the stream is terminated for some other reason. without this you end up with a lot of "stream closed by sending error" messages on the servers after they attempted to send an error to no one and then bubble that error up when they try to close or whatever. Change-Id: I12b02db5340eb2cc6613e5e8f396328bdb6b76c1 remove cancel message since we never send it Change-Id: Idf397de160644fe431ce488789542dcc19cc09c3 return context errors from socket writes after canceled Change-Id: Id51be713a643c4486806ab415442198135938f3e cancel should kill transport if necessary if we're stuck in a write syscall and the context is canceled the we must kill the transport in order to unblock. in order to reuse the connection as much as possible especially given that most contexts eventually become canceled, we don't kill the transport if we're certain we have already sent a final message for that stream to the remote end. Change-Id: Ie06e426bc8266ca846232958b8ef742f82047854 initial fuzzing package Change-Id: I654a483b8f2698910c53f132a0523e9e04b19a15 fixes to allow connection pooling first, we add a `Closed` method to the `drpcconn.Conn` so that you can do some best effort checking if the underlying transport is still usable. second, we ensure that the stream's context is only canceled after we know all of the sends/receives for that stream are completed. that way, users of the stream can be sure that the conn that provided it can be reused. Change-Id: I6814b8c6bda79d3e3a73672ef0d2d5374b06d84b ensure that we return io.EOF unwrapped Change-Id: I6ced1afac19a280bc0459f6302115255cfcae2c3 ensure the stream is terminated if we stop managing it Change-Id: If1b2a4f7ba872163a64f98835ba140d0d4d41b5d require id monotonicity across read calls we require id monotonicity within a read call when reconstructing a packet to make reading packets not require extra memory: we can throw out any results when the id increases. but, we don't require it across consecutive calls. Consider incomplete frame for packet 1 complete frame for packet 2 complete frame for packet 1 without this commit, that would successfully return packet 2, then successfully return a partial result for packet 1, violating both guarantees about complete packets but also monotonicity. this commit adds an id that is stored across calls so that for the above case we would return an error on the 3rd frame. Change-Id: Ib979112a37b73b44e25009e2129d74b1bc8608c2 add grpccompat module with skeleton for writing tests also, refactor the manager and stream to be simpler and pass the first round of tests Change-Id: I3a2a362e5bf1ab781ca0ed004904e67107476b3b rename Jenkinsfile.gerrit to just Jenkinsfile Change-Id: I1610be3ce1de308f959c775766c1a89688b404ad bug fixes, more tests, and some debug logging The main issue was that drpc could deadlock in the case that the manager received messages with no stream active. That's fixed by having the select to send the message to the active stream also attempt to enter the stream semaphore. If it can then that means there's no stream, so it should just drop the message. I'm not convinced there are no more deadlocks. I probably need to do some more low level tests to figure this out. Change-Id: I56e1b2b12ca290743e7edecc4274b7bade7aa512 remove drpc prefix from method names and add register functions Change-Id: I8316c12e91d4e921711434fc147dd285d7c66c07 drpcserver: associate serving clients with a context Change-Id: Ia3d88ad49da05df61c4876b8d5c9c1031551af23 attempted minimal jenkins pipeline Change-Id: I1e81b497f3bd76cbc82dbaa3b12af23e30638070 Reviewed-on: https://review.dev.storj.io/c/storj/drpc/+/21 Tested-by: Storj Robot Reviewed-by: JT Olio test error codes a small amount and fix some bugs marshal/unmarshal coded errors drpcerr package to add error codes to errors little cleanups add scripts readme tiny cleanup and basic docs go mod tidy add back stream ids and make more robust add back message/stream ids change to use serial rpcs on a connection initial code Initial commit --- drpcmanager/flow_control_test.go | 157 +++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 drpcmanager/flow_control_test.go diff --git a/drpcmanager/flow_control_test.go b/drpcmanager/flow_control_test.go new file mode 100644 index 00000000..c6c4c287 --- /dev/null +++ b/drpcmanager/flow_control_test.go @@ -0,0 +1,157 @@ +// Copyright (C) 2026 Cockroach Labs. +// See LICENSE for copying information. + +package drpcmanager + +import ( + "context" + "errors" + "io" + "net" + "sync/atomic" + "testing" + + "github.com/zeebo/assert" + + "storj.io/drpc" + "storj.io/drpc/drpcstream" + "storj.io/drpc/drpctest" + "storj.io/drpc/drpcwire" + "storj.io/drpc/internal/drpcopts" +) + +// fcManagerOptions returns manager Options whose streams have flow control +// enabled via the internal stream option. +func fcManagerOptions(window, highWater, threshold int64) Options { + stream := drpcstream.Options{SplitSize: 64 << 10} + drpcopts.SetStreamFlowControl(&stream.Internal, drpcopts.FlowControl{ + Enabled: true, + StreamWindow: window, + HighWater: highWater, + GrantThreshold: threshold, + }) + return Options{Stream: stream} +} + +// Enabling flow control through the manager's Stream options installs windows +// on every stream it creates (client and server), so a message larger than the +// send window only completes if credit grants flow across the real connection. +func TestManager_FlowControlEndToEnd(t *testing.T) { + ctx := drpctest.NewTracker(t) + defer ctx.Close() + + cconn, sconn := net.Pipe() + defer func() { _ = cconn.Close() }() + defer func() { _ = sconn.Close() }() + + // 128 KiB window = 2 frames of initial credit. + opts := fcManagerOptions(128<<10, 1<<20, 64<<10) + + cman := NewWithOptions(cconn, Client, opts) + defer func() { _ = cman.Close() }() + sman := NewWithOptions(sconn, Server, opts) + defer func() { _ = sman.Close() }() + + // 256 KiB is four frames: two fit in the initial window, the rest can only + // be sent as the server dispatches frames and returns credit. + msg := make([]byte, 256<<10) + + ctx.Run(func(ctx context.Context) { + stream, err := cman.NewClientStream(ctx, "rpc", drpc.CompressionNone) + assert.NoError(t, err) + assert.NoError(t, stream.RawWrite(drpcwire.KindInvoke, []byte("rpc"))) + assert.NoError(t, stream.RawWrite(drpcwire.KindMessage, msg)) + assert.NoError(t, stream.Close()) + }) + + ctx.Run(func(ctx context.Context) { + stream, _, err := sman.NewServerStream(ctx) + assert.NoError(t, err) + defer func() { _ = stream.Close() }() + + got, err := stream.RawRecv() + assert.NoError(t, err) + assert.Equal(t, len(got), len(msg)) + + _, err = stream.RawRecv() + assert.That(t, errors.Is(err, io.EOF)) + }) + + ctx.Wait() +} + +// sniffConn tees every byte read off the connection into w, where a frame +// parser classifies it. +type sniffConn struct { + net.Conn + w io.Writer +} + +func (s sniffConn) Read(p []byte) (int, error) { + n, err := s.Conn.Read(p) + if n > 0 { + _, _ = s.w.Write(p[:n]) + } + return n, err +} + +// Tripwire for accidental enablement: a default-configuration connection must +// never put a KindWindowUpdate on the wire. Both directions are sniffed; the +// KindMessage assertion proves the sniffer actually parsed the traffic. +func TestManager_DefaultConfigEmitsNoWindowUpdates(t *testing.T) { + tr := drpctest.NewTracker(t) + defer tr.Close() + + var sawMessage, sawWindowUpdate atomic.Bool + sniff := func(c net.Conn) net.Conn { + pr, pw := io.Pipe() + go func() { + rd := drpcwire.NewReader(pr) + for { + fr, err := rd.ReadFrame() + if err != nil { + _, _ = io.Copy(io.Discard, pr) // keep the tee unblocked + return + } + switch fr.Kind { + case drpcwire.KindMessage: + sawMessage.Store(true) + case drpcwire.KindWindowUpdate: + sawWindowUpdate.Store(true) + } + } + }() + t.Cleanup(func() { _ = pw.Close(); _ = pr.Close() }) + return sniffConn{Conn: c, w: pw} + } + + cconn, sconn := net.Pipe() + defer func() { _ = cconn.Close() }() + defer func() { _ = sconn.Close() }() + + cman := NewWithOptions(sniff(cconn), Client, Options{}) + defer func() { _ = cman.Close() }() + sman := NewWithOptions(sniff(sconn), Server, Options{}) + defer func() { _ = sman.Close() }() + + tr.Run(func(ctx context.Context) { + stream, _, err := sman.NewServerStream(ctx) + assert.NoError(t, err) + defer func() { _ = stream.Close() }() + got, err := stream.RawRecv() + assert.NoError(t, err) + assert.Equal(t, len(got), 256<<10) + _, err = stream.RawRecv() + assert.That(t, errors.Is(err, io.EOF)) + }) + + stream, err := cman.NewClientStream(context.Background(), "rpc", drpc.CompressionNone) + assert.NoError(t, err) + assert.NoError(t, stream.RawWrite(drpcwire.KindInvoke, []byte("rpc"))) + assert.NoError(t, stream.RawWrite(drpcwire.KindMessage, make([]byte, 256<<10))) + assert.NoError(t, stream.Close()) + tr.Wait() + + assert.That(t, sawMessage.Load()) + assert.That(t, !sawWindowUpdate.Load()) +} From 95f2e1b172eb528e78f5a8857bf1fbebfb43643c Mon Sep 17 00:00:00 2001 From: Sujatha Sivaramakrishnan Date: Thu, 16 Jul 2026 11:47:30 +0530 Subject: [PATCH 5/5] drpcmanager: test that context cancellation wakes a parked sender Add an integration test for the production wake path of a sender parked on flow-control credit. A flow-control-enabled client sends a message larger than its window to a server that has no flow control (so it never returns credit); the send parks, and cancelling the RPC context wakes it. This exercises manageStream -> Cancel -> terminate -> sendWindow.close end to end, which the unit tests previously covered only in pieces (the primitive's ctx path, and stream-level Cancel). Co-Authored-By: roachdev-claude drpcmanager: end-to-end test for flow-control enablement The manager already threads drpcstream.Options through newStream, so enabling FlowControl in Options.Stream installs windows on every stream the manager creates (client and server) with no additional wiring. Add an end-to-end test over a real net.Pipe connection between two --- drpcmanager/flow_control_test.go | 61 ++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/drpcmanager/flow_control_test.go b/drpcmanager/flow_control_test.go index c6c4c287..be8ed78f 100644 --- a/drpcmanager/flow_control_test.go +++ b/drpcmanager/flow_control_test.go @@ -10,6 +10,7 @@ import ( "net" "sync/atomic" "testing" + "time" "github.com/zeebo/assert" @@ -80,6 +81,66 @@ func TestManager_FlowControlEndToEnd(t *testing.T) { ctx.Wait() } +// Cancelling an RPC's context wakes a sender parked on flow-control credit. The +// client has flow control with a small window; the server does not, so it never +// returns credit and the client's oversized send parks. This exercises the +// production wake path: manageStream sees the cancellation and calls +// Cancel -> terminate -> sendWindow.close. +func TestManager_FlowControlContextCancelWakesParkedSend(t *testing.T) { + tr := drpctest.NewTracker(t) + defer tr.Close() + + cconn, sconn := net.Pipe() + defer func() { _ = cconn.Close() }() + defer func() { _ = sconn.Close() }() + + cman := NewWithOptions(cconn, Client, fcManagerOptions(128<<10, 1<<20, 64<<10)) + defer func() { _ = cman.Close() }() + // Server has no flow control, so it never returns credit. + sman := New(sconn, Server) + defer func() { _ = sman.Close() }() + + // Accept the stream and drain; the message never completes (the sender + // parks mid-message), so RawRecv just blocks until teardown. + tr.Run(func(ctx context.Context) { + stream, _, err := sman.NewServerStream(ctx) + if err != nil { + return + } + defer func() { _ = stream.Close() }() + for { + if _, err := stream.RawRecv(); err != nil { + return + } + } + }) + + streamCtx, cancel := context.WithCancel(context.Background()) + stream, err := cman.NewClientStream(streamCtx, "rpc", drpc.CompressionNone) + assert.NoError(t, err) + assert.NoError(t, stream.RawWrite(drpcwire.KindInvoke, []byte("rpc"))) + + // A 512 KiB message exceeds the 128 KiB window; once the initial credit is + // spent the send parks waiting for grants that never come. + done := make(chan error, 1) + go func() { done <- stream.RawWrite(drpcwire.KindMessage, make([]byte, 512<<10)) }() + + select { + case <-done: + t.Fatal("send returned before it could park on credit") + case <-time.After(50 * time.Millisecond): + } + + cancel() + + select { + case err := <-done: + assert.Error(t, err) + case <-time.After(time.Second): + t.Fatal("parked send did not wake on context cancellation") + } +} + // sniffConn tees every byte read off the connection into w, where a frame // parser classifies it. type sniffConn struct {