[pmoust]

The S3-FIFO read cache gains AccessIntent { Interactive, BulkScan, Maintenance } and intent-aware entry points on S3Fifo, Cache and TieredCache (ADR-3072 decision 3b, the cache side of the shape ADR-2677's deferral of decision 3 named). Interactive is exactly today's policy and the old method names stay as Interactive wrappers, so every caller in ravel-query and ravel-catalog compiles unchanged and production behaviour is unchanged until #3084 sets BulkScan from the SQL executor. BulkScan reads do not raise freq or promote on a ghost hit and are admitted into a bulk region bounded to CacheLimits.bulk_share (default 0.5) of max_bytes as a deterministic stable subset by hash rank, so a loop larger than the region serves the same subset on every later pass; a bulk admission never evicts a main-queue interactive entry. Maintenance reads stay in the small queue and never promote. The region's bookkeeping (a per-tenant bottom-k sketch of distinct bytes over a traffic window standing in for pass boundaries, since the cache cannot see them) is charged inside max_bytes and capped at 1 %. The disk tier is untouched; a disk hit re-admits to RAM under the request's intent. Per-intent hits and bytes served are on a new IntentMetricsSnapshot. docs/guides/caching.md now describes both caps as S3-FIFO (part of #2738; the flag help and reference still say LRU, so #2738 stays open).

Fleet task 6503eae7 (epic #3072 track P5), landed by hand after the Stage 4 checkpoint review passed with no blocker. Tests: a 2,000-key loop over 1,000 entries serves 997 touches on passes 2 and 3 with the same set; the 11.5 GB server-geometry hot-set cases (33 MB of 16 KiB interactive ranges against 25 MB bulk objects, and the hot set first touched mid-loop) keep at least 99 % hits; bookkeeping stays inside the cap; the three existing scan-resistance tests are unchanged; every mutant the executor ran (intent ignored, bulk promoted to main, FIFO in the region, bulk evicting main, maintenance promoted, bookkeeping outside the cap) fails its test. Executor gates: clippy over the 15-crate closure, doctests and every feature lane green; closure nextest 8,320 of 8,323 with the 3 failures being ravel-server jemalloc background-thread tests that fail identically on the dispatch ref on that macOS executor. No local gate ran here; CI is the gate.

Three review items are recorded on #3083 as the gate for #3084, not changed here: the region is reclaimed by interactive admissions only when both other queues are empty, so after bulk traffic stops the share stays held until the age sweep; an entry's life in the region is bounded at 16 to 32 x max_bytes of traffic, so a loop beyond that serves nothing, which the guide does not yet say; the small-queue eviction added for a cache full of interactive entries has no test for a hot set first read while the region fills.

Refs: #3083, #3072, #2738

[pmoust]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: 1559b0f9-eb7e-4866-88bc-06f740675b1a
Commit: 06f8068
Model: claude-opus-5 (effort medium)
Executor: ravel-clickbench-ubuntu-2
Status: done. Review posted.

[claude-fleet]

Request changes: one should_fix finding on the admission path the feature is built for.

Findings:

• should_fix, crates/ravel-cache/src/cache.rs:514 — a declined bulk admission still pays owned_exact, copying the whole payload and counting it as an admission copy
• nit, crates/ravel-cache/src/s3fifo.rs:447 — a Maintenance/BulkScan admission leaves the key in ghost_set, so ghost and ghost_set can drift
• nit, crates/ravel-cache/src/lib.rs:149 — AccessIntent::as_str has no caller in the workspace

Verified:

• Interactive behaviour is unchanged with an empty region: charged_bytes() equals total_bytes() when bookkeeping_bytes() is 0 — s3fifo.rs:338-345, 820; promote_to_main adds a term that is true for the default intent — s3fifo.rs:447; the final else of evict_to_bounds returns Step::Empty from evict_bulk_top on an empty region — s3fifo.rs:832, 888-891.
• Bookkeeping is charged inside max_bytes and held to 1%: both admission paths test against bookkeeping_budget = max_bytes / 100 — s3fifo.rs:316, 525-534, 630-633.
• A bulk admission never evicts main: the room loop calls only evict_bulk and evict_from_small — s3fifo.rs:486-501.
• Per-intent hits sum to the totals on a RAM tier: record_hit_with_intent is the only caller of record_hit outside the disk tier — metrics.rs:52, disk.rs:811.
• The max-age sweep keeps region accounting consistent: drain_where goes through remove, which clears by_rank and region.bytes for a bulk entry — s3fifo.rs:775-782, 799-812.
• The guide's "Probation is a tenth of the cap" matches small_quota_bytes = (max_bytes / 10).max(1) — s3fifo.rs:280, docs/guides/caching.md:164-172.
• Cache::get_uncounted and Cache::admit changed signature but are pub(crate), so the "callers compile unchanged" claim does not rest on them — cache.rs:224, 275.

Checks run:

• cargo test -p ravel-cache → ok, 104 passed, 0 failed (includes all nine new s3fifo tests and the tiered disk-hit intent test).

Smaller things:

• Inner::lookup now calls bookkeeping_bytes() (two len() reads) and an atomic store on every RAM lookup while holding the fifo lock — cache.rs:500-501. On the interactive path it stores 0 forever. A cheap guard (store only when the region is non-empty, or when the value changed) keeps the hot path as it was.
• In track_tenant, evict_from_small is called without the !self.small.is_empty() guard the other call sites use — s3fifo.rs:642. It is safe today because an empty small queue returns Step::Empty, which the match turns into return false, but the asymmetry with evict_to_bounds's guarded calls invites a future edit that loops.
• A bulk admission can promote a small-queue entry with freq > 0 into main via evict_from_small's Step::Skipped branch — s3fifo.rs:495, 855-860. Not a defect (the entry earned the promotion), but "a bulk admission never evicts a main-queue interactive entry" in the description is about eviction only; main can still grow because of bulk traffic.
• limits.rs:71-78 says values outside 0.0..=1.0 are clamped. f64::clamp returns NaN for a NaN input, and NaN as u64 saturates to 0, so a NaN bulk_share silently disables the region rather than clamping. No caller can produce it today.

Not checked:

• That ravel-query, ravel-catalog and ravel-server still compile against the changed crate (only cargo test -p ravel-cache was run; the pub(crate) signature changes make a break unlikely, and CI covers it).
• The sql / flight-sql / esql feature lanes and the workspace gate.
• ADR-3072 decision 3b and ADR-2677's decision 3 deferral themselves; the PR's claims about them were taken as written.
• The executor's mutant results and the quoted hit-rate figures beyond re-running the committed tests.

Findings outside the diff:

• crates/ravel-cache/src/cache.rs:514 (should_fix): Inner::insert runs owned_exact before the eviction structure decides, so a bulk admission the region declines still pays the copy and records it as an admission copy.
  Failure: a BulkScan read of a 25 MB object whose rank is above bulk_rank_bound, with fetched Bytes that are shared or over-capacity (the ordinary object-store case), copies all 25 MB in copy_counted and then insert_with_intent returns (false, _) at s3fifo.rs:484 without storing anything. Under the workload this feature exists for -- a loop larger than the region, where the rank bound declines most keys by design -- that is a full memcpy per declined read, and admission_copies/admission_copied_bytes (cache.rs:61-64) count copies for entries that were never admitted, so the metric no longer means what its name says.
  Evidence: cache.rs:509-519 copies first and calls the fifo second; s3fifo.rs:483-485 and 500 are the two decline returns; the over-size decline at cache.rs:511-512 already skips the copy for exactly this reason, and the new decline path was not given the same exemption.
  Fix: ask the fifo whether it would admit before copying (a would_admit(key, size, intent) that reproduces the rank-bound and bulk_fits checks), or move the copy inside insert_with_intent past the decline points.

Task 1559b0f9-eb7e-4866-88bc-06f740675b1a on ravel-clickbench-ubuntu-2, model claude-opus-5, effort medium.

[claude-fleet]

nit: A Maintenance admission short-circuits before ghost_set.remove(&key), so a key can be resident and still in the ghost queue at the same time; BulkScan returns at line 443-445 and skips the removal too.
Failure: a maintenance read admits key K to small (K stays in ghost_set and in the ghost deque), K is evicted from small again, and lines 863-864 push a second copy of K into ghost. The trim at 865-869 then pops the older copy and calls ghost_set.remove(&K) while the newer copy is still in the deque, so ghost and ghost_set disagree and a later interactive re-admission of K fails to promote even though the deque still remembers it. Before this change a re-admission always cleared the ghost entry, so a duplicate could not be created. No production impact, since nothing sets Maintenance or BulkScan yet.
Evidence: s3fifo.rs:443-450 (both early exits), 863-869 (push and trim).
Fix: remove the key from ghost_set/ghost for every intent and use the removal's result only to decide promote_to_main.

[claude-fleet]

nit: AccessIntent::as_str has no caller anywhere in the workspace.
Failure: none at runtime; it is a public method documented as "the intent's metric label value" while no metric emits a label, so the first reader to wire up labels has no way to tell whether these strings are already a published contract.
Evidence: grep -rn "as_str()" crates/ravel-cache/src returns nothing, and a workspace grep for IntentMetricsSnapshot/intent_snapshot outside crates/ravel-cache/src returns nothing. Either land it with the exporter in #3084 or say in the doc comment that it is unused until then.

[pmoust]

Round-1 findings fixed in 6d6a2a6 (fleet fix round 7a538f81, re-authored here): the cache decides bulk admission before copying (insert_with_intent_lazy takes the value as a closure and builds and counts the copy only where the entry is stored, since insert_bulk mutates tenant and window state before deciding, so a separate predicate could disagree; interactive and maintenance values are still copied before the lock); a Maintenance or BulkScan admission now clears the ghost entry for every intent and only Interactive promotes; as_str is documented as unused until #3084; the lookup stores the bookkeeping gauge only when it changed; track_tenant's small-queue eviction carries the empty guard; a NaN bulk_share reads as the default; the guide says an interactive hit on a region entry moves it to probation and states the 16 to 32 x max_bytes life bound with a test (a_bulk_loop_past_32_times_the_cap_is_served_nothing). Seven mutants each fail a named test. One trade-off to know: an admitted bulk value that needs a copy is now copied under the cache lock. Executor gates: every lane green, the ravel-bench lane rerun by hand after a transient dep-info error. Head moved; review re-requested below.

[pmoust]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: 4b6b1fef-a631-40a9-a074-515a3b748f82
Commit: 6d6a2a6
Model: claude-opus-5 (effort medium)
Executor: rp1
Status: done. Review posted.

[claude-fleet]

Previous round at 06f8068: 3 findings.

Previous finding	Classification	Cited line
crates/ravel-cache/src/cache.rs:514 (should_fix): Inner::insert runs owned_exact before the eviction structure decides, so a bulk admission the region declines still pays the copy and records it as an admission copy. Failure: a BulkScan read of a 25 MB object whose rank is above bulk_rank_bound, with fetched Bytes that are shared or over-capacity (the ordinary object-store case), copies all 25 MB in copy_counted and then insert_with_intent returns (false, _) at s3fifo.rs:484 without storing anything. Under the workload this feature exists for -- a loop larger than the region, where the rank bound declines most keys by design -- that is a full memcpy per declined read, and admission_copies/admission_copied_bytes (cache.rs:61-64) count copies for entries that were never admitted, so the metric no longer means what its name says. Evidence: cache.rs:509-519 copies first and calls the fifo second; s3fifo.rs:483-485 and 500 are the two decline returns; the over-size decline at cache.rs:511-512 already skips the copy for exactly this reason, and the new decline path was not given the same exemption. Fix: ask the fifo whether it would admit before copying (a would_admit(key, size, intent) that reproduces the rank-bound and bulk_fits checks), or move the copy inside insert_with_intent past the decline points.	fixed	crates/ravel-cache/src/cache.rs:518
crates/ravel-cache/src/s3fifo.rs:447 (nit): A Maintenance admission short-circuits before ghost_set.remove(&key), so a key can be resident and still in the ghost queue at the same time; BulkScan returns at line 443-445 and skips the removal too. Failure: a maintenance read admits key K to small (K stays in ghost_set and in the ghost deque), K is evicted from small again, and lines 863-864 push a second copy of K into ghost. The trim at 865-869 then pops the older copy and calls ghost_set.remove(&K) while the newer copy is still in the deque, so ghost and ghost_set disagree and a later interactive re-admission of K fails to promote even though the deque still remembers it. Before this change a re-admission always cleared the ghost entry, so a duplicate could not be created. No production impact, since nothing sets Maintenance or BulkScan yet. Evidence: s3fifo.rs:443-450 (both early exits), 863-869 (push and trim). Fix: remove the key from ghost_set/ghost for every intent and use the removal's result only to decide promote_to_main.	fixed	crates/ravel-cache/src/s3fifo.rs:463
crates/ravel-cache/src/lib.rs:149 (nit): AccessIntent::as_str has no caller anywhere in the workspace. Failure: none at runtime; it is a public method documented as "the intent's metric label value" while no metric emits a label, so the first reader to wire up labels has no way to tell whether these strings are already a published contract. Evidence: grep -rn "as_str()" crates/ravel-cache/src returns nothing, and a workspace grep for IntentMetricsSnapshot/intent_snapshot outside crates/ravel-cache/src returns nothing. Either land it with the exporter in #3084 or say in the doc comment that it is unused until then.	fixed	crates/ravel-cache/src/lib.rs:148

Re-reviewed at 6d6a2a6. All three findings from the previous round are addressed; the new commit introduces one issue of its own.

Request changes: one should_fix finding.

Findings:

• should_fix, crates/ravel-cache/src/cache.rs:530 — the bulk admission's memcpy now runs while the fifo mutex is held
• nit, crates/ravel-cache/src/cache.rs:532 — a copy made for an already-resident interactive key is no longer counted

Previous round:

• 1 (bulk copy before the decision) — fixed. Inner::insert no longer pre-copies a BulkScan value (cache.rs:517-523) and builds it in the make closure the fifo runs only when it stores (cache.rs:527-540, s3fifo.rs:442-455). The decline paths at s3fifo.rs:505-508 and 521-526 return before make().
• 2 (ghost entry left behind) — fixed. forget_ghost runs on every storing admission: s3fifo.rs:463 for interactive/maintenance, s3fifo.rs:533 for bulk, with the promotion decision taken from its result rather than gating it.
• 3 (as_str has no caller) — fixed as "say so in the doc comment", lib.rs:148-149.

Verified:

• "A bulk read the region declines is not copied into the cache" — cache.rs:518-523 skips the pre-copy for BulkScan, and the only owned_exact for that intent is inside the closure at cache.rs:530.
• "an admission that stores a key, whatever its intent, drops it from the ghost queue" (s3fifo.rs:47-50) — s3fifo.rs:463, 482-491, 533.
• NaN bulk_share reads as the default rather than collapsing the region to zero — limits.rs:117-128, applied at s3fifo.rs:305.
• The guide's new life-of-a-region-entry paragraph (docs/guides/caching.md:210-219) matches the code: window = clamp(2 * pass_bytes, 2 * max_bytes, 16 * max_bytes) (s3fifo.rs:112-113, 628-629), an entry surviving one to two windows (s3fifo.rs:866-871), so 16-32x for a loop at or past 8x.
• The added !self.small.is_empty() guard in track_tenant (s3fifo.rs:668-670) changes no outcome: evict_from_small on an empty queue already returned Step::Empty, which the match turns into the same return false.

Checks run:

• cargo test -p ravel-cache --lib → ok, 109 passed.
• Reverted cache.rs:518 to the pre-fix order (copy every in-size value before the lock) → a_declined_bulk_admission_copies_nothing_and_counts_no_admission_copy fails at cache.rs:1329, "the value itself". Restored.
• Reverted s3fifo.rs:463 to intent == AccessIntent::Interactive && self.ghost_set.remove(&key) → a_maintenance_or_bulk_admission_clears_the_ghost_entry fails at s3fifo.rs:1699, "Maintenance admission left the key in the ghost queue". Restored.

Smaller things:

• The FLIP note on the new cache.rs test (cache.rs:1277-1282) claims the pre-fix order makes "both declined admissions count a copy". It does not: the counting also moved into the closure, so under that one-line revert admission_copies() stays 0 and only the returned.as_ptr() assertion fails. That is still a real failure, so the test is non-vacuous, but the note names assertions that do not fire.
• Cache::admit's doc (cache.rs:273-275) still says it returns value itself only "if it was over the size limit". It now also does so for a bulk admission the region declined, which insert_with_intent (cache.rs:267-268) does say.

Not checked: the feature lanes and the rest of the workspace (CI's job), and the parts of the PR that are unchanged since 06f8068 beyond re-reading the current files — the previous round covered them.

Not checked:

• the sql/flight-sql/esql feature lanes and the rest of the workspace build (CI's job)
• parts of the PR unchanged since 06f8068 beyond re-reading the current files

Task 4b6b1fef-a631-40a9-a074-515a3b748f82 on rp1, model claude-opus-5, effort medium.

[claude-fleet]

should_fix: Moving the bulk copy into the make closure fixes the wasted copy but runs that copy while self.fifo is locked, which is the same mutex every interactive read takes.
Failure: a BulkScan admission of a 25 MB object whose Bytes are shared or over-capacity (the ordinary object-store case) now runs Bytes::copy_from_slice of 25 MB at cache.rs:530 inside the fifo.insert_with_intent_lazy call at cache.rs:527, with the guard taken at cache.rs:526 still held. Inner::lookup takes that same lock (cache.rs:477), so every concurrent interactive get blocks for the duration of the memcpy, a few milliseconds per admitted bulk object. Before this commit the copy happened before the lock was taken, and the interactive path still does (cache.rs:517-523). That is a tail-latency regression on exactly the workload this feature exists to protect: interactive reads served while a bulk scan runs. No production impact until #3084 sets BulkScan.
Evidence: cache.rs:526 takes the lock, 527-540 calls the closure under it, 530 is the copy; cache.rs:477 and 526 are the same self.fifo. Pre-fix order at 06f8068 had the single owned_exact call above the lock() line.
Fix: keep the decision cheap and the copy outside the lock -- take the lock, run the rank-bound and room checks (would_admit(key, size, intent)), drop it, copy, then admit; a key that stops fitting between the two is just an ordinary decline.

[claude-fleet]

nit: record_admission_copy now runs only inside the closure, so a copy made for an admission the fifo declines as already-resident is performed and never counted.
Failure: an interactive admit of a key already in RAM with shared Bytes copies the whole value at cache.rs:519, then insert_with_intent_lazy returns at s3fifo.rs:457-460 without calling the closure, so admission_copies/admission_copied_bytes stay flat. The memcpy still happened, and Cache::insert's doc (cache.rs:255-259) tells the reader those copies are counted by CacheMetrics::admission_copies. Before this commit owned_exact counted at copy time, so this case was visible. Reachable today without #3084: TieredCache::insert_with_intent and the resolve_peeked_miss recheck race both re-admit resident keys.
Evidence: cache.rs:519 copies, cache.rs:532-534 counts only under the closure, s3fifo.rs:457-460 is the resident early return.
Fix: either count the pre-lock copy where it is made, or say in the insert doc that the counter tracks copies that were stored.

[pmoust]

Round 2 acknowledged: the copy-under-the-lock finding is real (it was the trade-off the fix round named) and gets the fix you describe, a cheap would_admit under the lock, the copy outside it, then an admission that re-checks and may decline; the uncounted resident-key copy is settled either way with the doc and code agreeing. Second fix round dispatched from this head; the next review is round 3.

[pmoust]

Round 2's two findings are fixed in 7638e25 (fleet task 9ad6168b, folded onto the previous head 6d6a2a6 as one re-authored commit, tree identical to the task result).

• The bulk copy no longer runs under the fifo lock. insert_checked_then asks S3Fifo::would_admit under the lock (&self, mutates nothing; it runs insert_bulk's own steps against the tenant's pass as the read would leave it: rank collision, rank bound, and the room the eviction loop would make, through the shared TenantPass::observe, window_full, bulk_rank_bound_with and bulk_fits_with). A declined read copies nothing and is declined there. An accepted read is copied with the lock released and admitted under a fresh lock, where insert_bulk rechecks and may still decline. Interactive and maintenance reads copy before taking the lock, as before. would_admit answers yes without modelling the eviction for a tenant not yet tracked or a read that closes the window; a wrong yes costs one copy made outside the lock.
• The admission copy counters now count every copy made, where it is made, including a copy whose entry is then not stored (an interactive admission of a resident key, a bulk read declined on the recheck); the metrics.rs and cache.rs docs and the changelog say the same thing, which the interactive path already did.

Tests, each shown failing under a deliberate change and green after: a_stored_bulk_value_is_copied_outside_the_fifo_lock (copy moved back under the lock), a_bulk_admission_declined_on_recheck_is_an_ordinary_decline (recheck disabled), would_admit_predicts_the_bulk_admission (small-queue step removed from bulk_room; rank bound without the replaced tenant), the_admission_copy_counter_matches_its_doc (interactive copy counted only when not resident), would_admit_never_mutates (pins state; a mutating version does not compile). Executor gates: fmt, every clippy lane, closure nextest 8,366, sql 1,810, flight-sql 3,496, esql 1,831, bench lanes 692 and 399, stage-timing 1,115, cli sql-explain 715, all green, run one lane at a time after the volume filled; PR CI is the gate.

This is review round 3 on this pull request.

[pmoust]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: a5109399-0dc8-4f06-bbe0-fd3c60d707cf
Commit: 7638e25
Model: claude-opus-5 (effort medium)
Executor: ci-16gb-fsn1-1
Status: done. Review posted.

[claude-fleet]

Previous round at 6d6a2a6: 5 findings.

Previous finding	Classification	Cited line
crates/ravel-cache/src/cache.rs:518 (should_fix): Inner::insert runs owned_exact before the eviction structure decides, so a bulk admission the region declines still pays the copy and records it as an admission copy. Failure: a BulkScan read of a 25 MB object whose rank is above bulk_rank_bound, with fetched Bytes that are shared or over-capacity (the ordinary object-store case), copies all 25 MB in copy_counted and then insert_with_intent returns (false, _) at s3fifo.rs:484 without storing anything. Under the workload this feature exists for -- a loop larger than the region, where the rank bound declines most keys by design -- that is a full memcpy per declined read, and admission_copies/admission_copied_bytes (cache.rs:61-64) count copies for entries that were never admitted, so the metric no longer means what its name says. Evidence: cache.rs:509-519 copies first and calls the fifo second; s3fifo.rs:483-485 and 500 are the two decline returns; the over-size decline at cache.rs:511-512 already skips the copy for exactly this reason, and the new decline path was not given the same exemption. Fix: ask the fifo whether it would admit before copying (a would_admit(key, size, intent) that reproduces the rank-bound and bulk_fits checks), or move the copy inside insert_with_intent past the decline points.	fixed	crates/ravel-cache/src/cache.rs:545
crates/ravel-cache/src/s3fifo.rs:463 (nit): A Maintenance admission short-circuits before ghost_set.remove(&key), so a key can be resident and still in the ghost queue at the same time; BulkScan returns at line 443-445 and skips the removal too. Failure: a maintenance read admits key K to small (K stays in ghost_set and in the ghost deque), K is evicted from small again, and lines 863-864 push a second copy of K into ghost. The trim at 865-869 then pops the older copy and calls ghost_set.remove(&K) while the newer copy is still in the deque, so ghost and ghost_set disagree and a later interactive re-admission of K fails to promote even though the deque still remembers it. Before this change a re-admission always cleared the ghost entry, so a duplicate could not be created. No production impact, since nothing sets Maintenance or BulkScan yet. Evidence: s3fifo.rs:443-450 (both early exits), 863-869 (push and trim). Fix: remove the key from ghost_set/ghost for every intent and use the removal's result only to decide promote_to_main.	fixed	crates/ravel-cache/src/s3fifo.rs:592
crates/ravel-cache/src/lib.rs:148 (nit): AccessIntent::as_str has no caller anywhere in the workspace. Failure: none at runtime; it is a public method documented as "the intent's metric label value" while no metric emits a label, so the first reader to wire up labels has no way to tell whether these strings are already a published contract. Evidence: grep -rn "as_str()" crates/ravel-cache/src returns nothing, and a workspace grep for IntentMetricsSnapshot/intent_snapshot outside crates/ravel-cache/src returns nothing. Either land it with the exporter in #3084 or say in the doc comment that it is unused until then.	fixed	crates/ravel-cache/src/lib.rs:148
crates/ravel-cache/src/cache.rs:530 (should_fix): Moving the bulk copy into the make closure fixes the wasted copy but runs that copy while self.fifo is locked, which is the same mutex every interactive read takes. Failure: a BulkScan admission of a 25 MB object whose Bytes are shared or over-capacity (the ordinary object-store case) now runs Bytes::copy_from_slice of 25 MB at cache.rs:530 inside the fifo.insert_with_intent_lazy call at cache.rs:527, with the guard taken at cache.rs:526 still held. Inner::lookup takes that same lock (cache.rs:477), so every concurrent interactive get blocks for the duration of the memcpy, a few milliseconds per admitted bulk object. Before this commit the copy happened before the lock was taken, and the interactive path still does (cache.rs:517-523). That is a tail-latency regression on exactly the workload this feature exists to protect: interactive reads served while a bulk scan runs. No production impact until #3084 sets BulkScan. Evidence: cache.rs:526 takes the lock, 527-540 calls the closure under it, 530 is the copy; cache.rs:477 and 526 are the same self.fifo. Pre-fix order at 06f8068 had the single owned_exact call above the lock() line. Fix: keep the decision cheap and the copy outside the lock -- take the lock, run the rank-bound and room checks (would_admit(key, size, intent)), drop it, copy, then admit; a key that stops fitting between the two is just an ordinary decline.	fixed	crates/ravel-cache/src/cache.rs:551
crates/ravel-cache/src/cache.rs:532 (nit): record_admission_copy now runs only inside the closure, so a copy made for an admission the fifo declines as already-resident is performed and never counted. Failure: an interactive admit of a key already in RAM with shared Bytes copies the whole value at cache.rs:519, then insert_with_intent_lazy returns at s3fifo.rs:457-460 without calling the closure, so admission_copies/admission_copied_bytes stay flat. The memcpy still happened, and Cache::insert's doc (cache.rs:255-259) tells the reader those copies are counted by CacheMetrics::admission_copies. Before this commit owned_exact counted at copy time, so this case was visible. Reachable today without #3084: TieredCache::insert_with_intent and the resolve_peeked_miss recheck race both re-admit resident keys. Evidence: cache.rs:519 copies, cache.rs:532-534 counts only under the closure, s3fifo.rs:457-460 is the resident early return. Fix: either count the pre-lock copy where it is made, or say in the insert doc that the counter tracks copies that were stored.	fixed	crates/ravel-cache/src/cache.rs:569

No longer carried to the next round, settled in rounds 2 and 3:

• crates/ravel-cache/src/cache.rs:518 (should_fix): Inner::insert runs owned_exact before the eviction structure decides, so a bulk admission the region declines still pays the copy and records it as an admission copy. Failure: a BulkScan read of a 25 MB object whose rank is above bulk_rank_bound, with fetched Bytes that are shared or over-capacity (the ordinary object-store case), copies all 25 MB in copy_counted and then insert_with_intent returns (false, _) at s3fifo.rs:484 without storing anything. Under the workload this feature exists for -- a loop larger than the region, where the rank bound declines most keys by design -- that is a full memcpy per declined read, and admission_copies/admission_copied_bytes (cache.rs:61-64) count copies for entries that were never admitted, so the metric no longer means what its name says. Evidence: cache.rs:509-519 copies first and calls the fifo second; s3fifo.rs:483-485 and 500 are the two decline returns; the over-size decline at cache.rs:511-512 already skips the copy for exactly this reason, and the new decline path was not given the same exemption. Fix: ask the fifo whether it would admit before copying (a would_admit(key, size, intent) that reproduces the rank-bound and bulk_fits checks), or move the copy inside insert_with_intent past the decline points.
• crates/ravel-cache/src/s3fifo.rs:463 (nit): A Maintenance admission short-circuits before ghost_set.remove(&key), so a key can be resident and still in the ghost queue at the same time; BulkScan returns at line 443-445 and skips the removal too. Failure: a maintenance read admits key K to small (K stays in ghost_set and in the ghost deque), K is evicted from small again, and lines 863-864 push a second copy of K into ghost. The trim at 865-869 then pops the older copy and calls ghost_set.remove(&K) while the newer copy is still in the deque, so ghost and ghost_set disagree and a later interactive re-admission of K fails to promote even though the deque still remembers it. Before this change a re-admission always cleared the ghost entry, so a duplicate could not be created. No production impact, since nothing sets Maintenance or BulkScan yet. Evidence: s3fifo.rs:443-450 (both early exits), 863-869 (push and trim). Fix: remove the key from ghost_set/ghost for every intent and use the removal's result only to decide promote_to_main.
• crates/ravel-cache/src/lib.rs:148 (nit): AccessIntent::as_str has no caller anywhere in the workspace. Failure: none at runtime; it is a public method documented as "the intent's metric label value" while no metric emits a label, so the first reader to wire up labels has no way to tell whether these strings are already a published contract. Evidence: grep -rn "as_str()" crates/ravel-cache/src returns nothing, and a workspace grep for IntentMetricsSnapshot/intent_snapshot outside crates/ravel-cache/src returns nothing. Either land it with the exporter in #3084 or say in the doc comment that it is unused until then.

Request changes: one should_fix finding on the bulk eviction loop, plus a stale line in the guide.

The previous round's five findings are all addressed: the bulk decision now runs before the copy, the copy runs with the fifo lock dropped, every copy is counted where it is made, forget_ghost runs for all three intents, and as_str says it is unused until #3084. would_admit is a faithful model of insert_bulk's checks as far as I could follow them, and would_admit_predicts_the_bulk_admission pins the direction that matters (never false where the admission stores).

Findings:

• should_fix, crates/ravel-cache/src/s3fifo.rs:576 — a bulk admission blocked by the bookkeeping budget drains the whole small queue and admits nothing
• nit, docs/guides/caching.md:220 — "a bulk read the region declines is not copied" is no longer true for a recheck decline

Verified:

• Previous finding 1 and 4: the bulk decision is made under the lock at cache.rs:544-550, the lock is dropped at 551 and the copy made at 553, so no admission copy runs under self.fifo — the same mutex Inner::lookup takes at cache.rs:484.
• Previous finding 5: owned_copy (cache.rs:569-576) counts at the copy site, so a copy for a key the fifo finds resident is counted; Cache::insert's doc (cache.rs:254-259) now states exactly that.
• Previous finding 2: forget_ghost runs on the interactive/maintenance path (s3fifo.rs:484) and on the bulk path (s3fifo.rs:592), with the result used only to decide promotion.
• "Interactive is exactly today's policy": every pre-existing entry point delegates with AccessIntent::Interactive (cache.rs:205-208, 231-234, 255-262; tiered.rs:287-288, 326-333, 475-482, 610-613, 652-656, 681-686, 722-728).
• Per-intent hits sum to the totals: record_hit_with_intent calls record_hit first (metrics.rs:51-56), and the only RAM hit site is Inner::get (cache.rs:468-480).

Smaller things:

• insert_checked_then's between parameter exists only so one test can mutate the region between the check and the copy. It is cheap and documented, but it is a test seam in production code; a #[cfg(test)] variant would keep Inner::insert the single shape.
• bulk_room uses non-saturating -= on total, region and bookkeeping. The subtractions are bounded by the structures they iterate, so they cannot underflow as long as a key is never in small and by_rank twice; worth a saturating form anyway, since a debug build turns any future accounting drift into a panic on a read path.

Not checked:

• I ran no cargo command: no build, no tests, no revert check. The scenario in the finding is from reading insert_bulk and bulk_fits_with, not from an executed test.
• The executor's claimed mutant runs, the 11.5 GB geometry test figures, and the nextest/jemalloc result in the PR description.
• crates/ravel-cache/src/disk.rs and the single-flight path beyond the intent plumbing shown in the diff.

Not checked:

• No cargo command run: no build, no test, no revert check; the should_fix finding is from reading insert_bulk and bulk_fits_with, not from an executed test
• The executor's mutant runs, the 11.5 GB geometry hot-set figures, and the nextest/jemalloc results claimed in the PR description
• crates/ravel-cache/src/disk.rs and the single-flight internals beyond the intent plumbing visible in the diff

Task a5109399-0dc8-4f06-bbe0-fd3c60d707cf on ci-16gb-fsn1-1, model claude-opus-5, effort medium.

[claude-fleet]

should_fix: insert_bulk's eviction loop evicts small-queue entries to satisfy a bookkeeping-budget shortfall that those evictions cannot change, so it drains the entire probation queue and then declines anyway.
Failure: the region is at its bookkeeping cap (reached first whenever region entries are smaller than about 9.6 KB: the cap allows max_bytes/19200 entries while the byte share allows 0.5*max_bytes/size), the tenant's pass fits the region capacity so bulk_rank_bound is full and the rank check passes, and the arriving key's rank is above every resident region rank. Line 569-571 finds no higher-ranked victim, so each iteration falls to line 576: region.bytes + size <= region_share_bytes holds (the region is bookkeeping-bound, not byte-bound) and evict_from_small runs. But bulk_fits_with fails on its first term, bookkeeping + REGION_ENTRY_BOOKKEEPING_BYTES <= self.bookkeeping_budget (s3fifo.rs:623-624), which no small-queue eviction moves. The loop therefore evicts or promotes every entry in the small queue, returns (false, evicted) at line 582, and stores nothing. Concretely at max_bytes = 1 GB with 4 KB bulk objects: the region caps at 54,613 entries, and a bulk read whose rank happens to top the resident set evicts every once-read interactive entry in probation (up to the 10% small quota) for an admission that never happens. would_admit models the same loop and returns false (s3fifo.rs:643-666), but the decline path still calls insert_with_intent_lazy (cache.rs:546), which runs the real loop, so the new check avoids the copy and not the drain. No production impact until #3084 sets BulkScan.
Evidence: s3fifo.rs:568-583 (the loop), 611-627 (bulk_fits_with, the bookkeeping term), 996-1021 (evict_from_small), 357-360 (bookkeeping is tenants and region entries only), cache.rs:544-550 (the decline path still runs the loop).
Fix: before the small-queue step, decline when bookkeeping_bytes() + REGION_ENTRY_BOOKKEEPING_BYTES > bookkeeping_budget and no higher-ranked region entry is available, since only a region eviction can relieve that term.

[claude-fleet]

nit: The guide still says a declined bulk read is never copied, which this commit made untrue for the recheck decline.
Failure: an operator reading "A bulk read the region declines is not copied" concludes ravel_cache_admission_copies_total counts only stored entries, while a read would_admit accepts and the admission's own recheck then declines is copied and counted (cache.rs:551-562, 569-576).
Evidence: docs/guides/caching.md:220-221 against cache.rs:254-259, which the same commit updated to the new wording, and changelog.d/3083.added.md:7-9, which says "declines when it arrives". Mirror that qualifier in the guide.

[pmoust]

Owner decision: a fourth round rather than merging with the finding. Fix round dispatched as fleet task c04ab7aa (opus) from this head 7638e25: decline before the small-queue step when the bookkeeping term cannot be relieved by a small-queue eviction (only a region eviction can), the would_admit-declined path no longer runs the real eviction loop, the caching guide gets the "when it arrives" qualifier and the recheck-copy sentence, and AccessIntent::as_str says nothing emits the label until #3084. Tests pin the small queue's population across a declined admission and the region-eviction case that must still admit. Round 4 is requested when it lands.

[pmoust]

Round 3's should_fix and both nits are fixed in the commit on top of 7638e25 (fleet task c04ab7aa, one re-authored commit, tree identical to the task result).

• insert_bulk's loop declines, after the higher-ranked region step and before the small-queue step, when bookkeeping + REGION_ENTRY_BOOKKEEPING_BYTES > bookkeeping_budget: only a region eviction or a tenant retirement can relieve that term, so the small queue is left alone. bulk_room mirrors the check so would_admit stays in step. At the bookkeeping cap the review's shape now leaves all 20 once-read interactive entries in the small queue (the PR head evicted all 20 and still declined), and the resident population stays 227 with 31 region entries (PR head: 207 and 30).
• The decline path in cache.rs no longer runs the eviction loop: a read would_admit declines goes to the new S3Fifo::decline_bulk, which only records the read in the tenant's sketch; the accept path's copy outside the lock and the recheck are unchanged.
• docs/guides/caching.md says a bulk read declined when it arrives is not copied and evicts nothing, and that a read accepted before the copy and declined by the recheck is copied and counted; the changelog fragment matches.
• AccessIntent::as_str removed (nothing in the workspace, tests included, called it).

Mutants, each shown failing then reverted: the PR head logic (small queue 0 vs 20; resident 207 vs 227; region 30 vs 31), the check placed after one small-queue eviction (19 vs 20), the check placed before the region step (the higher-ranked region entry must still be evicted and the read admitted), the decline path still calling insert_with_intent_lazy (30 vs 31). Class closure: bulk_fits_with's three terms mapped to the eviction step that relieves each; track_tenant, evict_to_bounds and evict_from_main unchanged with reasons. Executor gates: every lane green (run one at a time after its own target filled the volume). No local gate here; CI is the gate.

This is review round 4, requested on the owner's decision.

[pmoust]

@claude-fleet review

[claude-fleet]

Review task queued.

Task: 5bf7685c-5f71-43ab-9384-920450ff54d8
Commit: 5096b8c
Model: claude-opus-5 (effort medium)
Executor: rp1
Status: done. Review posted.

[claude-fleet]

Previous round at 7638e25: 4 findings.

Previous finding	Classification	Cited line
crates/ravel-cache/src/cache.rs:551 (should_fix): Moving the bulk copy into the make closure fixes the wasted copy but runs that copy while self.fifo is locked, which is the same mutex every interactive read takes. Failure: a BulkScan admission of a 25 MB object whose Bytes are shared or over-capacity (the ordinary object-store case) now runs Bytes::copy_from_slice of 25 MB at cache.rs:530 inside the fifo.insert_with_intent_lazy call at cache.rs:527, with the guard taken at cache.rs:526 still held. Inner::lookup takes that same lock (cache.rs:477), so every concurrent interactive get blocks for the duration of the memcpy, a few milliseconds per admitted bulk object. Before this commit the copy happened before the lock was taken, and the interactive path still does (cache.rs:517-523). That is a tail-latency regression on exactly the workload this feature exists to protect: interactive reads served while a bulk scan runs. No production impact until #3084 sets BulkScan. Evidence: cache.rs:526 takes the lock, 527-540 calls the closure under it, 530 is the copy; cache.rs:477 and 526 are the same self.fifo. Pre-fix order at 06f8068 had the single owned_exact call above the lock() line. Fix: keep the decision cheap and the copy outside the lock -- take the lock, run the rank-bound and room checks (would_admit(key, size, intent)), drop it, copy, then admit; a key that stops fitting between the two is just an ordinary decline.	fixed	crates/ravel-cache/src/cache.rs:555
crates/ravel-cache/src/cache.rs:569 (nit): record_admission_copy now runs only inside the closure, so a copy made for an admission the fifo declines as already-resident is performed and never counted. Failure: an interactive admit of a key already in RAM with shared Bytes copies the whole value at cache.rs:519, then insert_with_intent_lazy returns at s3fifo.rs:457-460 without calling the closure, so admission_copies/admission_copied_bytes stay flat. The memcpy still happened, and Cache::insert's doc (cache.rs:255-259) tells the reader those copies are counted by CacheMetrics::admission_copies. Before this commit owned_exact counted at copy time, so this case was visible. Reachable today without #3084: TieredCache::insert_with_intent and the resolve_peeked_miss recheck race both re-admit resident keys. Evidence: cache.rs:519 copies, cache.rs:532-534 counts only under the closure, s3fifo.rs:457-460 is the resident early return. Fix: either count the pre-lock copy where it is made, or say in the insert doc that the counter tracks copies that were stored.	fixed	crates/ravel-cache/src/cache.rs:576
crates/ravel-cache/src/s3fifo.rs:576 (should_fix): insert_bulk's eviction loop evicts small-queue entries to satisfy a bookkeeping-budget shortfall that those evictions cannot change, so it drains the entire probation queue and then declines anyway. Failure: the region is at its bookkeeping cap (reached first whenever region entries are smaller than about 9.6 KB: the cap allows max_bytes/19200 entries while the byte share allows 0.5*max_bytes/size), the tenant's pass fits the region capacity so bulk_rank_bound is full and the rank check passes, and the arriving key's rank is above every resident region rank. Line 569-571 finds no higher-ranked victim, so each iteration falls to line 576: region.bytes + size <= region_share_bytes holds (the region is bookkeeping-bound, not byte-bound) and evict_from_small runs. But bulk_fits_with fails on its first term, bookkeeping + REGION_ENTRY_BOOKKEEPING_BYTES <= self.bookkeeping_budget (s3fifo.rs:623-624), which no small-queue eviction moves. The loop therefore evicts or promotes every entry in the small queue, returns (false, evicted) at line 582, and stores nothing. Concretely at max_bytes = 1 GB with 4 KB bulk objects: the region caps at 54,613 entries, and a bulk read whose rank happens to top the resident set evicts every once-read interactive entry in probation (up to the 10% small quota) for an admission that never happens. would_admit models the same loop and returns false (s3fifo.rs:643-666), but the decline path still calls insert_with_intent_lazy (cache.rs:546), which runs the real loop, so the new check avoids the copy and not the drain. No production impact until #3084 sets BulkScan. Evidence: s3fifo.rs:568-583 (the loop), 611-627 (bulk_fits_with, the bookkeeping term), 996-1021 (evict_from_small), 357-360 (bookkeeping is tenants and region entries only), cache.rs:544-550 (the decline path still runs the loop). Fix: before the small-queue step, decline when bookkeeping_bytes() + REGION_ENTRY_BOOKKEEPING_BYTES > bookkeeping_budget and no higher-ranked region entry is available, since only a region eviction can relieve that term.	fixed	crates/ravel-cache/src/s3fifo.rs:601
docs/guides/caching.md:220 (nit): The guide still says a declined bulk read is never copied, which this commit made untrue for the recheck decline. Failure: an operator reading "A bulk read the region declines is not copied" concludes ravel_cache_admission_copies_total counts only stored entries, while a read would_admit accepts and the admission's own recheck then declines is copied and counted (cache.rs:551-562, 569-576). Evidence: docs/guides/caching.md:220-221 against cache.rs:254-259, which the same commit updated to the new wording, and changelog.d/3083.added.md:7-9, which says "declines when it arrives". Mirror that qualifier in the guide.	fixed	docs/guides/caching.md:220

No longer carried to the next round, settled in rounds 3 and 4:

• crates/ravel-cache/src/cache.rs:551 (should_fix): Moving the bulk copy into the make closure fixes the wasted copy but runs that copy while self.fifo is locked, which is the same mutex every interactive read takes. Failure: a BulkScan admission of a 25 MB object whose Bytes are shared or over-capacity (the ordinary object-store case) now runs Bytes::copy_from_slice of 25 MB at cache.rs:530 inside the fifo.insert_with_intent_lazy call at cache.rs:527, with the guard taken at cache.rs:526 still held. Inner::lookup takes that same lock (cache.rs:477), so every concurrent interactive get blocks for the duration of the memcpy, a few milliseconds per admitted bulk object. Before this commit the copy happened before the lock was taken, and the interactive path still does (cache.rs:517-523). That is a tail-latency regression on exactly the workload this feature exists to protect: interactive reads served while a bulk scan runs. No production impact until #3084 sets BulkScan. Evidence: cache.rs:526 takes the lock, 527-540 calls the closure under it, 530 is the copy; cache.rs:477 and 526 are the same self.fifo. Pre-fix order at 06f8068 had the single owned_exact call above the lock() line. Fix: keep the decision cheap and the copy outside the lock -- take the lock, run the rank-bound and room checks (would_admit(key, size, intent)), drop it, copy, then admit; a key that stops fitting between the two is just an ordinary decline.
• crates/ravel-cache/src/cache.rs:569 (nit): record_admission_copy now runs only inside the closure, so a copy made for an admission the fifo declines as already-resident is performed and never counted. Failure: an interactive admit of a key already in RAM with shared Bytes copies the whole value at cache.rs:519, then insert_with_intent_lazy returns at s3fifo.rs:457-460 without calling the closure, so admission_copies/admission_copied_bytes stay flat. The memcpy still happened, and Cache::insert's doc (cache.rs:255-259) tells the reader those copies are counted by CacheMetrics::admission_copies. Before this commit owned_exact counted at copy time, so this case was visible. Reachable today without #3084: TieredCache::insert_with_intent and the resolve_peeked_miss recheck race both re-admit resident keys. Evidence: cache.rs:519 copies, cache.rs:532-534 counts only under the closure, s3fifo.rs:457-460 is the resident early return. Fix: either count the pre-lock copy where it is made, or say in the insert doc that the counter tracks copies that were stored.

Approve: every previous finding is fixed, one new nit.

The two fixes both land where they should: the decline path no longer runs the admission at all (decline_bulk counts the read against the tenant pass and stops), and the admission loop refuses before the small-queue step when only a region eviction could relieve the bookkeeping term. I verified each with its own revert, and the invariant that would_admit is never false where the admission would store survives the change, since the new check sits at the same position in bulk_room as in insert_bulk.

Findings:

• nit, docs/guides/caching.md:208 — "up to half of the cap" is unreachable for bulk objects under ~9.6 KB

Verified:

• The accepted bulk copy now happens with the lock released — cache.rs:553-555 drops the guard, then owned_copy; the admission retakes it at 563.
• Every copy is counted where it is made, stored or not — cache.rs:571-578, with Cache::insert's doc updated to match at cache.rs:252-262 and record_admission_copy's at metrics.rs:129-133.
• decline_bulk makes none of the admission's evictions, and its window-rotation return is unreachable from the decline path: would_admit returns true at s3fifo.rs:527-528 for a read that would close the window, so a read that reaches decline_bulk cannot rotate — s3fifo.rs:542-556, 764-796.
• The decline for an untracked tenant changing nothing is sound: would_admit is false there only when TENANT_BOOKKEEPING_BYTES + REGION_ENTRY_BOOKKEEPING_BYTES > bookkeeping_budget, i.e. max_bytes under ~224 KB — s3fifo.rs:519-522.
• The guide and changelog now carry the "when it arrives" qualifier and state that the counters count copies made, not entries stored — docs/guides/caching.md:220-226, changelog.d/3083.added.md:7-10.
• AccessIntent::as_str is gone and nothing referenced it: grep -rn "as_str()" crates/ravel-cache/src returns nothing.

Checks run:

• cargo test -p ravel-cache --lib → ok, 118 passed.
• Removed the bookkeeping guard at s3fifo.rs:599-603 → a_bulk_read_at_the_bookkeeping_cap_leaves_the_small_queue_alone fails on "small-queue population", left 0 right 20; a_bulk_read_at_the_bookkeeping_cap_evicts_a_higher_ranked_entry still passes, so the guard is not placed too early. Restored.
• Reverted cache.rs:548 to insert_with_intent_lazy(..., || None) → a_declined_bulk_read_evicts_no_region_entry fails on "no region entry evicted", left 30 right 31. Restored; git status --porcelain clean.

Smaller things:

• decline_bulk's return value is documented as the keys a window rotation evicted, and the one caller discards it with a comment about cleanup. Given the point above it is always empty on that path; a debug_assert or a sentence saying so would save the next reader the derivation.
• copy_probe::record(self.fifo.is_locked()) (cache.rs:573) reads whether any thread holds the lock, not this one. Fine for the single-threaded tests that read it, worth a word if it ever gets used from a concurrent test.

Not checked:

• Clippy and the feature lanes (CI's job), and the crate's doctests and integration tests — only --lib ran here.
• The 11.5 GB server-geometry and 2,000-key loop results quoted in the description; I read the tests that encode them but did not re-run the executor's mutant set.

Not checked:

• clippy and the sql/flight-sql/esql/ravel-bench feature lanes (CI's gate)
• ravel-cache doctests and integration tests; only cargo test -p ravel-cache --lib ran
• the 11.5 GB server-geometry and 2,000-key loop figures quoted in the description, and the executor's mutant set

Task 5bf7685c-5f71-43ab-9384-920450ff54d8 on rp1, model claude-opus-5, effort medium.

[claude-fleet]

nit: The guide says the bulk region "holds up to half of the cap", but the 1% bookkeeping budget caps the region by entry count, which binds first for any bulk object under about 9.6 KB.
Failure: an operator sizing --cache-max-bytes for a tenant whose bulk objects are 4 KiB expects half the cap available to the scan and gets about a fifth. The region admits at most (max_bytes/100 - 2048*tenants)/192 entries (s3fifo.rs:95, 108, 652), so at max_bytes = 4 MiB with one tenant and 4 KiB objects it stalls at 207 entries, 848 KB, against a 2 MiB share. The crossover is at an entry size of 9600 bytes; above it the byte share binds and the guide is right. This is now a hard decline rather than a drain, which is the right behaviour, but the cap it declines against is not the one the guide names.
Evidence: docs/guides/caching.md:208-210 and 228-229 against s3fifo.rs:95 (192 bytes per region entry), 108 (budget is max_bytes/100), 650-655 (bulk_fits_with's bookkeeping term), and this PR's own fixture at s3fifo.rs:2126-2135, which asserts the cap is reached while region.bytes + CAP_ENTRY <= region_share_bytes.
Fix: one clause saying the region also holds at most about max_bytes/19200 entries, so the half-the-cap figure is reached only for objects above roughly 10 KB.