drpcstream,drpcmanager: enable per-stream flow control (grant-on-consume)

[suj-krishnan]

Installs the per-stream windows when FlowControl is set: the send window seeded with StreamWindow credit and watching the send signal for termination; the receive window returning consumed-byte credit coalesced at GrantThreshold.

Enablement is internal-only (merge safety). The option lives in internal/drpcopts (SetStreamFlowControl), not on the public drpcstream.Options — so a consumer bumping the drpc dependency mid-implementation cannot enable flow control: no public API reaches it. Promotion to a public option is a later, deliberate change.

Validation at the installation site. An invalid configuration resorts to mutually-consistent defaults (and logs the override) instead of failing the stream:

• StreamWindow and GrantThreshold must be positive;
• a non-positive SplitSize uses SplitData's 64 KiB default as the frame size;
• GrantThreshold + frame ≤ StreamWindow — the liveness guarantee for grant coalescing. With G = GrantThreshold, W = StreamWindow, F = frame: once the receive side has consumed more than W − F of the window, the sender has < F credit left and halts; if G > W − F, the receiver cannot emit a grant to unblock it either — deadlock. Requiring G ≤ W − F prevents that.

The liveness condition (enforced by validateFlowControl in drpcstream/stream.go) is:

GrantThreshold ≤ StreamWindow − frame (equivalently, G + F ≤ W)

where G = GrantThreshold, W = StreamWindow, F = the per-frame size (SplitSize).

Why it exists

Grant coalescing means the receiver withholds credit until the app has consumed at least G bytes, then sends one window update. But the sender can only have W bytes in flight, and it stalls the moment it can't afford the next frame — so it can be left holding up to F of credit it can't spend. That means only about W − F is guaranteed to be delivered-and-consumable before the sender is stuck. If the coalescing threshold G is bigger than that, the receiver may never accumulate enough consumed bytes to fire a grant — and if it never grants, the stalled sender never wakes. Deadlock.

Example that deadlocks (condition violated)

Say W = 96 KiB, F = 64 KiB (one 64 KiB message per frame), and someone sets G = 80 KiB.
Check: G ≤ W − F → 80 ≤ 32? No — violated.

1. Sender sends message 1 (64 KiB): credit 96 → 32.
2. Sender wants to send message 2 (needs 64 KiB) but only has 32 KiB credit → blocks.
3. Receiver consumes message 1 → accrued consumed = 64 KiB, which is < G (80 KiB) → withholds the grant.
4. Nothing more has arrived to consume, and the sender can't send more without a grant → both sides stuck forever.

Example that's healthy (condition satisfied)

Same W = 96 KiB, F = 64 KiB, but G = 32 KiB → 32 ≤ 32 ✓.

1. Sender sends message 1 (64 KiB): credit 96 → 32; tries message 2, blocks.
2. Receiver consumes message 1 → accrued 64 KiB ≥ G (32 KiB) → grants 64 KiB → sender credit back to 96 → it sends the next message. Progress.

The defaults that we set have huge margin:
W = 2 MiB, F = 64 KiB, G = 512 KiB:
G + F = 576 KiB ≤ 2 MiB ✓ — satisfied with room to spare.

[shubhamdhama]

One thing I want to clarify before we move forward. We decided to keep things simpler on the receive window side by sending ack only after we have a complete packet. That means, if we have a 1MiB packet and a send window of 256KiB, the sender will be deadlocked because receiver won't send any update until it receive the complete packet.

One cure to this was the suggestion that flow control should be applied at the packet level. If we have a message and hence a packet of 1MiB size, we send that whole packet to the other side and make the available grant negative (256KiB-1MiB). This loosens up the guarantees that flow control gives us around the memory but I doubt that most of the requests are this big so this simple model may just work fine with cockroach.

[shubhamdhama]

It's an unnecessary ceremony to keep FlowControl configuration in internal/drpcopts/stream.go. We can have that configuration right here in the stream. It's adding unnecessary indirection. I think the code will be more readable if that's in this package and even this stream file itself.

[shubhamdhama]

In fact, I would prefer if we eventually remove the internal/drpcopts package altogether.

[suj-krishnan]

This is temporary, so we can write end-end tests until we are integration ready (see PR description/comments). These options will eventually reside directly in drpcstream/stream.go. Integration with cockroach/enablement is tracked separately.

[shubhamdhama]

Ignore if you've already removed this in a stacked PR. If not, I would still recommend to fix what I've said above. I'm not saying move this to the public Options. I'm saying, can we not just move FlowControl object here. I will leave this up to you.

[suj-krishnan]

Got it - we can reconsider during enablement.

[suj-krishnan]

One thing I want to clarify before we move forward. We decided to keep things simpler on the receive window side by sending ack only after we have a complete packet. That means, if we have a 1MiB packet and a send window of 256KiB, the sender will be deadlocked because receiver won't send any update until it receive the complete packet.

One cure to this was the suggestion that flow control should be applied at the packet level. If we have a message and hence a packet of 1MiB size, we send that whole packet to the other side and make the available grant negative (256KiB-1MiB). This loosens up the guarantees that flow control gives us around the memory but I doubt that most of the requests are this big so this simple model may just work fine with cockroach.

Yes, that is implemented in the next stacked PR - #92.

but I doubt that most of the requests are this big

This is not true unfortunately - we do have requests/responses that can carry significant payload and we may need to figure just how much of a blocker this is. I'm working on ascertaining the worst case size for certain APIs.

[shubhamdhama]

This deserves a comment with an example of what would happen if we don't hold this condition true.

[suj-krishnan]

done - added an example in the PR description and also a comment.

[shubhamdhama]

Ignore if you've already removed this in a stacked PR. If not, I would still recommend to fix what I've said above. I'm not saying move this to the public Options. I'm saying, can we not just move FlowControl object here. I will leave this up to you.