Repository navigation
perf(reload): reclaim predecessor memory during handoff - #479
Merged
Merged
Conversation
added 14 commits
November 3, 2026 09:14
Sync the decoder copy with the receiver: reserve an upper bound for an uncompressed snapshot instead of growing it while many connections hand over their state together.
During a SIGUSR2 replacement the old and new relay processes run side by side. With large stream dictionaries the predecessor keeps tens of GB of pages it no longer needs while the successor restores state, so the pair can exceed a memory limit; an OOM kill at that point loses queued records. Scavenge before fork/exec and after successor readiness. Fence each successful transfer on reader cleanup, then batch reclamation using released reader reservations. Keep GC configuration, wire state, connections, routing and queued-record drain unchanged. The batch size is heap/8, clamped to 8-256 MiB; heaps below 64 MiB skip collection. In a scaled local test (85 stream inputs, plaintext/zstd, 33 output sockets and 672k shared output names), combined reload RSS falls from 889.6 to 615.1 MB, or 1.69x to 1.20x steady RSS. All 11,379,285 records arrive exactly once at each of three sinks with no dictionary resets. A smaller control is 1.83x to 1.32x, exposing the fixed-workspace floor. These are scaled measurements, not a whole-process RSS guarantee; keep a release RSS guard, especially for the first upgrade whose predecessor does not contain this reclamation policy. Add batching, cleanup-fence and large-dictionary release checks. Co-Authored-By: Codex <noreply@openai.com>
Waiting for c.done after every accepted-socket transfer serializes reload behind deferred BatchChan.Close. Under TCPMainQueueBytes backpressure, its final Flush can wait for both budget admission and queue space. This hurts most on relays with tens of thousands of plaintext connections, whose releaseBytes is zero. Observe cleanup asynchronously only for nonzero reader state. A single collector consumes completion notifications; neither waiting for c.done nor reporting to the collector runs on the transfer goroutine. End the state exchange without a cleanup or collector join. Keep pre-fork and post-readiness collection, existing parsed-record drain and input state. Replace the 256 MiB collection cap with HeapAlloc/16 (minimum 8 MiB), at most five batch collections and one final collection. Including the two initial calls, a replacement makes at most eight explicit calls. For a 20-25 GB heap and 5 GB of reader state this models about 6-8 calls, rather than roughly 19 with 85 similarly sized readers under the old cap. The regression holds a reader in actual BatchChan.Close with either a full budget or full queue, transfers the following sockets, then verifies its record and state release after unblocking. Zero-state readers do not hold the collector open. Restoring the old wait makes this test fail. A scaled local case delivers 11,379,285 exact records at each of three replicas, with 87 successful input transfers and no output resets; peak to steady RSS is 1.22x/1.07x with eight explicit collections. Co-Authored-By: Codex <noreply@openai.com>
timtofan
requested review from
Joypaul11,
deniszh,
emadolsky and
grzkv
as code owners
November 4, 2026 12:12
deniszh
approved these changes
Nov 4, 2026
deniszh
approved these changes
Nov 4, 2026
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
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.
Depends on #478
Why
A SIGUSR2 reload runs the old and new relay side by side while accepted connections, stream dictionaries and queued records move to the successor. On relays holding tens of GB of stream dictionaries, the predecessor keeps pages it no longer needs while the successor restores state, so the overlap can reach roughly twice steady RSS. When that exceeds a memory limit the kernel kills a process mid-handoff and queued records are lost.
A second problem: the handoff waited for every transferred reader to finish its final flush. Under main-queue byte backpressure that flush waits for budget and queue space, so on relays with tens of thousands of plaintext connections the whole reload serialized behind it.
Architecture
Before / after
Usage
No configuration change. Keep a release RSS guard for the first upgrade, because the predecessor of that reload does not have this reclamation yet; later reloads have it on both sides.
Validation
Go 1.27.0: build, vet and full
go test ./...pass on this branch. New tests cover batching, the cleanup fence, large-dictionary release, and a regression that holds a reader insideBatchChan.Closewith a full budget or a full queue, transfers the following sockets, then checks its records and state are released after unblocking; restoring the old wait makes it fail. The same commits have run in production reloads of large stream relays: connections and dictionaries handed over with no restarts and no write errors.