feat(go-redis): store surrogate tags as native sets - #62
Open
mohammed90 wants to merge 5 commits into
Open
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.
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
Surrogate tags are stored as a single comma-joined string per tag. Every stored
response rereads the whole value, appends one cache key and rewrites it. In a
production allocs profile this showed up as ~720 KB single
GETs on the hotstore path (
storeTag→Redis.Get), executed twice per tag per storedresponse. Tag values grow without bound (no expiration for go-redis), so both
the value size and the per-write cost increase forever — the last unbounded
memory-growth path left after #.
Changes
SetStorerinterface —AddToSet,GetSetandWalkSets— for storers that can represent a set of members natively.SetStorerwith native Redis sets:AddToSet=SADD(+ optionalEXPIRE) in aMULTI/EXEC: deduplicatesserver-side, no read-back of existing members.
GetSet=SMEMBERS.WalkSets= SCAN-based streaming with early stop, reusing the boundedbatch size from #.
converted to a native set transparently on their first write (
DEL+SADDof old+new members in one transaction).shortens a longer remaining one; legacy keys without expiration become
bounded on their next write.
Compatibility
SetStoreris optional; no existing interface changes.and atomic within a transaction.
previous string-based code cannot append to it (
WRONGTYPE); members arestill purgeable and the underlying cache keys expire via their own TTLs.
Testing
go test ./go-redis/ ./core/against Redis 8.x,golangci-lintclean. New:TestRedis_Sets: dedup across calls, TTL bounded by the given duration,shorter durations don't shorten the remaining lifetime.
TestRedis_Sets_LegacyStringMigration: legacy value readable, first writeconverts type to
set, merges members and bounds the TTL.TestRedis_WalkSets: visits only matching sets, early stop.