fix(storage): policy-aware eviction fail-close for disk-offload without a durability backstop

CHANGELOG.md

@@ -6,6 +6,55 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed — `--disk-offload` without a durability backstop broke the default LRU cache recipe
+
+- **GCP benchmark finding (2026-07-10)**: with `--disk-offload enable` (the
+  default) but `--appendonly no` and no `--save`, the durable-spill eviction
+  path added to fix the crash window above (`evict_batch_durable_no_aof`)
+  never runs, because it needs a `ShardManifest` that is only threaded
+  through the tick-driven memory-pressure cascade
+  (`shard::persistence_tick::handle_memory_pressure`), itself gated on
+  `persistence_dir`, which `main.rs` only constructs when
+  `appendonly == "yes" || save.is_some()`. The inline write-path eviction
+  gate has no manifest access, so durable spill is impossible from that call
+  site — but the pre-fix `manifest is None` branch OOM'd **unconditionally**
+  regardless of `--maxmemory-policy`, silently breaking the documented
+  "Pure cache (no durability)" recipe
+  (`--appendonly no --maxmemory <bytes> --maxmemory-policy allkeys-lru`): a
+  classic LRU cache rejected writes at the cap instead of evicting.
+- **Behavior fix** (`src/storage/eviction.rs`,
+  `try_evict_if_needed_async_spill_with_total_budget`'s `manifest is None`
+  branch): made the fail-close policy-aware, matching Redis semantics. When
+  `total_memory <= budget` nothing happens; when `policy == NoEviction` it
+  still OOMs (correct — noeviction always rejects); otherwise (any evicting
+  policy — `allkeys-*`/`volatile-*`) it now reclaims the budget by
+  **dropping** victims via `evict_one_with_spill(.., spill: None)` (the same
+  plain-drop helper the disk-offload-disabled write path already uses — no
+  new victim-selection logic), looping until the budget is satisfied or no
+  eligible victim remains (then OOM). No durable spill is attempted here —
+  that still only happens via the tick's `evict_batch_durable_no_aof` when a
+  manifest is reachable. Three new tests in `src/storage/eviction.rs`:
+  `async_spill_no_aof_backstop_no_manifest_allkeys_lru_evicts_pure_cache`
+  (the regression test — fails with OOM on pre-fix code, passes after),
+  `async_spill_no_aof_backstop_no_manifest_noeviction_still_rejects`
+  (unchanged noeviction OOM behavior), and
+  `async_spill_no_aof_backstop_no_manifest_volatile_lru_scoped_to_ttl_keys`
+  (`volatile-*` only evicts keys with a TTL, never a persistent key). The
+  `--appendonly yes` async-spill path and the manifest-reachable durable
+  batch-spill path are untouched.
+- **Startup warning**: `ServerConfig::disk_offload_spill_inert` (new
+  predicate, `src/config.rs`) plus
+  `ServerConfig::warn_disk_offload_without_durability` (mirrors
+  `warn_deprecated_cold_tier_flags`) emit a single `tracing::warn!` at
+  startup when `--disk-offload enable` + no durability backstop is
+  detected, called from `main.rs` right after the existing cold-tier-flags
+  warning: disk-offload's cold-spill tiering is still inert in this
+  combination (evicting policies drop eligible victims instead of tiering;
+  `noeviction` — and any evicting policy with no eligible victim left, e.g.
+  `volatile-*` once its TTL-bearing keys are gone — returns OOM). Documented
+  in `docs/guides/tuning.md`'s "Tiered
+  memory offload" section.
+
 ### Fixed — test flakiness: oom_bypass_closure readiness on loaded CI runners
 
 - `tests/oom_bypass_closure.rs`: the readiness path used a fixed 30s connect

docs/guides/tuning.md

@@ -210,6 +210,29 @@ resident forever:
   backing both the KV cold-read cache and vector/graph page I/O; it starts
   empty and grows lazily, so setting it high does not pre-commit RAM.
 
+> **Requires a durability backstop.** The KV cold-spill path above needs a
+> `ShardManifest`, which is only threaded through the tick-driven
+> memory-pressure cascade — itself gated on `--appendonly yes` or `--save`
+> being configured. With `--disk-offload enable` (the default) but
+> `--appendonly no` and no `--save`, the inline write-path eviction gate has
+> no manifest access and cannot durably spill: cold data is **never spilled
+> to disk** in this combination, regardless of `--maxmemory-policy`. The
+> **"Pure cache (no durability)" recipe above still works correctly** —
+> `allkeys-lru` (and the other evicting policies) fall back to Redis-style
+> cache eviction: victims are DROPPED outright (no tiering, no durability
+> claim needed since nothing is meant to survive a restart) to keep
+> `--maxmemory` honored. `noeviction` rejects writes with OOM once the budget
+> is hit, and an evicting policy also returns OOM when no eligible victim
+> remains (e.g. no TTL-bearing key is left under a `volatile-*` policy) — same
+> as with disk-offload off. This
+> spill-inertness is intentional (correctness over availability for the
+> *tiering* feature specifically) — Moon warns about it once at startup
+> (`ServerConfig::warn_disk_offload_without_durability`). Enable
+> `--appendonly yes` or configure `--save` to activate disk-offload spill
+> (durable tiering instead of dropping), or pass `--disk-offload disable` if
+> you only want in-memory `--maxmemory-policy` eviction with no spill code
+> path involved at all.
+
 ## Vector/FTS/graph idle-unload
 
 Immutable vector segments (`ImmutableSegment`: full in-memory HNSW graph +

src/config.rs

@@ -931,6 +931,54 @@ impl ServerConfig {
         }
     }
 
