feat(perf): in-process micro-cache on /get with singleflight stampede protection - #27
Merged
Merged
Conversation
… protection
The /get hot path sees 13.5M reads across ~2,800 unique keys lifetime
(~4,800 reads/key). A 250ms TTL micro-cache in front of Client.Get
collapses that to ~150 Redis fills/sec at worst — a 95-99% reduction
in Redis GET traffic on the dominant endpoint.
GetCache (utils/microcache.go):
- sync.RWMutex map + atomic size counter (O(1) Size())
- golang.org/x/sync/singleflight collapses concurrent fills for the
same key into ONE Redis call (cache stampede protection on TTL
expiry under high RPS — without this, every expiry would trigger
a thundering herd of duplicate fills)
- Negative caching: redis.Nil results are stored too, so bot probes
of nonexistent keys don't hit Redis on every probe
- Errors are NOT cached — transient Redis failures don't poison the
cache for the full TTL window
- Background sweeper evicts stale entries only when size > maxSize
- Stop()/InitGetCache pattern matches ExpireGate — no goroutine leak
on re-init
Wiring:
- routes.go: GetView and GetShieldView use utils.GetCacheV.Fetch
with RedisGetThrough as the fill func. EXPIRE TTL refresh still
fires on every successful read (gated by ExpireGate to ~1/hour).
- main.go: GET_CACHE_TTL (default 250ms) and GET_CACHE_MAX_ENTRIES
(default 100k) tunable via env. TTL<=0 disables.
Metrics (Prometheus):
- abacus_get_cache_hits_total / misses_total / evicted_total
- abacus_get_cache_size (gauge)
- Hit ratio = hits / (hits + misses); >0.9 means working
Multi-instance safe: each Fly machine owns its cache, no coordination.
Cross-instance staleness bounded by TTL (250ms is invisible for
counter semantics where readers only ever see slightly-older-than-
latest, never wrong).
Tests (-race clean, 11 cases):
- basic hit/miss on TTL window
- expiry triggers fresh fill
- negative caching (redis.Nil cached)
- 200 concurrent fills for same key collapse to exactly 1 fill call
- 100 distinct keys each get their own fill (no false collapsing)
- errors are not cached (next caller retries)
- sweeper evicts only when above cap AND entries are stale
- sweeper is a no-op when below cap
- ttl<=0 fully disables (pass-through to fill)
- InitGetCache replaces cleanly without leaking the old sweeper
- global GetCacheV is non-nil at package load
There was a problem hiding this comment.
Pull request overview
Adds an in-process micro-cache in front of the /get hot path to reduce Redis GET load, with singleflight-based stampede protection and Prometheus visibility. This fits the existing pattern of local, per-instance mitigation already used by the EXPIRE coalescer.
Changes:
- Introduces
utils.GetCache(TTL micro-cache + singleflight) with a global instance and init hook. - Wires
/getand/getshieldto fetch via the micro-cache and adds env tunables (GET_CACHE_TTL,GET_CACHE_MAX_ENTRIES). - Adds Prometheus metrics and a test suite covering caching, expiry, negative caching, concurrency, and sweeper behavior.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/prometheus.go | Registers new GetCache hit/miss/eviction/size metrics. |
| utils/microcache.go | Implements the in-process TTL micro-cache, sweeper, and Redis adapter; adds global instance + init. |
| utils/microcache_test.go | Adds unit tests for cache semantics and concurrency behavior. |
| routes.go | Routes /get and /getshield through the micro-cache instead of direct Redis GET. |
| main.go | Initializes the global cache from env vars and logs configuration. |
| go.mod | Adds golang.org/x/sync dependency (singleflight). |
| go.sum | Adds checksums for the new dependency. |
Comments suppressed due to low confidence (1)
routes.go:246
- The comment says the TTL refresh fires “after the response is committed”, but the goroutine is started before the handler writes the response body. Either move the goroutine launch after
c.JSON/c.JSONP(if that ordering matters) or adjust the comment so it doesn’t claim post-commit behavior.
// Fire the TTL refresh after the response is committed. The coalescer
// suppresses ~99% of these so most cache hits incur zero Redis traffic.
go func() {
if utils.ExpireGate.ShouldRefresh(dbKey) {
Client.Expire(context.Background(), dbKey, utils.BaseTTLPeriod)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…dering, test races Five issues from the PR review: - microcache.go: Fetch panicked on nil *GetCache because c.enabled deref'd before any nil-check. Now treats nil identically to disabled, matching Enabled()/Stop() behavior. nil-receiver test added. - microcache.go: store() updated c.size AFTER releasing the mutex, which let it diverge from len(entries) during concurrent sweeps (size could even go negative). size.Add now happens inside the same critical section as the map mutation. Same lock that sweep() holds when decrementing, so the two paths are fully serialized. - microcache.go: When the cache is disabled (ttl<=0) or nil, Fetch now increments Misses on the pass-through. abacus_get_cache_misses_total stays meaningful as 'Redis GETs going through this wrapper' during rollback (GET_CACHE_TTL=0) or before main() runs InitGetCache. Hits stays untouched so the hit-rate panel reads 0%. - routes.go: The comment claimed the EXPIRE goroutine fired 'after the response is committed', but it was spawned BEFORE c.JSON/c.JSONP. Moved the goroutine spawn to after the response write in both GetView and GetShieldView. Now the comment matches reality and the response-path goroutine returns to gin one statement sooner. - microcache_test.go: TestGetCache_ConcurrentFillsCollapseToOne and TestGetCache_DistinctKeysFillIndependently called require.NoError from spawned goroutines. require uses t.FailNow which is not safe to call off the main test goroutine. Errors now flow back through a channel and get asserted in the parent goroutine after wg.Wait().
Comment on lines
+96
to
+104
| // nil receiver is treated as disabled (Misses is also bumped on the | ||
| // disabled/nil pass-through so abacus_get_cache_misses_total stays | ||
| // meaningful as a Redis-call counter during rollback or before init). | ||
| func (c *GetCache) Fetch(key string, fill func() (string, bool, error)) (string, bool, error) { | ||
| if c == nil || !c.enabled { | ||
| if c != nil { | ||
| c.Misses.Add(1) | ||
| } | ||
| return fill() |
…eper test Two more Copilot review findings + one flakiness fix: - microcache.go: Fetch ignored the coalescing dimension of singleflight. When N goroutines hit the same key and one runs fill while N-1 wait for the shared result, the leader incremented Misses but the waiters incremented nothing. The hits/(hits+misses) ratio undercounted both sides — Hits were silently lost on every coalescing burst. Now uses a ranFill flag inside the closure to distinguish leader from waiter: leader bumps Misses, waiters bump Hits. Property pinned by extending TestGetCache_ConcurrentFillsCollapseToOne to assert Misses=1, Hits=N-1, Hits+Misses=N. - microcache.go: Doc comment claimed nil-receiver bumps Misses but the code only does so on the disabled-but-non-nil path. Comment rewritten to enumerate all four cases (hit / fill / coalesced / disabled / nil) explicitly so future readers know what each path records. - microcache_test.go: TestGetCache_SweeperBoundedAndStaleOnly was flaky under repeat (-count=20) because the background sweeper goroutine ticks at ttl=10ms and races with the test's explicit sweep() calls. Once the BG sweeper drops size below maxSize, the next sweep early- returns and any straggler entry survives, leaving size=1 instead of 0. Fix: stop the BG sweeper at the start of these tests so they fully own timing. The test is for the sweep() function, not the loop that drives it — stopping the loop isolates correctly. Same fix on TestGetCache_SweeperBelowCapNoOp.
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.
Summary
Largest remaining lever from the cascade-mitigation investigation. Today's data shows the workload is wildly cacheable:
A 250ms TTL micro-cache collapses that into ~150 Redis fills/sec at worst. Expected reduction in Redis GET traffic: 95-99%.
Why micro-cache, why 250ms, why singleflight
Why micro-cache: Each Fly instance keeps its own cache. No coordination, no cross-instance invalidation. Cross-instance staleness is bounded by TTL (250ms), which is invisible for counter semantics — readers only ever see slightly-older-than-latest values, never wrong ones. Two readers on different instances seeing different values isn't a new problem; that's distributed-system reality whether you cache or not.
Why 250ms: Aggressive enough that staleness is invisible (a counter that increments many times per second looks identical at 250ms resolution). Conservative enough that single-instance traffic at hot keys still hits cache. The TTL is the one knob — go shorter for fresher reads, longer for less Redis traffic.
GET_CACHE_TTLenv var allows tuning per-deploy.Why singleflight: When a cached entry expires under high RPS, every concurrent reader sees a miss simultaneously and races to refill. Without protection this is a "cache stampede" — N concurrent Redis calls for the same key.
singleflight.Group.Docollapses N concurrent fills into one Redis call with N-1 waiters. The testTestGetCache_ConcurrentFillsCollapseToOneproves this directly: 200 concurrent Fetch calls for one key, exactly 1 fill() invocation.Three design choices worth highlighting
Negative caching on
redis.Nil. Bot probes for nonexistent keys account for some of the 11%4xxrate on/get. Without negative caching each probe hits Redis at full RPS. With it, the cached "not found" answer is served from memory for the TTL window. TestTestGetCache_NegativeCachingWorksenforces this.Errors are NOT cached. A transient Redis failure (network blip, brownout) returns through Fetch but doesn't store anything in the cache. The next caller retries. This prevents a single bad moment from poisoning the cache for 250ms. Test
TestGetCache_ErrorsAreNotCachedenforces this.EXPIRE refresh still fires on cache hits. Each successful Fetch (cache hit or miss) still spawns the
ExpireGate.ShouldRefreshgoroutine. The gate suppresses ~99% of those calls. The remaining ~1% keeps the 6-month Redis TTL fresh even on hot keys that always hit cache.New metrics
abacus_get_cache_hits_totalabacus_get_cache_misses_totalabacus_get_cache_evicted_totalabacus_get_cache_sizeThe ratio
hits / (hits + misses)is the hit rate. Target after warm-up: >0.9.Env tunables
GET_CACHE_TTL250ms0disablesGET_CACHE_MAX_ENTRIES100000Tests (all
-raceclean)11 tests covering:
TestGetCache_HitOnSecondCall)TestGetCache_ExpiryTriggersRefill)redis.Nil(TestGetCache_NegativeCachingWorks)TestGetCache_ConcurrentFillsCollapseToOne)TestGetCache_DistinctKeysFillIndependently)TestGetCache_ErrorsAreNotCached)maxSizeAND only evicts stale (TestGetCache_SweeperBoundedAndStaleOnly)TestGetCache_SweeperBelowCapNoOp)ttl<=0is a clean pass-through (TestGetCache_DisabledPassesThrough)InitGetCachereplaces cleanly without leaking sweeper goroutines (TestInitGetCache_ReplacesCleanly)TestGetCacheV_NotNilByDefault)Full suite (excluding the pre-existing
TestStreamValueViewrace) passes.Expected impact in metrics
After deploy, watch the dashboard:
rate(abacus_get_cache_hits_total[5m]) / (rate(hits) + rate(misses))climbs above 0.9 within minutes.rate(abacus_redis_cmd_duration_seconds_count{cmd="get",pool="main"}[5m])drops by ~95-99%.go_goroutinesduring the next brownout stays bounded — fewer pending requests because most reads short-circuit to cache.rate(abacus_redis_pool_timeouts[5m])stays at 0 through the next brownout (withMaxRetries=1already capping per-request damage).Rollback
If anything misbehaves:
fly secrets set GET_CACHE_TTL=0 -a j-abacusdisables the cache instantly without a redeploy. Fetch becomes a pass-through to Redis.🤖 Generated with Claude Code