Parquet metadata cache max-size setting is semantically broken (entries counted as weight 1, not bytes)

[Selfeer]

Summary

The input_format_parquet_metadata_cache_max_size server setting is documented and intended as a byte limit for the Parquet file metadata cache, but the implementation uses the default per-entry weight of 1 (entry count), not bytes. As a result, the cache can grow far beyond the configured "maximum size," leading to unbounded memory use, OOM risk, and availability loss when querying many unique Parquet objects.

Source: Audit of PR #1385 – Antalya 26.1 – Forward port of parquet metadata caching.

---

Impact

• Correctness/availability: High — resource exhaustion and potential process crash.
• Likelihood: Realistic for object-storage workloads with high file cardinality.
• Blast radius: Process-wide (global singleton cache).
• Exploitability: Operationally easy (querying many unique Parquet objects is normal usage).

---

Root cause

• Product contract: Setting is described as "Maximum size of parquet file metadata cache" and defaults to 500000000 (bytes).
• Implementation: ParquetFileMetaDataCache extends CacheBase with the default policy, which uses EqualWeightFunction — every cache entry has weight 1, so the limit is enforced as number of entries, not bytes.
• Result: With many unique path:etag keys and non-trivial metadata per file, the cache can retain far more than the intended byte budget because eviction is driven by entry count, not memory size.

---

Affected code / anchors

Location	Relevance
src/Processors/Formats/Impl/ParquetFileMetaDataCache.h	Cache class inherits from CacheBase with default (entry-count) weight
src/Common/ICachePolicy.h	EqualWeightFunction returns 1 for any entry
src/Core/ServerSettings.cpp	input_format_parquet_metadata_cache_max_size declared as bytes (e.g. 500000000)

---

Minimal reproduction

1. Set input_format_parquet_use_metadata_cache=1 and leave the server default for input_format_parquet_metadata_cache_max_size (500000000).
2. Query Parquet files in object storage with many unique keys (distinct path/etag).
3. Observe: Cache admission continues well past the intended byte budget because each entry contributes weight 1, so the "max size" is effectively a cap on entry count, not memory.

---

Affected transitions / subsystems

• Transitions: T2 (cache insert), T5 (configured-size enforcement at server start).
• Subsystem: Parquet object-storage read path; both Parquet and ParquetMetadata input formats; all server threads using the global cache.

---

Labels / metadata suggestions

• Severity: High
• Component: Parquet / Formats / Caching
• Type: Bug (correctness / resource contract)

[arthurpassos]

The defect found by QA is a very silly (as in dumb developer) but potentially dangerous one. The calculation of the size entries in the cache is using the default implementation, which simply sets all entries as 1 even if the entry is like 5MB. So the setting to limit the size of the cache is not really respected. I would say it is a form of leaking memory? At least we have the super power of clearing the cache.

Being a way to leak memory I would classify it as high risk, but at the same time it is only applicable to the old reader, and I suppose people are now completely focused on the new one?

To fix this, what we need to do is to calculate the size of each and every field of the metadata object IN MEMORY. I started doing this, but soon I realized the Metadata object abstraction we were using did not provide the tools to do so. So I was like: let's see how upstream implemented it, and to my surprise they were using a different abstraction. But if they are using a different abstraction, that means they had to refactor the parquet reader to use that one. Guess what? They didn't. They are implementing the metadata cache only for the new v3 reader, the old reader will not have it. So their calculation code is useless to us.

Being that said, there are two things I can do:

1. Use the serialized (bytes on disk) size offered to calculate the entry size - this is much better than 1, but it is smaller than in memory - so not completely accurate.
2. Refactor old reader so it uses the abstraction that allows us to properly calculate the size - not sure it is worth the effort.

n1 is a quick patch to make things slightly better, n2 is a real solution.

I would stick with n1 because soon we'll have the upstream one, and we can always backport.