matanper opened a new issue, #70228: URL: https://github.com/apache/doris/issues/70228
### Search before asking - [x] I searched the issues and found no similar issue. Related: #68304 (account for pinned PK roots in segment cache eviction), which added the charge-update callback used below. ### Description `SegmentCache` is bounded by memory (`segment_cache_memory_percentage`, 5% by default), but each entry is charged `Segment::cache_charge()`, which is `_meta_mem_usage` plus pinned PK index bytes. `_meta_mem_usage` is a fixed estimate computed once in `Segment::_open()` (`be/src/storage/segment/segment.cpp`): - `sizeof(Segment)` - `min(num_columns, max_segment_partial_column_cache_size) * estimated_mem_per_column_reader` (100 × 512 B at most) - a few terms proportional to the row count Most of what a cached segment actually holds is built later, lazily, by the queries that read it, and none of it is added to the charge: - VARIANT external meta loaded by `VariantExternalMetaReader::load_all()`: the subcolumn tree, `PathInData`, data types and statistics for every path - the path map built in `VariantColumnReader::init()`, and the per-path `ColumnReader`s created through `ColumnReaderCache` - ordinal indexes loaded on first seek (`OrdinalIndexReader::_load`) - index file readers (`IndexFileReader::_init_from`) A VARIANT column with thousands of paths counts as one column in the estimate. On tables with VARIANT columns the segment cache therefore believes it is far below its bound and never evicts for memory. The metadata of every segment ever read stays resident until the entry ages out by count or stale sweep, and the 5% bound does not limit it. From a heap profile of a query BE serving VARIANT-heavy log tables, the live allocations made in one hour included: | Allocation site | Live bytes | |---|---| | `VariantExternalMetaReader::load_all` | 4.15 GB | | path map in `VariantColumnReader::init` | 2.49 GB | | subcolumn trees, `PathInData` and data types from external meta | about 2.8 GB | | `OrdinalIndexReader::_load` | 2.85 GB | | index file readers and footer parsing | about 2 GB | The cache's charge for those same segments stayed at a few tens of KB each. ### Solution Charge segments with the heap they actually retain, and keep the charge current as it grows: 1. Measure the retained bytes of each lazy loader by the calling thread's net allocation across the load (the thread-local allocation counters the memory tracker already keeps), and add them to a per-segment `retained_bytes`. 2. Re-charge the cache entry when that grows, through the `_cache_charge_callback` / `update_charge` path #68304 added for PK index bytes, either right after each load or on a cache hit once the retained bytes exceed the charge by some ratio, so updates stay rare. 3. Keep the metadata memory snapshot consistent. `Segments(not in SegmentCache)` is computed in `memory_profile.cpp` as `MetadataAdder<Segment>::get_all_segments_estimate_size() - SegmentLoader::cache_mem_usage()`, and in production `cache_mem_usage()` is the cache's tracked value. If the tracked value is raised to the retained bytes, this subtraction goes negative by the difference. Either keep tracking the estimate there, or subtract the cached segments' estimate explicitly. With segments charged by measured retained bytes, the cache's charge matched independently measured retained memory (7.27 GB charged against 7.31 GB measured), and the memory bound started evicting. We are happy to contribute a PR for master if this direction is welcome. ### Are you willing to submit PR? - [x] Yes I am willing to submit a PR! -- 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]
