Repository navigation
relay: keep a short batch's remainder; per-source cap to 4000/s - #61
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
#62's destID-sharded relay queue already gives a flow per-worker ordering, so this keeps only what #61 still adds on top of main: - The batch flush falls back per-packet on any short write (n < len(msgs)), not only on an error. WriteBatch reports no error when a platform without sendmmsg sends one message per call, so the rest of the batch was silently discarded; #61 measured 19 of 600 relayed packets lost that way. - maxRelaysPerSourcePerSecond 1000 -> 4000. The cap counts packets, and daemons are moving from ~4.2 KB stream datagrams (IP-fragmented) to ~1.2 KB ones, so 4000 keeps the same ceiling in bytes (~4 MB/s). - relayQueueFor extracted so the per-dest queue invariant has a test, plus the order test from #61 and the WSS test enable-before-serve race fix.
TeoSlayer
force-pushed
the
fix/relay-cap-for-small-segments
branch
from
October 1, 2026 23:20
94ca805 to
e200328
Compare
TeoSlayer
changed the title
TeoSlayer
added a commit
to pilot-protocol/pilotprotocol
that referenced
this pull request
Oct 5, 2026
…ork with them (#509) daemon: stream segments fit one packet; loss recovery and short writes that work with them A full stream segment was 4096 bytes: about 4.2 KB as a UDP datagram and three IP fragments on a 1500-byte path. NATs, firewalls and some virtual networks drop fragments, so on those paths handshakes, pings and messages under ~1.4 KB worked while every full segment was lost — larger replies never arrived and send-file failed. Segments are now at most SendSegmentSize (1152) bytes: a 1222-byte datagram direct, 1231 relayed, within the 1232 that fit IPv6's minimum MTU. Sender-side only; receivers accept up to MaxSegmentSize as before, so old and new nodes interoperate in both directions. Smaller segments exposed faults the large ones had hidden. They are on main today and are fixed here because the change is not safe without them. Window accounting - The peer's window, advertised in segments, is enforced in segments. Counting bytes only, short segments overran the peer's receive slots. - The congestion window's growth and floors stay in MaxSegmentSize units; only adjustments that stand for one segment leaving the network use SendSegmentSize. Scaling everything down made a lossy path markedly slower (5 MB at 30ms RTT, 0.5% loss: 17s against 10s). Loss recovery (on main, 2 of 5 5 MB transfers failed at 2% loss) - In fast recovery SACKed segments were left out of the amount in flight while the window also grew per duplicate ACK, counting each twice; the amount outstanding doubled every round trip. - The peer buffers at most MaxOOOBuf (128) out-of-order segments and drops the rest silently. The sender now keeps at most that many unacknowledged (MaxSegmentsOutstanding). - After a retransmission timeout each further missing segment waited for its own timeout, which doubles each time. A partial ACK in timeout recovery now retransmits the next missing segment at once. Short writes Nagle held a short segment until everything before it was ACKed and blocked the writer until it had gone out. Nothing could join a held write, a stream of short writes moved at one write per round trip, and because a client's IPC sends are handled in order, its other requests waited too. With 1152-byte segments every 4 KB write ends in a short tail. - The tail of a write of a segment or more leaves with it. - A write shorter than a segment waits only for an earlier short segment. - A held write does not block: it stays buffered, later writes join it, and flushHeldTail sends it when that segment is ACKed, after NagleTimeout at the latest, or CloseConnection sends it ahead of the FIN. SendMu keeps segments in write order whichever goroutine sends them. - A receiver ACKs a lone segment at once for the first QuickACKBudget (32) segments of a connection and again after each time its delayed-ACK timer has had to fire, so a peer that waits for an ACK per small write (any peer up to v1.14.1) is not stalled for the timer each time. Measured in a container lab (details in the pull request): 5 MB over a 30ms path, five runs each, before -> after 2% loss: 9.7-12.4s with 2 of 5 failed -> 10.3-11.8s, none failed 0.5% loss: 9.4-10.5s -> 7.3-7.6s no loss: 8.2-8.4s -> 7.6-7.7s 60 MB on one host: 4.8s -> 1.2s; into v1.13.5: 1.8s (v1.13.5 to itself 22.7s) with non-first fragments dropped at the receiver: messages over one packet and file transfers failed -> all delivered, no fragments sent Interop was checked both ways against v1.13.5, v1.14.1 and main. Two costs: a node that is not upgraded and echoes back what it reads now sends one 1152-byte piece per round trip (its own Nagle rule); and the current beacon reorders relayed packets and caps packets per source, which costs smaller segments more (5 MB relayed: 5.8s against 3.6s) until pilot-protocol/beacon#61 is deployed (1.8s). Twelve tests pinned the old behaviour and are updated: three segmentation tests (1152-byte segments), four fast-recovery tests (window grows by 1152 per duplicate ACK), the window-update wake test (two free slots, one taken by the segment in flight), the delayed-ACK scheduling test (starts with the quick-ACK budget spent), and three Nagle tests that asserted the writer blocks on a held write. Co-authored-by: Teo Calin <calinteodor@Teos-MacBook-Pro.local> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Rebased after #62
#62 (destID-sharded relay queues) shipped the per-flow ordering design this PR originally carried, so the branch was rebased onto
mainand now keeps only what is still missing there:sendmmsg(whereWriteBatchsends one message per call and reports no error), or a shortsendmmsg, silently dropped the rest of the batch. The fallback now triggers onn < len(msgs).TestRelayQueueIsStablePerDestAndSpreadsDests(dest→queue affinity and spread over workers) andTestRelayKeepsAFlowsPacketsInOrder(600 packets through a real beacon, in order) lock in #62s guarantees.relayQueueForextracted from the inlinerelayChs[destID%N]lookup so the invariant is testable.EnableCompatWSSdocuments; the race detector flagged the old order intermittently.Deploy note (from the original PR): watch
relay statsdroppedafter rollout, and note the raised cap also raises what one abusive source can push by the same factor.Testing
go build ./...,go vet ./...clean.go test -race -parallel 4 ./...green.