perf(go-redis): bound memory usage of the mapping index (streaming eviction, mapping TTL, smaller lz4 blocks) - #59
Open
mohammed90 wants to merge 4 commits into
Open
perf(go-redis): bound memory usage of the mapping index (streaming eviction, mapping TTL, smaller lz4 blocks)#59mohammed90 wants to merge 4 commits into
mohammed90 wants to merge 4 commits into
Conversation
Storer.MapKeys forces implementations to materialize every mapping key and value in a single map. On large deployments the mapping index can reach hundreds of MB, so any caller doing periodic maintenance pays that allocation on every run. MappingWalker is an optional interface that lets a storer stream mapping entries in bounded batches instead. Callers fall back to MapKeys when the storer doesn't implement it. Also expose Lz4WriterPool so downstream storers can reuse lz4 writers. Writers are safe to pool once Close has flushed the frame; readers must never be pooled this way because they escape through http.Response.Body. Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
MapKeys collected every key from SCAN into one slice, then issued a single MGET for all of them and copied every value into a map. With a large mapping index this materializes the entire index in memory at once (observed: 325 MB live from a single caller in a production heap profile). Implement MappingWalker with SCAN + MGET in batches of 100 keys and rewire MapKeys through it, so even the compatibility path no longer issues one unbounded MGET.
SetMultiLevel stored the mapping key with duration -1, which maps to KeepTTL: mapping keys never expired and grew by one entry per varied key forever. The index could only shrink via the eviction job, and its size was unbounded between runs. Give the mapping key a TTL of max(existing TTL, duration + stale) so it always outlives the longest-lived entry it references, never shortens an expiration owned by a longer-lived entry, and converts legacy unbounded keys to bounded ones on their next update. Also configure the lz4 writer with 64 KB blocks instead of the 4 MB default. Every compression and decompression churned 4 MB pooled blocks even for tiny payloads. Readers pick the block size up from the frame header, so old entries remain readable and new entries are cheap on both paths.
Contributor
Author
|
@darkweak, if you have the time, can you take a look? It'd be great if we can resolve this at the earliest to improve the ecosystem for Caddy and Souin. |
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
Caddy instances running Souin with the go-redis storage OOM under production
traffic. A heap/allocs profile from an affected instance (HeapAlloc 2.13 GB,
MaxRSS 2.22 GB, 20.9 GB allocated over ~3.5 min) shows three dominant causes,
all in the storage layer:
MapKeysmaterializes the whole mapping index. The mapping-eviction jobcalls
MapKeys(IDX_), which SCANs every mapping key and issues one MGETfor all of them. In the profile this single call stack holds 325 MB +
43 MB live (~83% of sampled live heap), and it reruns every eviction
interval (default 1 minute).
SetMultiLevelstores the mapping key withduration
-1(KeepTTL). The index grows by one entry per varied key,forever, so the spike in (1) grows without bound.
SetMultiLevelcompression and everyMappingElectiondecompression cycles 4 MB pooled blocks even for tinybodies — ~65% of all allocated bytes (~13.5 GB of 20.9 GB) in the profile.
Changes
MappingWalkerinterface — storers can stream mappingentries in bounded batches; callers fall back to
MapKeyswhen unimplemented.Also exposes
Lz4WriterPool(writer pooling only; see the doc comment for whyreaders must never be pooled).
WalkMappings(SCAN + MGET in batches of 100, earlystop supported);
MapKeysnow delegates to it.max(existing TTL, duration + stale)insteadof
-1. Never shortens an expiration owned by a longer-lived entry; legacyunbounded keys become bounded on their next update.
Block64Kb. Readers derive block size from theframe header, so previously stored entries remain readable.
Compatibility
MappingWalkeris optional,MapKeysbehavioris preserved (same results, bounded batches internally).
keys converge to bounded TTLs as they are updated or evicted.
Testing
go test ./go-redis/ ./core/against a real Redis 8.x — all green.TestRedis_WalkMappings: 250 keys (crosses batch boundaries), prefixstripping, early stop after the first entry.
TestRedis_SetMultiLevel_MappingTTL: TTL is set within the entry lifetime,extended by longer-lived entries, not shortened by shorter-lived ones, and
applied to legacy keys without expiration.
AI Disclosure
Copilot (Claude Fable 5) analyzed and developed the changes and the tests. I understand the reasoning and the code changes make sense. The changes aren't deployed yet due to how this repo couples with Souin repo.