[detail-app]

Detail bug report: View on Detail

Bug

S3Fifo::update (foyer-memory/src/eviction/s3fifo.rs) validated small_queue_capacity_ratio with an inverted guard: it returned Err for in-range values (rejecting even the documented default 0.1) and silently accepted/stored out-of-range values (<= 0 or >= 1).

• The predicate was the exact inverse of the constructor S3Fifo::new's assertion and of the sibling Lru/Lfu update validations, which all reject out-of-range ratios.
• Because the Err returned before the capacity recompute, a valid config update also aborted the always-performed capacity resize, leaving the cache at its old capacity.
• An invalid ratio silently applied (e.g. 2.0 set small_weight_capacity = 2 * capacity), which defeats S3-FIFO's small/main/ghost partition trigger (small_weight > small_weight_capacity) and collapses eviction into a degenerate mode.
• Latent today: the only in-tree caller (RawCache::resize) passes config: None, so the broken Some(config) branch is never exercised in production or tests — but it's the branch the Option<&Config> signature explicitly intends to support.

Fix

Negated the guard so the error fires when the ratio is out of range, matching lfu.rs/lru.rs:

if config.small_queue_capacity_ratio <= 0.0 || config.small_queue_capacity_ratio >= 1.0 {
    return Err(...);
}
self.config = config.clone();

Also appended "new configuration ignored" to the error message for parity with the sibling implementations. No other behavior changed; the new, push/pop/evict, acquire, release, remove, and clear paths are untouched.

Testing

Added two focused unit tests in the s3fifo tests module (there was previously no coverage of S3Fifo::update):

• test_s3fifo_update_with_valid_config — asserts valid ratios (incl. the default 0.1 and near-boundary 0.01/0.999) return Ok, the new config is stored, and small_weight_capacity/ghost_queue.capacity are recomputed (i.e. the capacity resize is not aborted).
• test_s3fifo_update_with_invalid_config — asserts out-of-range values (0.0, 1.0, -0.5, 2.0, ±∞) return Err with ErrorKind::Config, the message recites the valid range, and the existing config and derived capacities are left untouched on rejection.

Both tests use the existing in-module test patterns (TestS3Fifo, direct field access like the existing test_s3fifo). A proposed capacity-only-resize (config: None) test was excluded as redundant with the existing raw::tests::test_s3fifo_cache_resize, which exercises that production path end-to-end.

Verification run (with --features strict_assertions, matching CI):

• New + existing s3fifo unit tests: 3/3 pass.
• Full foyer-memory suite: 33/33 pass (incl. raw::tests::test_s3fifo_cache_resize and all eviction *_cache_fuzzy tests).
• foyer umbrella crate: cargo check -p foyer and cargo nextest run -p foyer (12/12) pass.
• Build clean (cargo check/cargo build).
• cargo fmt -p foyer-memory -- --check and whole-workspace nightly fastidious cargo +nightly fmt --all -- --config-path rustfmt.nightly.toml --check clean.
• cargo clippy -p foyer-memory --all-targets --features strict_assertions -- -D warnings exits 0. The only emitted warning is a pre-existing environment artifact (unknown_lints for the workspace-allowed clippy::chunks_exact_to_as_chunks, unrecognized by the local rustc-1.91 toolchain) — confirmed identical on an unmodified git stash baseline and not attributable to this change.
• Doc tests, cargo-machete (no unused deps), typos, taplo fmt --check, Grafana JSON minification check, and the CI license gate (hawkeye check) all pass.

Two procedures could not be run verbatim and are noted for transparency:

• cargo x --fast (the README's local gate) runs a mutating check() step (cargo sort -w, taplo fmt, json rewrites) plus 8 example binaries; I ran the read-only --check equivalents and the non-mutating xtask sub-steps instead to avoid spurious diffs. The one component that failed, cargo sort --check, fails identically on an unmodified baseline (tool-version drift on untouched Cargo.toml files) and is not a CI gate.
• The xtask license subcommand invokes the legacy license-eye, which cannot parse the repo's .licenserc.toml in this environment; CI uses hawkeye check instead, which passes.

---

Automatic Fixes PRs can be configured here.

[detail-app]

Closing this pull request because it has had no human activity in 30 days (since 2026-09-11) and was opened 30 days ago. Detail closes its pull requests once they are more than 30 days old and have had no human activity for 14 days.

If this change is still useful, reopen the pull request and Detail won't close it again. To keep a Detail pull request open, add the detail-keep-open label to it.