From 5153e9d84f1f4847b30c41a60e6a7fe50113ec1c Mon Sep 17 00:00:00 2001 From: Mohamad Rostami Date: Tue, 11 Aug 2026 17:49:06 +0200 Subject: [PATCH 1/6] fix single-flight cancellation Waiters in fetchOnce blocked on wg.Wait() with no way to honor their own context, and the shared fetch ran on the initiating caller's context, so one caller giving up canceled the fetch and poisoned the result for every waiter. The shared fetch now runs in its own goroutine on a cancel-free copy of the initiator's context (values kept for tracing, cancellation dropped), completion is signaled by closing a channel, and every caller selects on that against its own ctx.Done(). A caller that gives up gets ctx.Err(); the fetch still completes and its result is cached for the others. context.WithoutCancel requires go 1.21, so the module floor moves up from 1.18. Co-Authored-By: Claude Fable 5 --- go.mod | 2 +- lastcache.go | 42 ++++++++++++++---------- lastcache_test.go | 81 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index 8c3f204..a298678 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/mbrostami/lastcache/v2 -go 1.18 +go 1.21 diff --git a/lastcache.go b/lastcache.go index 5e1a2ee..8d7e4d8 100644 --- a/lastcache.go +++ b/lastcache.go @@ -67,9 +67,9 @@ type item[V any] struct { } type call[V any] struct { - wg sync.WaitGroup - val V - err error + done chan struct{} + val V + err error } // Cache is a generic, concurrency-safe cache. Use New to construct one; it must @@ -196,23 +196,33 @@ func (c *Cache[K, V]) Range(f func(key K, value V, ttl time.Duration) bool) { // fetchOnce runs fetch for key, collapsing concurrent calls for the same key // into a single fetch and caching a successful result. +// +// The shared fetch runs on a cancel-free copy of the initiating caller's +// context, so one caller giving up does not abort the fetch for the others. +// Each caller (the initiator included) waits with its own context and returns +// ctx.Err() if that is done first; the fetch still completes and is cached. func (c *Cache[K, V]) fetchOnce(ctx context.Context, key K, fetch Fetch[K, V]) (V, error) { - cl := &call[V]{} - cl.wg.Add(1) - actual, loaded := c.inflight.LoadOrStore(key, cl) - if loaded { - existing := actual.(*call[V]) - existing.wg.Wait() - return existing.val, existing.err + cl := &call[V]{done: make(chan struct{})} + if actual, loaded := c.inflight.LoadOrStore(key, cl); loaded { + cl = actual.(*call[V]) + } else { + go func() { + cl.val, cl.err = fetch(context.WithoutCancel(ctx), key) + if cl.err == nil { + c.Set(key, cl.val) + } + c.inflight.Delete(key) + close(cl.done) + }() } - cl.val, cl.err = fetch(ctx, key) - if cl.err == nil { - c.Set(key, cl.val) + select { + case <-cl.done: + return cl.val, cl.err + case <-ctx.Done(): + var zero V + return zero, ctx.Err() } - c.inflight.Delete(key) - cl.wg.Done() - return cl.val, cl.err } // triggerRefresh starts at most one background refresh per key, bounded across diff --git a/lastcache_test.go b/lastcache_test.go index 6bf0eb7..c652c10 100644 --- a/lastcache_test.go +++ b/lastcache_test.go @@ -139,6 +139,87 @@ func TestGet_SingleFlight(t *testing.T) { } } +// A waiter whose context is canceled gets ctx.Err() immediately instead of +// blocking until the shared fetch finishes. +func TestGet_WaiterHonorsOwnContext(t *testing.T) { + c := New[string, string](Config{TTL: time.Minute}) + + release := make(chan struct{}) + started := make(chan struct{}) + fetch := func(context.Context, string) (string, error) { + close(started) + <-release + return "v", nil + } + + // Initiate the shared fetch and keep it blocked. + leaderDone := make(chan struct{}) + go func() { + defer close(leaderDone) + if v, err := c.Get(context.Background(), "k", fetch); err != nil || v != "v" { + t.Errorf("initiator got (%v,%v), want (v,nil)", v, err) + } + }() + <-started + + // A second caller joins the in-flight fetch but cancels while waiting. + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := c.Get(ctx, "k", fetch); !errors.Is(err, context.Canceled) { + t.Fatalf("canceled waiter got %v, want context.Canceled", err) + } + + close(release) + <-leaderDone +} + +// Canceling the context of the caller that initiated the shared fetch must not +// poison the result for the other callers: the fetch runs on a cancel-free +// context and its result is still cached. +func TestGet_InitiatorCancelDoesNotAbortSharedFetch(t *testing.T) { + c := New[string, string](Config{TTL: time.Minute}) + + release := make(chan struct{}) + started := make(chan struct{}) + fetch := func(ctx context.Context, _ string) (string, error) { + close(started) + <-release + // The fetch context must outlive the initiator's cancellation. + if err := ctx.Err(); err != nil { + return "", err + } + return "v", nil + } + + ctx, cancel := context.WithCancel(context.Background()) + initiatorDone := make(chan struct{}) + go func() { + defer close(initiatorDone) + if _, err := c.Get(ctx, "k", fetch); !errors.Is(err, context.Canceled) { + t.Errorf("initiator got %v, want context.Canceled", err) + } + }() + <-started + + // A second caller joins, then the initiator gives up. + waiterDone := make(chan struct{}) + go func() { + defer close(waiterDone) + if v, err := c.Get(context.Background(), "k", fetch); err != nil || v != "v" { + t.Errorf("waiter got (%v,%v), want (v,nil)", v, err) + } + }() + cancel() + <-initiatorDone + + close(release) + <-waiterDone + + if v, ok := c.load("k"); !ok || v.value != "v" { + t.Fatalf("fetch result was not cached, got (%v,%v)", v.value, ok) + } +} + func TestGetAsync_ColdMiss_Sync(t *testing.T) { c := New[string, string](Config{TTL: time.Minute}) res := c.GetAsync(context.Background(), "k", func(context.Context, string) (string, error) { From e03e2abb39a881fe336e8cefc2bbbb724a91c8ea Mon Sep 17 00:00:00 2001 From: Mohamad Rostami Date: Tue, 11 Aug 2026 17:50:51 +0200 Subject: [PATCH 2/6] make StaleTTL a hard cap anchored at the last successful fetch Previously a failed refresh re-put the stale value with a fresh StaleTTL, so every failure pushed the expiry out again: under a long outage staleness was unbounded, and the re-put could overwrite a value a concurrent successful fetch had just stored. Items now record fetchedAt instead of a mutable expiry. A value is fresh until fetchedAt+TTL and may be served stale until fetchedAt+TTL+StaleTTL; past that it is dead and the next lookup fetches synchronously, GetAsync included. Failed refreshes no longer touch the stored item, so the cap never moves and the overwrite race is gone. "Serve stale for at most StaleTTL" is now a guarantee an operator can state. StaleTTL == 0 keeps its existing meaning: Get never serves stale, and GetAsync's serve-while-refreshing staleness stays unbounded. Co-Authored-By: Claude Fable 5 --- lastcache.go | 56 ++++++++++++++++++++++++++++++++--------------- lastcache_test.go | 46 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 18 deletions(-) diff --git a/lastcache.go b/lastcache.go index 8d7e4d8..00727f0 100644 --- a/lastcache.go +++ b/lastcache.go @@ -4,7 +4,9 @@ // // stale-if-error (Get / GetStale) // When a fetch fails and a previous value is still around, the cache serves -// that stale value for up to Config.StaleTTL instead of returning the error. +// that stale value instead of returning the error, for at most +// Config.StaleTTL past expiry. The cap is anchored at the last successful +// fetch: no amount of failed refreshes extends it. // // stale-while-revalidate (GetAsync) // An expired value is returned immediately while a single background goroutine @@ -29,8 +31,14 @@ type Config struct { TTL time.Duration // StaleTTL is how long a stale value may be served after expiry when a - // refresh fails (stale-if-error). 0 disables serving stale: every expired - // Get then re-runs the fetch until it succeeds. + // refresh fails (stale-if-error). It is a hard wall-clock cap anchored at + // the last successful fetch: past fetchedAt+TTL+StaleTTL the value is + // treated as gone, however many refreshes failed in between, and the next + // lookup fetches synchronously (GetAsync included). + // + // 0 disables serving stale on error: every expired Get re-runs the fetch + // until it succeeds. GetAsync then keeps its serve-stale-while-refreshing + // behavior without a staleness bound. StaleTTL time.Duration // MaxConcurrentRefresh bounds the number of background refreshes running at @@ -62,8 +70,8 @@ type Result[V any] struct { } type item[V any] struct { - value V - expiry time.Time + value V + fetchedAt time.Time // when the value was last fetched successfully (or Set) } type call[V any] struct { @@ -129,11 +137,11 @@ func (c *Cache[K, V]) GetStale(ctx context.Context, key K, fetch Fetch[K, V]) (R return Result[V]{Value: val}, nil } - // Fetch failed: serve the last known value if we still have one. + // Fetch failed: serve the last known value while it is within the hard + // staleness cap. The item is left untouched, so the cap never moves and a + // concurrent successful refresh is never overwritten with a stale copy. if c.config.StaleTTL > 0 { - if it, ok := c.load(key); ok { - // Push the expiry out so a failing upstream isn't hammered. - c.put(key, it.value, c.config.StaleTTL) + if it, ok := c.load(key); ok && !c.dead(it) { return Result[V]{Value: it.value, Stale: true, Err: err}, nil } } @@ -144,11 +152,12 @@ func (c *Cache[K, V]) GetStale(ctx context.Context, key K, fetch Fetch[K, V]) (R // GetAsync returns the current value immediately. If it is expired, the stale // value is returned (Result.Stale == true) and a single background goroutine -// refreshes the key. On a cold miss the fetch runs synchronously; if it fails, -// Result.Err is set. Background refresh errors are reported via Config.OnError. +// refreshes the key. On a cold miss — or once a value is past the hard +// staleness cap — the fetch runs synchronously; if it fails, Result.Err is +// set. Background refresh errors are reported via Config.OnError. func (c *Cache[K, V]) GetAsync(ctx context.Context, key K, fetch Fetch[K, V]) Result[V] { it, ok := c.load(key) - if !ok { + if !ok || c.dead(it) { val, err := c.fetchOnce(ctx, key, fetch) if err != nil { c.onError(key, err) @@ -167,7 +176,7 @@ func (c *Cache[K, V]) GetAsync(ctx context.Context, key K, fetch Fetch[K, V]) Re // Set stores value for key with the configured TTL. func (c *Cache[K, V]) Set(key K, value V) { - c.put(key, value, c.config.TTL) + c.put(key, value) } // Delete removes key from the cache. @@ -179,7 +188,7 @@ func (c *Cache[K, V]) Delete(key K) { // item is expired; zero means the key is not present. func (c *Cache[K, V]) TTL(key K) time.Duration { if it, ok := c.load(key); ok { - return it.expiry.Sub(c.clock()) + return c.expiry(it).Sub(c.clock()) } return 0 } @@ -190,7 +199,7 @@ func (c *Cache[K, V]) TTL(key K) time.Duration { func (c *Cache[K, V]) Range(f func(key K, value V, ttl time.Duration) bool) { c.store.Range(func(k, v any) bool { it := v.(item[V]) - return f(k.(K), it.value, it.expiry.Sub(c.clock())) + return f(k.(K), it.value, c.expiry(it).Sub(c.clock())) }) } @@ -261,12 +270,23 @@ func (c *Cache[K, V]) load(key K) (item[V], bool) { return v.(item[V]), true } -func (c *Cache[K, V]) put(key K, value V, ttl time.Duration) { - c.store.Store(key, item[V]{value: value, expiry: c.clock().Add(ttl)}) +func (c *Cache[K, V]) put(key K, value V) { + c.store.Store(key, item[V]{value: value, fetchedAt: c.clock()}) +} + +func (c *Cache[K, V]) expiry(it item[V]) time.Time { + return it.fetchedAt.Add(c.config.TTL) } func (c *Cache[K, V]) expired(it item[V]) bool { - return c.clock().After(it.expiry) + return c.clock().After(c.expiry(it)) +} + +// dead reports whether it is past the hard staleness cap and may no longer be +// served, even stale. With StaleTTL == 0 nothing is ever dead: Get already +// refuses to serve stale, and GetAsync's staleness is deliberately unbounded. +func (c *Cache[K, V]) dead(it item[V]) bool { + return c.config.StaleTTL > 0 && c.clock().After(c.expiry(it).Add(c.config.StaleTTL)) } func (c *Cache[K, V]) onError(key K, err error) { diff --git a/lastcache_test.go b/lastcache_test.go index c652c10..e51e77b 100644 --- a/lastcache_test.go +++ b/lastcache_test.go @@ -112,6 +112,52 @@ func TestGetStale_NoStaleTTL_ReturnsError(t *testing.T) { } } +// The staleness cap is a hard wall-clock bound from the last successful fetch: +// repeated failed refreshes do not extend it, and past it the error is +// returned instead of the stale value. +func TestGetStale_HardCap(t *testing.T) { + now := fixedTime + c := New[string, string](Config{TTL: time.Second, StaleTTL: 10 * time.Second}) + withClock(c, &now) + c.Set("k", "stored") + failing := func(context.Context, string) (string, error) { + return "", errors.New("upstream down") + } + + // Expired but within the cap: served stale, repeatedly. + now = now.Add(5 * time.Second) + for i := 0; i < 3; i++ { + res, err := c.GetStale(context.Background(), "k", failing) + if err != nil || res.Value != "stored" || !res.Stale { + t.Fatalf("within cap: got (%+v,%v), want stale 'stored'", res, err) + } + } + + // Past fetchedAt+TTL+StaleTTL: the failed refreshes above must not have + // pushed the cap out. + now = fixedTime.Add(12 * time.Second) + if _, err := c.GetStale(context.Background(), "k", failing); err == nil { + t.Fatal("past the hard cap a failing fetch must return the error") + } +} + +// GetAsync stops serving a value past the hard cap and falls back to a +// synchronous fetch, like a cold miss. +func TestGetAsync_HardCap(t *testing.T) { + now := fixedTime + c := New[string, string](Config{TTL: time.Second, StaleTTL: 10 * time.Second}) + withClock(c, &now) + c.Set("k", "stored") + + now = now.Add(12 * time.Second) // past the cap + res := c.GetAsync(context.Background(), "k", func(context.Context, string) (string, error) { + return "new", nil + }) + if res.Value != "new" || res.Stale || res.Err != nil { + t.Fatalf("got %+v, want fresh 'new' fetched synchronously", res) + } +} + // The core fix: concurrent requests for the same expired key share one fetch. func TestGet_SingleFlight(t *testing.T) { now := fixedTime From 2c8fe9d5f57a87222f8ba0e0c97416e4a684ebb4 Mon Sep 17 00:00:00 2001 From: Mohamad Rostami Date: Tue, 11 Aug 2026 17:52:59 +0200 Subject: [PATCH 3/6] add optional Capacity bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The store was an unbounded sync.Map, which rules the cache out for attacker-influenced key spaces (caching by API key or request path lets a client grow memory without limit). Config.Capacity (<= 0 keeps today's unbounded behavior) now bounds the entry count; on overflow dead entries — past the staleness cap — are evicted first, then arbitrary ones. Enforcing a size atomically needs a size-aware store, so the value store moves from sync.Map to an RWMutex-guarded map; the read path stays a single RLock'd map lookup. Range now iterates a snapshot, so its callback may safely mutate the cache. The inflight/refreshing bookkeeping is untouched. Co-Authored-By: Claude Fable 5 --- README.md | 7 +++-- lastcache.go | 75 +++++++++++++++++++++++++++++++++++++---------- lastcache_test.go | 52 ++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index bd93ec2..87710a4 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,9 @@ go get github.com/mbrostami/lastcache/v2 ### stale-if-error (`Get` / `GetStale`) When a fetch fails and a previous value is still around, the cache serves that -stale value for up to `Config.StaleTTL` instead of returning the error. +stale value instead of returning the error — for at most `Config.StaleTTL` +past expiry. The cap is a hard wall-clock bound anchored at the last +*successful* fetch: failed refreshes never extend it. ### stale-while-revalidate (`GetAsync`) An expired value is returned immediately while a single background goroutine @@ -88,8 +90,9 @@ type Result[V any] struct { | Field | Meaning | |-------|---------| | `TTL` | How long a fetched value stays fresh. Defaults to 1 minute. | -| `StaleTTL` | How long a stale value may be served after expiry when a refresh fails. `0` disables serving stale. | +| `StaleTTL` | How long a stale value may be served after expiry when a refresh fails. A hard cap anchored at the last successful fetch; past it the value is treated as gone. `0` disables serving stale on error. | | `MaxConcurrentRefresh` | Caps concurrent background refreshes (`GetAsync`) across all keys. Defaults to `1`. | +| `Capacity` | Bounds the number of entries; dead entries are evicted first, then arbitrary ones. `<= 0` means unbounded. Set it whenever keys come from user input. | | `OnError` | Optional `func(key any, err error)` called when a background refresh fails. | | `Context` | Base context for background refreshes (they outlive the request). Defaults to `context.Background()`. | diff --git a/lastcache.go b/lastcache.go index 00727f0..fdb7486 100644 --- a/lastcache.go +++ b/lastcache.go @@ -45,6 +45,13 @@ type Config struct { // once across all keys (GetAsync). Values <= 0 use 1. MaxConcurrentRefresh int + // Capacity bounds the number of entries; values <= 0 mean unbounded. + // When the cache is full, dead entries (past the staleness cap) are + // evicted first, then arbitrary ones. Set a capacity whenever keys come + // from outside (request paths, API keys, user input) so the cache cannot + // grow without limit. + Capacity int + // OnError, if set, is called with the underlying error when a background // refresh (GetAsync) fails. Foreground errors are returned to the caller. OnError func(key any, err error) @@ -86,7 +93,8 @@ type Cache[K comparable, V any] struct { config Config clock func() time.Time baseCtx context.Context - store sync.Map // K -> item[V] + mu sync.RWMutex + entries map[K]item[V] inflight sync.Map // K -> *call[V], for single-flight foreground fetches refreshing sync.Map // K -> struct{}, one background refresh per key semaphore chan struct{} @@ -112,6 +120,7 @@ func New[K comparable, V any](config Config) *Cache[K, V] { config: config, clock: time.Now, baseCtx: base, + entries: make(map[K]item[V]), semaphore: make(chan struct{}, sem), } } @@ -181,7 +190,9 @@ func (c *Cache[K, V]) Set(key K, value V) { // Delete removes key from the cache. func (c *Cache[K, V]) Delete(key K) { - c.store.Delete(key) + c.mu.Lock() + delete(c.entries, key) + c.mu.Unlock() } // TTL returns the remaining time before key expires. A negative value means the @@ -194,13 +205,23 @@ func (c *Cache[K, V]) TTL(key K) time.Duration { } // Range calls f for each key with its value and remaining TTL. Iteration stops -// if f returns false. It follows sync.Map.Range semantics (no consistent -// snapshot). +// if f returns false. It iterates over a snapshot taken when Range is called, +// so f may safely mutate the cache; mutations are not reflected in the +// iteration. func (c *Cache[K, V]) Range(f func(key K, value V, ttl time.Duration) bool) { - c.store.Range(func(k, v any) bool { - it := v.(item[V]) - return f(k.(K), it.value, c.expiry(it).Sub(c.clock())) - }) + c.mu.RLock() + snapshot := make(map[K]item[V], len(c.entries)) + for k, it := range c.entries { + snapshot[k] = it + } + c.mu.RUnlock() + + now := c.clock() + for k, it := range snapshot { + if !f(k, it.value, c.expiry(it).Sub(now)) { + return + } + } } // fetchOnce runs fetch for key, collapsing concurrent calls for the same key @@ -262,16 +283,40 @@ func (c *Cache[K, V]) triggerRefresh(key K, fetch Fetch[K, V]) { } func (c *Cache[K, V]) load(key K) (item[V], bool) { - v, ok := c.store.Load(key) - if !ok { - var zero item[V] - return zero, false - } - return v.(item[V]), true + c.mu.RLock() + it, ok := c.entries[key] + c.mu.RUnlock() + return it, ok } func (c *Cache[K, V]) put(key K, value V) { - c.store.Store(key, item[V]{value: value, fetchedAt: c.clock()}) + now := c.clock() + c.mu.Lock() + c.entries[key] = item[V]{value: value, fetchedAt: now} + c.evictLocked() + c.mu.Unlock() +} + +// evictLocked bounds the cache to Capacity, dropping dead entries first and +// then arbitrary ones. Must hold c.mu. +func (c *Cache[K, V]) evictLocked() { + if c.config.Capacity <= 0 || len(c.entries) <= c.config.Capacity { + return + } + for k, it := range c.entries { + if len(c.entries) <= c.config.Capacity { + return + } + if c.dead(it) { + delete(c.entries, k) + } + } + for k := range c.entries { + if len(c.entries) <= c.config.Capacity { + return + } + delete(c.entries, k) + } } func (c *Cache[K, V]) expiry(it item[V]) time.Time { diff --git a/lastcache_test.go b/lastcache_test.go index e51e77b..98d737f 100644 --- a/lastcache_test.go +++ b/lastcache_test.go @@ -504,6 +504,58 @@ func TestGetAsync_SkipsRefreshIfAlreadyFresh(t *testing.T) { } } +// len reports the current number of entries (test helper). +func cacheLen[K comparable, V any](c *Cache[K, V]) int { + c.mu.RLock() + defer c.mu.RUnlock() + return len(c.entries) +} + +func TestCapacity_Bounded(t *testing.T) { + c := New[string, int](Config{TTL: time.Minute, Capacity: 3}) + for i := 0; i < 10; i++ { + c.Set(string(rune('a'+i)), i) + } + if n := cacheLen(c); n != 3 { + t.Fatalf("len = %d, want 3", n) + } +} + +// Dead entries (past the staleness cap) are evicted before live ones. +func TestCapacity_EvictsDeadFirst(t *testing.T) { + now := fixedTime + c := New[string, int](Config{TTL: time.Second, StaleTTL: 10 * time.Second, Capacity: 2}) + withClock(c, &now) + + c.Set("dead", 1) + now = now.Add(time.Minute) // "dead" is now past TTL+StaleTTL + c.Set("live", 2) + c.Set("live2", 3) // over capacity: must evict "dead", not a live entry + + if _, ok := c.load("dead"); ok { + t.Error("dead entry should have been evicted") + } + if _, ok := c.load("live"); !ok { + t.Error("live entry was evicted while a dead one existed") + } + if _, ok := c.load("live2"); !ok { + t.Error("just-inserted entry must survive eviction") + } + if n := cacheLen(c); n != 2 { + t.Fatalf("len = %d, want 2", n) + } +} + +func TestCapacity_ZeroMeansUnbounded(t *testing.T) { + c := New[string, int](Config{TTL: time.Minute}) // Capacity unset + for i := 0; i < 100; i++ { + c.Set(string(rune(i)), i) + } + if n := cacheLen(c); n != 100 { + t.Fatalf("len = %d, want 100 (unbounded)", n) + } +} + func BenchmarkGet(b *testing.B) { c := New[string, string](Config{TTL: time.Minute}) c.Set("key", "value") From 6249eb7f72b51cd4e8a7c332862c8dedd2c00096 Mon Sep 17 00:00:00 2001 From: Mohamad Rostami Date: Tue, 11 Aug 2026 18:10:34 +0200 Subject: [PATCH 4/6] add negative caching with an authoritative-miss classifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fetch error was always treated as transient, which made stale-if-error dangerous for lookups where the upstream can authoritatively say a key is gone: a deleted model or revoked API key kept being served stale for up to StaleTTL. Config gains two opt-in fields. NotFound classifies a fetch error as an authoritative "does not exist"; such a miss evicts any cached value (authoritative-wins — it is never served stale, in GetStale's error path and in background refreshes alike). NegativeTTL then caches the miss itself: lookups within the TTL are answered from the negative entry without touching the upstream, and misses are never served past it (no stale negatives). NegativeTTL of 0 keeps evicting without caching. OnError no longer fires for authoritative background misses — they are cache state, not failures. With NotFound unset every error stays transient and nothing changes. Co-Authored-By: Claude Fable 5 --- README.md | 10 ++++ lastcache.go | 115 +++++++++++++++++++++++++++++-------- lastcache_test.go | 143 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 245 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 87710a4..0ebad92 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,14 @@ past expiry. The cap is a hard wall-clock bound anchored at the last An expired value is returned immediately while a single background goroutine refreshes it. +### negative caching (`NotFound` + `NegativeTTL`) +An error your `NotFound` classifier reports as an authoritative "does not +exist" evicts any cached value — a deleted key is never served stale — and is +itself cached for `NegativeTTL`, so a hammered missing key is answered from +cache instead of hitting the upstream. Keep `NegativeTTL` short: a negative +entry makes a key that was just created upstream look missing until it +expires. + ## Usage ```go @@ -93,6 +101,8 @@ type Result[V any] struct { | `StaleTTL` | How long a stale value may be served after expiry when a refresh fails. A hard cap anchored at the last successful fetch; past it the value is treated as gone. `0` disables serving stale on error. | | `MaxConcurrentRefresh` | Caps concurrent background refreshes (`GetAsync`) across all keys. Defaults to `1`. | | `Capacity` | Bounds the number of entries; dead entries are evicted first, then arbitrary ones. `<= 0` means unbounded. Set it whenever keys come from user input. | +| `NotFound` | Optional `func(err error) bool` classifying a fetch error as an authoritative "does not exist". Such misses evict any cached value and are negatively cached. Nil treats every error as transient. | +| `NegativeTTL` | How long an authoritative miss is served from cache before re-fetching. `0` disables negative caching (misses still evict). | | `OnError` | Optional `func(key any, err error)` called when a background refresh fails. | | `Context` | Base context for background refreshes (they outlive the request). Defaults to `context.Background()`. | diff --git a/lastcache.go b/lastcache.go index fdb7486..b3516d5 100644 --- a/lastcache.go +++ b/lastcache.go @@ -11,6 +11,12 @@ // stale-while-revalidate (GetAsync) // An expired value is returned immediately while a single background goroutine // refreshes it. +// +// negative caching (Config.NotFound + Config.NegativeTTL) +// An error the NotFound classifier reports as an authoritative "does not +// exist" evicts any cached value (it is never served stale) and is itself +// cached for NegativeTTL, so lookups of a hammered missing key are answered +// from cache instead of hitting the upstream. package lastcache import ( @@ -52,8 +58,23 @@ type Config struct { // grow without limit. Capacity int + // NotFound reports whether a fetch error means the key authoritatively + // does not exist, rather than a transient upstream failure. Authoritative + // misses evict any cached value — they are never served stale — and are + // negatively cached for NegativeTTL. Nil treats every error as transient. + NotFound func(err error) bool + + // NegativeTTL is how long an authoritative miss (per NotFound) is served + // from cache before the next lookup re-fetches. Keep it short: a negative + // entry makes a key that was just created upstream look missing until it + // expires. 0 disables negative caching; authoritative misses then still + // evict, but every lookup re-fetches. + NegativeTTL time.Duration + // OnError, if set, is called with the underlying error when a background - // refresh (GetAsync) fails. Foreground errors are returned to the caller. + // refresh (GetAsync) fails transiently. Foreground errors are returned to + // the caller, and authoritative misses (per NotFound) are cache state, not + // errors, so they are not reported. OnError func(key any, err error) // Context is the base context used for background refreshes, which outlive @@ -76,9 +97,12 @@ type Result[V any] struct { Err error } +// item is either a value (err == nil) or a negative entry caching an +// authoritative "not found" (err != nil, value is the zero value). type item[V any] struct { value V - fetchedAt time.Time // when the value was last fetched successfully (or Set) + err error + fetchedAt time.Time // when the entry was last fetched (or Set) } type call[V any] struct { @@ -135,9 +159,13 @@ func (c *Cache[K, V]) Get(ctx context.Context, key K, fetch Fetch[K, V]) (V, err } // GetStale is like Get but reports whether the value was served stale via the -// returned Result. The error is non-nil only when nothing could be served. +// returned Result. The error is non-nil only when nothing could be served; a +// fresh negative entry counts as such and returns the cached error. func (c *Cache[K, V]) GetStale(ctx context.Context, key K, fetch Fetch[K, V]) (Result[V], error) { if it, ok := c.load(key); ok && !c.expired(it) { + if it.err != nil { + return Result[V]{Err: it.err}, it.err + } return Result[V]{Value: it.value}, nil } @@ -146,11 +174,13 @@ func (c *Cache[K, V]) GetStale(ctx context.Context, key K, fetch Fetch[K, V]) (R return Result[V]{Value: val}, nil } - // Fetch failed: serve the last known value while it is within the hard - // staleness cap. The item is left untouched, so the cap never moves and a - // concurrent successful refresh is never overwritten with a stale copy. + // Transient failure: serve the last known value while it is within the + // hard staleness cap. The item is left untouched, so the cap never moves + // and a concurrent successful refresh is never overwritten with a stale + // copy. Authoritative misses never serve stale (fetchOnce already evicted + // the value, so the load below misses or finds a negative entry). if c.config.StaleTTL > 0 { - if it, ok := c.load(key); ok && !c.dead(it) { + if it, ok := c.load(key); ok && it.err == nil && !c.dead(it) { return Result[V]{Value: it.value, Stale: true, Err: err}, nil } } @@ -163,13 +193,20 @@ func (c *Cache[K, V]) GetStale(ctx context.Context, key K, fetch Fetch[K, V]) (R // value is returned (Result.Stale == true) and a single background goroutine // refreshes the key. On a cold miss — or once a value is past the hard // staleness cap — the fetch runs synchronously; if it fails, Result.Err is -// set. Background refresh errors are reported via Config.OnError. +// set. A fresh negative entry returns its cached error without fetching. +// Background refresh errors are reported via Config.OnError. func (c *Cache[K, V]) GetAsync(ctx context.Context, key K, fetch Fetch[K, V]) Result[V] { it, ok := c.load(key) - if !ok || c.dead(it) { + if ok && it.err != nil && !c.expired(it) { + return Result[V]{Err: it.err} + } + + if !ok || it.err != nil || c.dead(it) { val, err := c.fetchOnce(ctx, key, fetch) if err != nil { - c.onError(key, err) + if !c.isNotFound(err) { + c.onError(key, err) + } return Result[V]{Err: err} } return Result[V]{Value: val} @@ -204,10 +241,10 @@ func (c *Cache[K, V]) TTL(key K) time.Duration { return 0 } -// Range calls f for each key with its value and remaining TTL. Iteration stops -// if f returns false. It iterates over a snapshot taken when Range is called, -// so f may safely mutate the cache; mutations are not reflected in the -// iteration. +// Range calls f for each key with its value and remaining TTL. Negative +// entries hold no value and are skipped. Iteration stops if f returns false. +// It iterates over a snapshot taken when Range is called, so f may safely +// mutate the cache; mutations are not reflected in the iteration. func (c *Cache[K, V]) Range(f func(key K, value V, ttl time.Duration) bool) { c.mu.RLock() snapshot := make(map[K]item[V], len(c.entries)) @@ -218,6 +255,9 @@ func (c *Cache[K, V]) Range(f func(key K, value V, ttl time.Duration) bool) { now := c.clock() for k, it := range snapshot { + if it.err != nil { + continue + } if !f(k, it.value, c.expiry(it).Sub(now)) { return } @@ -238,9 +278,7 @@ func (c *Cache[K, V]) fetchOnce(ctx context.Context, key K, fetch Fetch[K, V]) ( } else { go func() { cl.val, cl.err = fetch(context.WithoutCancel(ctx), key) - if cl.err == nil { - c.Set(key, cl.val) - } + c.storeResult(key, cl.val, cl.err) c.inflight.Delete(key) close(cl.done) }() @@ -274,11 +312,10 @@ func (c *Cache[K, V]) triggerRefresh(key K, fetch Fetch[K, V]) { } val, err := fetch(c.baseCtx, key) - if err != nil { + c.storeResult(key, val, err) + if err != nil && !c.isNotFound(err) { c.onError(key, err) - return } - c.Set(key, val) }() } @@ -297,6 +334,31 @@ func (c *Cache[K, V]) put(key K, value V) { c.mu.Unlock() } +// storeResult records a fetch outcome: a success stores the value, an +// authoritative miss evicts any cached value and (with NegativeTTL > 0) caches +// the error, and a transient failure leaves the cache untouched so any stale +// value keeps being served up to its cap. +func (c *Cache[K, V]) storeResult(key K, val V, err error) { + switch { + case err == nil: + c.put(key, val) + case c.isNotFound(err): + now := c.clock() + c.mu.Lock() + if c.config.NegativeTTL > 0 { + c.entries[key] = item[V]{err: err, fetchedAt: now} + c.evictLocked() + } else { + delete(c.entries, key) + } + c.mu.Unlock() + } +} + +func (c *Cache[K, V]) isNotFound(err error) bool { + return c.config.NotFound != nil && c.config.NotFound(err) +} + // evictLocked bounds the cache to Capacity, dropping dead entries first and // then arbitrary ones. Must hold c.mu. func (c *Cache[K, V]) evictLocked() { @@ -320,6 +382,9 @@ func (c *Cache[K, V]) evictLocked() { } func (c *Cache[K, V]) expiry(it item[V]) time.Time { + if it.err != nil { + return it.fetchedAt.Add(c.config.NegativeTTL) + } return it.fetchedAt.Add(c.config.TTL) } @@ -327,10 +392,14 @@ func (c *Cache[K, V]) expired(it item[V]) bool { return c.clock().After(c.expiry(it)) } -// dead reports whether it is past the hard staleness cap and may no longer be -// served, even stale. With StaleTTL == 0 nothing is ever dead: Get already -// refuses to serve stale, and GetAsync's staleness is deliberately unbounded. +// dead reports whether it may no longer be served at all. A negative entry +// dies at expiry (misses are never served stale). A value dies past the hard +// staleness cap; with StaleTTL == 0 values are never dead: Get already refuses +// to serve stale, and GetAsync's staleness is deliberately unbounded. func (c *Cache[K, V]) dead(it item[V]) bool { + if it.err != nil { + return c.expired(it) + } return c.config.StaleTTL > 0 && c.clock().After(c.expiry(it).Add(c.config.StaleTTL)) } diff --git a/lastcache_test.go b/lastcache_test.go index 98d737f..93c25a3 100644 --- a/lastcache_test.go +++ b/lastcache_test.go @@ -504,6 +504,149 @@ func TestGetAsync_SkipsRefreshIfAlreadyFresh(t *testing.T) { } } +var errNotFound = errors.New("not found") + +func notFoundConfig(negTTL time.Duration) Config { + return Config{ + TTL: time.Second, + StaleTTL: time.Minute, + NegativeTTL: negTTL, + NotFound: func(err error) bool { return errors.Is(err, errNotFound) }, + } +} + +// An authoritative miss is cached: repeat lookups within NegativeTTL are +// answered from cache, and after expiry the next lookup re-fetches. +func TestNegative_CachedAndExpires(t *testing.T) { + now := fixedTime + c := New[string, string](notFoundConfig(5 * time.Second)) + withClock(c, &now) + + var calls int32 + exists := false + fetch := func(context.Context, string) (string, error) { + atomic.AddInt32(&calls, 1) + if exists { + return "v", nil + } + return "", errNotFound + } + + for i := 0; i < 3; i++ { // one fetch, then served from the negative entry + if _, err := c.Get(context.Background(), "k", fetch); !errors.Is(err, errNotFound) { + t.Fatalf("want errNotFound, got %v", err) + } + } + if n := atomic.LoadInt32(&calls); n != 1 { + t.Fatalf("fetch ran %d times, want 1 (negative hit)", n) + } + + // Past NegativeTTL the miss is re-fetched and the new value found. + exists = true + now = now.Add(6 * time.Second) + if v, err := c.Get(context.Background(), "k", fetch); err != nil || v != "v" { + t.Fatalf("after negative expiry: got (%v,%v), want (v,nil)", v, err) + } +} + +// An authoritative miss evicts a cached value: despite StaleTTL, the stale +// value must not be served once the upstream said the key is gone. +func TestNegative_AuthoritativeMissEvictsValue(t *testing.T) { + now := fixedTime + c := New[string, string](notFoundConfig(5 * time.Second)) + withClock(c, &now) + c.Set("k", "stored") + now = now.Add(2 * time.Second) // expired, well within the stale cap + + var calls int32 + fetch := func(context.Context, string) (string, error) { + atomic.AddInt32(&calls, 1) + return "", errNotFound + } + + res, err := c.GetStale(context.Background(), "k", fetch) + if !errors.Is(err, errNotFound) || res.Stale { + t.Fatalf("authoritative miss must not serve stale, got (%+v,%v)", res, err) + } + // Evicted and negatively cached: answered without another fetch. + if _, err := c.Get(context.Background(), "k", fetch); !errors.Is(err, errNotFound) { + t.Fatalf("want errNotFound, got %v", err) + } + if n := atomic.LoadInt32(&calls); n != 1 { + t.Fatalf("fetch ran %d times, want 1", n) + } +} + +// With NegativeTTL = 0 an authoritative miss still evicts the value but is not +// cached: every lookup re-fetches. +func TestNegative_DisabledStillEvicts(t *testing.T) { + now := fixedTime + c := New[string, string](notFoundConfig(0)) + withClock(c, &now) + c.Set("k", "stored") + now = now.Add(2 * time.Second) + + var calls int32 + fetch := func(context.Context, string) (string, error) { + atomic.AddInt32(&calls, 1) + return "", errNotFound + } + for i := 0; i < 2; i++ { + if _, err := c.Get(context.Background(), "k", fetch); !errors.Is(err, errNotFound) { + t.Fatalf("want errNotFound, got %v", err) + } + } + if n := atomic.LoadInt32(&calls); n != 2 { + t.Fatalf("fetch ran %d times, want 2 (no negative caching)", n) + } + if _, ok := c.load("k"); ok { + t.Fatal("value must be evicted on authoritative miss") + } +} + +// A background refresh that comes back "not found" evicts the value, stores a +// negative entry, and does not fire OnError. +func TestGetAsync_BackgroundNotFound(t *testing.T) { + now := fixedTime + var onErrCalls int32 + cfg := notFoundConfig(5 * time.Second) + cfg.OnError = func(any, error) { atomic.AddInt32(&onErrCalls, 1) } + c := New[string, string](cfg) + withClock(c, &now) + c.Set("k", "stored") + now = now.Add(2 * time.Second) // expired (no clock writes after this) + + res := c.GetAsync(context.Background(), "k", func(context.Context, string) (string, error) { + return "", errNotFound + }) + if res.Value != "stored" || !res.Stale { + t.Fatalf("got %+v, want stale 'stored' while the refresh runs", res) + } + + deadline := time.Now().Add(time.Second) + for { + if it, ok := c.load("k"); ok && it.err != nil { + break // negative entry landed + } + if time.Now().After(deadline) { + t.Fatal("background refresh did not store a negative entry") + } + time.Sleep(time.Millisecond) + } + + var calls int32 + res = c.GetAsync(context.Background(), "k", func(context.Context, string) (string, error) { + atomic.AddInt32(&calls, 1) + return "x", nil + }) + if !errors.Is(res.Err, errNotFound) || atomic.LoadInt32(&calls) != 0 { + t.Fatalf("want cached errNotFound with no fetch, got (%+v, %d calls)", res, calls) + } + if n := atomic.LoadInt32(&onErrCalls); n != 0 { + t.Fatalf("OnError fired %d times for an authoritative miss, want 0", n) + } +} + // len reports the current number of entries (test helper). func cacheLen[K comparable, V any](c *Cache[K, V]) int { c.mu.RLock() From e0c69759427fb933450c7b9d996c2385c7d4d64d Mon Sep 17 00:00:00 2001 From: Mohamad Rostami Date: Tue, 11 Aug 2026 18:16:23 +0200 Subject: [PATCH 5/6] CI: test on go 1.21+ and update action versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The go.mod floor moved to 1.21 for context.WithoutCancel, so the 1.18 matrix leg fails to build (and fail-fast cancels the stable legs with it). Test 1.21 as the minimum plus stable. Also bump checkout/setup-go past their Node 20 deprecation warnings and disable the setup-go module cache — a zero-dependency module has no go.sum to key it on, which logged a restore warning every run. Co-Authored-By: Claude Fable 5 --- .github/workflows/coverage.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml index 3107c9a..784283f 100644 --- a/.github/workflows/coverage.yaml +++ b/.github/workflows/coverage.yaml @@ -15,14 +15,15 @@ jobs: strategy: matrix: os: [ubuntu-latest, windows-latest] - go: ['1.18', 'stable'] + go: ['1.21', 'stable'] # 1.21 is the module floor (context.WithoutCancel) steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v6 with: go-version: ${{ matrix.go }} + cache: false # zero-dependency module: no go.sum for the cache key - name: Build run: go build ./... From 06e040da05c0826fad899bddbf6d5ab9f9abf4b0 Mon Sep 17 00:00:00 2001 From: Mohamad Rostami Date: Tue, 11 Aug 2026 19:20:08 +0200 Subject: [PATCH 6/6] add Config.Clock for deterministic time in tests The clock was already injectable internally; expose it so consumers can pin time in their own tests instead of sleeping through TTLs. Nil keeps time.Now. Co-Authored-By: Claude Fable 5 --- README.md | 1 + lastcache.go | 13 ++++++++++++- lastcache_test.go | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 0ebad92..d23de3f 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,7 @@ type Result[V any] struct { | `NegativeTTL` | How long an authoritative miss is served from cache before re-fetching. `0` disables negative caching (misses still evict). | | `OnError` | Optional `func(key any, err error)` called when a background refresh fails. | | `Context` | Base context for background refreshes (they outlive the request). Defaults to `context.Background()`. | +| `Clock` | Optional `func() time.Time` used for all expiry checks. Defaults to `time.Now`. Override in tests to control TTLs deterministically; must be safe for concurrent use. | ## Migrating from v1 diff --git a/lastcache.go b/lastcache.go index b3516d5..a13e4df 100644 --- a/lastcache.go +++ b/lastcache.go @@ -80,6 +80,12 @@ type Config struct { // Context is the base context used for background refreshes, which outlive // the request that triggered them. Defaults to context.Background(). Context context.Context + + // Clock returns the current time and defaults to time.Now. Override it in + // tests to control TTL expiry deterministically instead of sleeping. The + // cache reads it from background goroutines too, so a test clock must be + // safe for concurrent use. + Clock func() time.Time } // Result carries a value plus whether it was served stale. @@ -140,9 +146,14 @@ func New[K comparable, V any](config Config) *Cache[K, V] { base = context.Background() } + clock := config.Clock + if clock == nil { + clock = time.Now + } + return &Cache[K, V]{ config: config, - clock: time.Now, + clock: clock, baseCtx: base, entries: make(map[K]item[V]), semaphore: make(chan struct{}, sem), diff --git a/lastcache_test.go b/lastcache_test.go index 93c25a3..4591633 100644 --- a/lastcache_test.go +++ b/lastcache_test.go @@ -34,6 +34,40 @@ func TestNew_Defaults(t *testing.T) { } } +// Config.Clock lets tests control expiry without sleeping or touching +// internals. +func TestConfig_Clock(t *testing.T) { + var mu sync.Mutex + now := fixedTime + c := New[string, string](Config{ + TTL: time.Minute, + Clock: func() time.Time { + mu.Lock() + defer mu.Unlock() + return now + }, + }) + c.Set("k", "v") + + var calls int32 + fetch := func(context.Context, string) (string, error) { + atomic.AddInt32(&calls, 1) + return "v2", nil + } + + if v, _ := c.Get(context.Background(), "k", fetch); v != "v" || atomic.LoadInt32(&calls) != 0 { + t.Fatalf("fresh hit: got %q with %d fetches, want \"v\" with 0", v, atomic.LoadInt32(&calls)) + } + + mu.Lock() + now = now.Add(2 * time.Minute) // expire + mu.Unlock() + + if v, _ := c.Get(context.Background(), "k", fetch); v != "v2" || atomic.LoadInt32(&calls) != 1 { + t.Fatalf("after expiry: got %q with %d fetches, want \"v2\" with 1", v, atomic.LoadInt32(&calls)) + } +} + func TestGet_FreshHit_NoFetch(t *testing.T) { now := fixedTime c := New[string, int](Config{TTL: time.Minute})