giladkl opened a new pull request, #3630: URL: https://github.com/apache/iceberg-rust/pull/3630
## Which issue does this PR close? - Closes #1720. ## What changes are included in this PR? The `ObjectCache` weigher used `size_of_val` on the cached `Manifest` and `ManifestList`, which is the inline size of the struct: 24 bytes for a manifest list and roughly 100 bytes for a manifest, regardless of how many entries either holds. The 32 MiB default budget therefore never bound the cache, and a table with many large manifests kept all of them resident. The weigher now estimates the bytes each item owns: the entry vectors, every path string, partition values, column stats maps, bounds, and the `Schema` that each parsed manifest carries (each manifest deserializes its own copy from the Avro user metadata, so it is not shared with the table metadata). The weight saturates at `u32::MAX` instead of truncating. The estimate deliberately stays approximate. It counts allocations that scale with entry count and skips allocator overhead, so it slightly undercounts a large item rather than requiring an exact accounting of every nested type. ## Are these changes tested? Unit tests in `object_cache.rs`: - The weight of a manifest grows by an exact per-entry amount (entry struct plus its owned path and partition value) when entries are added. - The weight of a manifest grows by the length of a data file path. - `moka`'s reported `weighted_size` equals the sum of the item estimates after loading a manifest list and a manifest through the cache. - A cache whose capacity is one byte short of holding both items keeps only one after eviction runs. ## AI Disclosure This PR was written with Claude Code. Areas a reviewer should focus on: - The per-field schema estimate multiplies the field name length by four to stand in for the id and name lookup maps inside `Schema`, which are private. That is a rough factor, not a measurement. - `Literal::Map` exposes no iterator, so its keys' and values' own heap is not counted. Partition values are rarely maps, so this should not matter in practice. - `HashMap` heap is estimated from `capacity()` times the slot size plus one control byte, which is the hashbrown layout at the time of writing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
