perf: port upstream memory budget — queue budgeting, contract stats, pool sizing#264
Merged
full-bars merged 9 commits intoJul 17, 2026
Merged
Conversation
full-bars
force-pushed
the
perf/upstream-memory-budget
branch
from
July 12, 2026 10:52
0fc397d to
912dfac
Compare
full-bars
force-pushed
the
perf/upstream-gopacket-removal
branch
from
July 12, 2026 21:49
82f9179 to
f6728b3
Compare
full-bars
force-pushed
the
perf/upstream-memory-budget
branch
3 times, most recently
from
July 13, 2026 00:27
18789ab to
8a4f47b
Compare
full-bars
force-pushed
the
perf/upstream-gopacket-removal
branch
2 times, most recently
from
July 13, 2026 10:39
4519357 to
069a964
Compare
…2, 3.3) Covers 7440e8b's fixes: - CheckpointContract now releases the client-side stats entry immediately (worker never started) or marks it closed for the epoch worker to reap (worker started) — the leak from 3.1. - ContractStatsEpoch<=0 is normalized to a safe default instead of spinning on time.After(0) — 3.3, plus a fire-count check over 200ms to distinguish a normalized epoch from a hot-spin. - An end-to-end two-client test drives 20 messages through a single receive-side contract, closes the sequence, and asserts the final contract-stats event's UsedByteCount exactly matches the ground truth ReceiveContractCloseByteCount from LocalStats — pins 3.2 (stats previously lagged by one ack, from being updated in updateContract's reservation step instead of receiveHead's ack step, permanently losing the last message's bytes on close). All three verified against the unfixed code (reverted 7440e8b's transfer.go/transfer_contract_manager.go diff locally, confirmed each test fails with the exact original failure mode, restored). Full suite (go test . -short -skip TestDohQuery) passes clean.
full-bars
force-pushed
the
perf/upstream-memory-budget
branch
from
July 13, 2026 10:41
e3f6007 to
8821a43
Compare
The previous commit ("prevent contract stats from closing on
checkpoints") guarded closeContractStats behind `if !checkpoint`,
copying the pattern used a few lines above for the unrelated
localStats.ContractOpenByteCounts/ContractOpenKeys bookkeeping. That
guard is correct there — the wire-level contract may still be reused
by a new sequence on checkpoint — but wrong for closeContractStats:
the per-contract stats entry's lifecycle is tied to this sequence's
Run() ending, not to the wire contract's lifecycle, so it must always
release here.
This reintroduced the exact leak TestContractStatsClosedOnCheckpoint
(finding 3.1) was written to catch, and broke
TestContractStatsReceiveSideNotLagging the same way — both now pass.
Per ReceiveSequence.Run's defer, checkpoint is the *common* path a
sequence takes on exit (only the previous, superseded contracts get a
hard CloseContract), so this leaked for most real contract lifecycles.
full-bars
added a commit
that referenced
this pull request
Jul 17, 2026
* chore: port upstream new infrastructure files (ee7a476 + 83dc999)
Adds 11 new files from upstream connect main (ahead of v2026.7.6-985989570):
New files — no behavioural changes to existing code:
- ip_assoc.go (1135L) — activity-based IP co-association matrix
- ip_block_action.go (570L) — block action overrides for security policy
- memory_budget.go (56L) — process-wide memory budget scaling
- transfer_memory_budget.go (74L) — shared byte budget for sequences
- transfer_contract_stats.go (164L) — per-contract usage event callbacks
- egress.go / egress_net.go / egress_other.go / egress_windows.go (181L) — egress interface binding
- transfer_peer_manager.go (166L) — peer tracking from control messages
- race_on_test.go / race_off_test.go — test helpers for race detection
Supporting additions to existing files:
- protocol/frame.pb.go, protocol/transfer.pb.go — regenerated for new message types
- transfer.go — Peer, NetworkPeer types
- util.go — memoryShedders, AddMemoryShedder, ShedMemory
- ip_remote_multi_client.go — ServerNameLookup interface, EventEpoch field + default
- transfer_contract_manager.go — SignStoredContract, VerifyStoredContract, NetworkEventTimeChangeHmac field, ContractStatsEpoch field, contract stats struct fields
New files compile against the fork's existing interfaces. Test files
that depend on not-yet-ported methods (block actions, packet stats,
IpAssoc wiring) are deferred to later PRs.
* chore: port upstream Peer API — ReceiveFunction type change + pause fix
Changes ReceiveFunction signature from `provideMode protocol.ProvideMode`
to `peer Peer` (upstream 83dc999). Updates all 7 source file signatures,
4 test file lambdas, and connectctl. Adds ProvideMode_Network to the
contract-manager pause filter so network peers don't fall back to stream
contracts when paused.
* perf: port upstream gopacket removal + IpAssoc wiring (ee7a476)
Replaces gopacket-based packet parsing/building with manual binary
encoding on all hot paths in ip.go. Adds packet stats, ipPath caching,
and lock-free multiClientConfig refactoring in ip_remote_multi_client.go.
- Removes github.com/google/gopacket dependency from ip.go
- Custom IPProtocol/UDPPort/TCPPort types + manual IPv4/IPv6/TCP/UDP
parse and write functions
- New fields: ipAssoc, blockAction state, packetStats on RemoteUserNatMultiClient
- Atomic config pointer replaces separate performanceProfile + localSecurityBypass fields
- Test file updated to use new types
* fix: don't tear down NAT shard on malformed packet, guard empty packets
runShard's bare `return` on parse failure exited the whole loop, firing
defer self.cancel() and killing the shard permanently on a single
malformed TCP/UDP segment from any client (remote DoS). Changed to
continue so only the bad packet is dropped.
Also add the len(ipPacket) == 0 guard in ParseIpPathWithPayload that
upstream's reference port includes, preventing an index-out-of-range
panic on an empty packet.
* fix: TCP checksum scope, option parsing, SynAck padding, pool-return gaps
Five issues found in independent review of the gopacket-removal port:
1. tcpPacket()/SynAck() computed the TCP checksum over the header only,
excluding payload/options. Every TCP data segment sent to clients
carried an invalid checksum and would be silently dropped by
standard client network stacks. Fixed to checksum the full segment,
matching the UDP path.
2. parseWindowScaleOpts didn't handle TCP option kinds 0 (EOL) and 1
(NOP), which have no length byte. Since real SYNs almost always
place a NOP before the Window Scale option, the parser desynced and
missed WS for nearly all real clients. Fixed to handle both.
3. SynAck() allocated an unpadded options buffer (3 bytes) while
writeTcpHeader's data-offset field rounds up to a 4-byte boundary
(24), producing a segment 1 byte shorter than its own header claims.
Receiving stacks reject this as truncated. Fixed to pad options to
a 4-byte boundary (zero-fill reads as EOL).
4. tcpSend's RST branch didn't return ipPacket to the message pool on
an existing-sequence teardown, unlike the sibling no-syn-drop branch.
Fixed to return it.
5. runShard's ipVersion switch had no default case, leaking the pool
buffer for any packet with a first nibble outside {4, 6}. Fixed to
add one, matching the existing per-protocol default branches.
* feat: extract ParseTcpWindowScaleOpts + comprehensive ip packet tests
- Extract the TCP window-scale option parser from TcpSequence.Run()'s
inline closure to a package-level exported function for testability.
- Add ip_packet_test.go with 33 tests covering:
- checksumAdd/checksumFinish (RFC 1071)
- transportChecksum (pseudo-header + segment)
- TCP checksum scope (payload, options)
- ConnectionState PureAck/RstAck packet construction
- parseIpv4/parseIpv6/parseUdpPacket/parseTcpPacket round trips
- ParseTcpWindowScaleOpts (nil, empty, WS, cap-at-14, NOP, EOL,
unknown options, multi-NOP)
- writeIpv4Header/writeIpv6Header/writeTcpHeader round trips
- flowHash (IPv4/IPv6 consistency, short packets)
- malformed packet guards (nil, empty, invalid data offset)
- SynAck data-offset/segment-length consistency
- UDP checksum non-zero invariant
- Fix TestChecksumAddAndFinish expected value.
* test: extend ip_packet_test.go to 55 wire-format tests
Adds unit-level checksumAdd/checksumFinish coverage, PureAck/RstAck
construction tests, parseIpv4/parseIpv6/parseUdpPacket/parseTcpPacket
round trips, writeIpv4Header/writeIpv6Header/writeTcpHeader round
trips, malformed/too-short input guards, empty-packet guard, and
flow-hash edge cases, on top of the checksum-scope and window-scale
option tests from the prior commit.
Fixed one flaky test in the process: TestWriteIpv4RoundTrip verified
the IPv4 checksum over the full allocated buffer (including 8 bytes
of unrelated UDP-header space past the 20-byte IP header), which only
holds when the message pool happens to hand back a zeroed buffer.
Pool buffers aren't guaranteed zeroed on reuse, so this failed
intermittently depending on prior test pool state. Scoped the
verification to the actual 20-byte IPv4 header, matching what
writeIpv4Header itself checksums. Verified with -shuffle=on across
multiple runs after the fix.
* fix: prevent MTU capacity panics and correct test checksum calculation sequence
* test: add MTU capacity guard regression tests (findings 2.1/2.2)
Covers c4065b4's fix directly: StreamState.DataPackets (UDP) and
ConnectionState.DataPackets (TCP) both now error on mtu <= header
size instead of divide-by-zero / negative-capacity panicking.
Verified against the unfixed code — reproduces the exact
"makeslice: cap out of range" panic from the audit report.
* perf: port upstream memory budget — queue budgeting, contract stats, pool sizing (#264)
* perf: port upstream memory budget — queue budgeting, contract stats, pool sizing
* fix: set ContractStatsEpoch default to 60s to prevent hot-spin
* fix: restore per-class pool ceiling sizing
* fix: contract stats memory leak, CPU loop, and lagging receive stats
* test: add regression tests for contract-stats audit findings (3.1, 3.2, 3.3)
Covers 7440e8b's fixes:
- CheckpointContract now releases the client-side stats entry
immediately (worker never started) or marks it closed for the
epoch worker to reap (worker started) — the leak from 3.1.
- ContractStatsEpoch<=0 is normalized to a safe default instead of
spinning on time.After(0) — 3.3, plus a fire-count check over
200ms to distinguish a normalized epoch from a hot-spin.
- An end-to-end two-client test drives 20 messages through a single
receive-side contract, closes the sequence, and asserts the final
contract-stats event's UsedByteCount exactly matches the ground
truth ReceiveContractCloseByteCount from LocalStats — pins 3.2
(stats previously lagged by one ack, from being updated in
updateContract's reservation step instead of receiveHead's ack
step, permanently losing the last message's bytes on close).
All three verified against the unfixed code (reverted 7440e8b's
transfer.go/transfer_contract_manager.go diff locally, confirmed each
test fails with the exact original failure mode, restored). Full
suite (go test . -short -skip TestDohQuery) passes clean.
* fix: restore 4 MiB ResendQueueMaxByteCount scaling base
* fix: ReceiveSequence nil map panic on evicted contracts
* fix: prevent contract stats from closing on checkpoints
* fix: closeContractStats must run on checkpoint, not just hard close
The previous commit ("prevent contract stats from closing on
checkpoints") guarded closeContractStats behind `if !checkpoint`,
copying the pattern used a few lines above for the unrelated
localStats.ContractOpenByteCounts/ContractOpenKeys bookkeeping. That
guard is correct there — the wire-level contract may still be reused
by a new sequence on checkpoint — but wrong for closeContractStats:
the per-contract stats entry's lifecycle is tied to this sequence's
Run() ending, not to the wire contract's lifecycle, so it must always
release here.
This reintroduced the exact leak TestContractStatsClosedOnCheckpoint
(finding 3.1) was written to catch, and broke
TestContractStatsReceiveSideNotLagging the same way — both now pass.
Per ReceiveSequence.Run's defer, checkpoint is the *common* path a
sequence takes on exit (only the previous, superseded contracts get a
hard CloseContract), so this leaked for most real contract lifecycles.
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.
Caution
Merge Blockers Resolved: This PR ports upstream memory budget changes which originally contained three critical regressions (an unconditional nil-map panic, an unbounded 100% CPU hot-spin on contract stats, and an 80% silent reduction in sharded message pool capacity). All are fixed in this stack.
Features
Stability
ContractStatsEpochdefault to 60s inDefaultContractManagerSettings(). Without this, the uninitialized0duration triggered an unbounded hot-spin (time.After(0)) inrunContractStats(), permanently pinning a core to 100% CPU.ResizeMessagePoolsPerClasswrapper inmessage_pool.goand updatesprovider/main.gocall sites. Restores the legacy per-class-ceiling pool allocation semantics. Bypasses the upstream division-by-pool-count logic which would have starved the proxy's 5-bucket sharded architecture under load and re-introduced heavy GC pressure.Bug Fixes
contractStatsEntriesandcontractStatsCallbacksinNewContractManager()to prevent an unconditional panic on the very first contract creation.What's Changed
8a4f47bca876900823ecaFull Changelog: perf/upstream-gopacket-removal...perf/upstream-memory-budget