Repository navigation
perf(surrogate): store tags via native sets when the storer supports it - #851
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.
storeTag reread the whole SURROGATE_ value on every stored response, appended one cache key and rewrote it under a global mutex. With tag values growing without bound (~720 KB single reads observed in a production allocs profile), every stored response paid O(value size) in reads, allocations and writes. Use the storer's SetStorer implementation when available: additions become one deduplicating set operation with no read-back, purges read members directly and listings stream sets instead of going through MapKeys, which cannot see set values. Storers without SetStorer keep the previous string behavior. Give go-redis surrogate tags a one-year rolling lifetime instead of an infinite one so abandoned tags eventually expire, and bump the storages replace to the fork commit providing SetStorer.
8c22112 to
7d032fe
Compare
|
Further relevant fixes and adjustment to the design flaw have a dedicated fix in Per-handler registration — eviction goroutines are now deduplicated Lock was a no-op in-process / too short for long walks — in-process No oversized-value guard — The root cause — whole-blob mappings that grow per variant — is fixed |
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
|
This includes #828 |
|
@darkweak, I don't know how to manage the plugins system. Is souin in the plugin upgraded after a tag, so CI fails now but succeeds later or what? |
2 participants
The
go.modreplaces are temporaryProblem
Follow-up to # (OOM fixes).
storeTagperforms aread-modify-write of the entire
SURROGATE_<tag>value on every storedresponse, twice per tag, under a global mutex:
GET(~720 KB observed in theproduction allocs profile) → string append →
SETwith no expiration. Cost perstored response grows linearly with tag size, and tag size grows forever —
quadratic work over time, and the last unbounded growth path in the profile.
Changes
storeTaguses the storer'score.SetStorerwhen implemented (go-redis):one deduplicating set-add, no read-back, no giant string values.
purgeTagreads members viaGetSet.Liststreams tags viaWalkSets— native sets are invisible toMapKeys,which only reads string values, so the surrogate-keys API endpoint keeps
working.
SetStorerkeep the existing string behavior unchanged.infinite one, so abandoned tags eventually expire.
SetStorer(darkweak/storages#).
Testing
go test ./pkg/surrogate/... ./pkg/api/... ./pkg/middleware/...— green(surrogate provider tests cover the string fallback; set semantics are
covered by integration tests in the storages PR).
Rollout
core, andgo-redisin the xcaddy build) must point at the same storages commit.until a tag is migrated; after migration they cannot append to that tag
(missed purge member at worst — the affected keys still expire via their own
TTLs).
Expected impact
SURROGATE_*value sizes bounded by unique membership; dead tags expirewithin a year of last touch instead of never.