Scan-resistant eviction + footprint-based cache admission gate - #8
Merged
Conversation
LiquidPolicy was FIFO-per-type with a no-op notify_access, so a large one-pass scan floods the cache and evicts the reused working set (and, once full, read-once batches still displace hot ones). Add a CLOCK/second-chance reference bit: notify_access sets it on a cache hit, and find_memory_victim passes over a referenced entry once (clearing the bit, moving it to the back) before evicting it. A batch read exactly once starts and stays cold, so it is evicted before any reused batch — the cache converges on the working set regardless of how large the surrounding column/scan is. FIFO behaviour is unchanged when nothing is reused.
A disk-served hit called notify_access before hydration, and the following Disk->Memory notify_insert preserved the bit, so a batch read once from disk arrived in memory looking reused and could evict a genuinely reused batch during a large one-pass scan (the exact overflow case scan-resistance targets). Gate the reference bit on memory batch types only. (codex review)
Replace LocalModeOptimizer's raw-file-size cap with a byte-accurate, filter-aware footprint gate. For a parquet scan it estimates the in-memory liquid footprint from DuckLake per-file stats: required columns (output projection UNION predicate columns), summed over the files that survive the predicate (PruningPredicate + PrunableStatistics), using only Exact per-column byte sizes and falling back to the whole-file size otherwise. A scan whose estimate (raw x expansion x safety) exceeds the cache memory budget is left as a plain parquet read (bypass -> mount) instead of thrashing the cache. Conservative by construction: pruning only drops proven-non-matching files; non-exact/missing byte sizes over-count; multipliers inflate the estimate, never the budget -- so the gate never wrongly admits an oversized scan. Complements the CLOCK eviction (admission vs. eviction). Pure byte-math unit-tested. Builder: with_max_scan_bytes -> with_admission_gate(expansion, safety).
…iew) - Prune with the full table schema (file + partition columns) so predicates on partition columns resolve; PartitionedFile stats are table-schema too. - Exclude partition columns from byte accounting -- they are literals, never materialized in LiquidCache; only file columns contribute to the footprint. - Guard per-file stats width against the schema to avoid a mismatch; a mismatched/absent file is treated as unknown (kept, not pruned).
Deny only when estimated liquid footprint exceeds budget x tolerance, not budget x1. LiquidCache compacts in RAM and beats the fallback mount until ~5x over budget, so gating at 1x over-bypasses a whole band of scans that would cache and win. Compare as a finite f64 pressure ratio (footprint/budget > tolerance) with a saturating raw-byte sum and an explicit budget==0 case. tolerance is the one relaxing knob: clamped to [1.0, 5.0], defaulting to 3.0, non-finite rejected.
file_column_projection_indices() delegates to the deprecated ProjectionExprs::ordered_column_indices(), which .expect()-panics on any non-Column projection expression. DataFusion pushes non-column exprs (get_field on struct columns, via enable_leaf_expression_pushdown) into the scan projection, so the gate panicked and killed the query. Use src.projection().column_indices() (collect_columns-based, handles arbitrary exprs) instead, matching the reader's own opener. Add a struct-column get_field regression test that runs without a cache mount.
The gate counted only Precision::Exact per-column byte_size and fell back to the whole-file size otherwise. DuckLake (the target backend) always labels byte_size Inexact — it is the real recorded compressed column size, marked Inexact because catalog stats can go stale after deletes/compaction — so the fallback fired on every scan, inflating the estimate ~50-100x (charging all columns for a single-column read) and leaving the cache near-empty. Count Inexact sizes too; only Absent (or missing file stats) still falls back to whole-file. expansion/safety remain the margin against stale-low drift. Sum with saturating_add.
The gate runs in the physical optimizer on every query, so a panic in footprint estimation (e.g. a DataFusion API that panics on an unusual plan shape) aborts the whole query. Since the gate is a pure performance optimization — caching a scan or not yields identical results — guard the decision with catch_unwind: on panic, log and cache the scan normally instead of taking down the query.
Add a strict flag to the admission gate. Either way the panic is caught and logged at ERROR with its message (never silently swallowed): - strict: log, advise disabling the flag to keep queries running, then re-raise so the query aborts and the bug surfaces immediately. - non-strict: log, advise enabling the flag to fail loud, then cache the scan normally so the query survives. The caller (runtimedb) defaults strict on.
The gate was a black box — no way to see why a scan was admitted or bypassed without a full benchmark run. Log one INFO line per decision (target liquid_cache::admission): file, projected cols, surviving/total files, raw and footprint bytes, budget, threshold, and the multipliers. Crucially, surface fallback_files: when a file lacks per-column byte sizes the estimate charges the whole file, so a non-zero count is the direct signal that the catalog has no column_size_bytes for the table (the DuckLake write-side stat). estimate_required_bytes now returns a FootprintEstimate breakdown and file_required_bytes reports whether it fell back.
DataFusion splits one file into several byte-range PartitionedFiles for parallelism, each cloning the whole file's statistics and object size. The estimate summed every range, multiplying a single file's footprint by the split count (e.g. 7x14.8GB). Charge each distinct file path once; pruning is file-granular, so a surviving file means its whole required columns regardless of how it was split for scanning.
Codex review follow-ups: key the file dedupe by (location, size) so two distinct objects sharing a path can't be collapsed, and rename the decision-log counters to charged_files / partitioned_files so a split-into-N file reads unambiguously (e.g. charged_files=1 partitioned_files=7).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
+423
to
+433
| "liquid-cache admission gate panicked during footprint estimation: \ | ||
| {msg}. Aborting query (strict mode). Set \ | ||
| LIQUID_CACHE_ADMISSION_STRICT=off to fall back to caching and keep \ | ||
| queries running while this is fixed." | ||
| ); | ||
| std::panic::resume_unwind(payload); | ||
| } | ||
| log::error!( | ||
| "liquid-cache admission gate panicked during footprint estimation: {msg}; \ | ||
| caching scan normally. Set LIQUID_CACHE_ADMISSION_STRICT=on to fail loud \ | ||
| instead." |
There was a problem hiding this comment.
super nit: (not blocking) These messages advise setting LIQUID_CACHE_ADMISSION_STRICT=off/=on, but that env var isn't read anywhere in this crate — strict is only set programmatically via with_admission_gate(..). For a user driving liquid-cache directly through the builder (not via runtimedb), this advice points at a knob that doesn't exist. Consider naming the actual builder parameter (with_admission_gate's strict) instead, or wiring the env var if it's meant to be honored here.
- squeeze_strings snapshot: CLOCK second-chance eviction reorders the squeeze victims ([851969,851968,917504] vs prior) — same victims, same outcomes, deterministic queue order. Accept the reordered trace. - admission log/doc: advise the gate's 'strict' builder flag rather than a runtimedb env var the crate doesn't read (review nit). - dev-tools cache_state_view: iterate map keys() (clippy for_kv_map, newly flagged by a clippy toolchain bump; pre-existing code unrelated to this change but blocks -D warnings CI).
📊 Benchmark ComparisonCurrent:
Compared Liquid vs DataFusionDefault on the same runner |
There was a problem hiding this comment.
Reviewed the CLOCK eviction and footprint-based admission gate. Logic is correct and well-tested:
- CLOCK: reference bit is set only on cache hits (
notify_access), while the initial miss/insert starts cold, so read-once scan batches are evicted before reused ones. Disk-hit exclusion is correct.find_memory_victimterminates (each entry gets at most one second chance) and second chances never reduce the victim count. - Admission gate: inputs sanitized/clamped, comparison done in f64 to avoid overflow,
budget == 0handled, projection∪predicate column accounting is index-consistent for file columns, split dedupe by (path, size) is sound, and the panic guard operates over a lock-free pure read. Thecolumn_indices()fix is covered by theget_fieldregression test.
The prior comment about the strict flag messaging is addressed — the logs now name the strict builder parameter rather than a nonexistent env var. No blocking issues.
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
1 participant
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.
Summary
Two complementary LiquidCache improvements for large-table workloads, plus observability.
1. Scan-resistant eviction (CLOCK)
LiquidPolicyeviction is now scan-resistant (CLOCK / second-chance): a single large sequential scan no longer floods out a hot working set. The reference bit is set on memory hits only (disk hits don't protect), and a bounded second-chance sweep guarantees termination.2. Footprint-based admission gate
Instead of caching every scan (and thrashing when a scan can't fit), the optimizer estimates a scan's liquid footprint at plan time and only admits scans that fit; the rest are read as vanilla parquet (bypassing the cache). The estimate is:
byte_sizefrom catalog stats (ExactorInexact); falls back to whole-file size only when a column size isAbsent.budget × tolerance(default caller value ~3, capped 5) because LiquidCache compacts in RAM and beats the fallback mount until ~5× over budget.PartitionedFiles DataFusion creates for one physical file (keyed by path+size), so a split file's footprint isn't multiplied by the split count.Parameters (
expansion,safety,tolerance,strict) are set by the caller viaLocalModeOptimizer::with_admission_gate/ the local builder; all are sanitized (finite, clamped).3. Robustness + observability
strictflag (default on in the caller) re-raises after logging so bugs fail loud in canary; non-strict logs at ERROR and caches normally.ProjectionExprs::column_indices(handles compound scan projections likeget_field) instead of the deprecatedordered_column_indices, which panicked.target: liquid_cache::admission) with the full breakdown — charged/partitioned files, raw + footprint bytes, threshold, andfallback_files(the signal that catalog per-column sizes are missing).Testing
get_fieldregression that reproduces the old projection panic.avg(UserID), ~270 MB) is admitted and cached; a wide multi-string scan (~7 GB) is bypassed.Notes