feat(go-redis): store mappings as one hash field per variant - #63
Merged
Merged
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.
Surrogate tags were stored as one comma-joined string per tag. Every stored response reread the whole value, appended one key and rewrote it, so tag values grew without bound (~720 KB single reads observed in a production allocs profile) and each write cost O(value size). Add a SetStorer optional interface to core and implement it with native Redis sets: SADD deduplicates members without reading the value back, SMEMBERS serves purges, and a SCAN-based WalkSets streams tags for listings. Legacy string values are migrated to sets transparently on first write and remain readable until then. A positive duration bounds the set lifetime without ever shortening a longer remaining one, so legacy unbounded tags become bounded too.
Mappings were one protobuf blob per base key, rewritten wholesale on every store: to add a variant the whole blob was read, unmarshalled, extended and written back. Blob size grows with variant cardinality, so hot keys with unbounded Vary values degrade into multi-MB (observed: 119 MB) values that every request and every eviction walk must materialize in memory. Store one hash field per varied key instead: adding a variant is a single HSET of a ~100 byte marshalled KeyIndex, no read-modify-write. Election reads the fields with HGETALL and elects from per-variant entries via the new core.MappingElectionEntries, sharing the election loop with the blob-based path used by other storers. Legacy blobs stay readable through the previous election path and are migrated to hashes on their next write. Blobs larger than the new core.MaxMappingSize bound are dropped undecoded on read and replaced on write: decoding a pathological value would defeat the purpose, and the response keys it references expire through their own TTLs. ListKeys now handles hash mappings first, since calling GET on them would raise WRONGTYPE and trigger a reconnect. Hash mappings need no eviction walking: fields are bounded by variant cardinality and the key expires through the existing TTL handling, so WalkMappings intentionally skips them.
Owner
|
@mohammed90 fix lint issue (mostly add |
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
darkweak
approved these changes
Sep 13, 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.
Problem
Mappings are stored as one protobuf blob per base key and rewritten wholesale on
every store: adding a variant reads the whole blob, unmarshals it, appends one
entry and writes it back. Blob size grows with variant cardinality, so keys with
high-cardinality
Varyvalues degrade into multi-MB blobs. Production numbersfrom a real deployment: 43.8M
IDX_*keys totaling 12.4 GB, individual valuesup to 119 MB — which every request for that key and every eviction walk must
materialize in memory (protobuf decode expands 3–5× over wire size).
Changes
MappingElectionrefactored around a sharedelectMappingloop;new
MappingElectionEntrieselects from per-variant entries (one marshalledKeyIndexeach); newMappingEntryencoder; exportedMaxMappingSize(1 MB) bound for legacy blob handling.
SetMultiLevel: oneHSETfield (~100 B) per varied key inside aMULTI/EXEC, TTL semantics unchanged (extend-never-shorten). Noread-modify-write remains — pathological values can no longer form.
GetMultiLevel:HGETALL+MappingElectionEntries.HGETALL(notHSCAN) is deliberate: field count per key is bounded by variantcardinality, which is small once blobs are gone.
a hash on their next write; blobs over
MaxMappingSizeare droppedundecoded (
UNLINKon read, replaced on write) — decoding a 119 MBvalue even once would defeat the purpose, and the response keys it
references expire via their own TTLs.
ListKeys: handles hash mappings first — aGETon a hash raisesWRONGTYPE, which previously triggered a reconnect loop.WalkMappingsintentionally skips hashes (MGETreturns nil for them):hash mappings are self-pruning via the key TTL and bounded field count, so
they need no eviction walking. As legacy data ages out, the eviction walk
converges to a no-op for Redis.
Compatibility & rollout
(
WRONGTYPEon theirSET) — worst case a missed mapping update for keyswhose responses expire via their own TTLs. Roll forward promptly; don't run
mixed for extended periods.
the souin eviction guard (perf(api): stream mapping eviction to fix OOM from whole-index loading souin#828), no offline migration needed.
Testing
go test ./go-redis/ ./core/against Redis 8.x — 18/18 green. New:TestRedis_MultiLevel_HashMapping: two variants → hash with 2 fields, freshresponse elected end-to-end.
TestRedis_MultiLevel_LegacyBlobMigration: blob written by the oldMappingUpdaterstill elects; next write converts to a hash preservingentries.
TestRedis_MultiLevel_OversizedLegacyBlob: >1 MB blob elects nothing, isdropped without decoding, and is replaced by a hash on the next write.