diff --git a/cache_test.go b/cache_test.go index e328a09..e03ac35 100644 --- a/cache_test.go +++ b/cache_test.go @@ -1140,8 +1140,9 @@ func TestAdaptiveCache_Close_WaitsForInFlightEpoch(t *testing.T) { &Settings{ EpochDuration: time.Millisecond, // Load-bearing: with partial filling disallowed the not-yet-full - // cache would make selectPolicyLocked return before ever calling - // bandit.SelectPolicy, and the goroutine would never park. + // cache would make snapshotEpochLocked report nothing, consultBandit + // would return without calling bandit.SelectPolicy, and the + // goroutine would never park. EvictPartialCapacityFilling: true, MigrationStrategy: MigrationCold, }, @@ -1149,7 +1150,7 @@ func TestAdaptiveCache_Close_WaitsForInFlightEpoch(t *testing.T) { require.NoError(t, err) // If an assertion below fails, unblock the bandit so the epoch goroutine - // (parked while holding the cache mutex) does not leak, then Close. + // (parked inside the bandit, holding banditMu) does not leak, then Close. var releaseOnce sync.Once t.Cleanup(func() { releaseOnce.Do(func() { close(bandit.release) }) diff --git a/docs/advisor-mode.md b/docs/advisor-mode.md index aedf720..8aa6396 100644 --- a/docs/advisor-mode.md +++ b/docs/advisor-mode.md @@ -43,6 +43,23 @@ and let the bandit do it. `Advice()` is safe to call at any time. Check `Epochs` before believing it: a handful of epochs is not evidence. +Three things about how the report is computed, each of which once made it +misleading: + +- **`Epochs` counts reporting epochs, not ticks.** Outside `ObserveOnly`, with + `EvictPartialCapacityFilling` off, an epoch in which the active policy is not + yet full measures nothing and is not counted -- a cache that has ticked a + thousand times behind a half-full policy reports the evidence it actually has. +- **A policy's measurements start over when it changes role.** Its active + tenure is measured at full capacity over all traffic, its shadow tenure on a + miniature over the sample, and pooling the two describes neither. Carrying + them across a switch also let the outgoing policy's long history outweigh the + incoming one's short one, so straight after a correct switch the report named + the policy just abandoned as best. +- **Ties are broken by policy type.** Arms performing identically produce the + same `Best` on every call, rather than whichever one a map iteration happened + to put first. + ## Observability A cache that changes its own eviction policy needs to be visible in staging. diff --git a/docs/design.md b/docs/design.md index b437d6e..a49bbc1 100644 --- a/docs/design.md +++ b/docs/design.md @@ -72,7 +72,11 @@ Then once per epoch: 1. Every arm reports its hits and misses — the active one included, measured over the same sampled substream, so no arm is judged on more evidence than - another. Counters reset; the epoch is the unit of evidence. + another. Counters reset; the epoch is the unit of evidence. The counts are + what was measured and are never scaled back up by `1/rate`: scaling would + restore the magnitude while inventing confidence, handing a Beta posterior + many times the evidence actually collected. `Stats()` still reports every + request the cache served; only the bandit sees the sample. Which goroutine runs this depends on how the epoch ends. `EpochDuration` ticks on a background goroutine; `EpochRequests` runs it on whichever @@ -103,11 +107,31 @@ AdaptiveCache |-- shadow policy (CacheWrapper -> real Cacher impl, zero-value adds only, | optionally a sampled miniature -- see ShadowSampleRate) |-- Bandit (an interface; ready-made ones in the bandit module) - |-- epoch driver (runEpoch -> selectPolicyLocked -> switchLocked -> migrateData, - on a background goroutine for EpochDuration, or on the - calling goroutine for EpochRequests) + |-- epoch driver (runEpoch: collectEpoch -> consultBandit -> applySelection + -> switchLocked -> migrateData, on a background goroutine + for EpochDuration, or on the calling goroutine for + EpochRequests) ``` +### Two rules the implementation keeps + +**Every mutation of a policy happens while that policy is not the active one.** +A switch resizes and migrates into the incoming policy before making it active, +and drops the outgoing policy's values only after it has stopped being active. +Reverse either half and a caller can read a policy mid-rewrite -- most +damagingly, taking a dropped value's zero for real data. Lock-free reads, listed +under [what is not done](#what-is-not-done), would rest on this rule: a reader +could only ever hold a policy nobody is mutating. + +**During a gradual migration window a policy can hold one of three roles, not +two.** Besides the active policy and the shadows there is the source the window +promotes out of. It is not active, and it is not a shadow either: it holds the +only copy of every value not yet promoted, so it keeps its full capacity, is not +filled with zero values when a read misses, and is demoted only when the window +closes. Code that ranges over the policies skipping only the active one has to +decide what it does to the source. Treating it as a shadow once filled it with +zeros that the window then promoted and served to callers as hits. + ## Implementing the Bandit Interface ```go @@ -133,6 +157,10 @@ an epoch runs it, so that one caller does wait for the bandit — under `EpochDuration` nobody does. And calls are serialised: no implementation is entered from two goroutines at once. +A selection naming `Undefined`, or any policy the cache does not hold, means no +change. That is the natural answer from a bandit that has not formed an opinion +yet, and the cache treats it as one rather than looking the policy up. + Ready-made bandits live in the `bandit` module: `bandit.NewThompson`, and `bandit.NewGreedy` as a control. Both examples use the first. diff --git a/docs/policies.md b/docs/policies.md index 38fdcab..6f9d830 100644 --- a/docs/policies.md +++ b/docs/policies.md @@ -276,3 +276,11 @@ broken here: an arm reading 0 as "no limit" turns a bounded miniature into an unbounded cache. It must also not start a goroutine it gives you no way to stop: a reaper per cache with no `Close` leaks the goroutine and the cache with it. + +And one thing the library deliberately does not assume about an arm: **the order +of `Keys()`.** 2Q returns its frequent queue followed by its recent one, ARC its +recent list followed by its frequent one, and neither is a single recency order, +so any rule of the form "keep the tail" is right for one and backwards for the +other. When an adapted cache shrinks, `Resize` replays every entry into the +rebuilt cache and lets the algorithm choose its own victims. Which entries +survive a shrink is therefore not meaningful, and nothing should rely on it. diff --git a/stability.go b/stability.go index 12accaf..8adbfc1 100644 --- a/stability.go +++ b/stability.go @@ -24,8 +24,10 @@ func (s *Settings) switchGated() bool { // place; the bandit still keeps the posterior it learned this epoch, so a // genuinely better policy wins again on a later epoch. // -// It must be called while the write lock is held, immediately after -// selectPolicyLocked, which populates epochStats. +// It must be called while the write lock is held, from applySelection. The +// epochStats it reads were populated by snapshotEpochLocked, and +// applySelection's staleness check guarantees no later epoch has overwritten +// them in the meantime. func (c *AdaptiveCache[K, V]) allowSwitchLocked(candidate PolicyType) bool { if !c.settings.switchGated() { return true @@ -40,7 +42,7 @@ func (c *AdaptiveCache[K, V]) allowSwitchLocked(candidate PolicyType) bool { cand, okCandidate := c.epochStats[candidate] if !okActive || !okCandidate { // The epoch produced no comparable measurement (see the - // EvictPartialCapacityFilling gate in selectPolicyLocked). Hold the + // EvictPartialCapacityFilling gate in snapshotEpochLocked). Hold the // current policy rather than switch on no evidence. return false }