Repository navigation
perf(api): stream mapping eviction to fix OOM from whole-index loading - #828
Closed
mohammed90 wants to merge 2 commits into
Closed
mohammed90 wants to merge 2 commits into
mohammed90 wants to merge 2 commits into
Conversation
EvictMapping fetched the entire mapping index via MapKeys before processing it. With the go-redis storage this materialized hundreds of MB per run (325 MB live in a production heap profile) on every eviction interval, which is the primary driver of the OOM crashes. Process mappings one entry at a time through the storer's MappingWalker when available, falling back to MapKeys for storers that don't implement it. Peak memory for an eviction run drops from O(index size) to O(batch). Temporarily replace darkweak/storages/core with the fork commit that provides MappingWalker, bounded mapping key TTLs and 64 KB lz4 blocks, so canary builds can validate the fix. The replace also moves off the perf/core/reduce-overhead pseudo-version, which pools lz4/bufio readers that escape through http.Response.Body and can serve truncated responses under concurrency. Swap the replace for a tagged upstream release once darkweak/storages merges the changes.
✅ Deploy Preview for teal-sprinkles-4c7f14 canceled.
|
Production goroutine profiles showed one eviction goroutine per configured cache handler for the same storer: eviction registration runs in the handler constructor, which executes once per handler and again on config reloads. All of them walked the same mapping index concurrently, multiplying its memory cost. Register at most one eviction goroutine per storer identity for the process lifetime. Extend the eviction lock expiry on every ownership check and keep extending it while a walk runs, so walks longer than the lock TTL don't let other replicas join mid-walk. Previously the in-process holder comparison also short-circuited all local goroutines to "owned", making the lock a no-op inside one process. Drop mapping values larger than 1 MB during eviction without decoding them: unmarshalling a pathological value materializes it in memory, and the response keys it references expire through their own TTLs.
✅ Deploy Preview for teal-sprinkles-4c7f14 canceled.
|
Contributor
Author
|
included in #851 |
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.
Problem
Production Caddy instances OOM after deploying the latest build. Heap profiling
shows
EvictMapping→MapKeysholding 325 MB + 43 MB live at snapshottime — the eviction job loads the entire Redis mapping index into memory every
interval (default: 1 minute).
Changes
EvictMappingnow streams the mapping index through the storer'score.MappingWalker(added in perf(go-redis): bound memory usage of the mapping index (streaming eviction, mapping TTL, smaller lz4 blocks) storages#59) and processesone mapping at a time. Storers without
WalkMappingskeep the previousMapKeyspath, unchanged.darkweak/storages/*to (TODO)The previous pin (
core v0.0.20-0.20260314133624-4df176921261) pointed at theperf/core/reduce-overheadbranch, which returns pooled lz4/bufio readers totheir pools before
http.Response.Bodyis consumed — the same use-after-putrace class fixed in storages v0.0.19. The bump moves off that branch; do not
re-pin to it.
Testing
go test ./pkg/api/... ./pkg/middleware/...— green.Expected impact
Rollout notes
GOMEMLIMIT(~80% of container limit) and reviewingmapping_eviction_intervalandmax_cacheable_body_bytes.