Add S3-FIFO and SIEVE arms, MSR and Meta trace loaders - #6
Merged
Conversation
Two policies from the FIFO-queue family join the arm set, both adapting scalalang2/golang-fifo through one shared adapter in policies/fifo. They come from one dependency and need the same four methods supplied - Keys, Values, Resize, Cap, plus Add's evicted flag - so giving each its own module would have duplicated three hundred lines of deadlock-sensitive glue. The adapter keeps its own key index, maintained through upstream's eviction callback, and rebuilds to resize. The two fail differently at the edges, which is why the adapter guards rather than trusting either: SIEVE panics at size zero where S3-FIFO's Set loops forever, and only S3-FIFO's Len reads its queues unlocked. Len is answered from the adapter's index for both. S3-FIFO is the best fixed policy on the Meta trace and within 0.05 points of the winner on Twitter; SIEVE matches it on key-value traffic at roughly half the per-operation cost, and collapses to 0.00% on LIRS loop because it carries no ghost queue. Two trace loaders come with them. Both formats expand, and that is the whole difficulty: an MSR record carries a byte length and stands for as many block accesses as fit in it, while a Meta row collapses a run of identical operations into an op_count that CacheBench replays that many times. Read either as one-record-one-request and the result is the same keys with far less reuse, and hit rates that are wrong and entirely plausible. MSR keys are namespaced by host and disk, since the thirteen servers number their volumes from zero independently. A shadow policy could only ever acquire a key the active policy had missed, because the only insert path was the caller's Add and a read-through caller calls Add just on an active-policy miss. Behind a strong incumbent the shadows went static, and a static cache covering most of a small keyspace scores well: measured on a cyclic workload with a 94%-hit incumbent, arms that truly serve 0.00% reported above 90% and Advice recommended switching from the best arm to the worst. A shadow that misses now fills itself, and the Add fan-out skips a key a shadow already holds so the fill is not double-counted as an access - SIEVE would otherwise mark every freshly filled key as visited, defeating the one-hit-wonder filtering it is carried for. Every measured number in the evidence tables was re-run against that fix. Adaptive selection now beats the best fixed policy on two of six real traces rather than one: LIRS loop moved from -7.45 points to +0.12, while P3's margin shrank from +1.13 to +0.05. The synthetic conclusion is unchanged.
While a MigrationGradual window is open the source policy is deliberately not demoted: it holds the only copy of every value not yet promoted, and promoteLocked reads those values back out with Peek. It is also not the active policy, so the shadow read fan-out treated it as a shadow and filled it with the zero value on a miss. Peek cannot tell such a zero from a real value still pending, so the zero was promoted into the active policy and returned to the caller as a hit. "Not active" is not the same as "is a shadow". For the duration of a gradual window there are three roles rather than two, and anything iterating the policy map and skipping only the active one has to say what it means to do to the migration source. The read fan-out now skips it until the window closes and it is demoted properly. Add's fan-out writes to the source too, but compensates by dropping the key from migrationRealKeys, so the poisoned entry is never promoted. Only the read path was wrong. The precondition is that the source evicts during the window, which needs a working set larger than the capacity. Every existing gradual test used either a working set that exactly fits or a policy double that does not enforce capacity at all, so none of them could have caught it. The new test uses three times the capacity and the capacity-enforcing double.
…anch The policies/fifo constructors accepted a size of zero and returned a working cache that could hold nothing. NewLRU, NewLFU and NewTwoQueue all reject that, and as a bandit arm it is worse than an error: the arm accepts nothing and reports no hits for its whole life without ever looking broken. Both constructors now return an error, and resizing an existing cache to zero stays legal, since AdaptiveCache.Resize passes its own capacity through to every arm. Values skipped an entry the key index held but the library did not, returning a slice shorter than and misaligned with Keys, so every value after the gap belonged to the wrong key. The stale key is dropped from the index instead. The state is unreachable today - upstream funnels every removal through its eviction callback - so this is defence rather than a repair, and the comment says so. Thompson.SelectPolicy carried a tie-break clause that could never fire: b.order is sorted ascending on every insert, so a strict comparison already leaves the lowest PolicyType holding a tie. The identical clause in Greedy.SelectPolicy is live, because that one ranges a map, and stays. The sortedness the removal depends on was asserted only by a comment and is now a test. Three findings from review came with it. The index reservation is capped, so a resize near the top of the int range no longer panics part-way through AdaptiveCache resizing its arms. A concurrent test called require from a spawned goroutine, where FailNow is not legal; it collects failures instead. And two tests asserted less than their names claimed - one checked only that a resize did not panic, where the documented cost is that the rebuild discards the algorithm's learned state, which is now measured directly: a key read six times outlives keys read none, and stops outliving them once the cache has been resized.
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.
Adds two eviction policies from the FIFO-queue family as bandit arms, two real-trace loaders, and fixes two correctness bugs in the core cache found along the way.
Policies
policies/fifois a new module adapting scalalang2/golang-fifo, exposing S3-FIFO (SOSP '23) and SIEVE (NSDI '24) through one shared adapter — they come from one dependency and need the same four methods supplied, so separate modules would have duplicated ~300 lines of deadlock-sensitive glue.The adapter keeps its own key index through upstream's eviction callback, and rebuilds to resize. The two algorithms fail differently at the edges, so it guards rather than trusting either: SIEVE panics at size zero where S3-FIFO's
Setloops forever, and only S3-FIFO'sLenreads its queues unlocked.S3-FIFO is the best fixed policy on the Meta trace and within 0.05 points of the winner on Twitter. SIEVE matches it on key-value traffic at roughly half the per-operation cost, and serves 0.00% on LIRS
loopbecause it carries no ghost queue.Trace loaders
bench/trace_msr.go(MSR Cambridge block I/O) andbench/trace_meta.go(Meta CacheBench key-value). Both formats expand — an MSR record covers a byte range and stands for every block in it, a Meta row collapses a run of identical operations into anop_count. Reading either as one-record-one-request gives the same keys with far less reuse, and hit rates that are wrong and entirely plausible. Pinned against an independent count of the published file.Bug fixes
Shadow policies could only acquire keys the active policy had missed. The only insert path was the caller's
Add, and a read-through caller callsAddjust on an active-policy miss — so behind a strong incumbent the shadows went static. Measured with a 94%-hit incumbent, arms that truly serve 0.00% reported above 90%, andAdvice()recommended switching from the best arm to the worst. Every deterministic arm's shadow now matches its standalone replay to within 0.005.A gradual migration could serve a shadow's zero as real data. Fixing the above introduced it: the migration source is not the active policy, so the read fan-out filled it with zeros, and
promoteLockedcannot tell such a zero from a real value still pending. "Not active" is not the same as "is a shadow".Evidence
Every documented number was re-measured against the shadow fix. Adaptive selection now beats the best fixed policy on two of six real traces rather than one — LIRS
loopmoved from −7.45 points to +0.12, while P3's margin shrank from +1.13 to +0.05. The synthetic conclusion is unchanged.Not addressed here
benchclient/go.modrequirespolicies/fifo v0.3.1, a tag that has never existed because the module is new. Correct as a pre-release placeholder — the release procedure rewrites everyrequireat tag time — butrelease-checkreports green on it and should not, since its filter only rejectsv0.0.0. Recorded in CLAUDE.md; must be fixed before anything is tagged.The fleet section of
docs/evidence.mdwas not re-measured and is marked stale: it works by pooling exactly the evidence the shadow defect distorted.