fix(io): reserve per-conn pending-send headroom for priority (gossip) streams#236
Merged
Conversation
… streams
A large req/resp response monopolized the whole 32 MB per-connection
pending-send byte budget (conn.pending_stream_send_bytes). When a peer
pulled a multi-MB blocks_by_range sync response, the embedder handed it
to zquic and it filled the entire budget. The persistent /meshsub gossip
stream's enqueue then returned false, the embedder's gossip outbox backed
up, and attestations/aggregates were dropped ("persistent gossip bulk
outbox cap (64) hit ... dropping oldest") — even though the transport was
otherwise healthy (srtt 4.6-26 ms). Pure budget monopolization, not
congestion.
Fix: priority-stream headroom reservation. The embedder marks the
persistent gossip stream priority via the new markStreamPriority API.
Non-priority streams (req/resp) enqueue pending bytes against the reduced
cap pending_stream_send_bytes_cap - pending_priority_reserve_bytes (24 MB),
so they can fill at most 24 MB and always leave 8 MB for gossip; priority
streams enqueue against the full 32 MB cap. All existing pending-queue
invariants (byte accounting, drain, double-free safety, byte reset) are
preserved — only the per-enqueue byte-cap ceiling is now stream-aware.
- ConnState.priority_stream_ids: AutoHashMapUnmanaged(u64, void), cleared on
conn reset alongside the other per-stream maps.
- pendingBytesCapForStream(): full cap for priority, reduced cap otherwise.
- All four enqueue byte-cap checks route through it.
- Server.markStreamPriority/unmarkStreamPriority +
Client.markStreamPriority/unmarkStreamPriority public API.
- Test: a full non-priority backlog cannot exceed the reduced cap, yet a
priority(gossip) enqueue still succeeds (headroom guaranteed) while a
further non-priority enqueue is rejected.
Bumps version 1.7.65 -> 1.7.66.
… enqueues Correction to the headroom fix: the reserve was implemented by LOWERING the non-priority cap (32 → 24 MB), which broke block sync. A blocks_by_range response is enqueued as ONE enqueuePendingStreamSend call carrying the whole ~32 MB payload (split into MTU chunks only at drain). With a 24 MB non-priority ceiling, 0 + 32 MB > 24 MB is true even from an empty queue, so the response could never be enqueued and sync stalled forever. Make the reserve ADDITIVE instead: raise pending_stream_send_bytes_cap 32 → 40 MB. Non-priority (req/resp) streams now get 40 - 8 = 32 MB — exactly the original budget, so no req/resp regression — while priority (gossip) streams get the full 40 MB, i.e. 8 MB of headroom ABOVE a full 32 MB req/resp backlog. pendingBytesCapForStream is unchanged (full cap for priority, cap - reserve otherwise); only the cap value moved. Updated the test to cover the regression: a SINGLE ~32 MB non-priority write succeeds from an empty queue, leaves the queue exactly at its 32 MB ceiling, a further non-priority byte is rejected, and a priority(gossip) enqueue still succeeds out of the reserved 8 MB. Version stays 1.7.66.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a per-connection pending-send budget monopolization bug: a large req/resp (
blocks_by_range) response — enqueued as one ~32MB entry — filled the entire per-conn pending budget, so the persistent/meshsubgossip stream's enqueue was rejected and attestations/aggregates were dropped (persistent gossip bulk outbox cap (64) hit). Live CC showed the transport was otherwise healthy (srtt 4.6–26ms) — pure budget monopolization, not congestion.Fix — additive gossip headroom reservation:
pending_stream_send_bytes_cap32MB → 40MB;pending_priority_reserve_bytes= 8MB.pendingBytesCapForStream: 40MB for priority streams (the persistent gossip stream, marked via newmarkStreamPriorityAPI), 32MB for non-priority (req/resp — unchanged from the original budget, so NO req/resp regression).The reserve is additive (raise total) rather than carved out of the existing budget — a 24MB non-priority cap would have made 32MB single-write responses un-enqueueable (infinite retry → sync stall). Test covers: single ~32MB non-priority write succeeds from empty, further non-priority byte rejected, priority enqueue still succeeds from the reserve.
zquic 280 passed / 1 skipped. Pairs with zig-libp2p #293 (marks the persistent gossip stream priority). Repoint zig-libp2p's zquic dep at the v1.7.66 tag at release. No Claude attribution.