Take into account key size for MemorySizedCache

[ncoiffier-celonis]

Description

Take into account key size for MemorySizedCache; this is particularly problematic for LeafSearchCache which uses the (large) SearchRequest, including the whole query, as a cache key and for queries that mostly return zero-hit splits.

Attempt to fix #6719.

• create a MemUsage trait
• implement MemUsage for CacheKey and SliceAddress
• modify the 3 base cache implementations to take into account the key mem_usage when inserting and reporting the cache in_cache_num_bytes and evict_num_bytes
• hits_num_bytes still return the value size only, without taking into account the key size.
• the changes should be mostly self-contained in the quickwit-storage/cache module

How was this PR tested?

Unit tests only.

[Copilot]

Pull request overview

This PR fixes MemorySizedCache under-accounting by charging cache entries for both key + value memory, addressing the reported key-driven memory growth (notably for LeafSearchCache where keys embed large SearchRequests).

Changes:

• Introduces a MemUsage trait (plus helpers) and implements it for relevant cache key types (SliceAddress, LeafSearchCache::CacheKey, plus foundational impls like String / tuples).
• Updates the underlying cache backends (LRU / S3Fifo / TinyLfu) to enforce capacity and metrics using entry size = key_mem_usage + value_len.
• Adjusts tests and documentation to reflect “keys included” cache capacity semantics.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
quickwit/quickwit-storage/src/lib.rs	Re-exports MemUsage and owned_mem_usage from the storage crate surface.
quickwit/quickwit-storage/src/cache/stored_item.rs	Adds KeyedEntry and tracks per-entry key memory to keep eviction paths cheap and accounting consistent.
quickwit/quickwit-storage/src/cache/slice_address.rs	Implements MemUsage for SliceAddress so slice caches can charge keys.
quickwit/quickwit-storage/src/cache/mod.rs	Adds the new mem_usage module and re-exports its API.
quickwit/quickwit-storage/src/cache/memory_sized_cache.rs	Requires K: MemUsage and expands tests to validate key charging across policies.
quickwit/quickwit-storage/src/cache/mem_usage.rs	New MemUsage trait + helper and unit tests.
quickwit/quickwit-storage/src/cache/base_cache.rs	Makes LRU/S3Fifo/TinyLfu capacity and metrics account for key size via stored key footprint.
quickwit/quickwit-search/src/leaf_cache.rs	Implements MemUsage for LeafSearchCache keys using SearchRequest::encoded_len() as a proxy.
quickwit/quickwit-config/src/node_config/mod.rs	Adds CacheConfig::with_capacity_and_policy helper (used in tests).
docs/configuration/node-config.md	Documents that cache capacities bound key+value, not just value bytes.

Suppressed comments (2)

quickwit/quickwit-storage/src/cache/base_cache.rs:358

• This warning message is now also triggered when the key makes the entry exceed capacity (even if the value itself is small), but the message still says "byte slice". Updating the wording will make logs less misleading.

                    capacity_in_bytes = ?self.capacity,
                    len = value.len(),
                    key_mem_usage,
                    "Downloaded a byte slice larger than the cache capacity."
                );

quickwit/quickwit-storage/src/cache/base_cache.rs:479

• This warning message is now also triggered when the key makes the entry exceed capacity (even if the value itself is small), but the message still says "byte slice". Updating the wording will make logs less misleading.

                    capacity_in_bytes = ?self.capacity,
                    len = value.len(),
                    key_mem_usage,
                    "Downloaded a byte slice larger than the cache capacity."
                );

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ncoiffier-celonis]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5dfaaf9374

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Account for SearchRequest's heap allocations

For searches spanning many indexes or containing many snippet fields, encoded_len() omits the heap allocation for each Vec element and nested structure—for example, every String in index_id_patterns occupies an inline header in the vector in addition to its encoded bytes. Because the request is cloned into a key for every cached split, these omitted allocations accumulate across entries, allowing the partial-request cache to retain substantially more memory than its configured capacity. Compute the request's transitive allocation sizes, including vector capacities and nested fields, rather than using its protobuf wire length.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Clarify the duplicated partial-cache budget

This does not bound total memory for partial_request_cache_capacity: SearcherContext::new passes the full configuration independently to both LeafSearchCache and ListFieldsCache (quickwit-search/src/service.rs lines 473–475), and each constructs its own MemorySizedCache. On a node receiving both search and list-fields traffic, cached entries can therefore consume roughly twice the configured capacity even before cache metadata overhead, so this new guarantee is misleading unless the caches share a budget or the documentation explicitly describes the per-cache limit.

Useful? React with 👍 / 👎.