+    /// True when `--disk-offload enable` is configured but neither AOF
+    /// (`--appendonly yes`) nor RDB (`--save`) durability is on (GCP
+    /// benchmark finding, 2026-07-10).
+    ///
+    /// The durable-spill eviction path (`evict_batch_durable_no_aof`) needs
+    /// a `ShardManifest`, which today is only threaded through the
+    /// tick-driven memory-pressure cascade
+    /// (`shard::persistence_tick::handle_memory_pressure`) — itself gated on
+    /// `persistence_dir`, which `main.rs` only constructs when
+    /// `appendonly == "yes" || save.is_some()` (see `main.rs`'s
+    /// `persistence_dir` binding). The inline per-connection write-path
+    /// eviction gate has no manifest access and, under this combination,
+    /// cannot durably spill — cold data is NOT tiered to disk regardless of
+    /// policy. What happens to the *write* itself is now policy-aware
+    /// (`src/storage/eviction.rs`, the `manifest is None` branch of
+    /// `try_evict_if_needed_async_spill_with_total_budget`): an evicting
+    /// policy (`allkeys-*`/`volatile-*`) still honors `--maxmemory` by
+    /// DROPPING victims outright (Redis cache semantics, no tiering, no
+    /// crash-durability claim needed since nothing needs to survive a
+    /// restart); `noeviction` — and any evicting policy once no eligible
+    /// victim remains (e.g. `volatile-*` with no TTL keys left) — rejects
+    /// writes with OOM at the cap. This
+    /// predicate stays orthogonal to `maxmemory_policy` — spill IS still
+    /// inert either way — and exists only to make that degradation loud.
+    pub fn disk_offload_spill_inert(&self) -> bool {
+        self.disk_offload_enabled() && self.appendonly != "yes" && self.save.is_none()
+    }
+
+    /// Warn once at startup when [`disk_offload_spill_inert`] holds — see
+    /// that method's docs for the full mechanism. Behavior is unchanged;
+    /// this only surfaces the pre-existing silent degradation.
+    ///
+    /// [`disk_offload_spill_inert`]: Self::disk_offload_spill_inert
+    pub fn warn_disk_offload_without_durability(&self) {
+        if self.disk_offload_spill_inert() {
+            tracing::warn!(
+                "--disk-offload is enabled but persistence is off (appendonly=no and no \
+                 --save). The disk-offload cold-spill tier requires a durability backstop \
+                 to function: without one, cold data is NOT spilled to disk. Evicting \
+                 policies (allkeys-*/volatile-*) fall back to DROPPING eligible \
+                 victims with no tiering; noeviction — and any evicting policy once \
+                 no eligible victim remains (e.g. volatile-* with no TTL keys) — \
+                 rejects writes with OOM at the cap. Enable \
+                 --appendonly yes or --save to activate durable spill."
+            );
+        }
+    }
+
     /// Validate `--databases` fits the `u8` db_index tag used by vector/text
     /// index scoping (WS5a round 2). Returns an error message (not a
     /// `Frame`/`anyhow::Error` — kept plain so both `main.rs`'s early-boot
@@ -2369,6 +2417,33 @@ mod tests {
         assert_eq!(config.pagecache_size, None);
     }
 
+    #[test]
+    fn disk_offload_spill_inert_true_without_durability_backstop() {
+        // disk-offload defaults to enabled; explicitly turning off both AOF
+        // and RDB durability with no backstop leaves cold-spill inert.
+        let config = ServerConfig::parse_from(["moon", "--appendonly", "no"]);
+        assert!(config.disk_offload_spill_inert());
+    }
+
+    #[test]
+    fn disk_offload_spill_inert_false_when_appendonly_yes() {
+        let config = ServerConfig::parse_from(["moon", "--appendonly", "yes"]);
+        assert!(!config.disk_offload_spill_inert());
+    }
+
+    #[test]
+    fn disk_offload_spill_inert_false_when_save_configured() {
+        let config = ServerConfig::parse_from(["moon", "--appendonly", "no", "--save", "3600 1"]);
+        assert!(!config.disk_offload_spill_inert());
+    }
+
+    #[test]
+    fn disk_offload_spill_inert_false_when_disk_offload_disabled() {
+        let config =
+            ServerConfig::parse_from(["moon", "--appendonly", "no", "--disk-offload", "disable"]);
+        assert!(!config.disk_offload_spill_inert());
+    }
+
     #[test]
     fn test_parse_size() {
         assert_eq!(ServerConfig::parse_size("256mb"), Some(268_435_456));

src/main.rs

@@ -140,6 +140,12 @@ fn main() -> anyhow::Result<()> {
     // once if an operator still sets any of them away from their default.
     config.warn_deprecated_cold_tier_flags();
 
+    // GCP benchmark finding (2026-07-10): --disk-offload enable without a
+    // durability backstop (--appendonly yes or --save) leaves the cold-spill
+    // tier silently inert — --maxmemory becomes a hard reject-at-cap instead
+    // of spilling. Warn once at startup; behavior is intentionally unchanged.
+    config.warn_disk_offload_without_durability();
+
     // ── --profile tuning presets (WS4, v0.6.0) ──────────────────────────────
     // Fill-only: a preset only sets flags the operator left at their default
     // (see `ServerConfig::apply_profile`); anything explicitly passed on the