[FumingPower3925]

The cache middleware's Config.SetTimeout default is 1 second. #1131 merged the bound with a placeholder of 5 s; the maintainer decided 1 s.

Refs #888 (item 30).

Why 1 s

A Store.Set that never returns holds its request and, on epoll and io_uring, the event loop a sync request runs on, for the whole bound. The L16 review of #1131 measured a bystander request on the held loop delayed 951 ms at a 1 s bound and 2.95 s at 3 s, so 5 s meant 5 s of a dead loop per hung fill. Celeris's Redis and Memcached drivers set conn deadlines from the context's deadline, so a Set on a healthy link takes well under a millisecond and a 1 s bound is far above it.

What changes

Only the default: defaultSetTimeout and the doc sentence "Zero selects the default, 1 second," in middleware/cache/config.go. The field, the bound, the override (any positive value) and the "negative waits for ever" behaviour are as before. No exported API change (mage apiCheck: matches 53 packages). The placeholder comments ("PENDING the maintainer's decision") are gone from config.go and from the test header.

Release note: cache: Config.SetTimeout now defaults to 1 second (the 5 s was a placeholder; v1.6.0 is unreleased). A Set not returned in 1 s is abandoned: the response is sent as usual with X-Cache: MISS and is not stored, and nothing is logged or counted unless the Set panics. A hung Set still holds the request, and on epoll and io_uring its event loop, for the whole bound. A store with a legitimately slower Set (Redis over a WAN, a saturated pool) should set cache.Config{SetTimeout: 5 * time.Second}; a negative value waits for ever. The default may change before the v1.6.0 tag.

Tests (failing first)

Commits 1 and 2 are tests then fix; commit 1 is the tests only, red on main (5 s placeholder), in a linux/epoll container at --cpus 2 under -race:

test	on main
TestSetTimeoutZeroSelectsTheDefault888 (now pins the literal 500 ms..1 s, not the constant)	red: context ends in 4.99996 s
TestDefaultSetTimeoutIsOneSecond888 (new: constant, applyDefaults, an explicit 3 s kept, a negative kept)	red: 5 s, want 1 s
TestDefaultSetTimeoutIsInTheDoc888 (doc says "1 second,")	red
TestDefaultSetTimeoutBoundsTheLoopHold888 (new, linux; epoll, 2 workers, a Set that ignores its context, SetTimeout unset)	red: miss answered after 5.003 s, bystander waited 5.003 s
TestUnsetSetTimeoutBoundsTheStoresContext888	green on main, unchanged

Commit 2 is the fix: all green. The bystander test sends sequential pings on new connections until the miss is answered and takes the longest wait (the engine needs 2 workers, so a ping can land on the free loop; the first one the kernel hands to the held loop waits out the hold). It asserts miss and bystander within 2.5 s, the miss not before 800 ms, and a longest wait of at least 300 ms (otherwise the test measured nothing). Epoll only.

Mutants, all killed: default 3 s (3 tests red, the loop-hold one at 3.005 s), default 500 ms (loop-hold red: answered after 503 ms), a negative value defaulted (IsOneSecond red), an explicit value ignored (IsOneSecond red).

Runs (docker golang:1.27.3, --cpus 2, one process at a time): the new tests -race -count=10 ok; the whole middleware/cache package once under -race: only the io_uring subtests fail (no io_uring on Docker Desktop; 0 non-io_uring failures; the same set as main per the L16 review). golangci-lint v2.14.0 0 issues (linux and darwin); go vet windows, darwin and linux clean; mage apiCheck ok.

Findings, not fixed here

From the L16 review of #1131 (review-r1-opus.md), decisions the maintainer left open:

• Wrappers stay bounded (m4). The bound exempts only *store.MemoryKV and MemoryStore themselves. A struct embedding *store.MemoryKV is +175% on a miss (CacheMissOtherStore888: 789 to 2204 ns, 9 to 20 allocs; A/A -3.9%). store.Prefixed's cost is inferred from the same mechanism (the doc says about 1.4 us and 11 allocs per fill), not measured. An opt-out needs a marker that store can implement (an exported method, or a module-internal package), and prefixedKV would forward it only when the store it wraps is in-process.
• No bound on Get. A hung connection hangs Get before Set is reached, so the Set bound protects only "Get works, Set hangs".
• No cap on abandoned Sets (m6). A store that ignores its context and never returns, SetTimeout 1 ms, 2000 misses: 2000 goroutines and +37 MiB live heap (each abandoned Set pins up to MaxBodyBytes). Celeris's drivers honour the deadline, so only a third-party store that ignores ctx grows. A per-middleware cap (skip the Set past it) is the follow-up.
• Miss path about 4% slower on MemoryStore (m3). CacheMissNoSingleflight +4.5% paired (A/A -1.9%), about 30 ns, allocations unchanged; most likely code placement. Optional: make the in-process fast path inlinable in the caller.
• m5, mostly addressed. A panic in a Set the middleware stopped waiting for is logged through slog at Error with its stack (cache.go); what remains is that a panic raised while the request is still waiting is re-raised with the value only, so the store goroutine's stack is lost.
• An abandoned Set, or any Set error, is invisible (M1). The Set's error is dropped and nothing is logged or counted; the client gets a normal X-Cache: MISS. Only a panic in an abandoned Set is logged (cache.go). A store merely slower than the bound degrades the cache to permanent misses with no signal but the MISS ratio, and this happens more often at 1 s. The godoc of SetTimeout and doc.go now say so; a counter or log is a follow-up (API).
• The 1 s default does not remove the loop hold, it shortens it. A dead store parks the loop of each sync miss for the bound (measured 1.00 s here).
• Slow-store connection churn (M3, read, not measured). The Redis driver closes the connection when the deadline passes (driver/redis/conn.go ~352), so each Set exceeding 1 s costs a reconnect.
• Driver defaults are 3 s, the cache bound is 1 s. Redis Read/Write timeouts are advisory (3 s, "unused currently"); memcached Timeout (3 s) is used only when the context has no deadline. Under the cache the Set always has the 1 s deadline, so the effective bound is 1 s: a user who raised the driver timeouts for a slow link also has to raise SetTimeout.
• The bound is a request-side wait: a Set that ignores its context keeps running after it, and may still write later (documented on SetTimeout).

CI note

The first run on 9a96459 failed Portable fallback (macos-latest): TestSyncFallThroughAfterTheCapKeepsOrderAndTheError1104 in driver/postgres (a data race, unexpected DataRow in extended query), which this PR does not touch. It is green on the current head.

[coderabbitai]

Caution

Review failed

The pull request is closed.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!