phongn opened a new pull request, #13669: URL: https://github.com/apache/trafficserver/pull/13669
## Summary `RamCacheLRU::put` evaluated the seen filter (`proxy.config.cache.ram_cache.use_seen_filter`, on by default) *before* looking the key up. The filter is a per-slot bit: first sighting sets it and declines, second sighting clears it and admits. So any put for a key the cache already held met a cleared slot and was declined once. Two kinds of put fall into that: 1. **A rewrite.** The auxkey is the object's directory offset; a rewrite or header update assigns a new one, and nothing in the write path touches the RAM cache (only evacuation calls `fixup`). The next read misses on the auxkey, goes to disk, and puts `(K, new_off)`. The walk discarded the stale copy, then the filter declined the put — so every rewrite of a hot object cost two disk reads before it was RAM-resident again. This is the routine production case. 2. **A put for the resident entry itself** (same key and auxkey). Narrower reach: the only put site is gated on `!doc_from_ram_cache`, so this arises only in the concurrent-miss race, but the put is a reference and should not be turned away. CLFUS has neither problem: it gates its filter on `!e`, and its filter is a persistent fingerprint (`_seen[s] == k`) rather than a toggle, so a replaced key matches and is admitted at once. S3-FIFO has no seen filter. Split out of #13380, where it surfaced; it is pre-existing LRU behavior and independent of the `copy` work there. ## Change Look the key up first. A same-auxkey match bumps recency and returns, as before. An auxkey conflict removes the stale entry and sets `replaced`; the filter then runs only for keys with no entry at all. Three lines of logic and a comment. ## Notes for reviewers - **Semantics change to be aware of:** resident puts no longer toggle `seen[j]`. Under the old order a resident put alternately set and cleared the slot, so an evicted hot object was re-admitted on its first put about half the time, and unrelated keys sharing the slot saw the flips. Now the slot is always clear after admission and every evicted object needs two puts to return. That is the documented semantics and matches the other policies; the toggle-on-resident-put was incidental to #10662. - Two side effects, both matching CLFUS's `_destroy`-then-filter order: the stale copy is removed even in the (now unreachable) case where the filter would have declined, and in threshold mode (`use_seen_filter > 1`) the fill check reads `bytes` after the stale copy has been reclaimed, i.e. actual occupancy. - `use_seen_filter` is read once at startup and both LRU and CLFUS size their filter state in `init()`, which is why the test sets it before constructing the cache. ## Tests New `test_RamCacheSeenFilter.cc`, four cases, 57 assertions: - *accepts a put for a resident entry under every policy* — the shared contract, over LRU/CLFUS/S3-FIFO; filter-agnostic by design. - *seen filter declines a new key once and a resident key never* — LRU; asserts the exact decline (`put` returns 0 then 1) so the case fails if the filter is not engaged, then two resident puts both return 1. - *admits a rewrite of a resident key without filtering it* — LRU; `(K,1)` admitted, `put(K,2)` returns 1 immediately, `get(K,1)` misses, `get(K,2)` hits. - *re-put of a resident entry refreshes its recency* — LRU; per-entry cost read off the `ram_cache_bytes` gauge, cache filled to derived capacity, first object re-put, one more admitted, the second-oldest is the victim. Each piece is mutation-verified: | Mutation | Result | |---|---| | master's ordering | all four cases fail | | `replaced` skip disabled (filter still runs for replaced keys) | only the rewrite case fails | | filter disabled in the tests | the three LRU cases fail at `REQUIRE(put == 0)` | ## Follow-ups (not in this PR) - `test_RamCacheCompressEntries.cc` carries its own copies of these fixtures; hoist them into `test_doubles.h`. - `seen.resize(size * 2)` and `% (nbuckets * 2)` are `int` arithmetic and overflow for the last `bucket_sizes[]` entry (needs ~257 GB of RAM cache in one stripe). 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
