Doris-Breakwater commented on issue #69924: URL: https://github.com/apache/doris/issues/69924#issuecomment-6558479272
The reported retention mechanism is supported by the code: page-zone-map allocations can grow without a corresponding increase in SegmentCache eviction charge. This warrants BE storage/memory-accounting investigation. The exact ~1.4 KB/page footprint and attribution of the reported process-limit failures still need runtime evidence; I have not reproduced the workload. I inspected public `master` at `8e10b235cf03` and `branch-4.1` at `b4afc8c693ef6`. The issue currently has no labels. **Verified findings** - `ZoneMapIndexReader::_load()` sizes `_page_zone_maps` to `reader.num_values()` and parses every page's protobuf into it. Loading is once per reader, and the bounds remain owned by that reader. Thus the growth is per page in each loaded column index, including pages subsequently pruned, rather than necessarily per data page actually scanned. Both [master](https://github.com/apache/doris/blob/8e10b235cf030d486b6840cfd64b6218ab515e6d/be/src/storage/index/zone_map/zone_map_index.cpp#L363-L410) and [branch-4.1](https://github.com/apache/doris/blob/b4afc8c693ef6fbb2a657f0a5fcbcd14e52a30d2/be/src/storage/index/zone_map/zone_map_index.cpp#L340-L386) have this behavior. - The ownership path is `Segment → ColumnReaderCache → ColumnReader → ZoneMapIndexReader → vector<ZoneMapPB>`. The column-reader cache limits **reader count**, not loaded index bytes. Master's `Segment::cache_charge()` adds PK-root bytes to the segment's estimate, but no lazy zone-map bytes; branch-4.1 inserts only the estimate. So “not bounded by any cache” is more precisely **not covered by a byte budget reflecting these retained allocations**. Reader/segment eviction can release ownership, while active shared references can delay reclamation. See [column-reader cache](https://github.com/apache/doris/blob/8e10b235cf030d486b6840cfd64b6218ab515e6d/be/src/storage/segment/column_reader_cache.cpp#L60-L78), [master charge](https://github.com/apache/doris/blob/8e10b235cf030d486b6840cfd64b6218ab515e6d/be/src/storage/segment/segment.h#L210-L229), and [4.1 insertion](https://github.com/apache/doris/blob/b4afc8c693ef6fbb2a657f0a5fcbcd14e52a30d2/be/src/storage/segment/segment_lo ader.cpp#L43-L49). - `ZoneMapIndexReaderMemBytes` uses `sizeof(ZoneMapIndexReader) + sum(ZoneMapPB::ByteSizeLong())`. This measures serialized payload plus the reader object, not the vector allocation, protobuf objects, string capacities, or allocator overhead. The writer caps logical bounds at **512 bytes**; 513 bytes can describe terminator-inclusive string storage, but is not a 513-byte protobuf bound. Consequently, neither ~1.4 KB/page nor the reported 2.2 MB → 13 KB reduction can be established from this counter alone. - The two referenced changes are present on the inspected master, but their corresponding implementations are absent from the inspected branch-4.1: its writer still increments only the last max-bound byte, and its SegmentCache lacks the PK-root charge callback. Please identify the exact tested build and any backports; the PR references do not establish applicability to every 4.1.x build. See [4.1 writer](https://github.com/apache/doris/blob/b4afc8c693ef6fbb2a657f0a5fcbcd14e52a30d2/be/src/storage/index/zone_map/zone_map_index.cpp#L236-L256). **Assessment of the proposed fixes** 1. **Dynamic cache charging (c)** addresses the retention-budget problem while preserving pruning. It should account for retained heap storage, handle loading before/after cache insertion, and reconcile column-reader eviction/replacement. Reuse the lifetime and identity protections in #68304 without double-counting physical allocations. This limits cache retention; it cannot impose a hard cap on memory held by active queries, and reload frequency needs measurement. 2. **Compact `pass_all` pages (b)** is a reasonable conservative mitigation to evaluate. Existing page predicate handling retains such pages, and expression range evaluation rejects their bounds. Preserve null flags, keep the change specific to eligible string page maps, and preserve segment MIN/MAX safeguards. It sacrifices range pruning, including potentially valuable pruning on sorted strings. Crucially, actually release the long-string storage: setting `pass_all` alone does not do that, and protobuf clearing can retain allocations ([protobuf memory semantics](https://protobuf.dev/reference/cpp/api-docs/google.protobuf.message_lite/#MessageLite.Clear.details)). A smaller `ByteSizeLong()` is insufficient proof of heap savings. Per-page vector/object overhead also remains. 3. **Short prefixes (a)** may retain useful pruning, but “nearly all” of its effectiveness is workload-dependent. For byte-ordered VARCHAR/STRING bounds, the shortened lower bound must stay below the original minimum, and the shortened upper bound needs a correct unsigned-byte successor. An all-`0xFF` prefix requires a conservative fallback. Preserve the original overflow/invalid-bound checks before shortening; shortening does not by itself repair unsafe legacy bounds. There is no explicit cut flag in `ZoneMapPB`, so a bound reaching 512 bytes is only a conservative proxy, including genuinely 512-byte values. **Information needed and next steps** - Provide exact BE version/commit, table DDL, data generator/load batches, predicate SQL, and the experimental compaction patch. Include page/segment counts, string-prefix distribution, and relevant cache settings. - Capture the memory-limit error and contemporaneous BE memory breakdown, plus before/after heap-allocation measurements, zone-map counters, and SegmentCache charge. Measure after queries release their references and after controlled eviction in a test environment. This distinguishes retained allocations, transient loading peaks, and allocator-retained RSS. - Add correctness comparisons against row evaluation for equality/ranges, 511/512/513-byte boundaries, long shared prefixes, embedded NUL, carry/all-`0xFF`, nullable data, and delete predicates. Exercise both column-predicate and expression pruning, and verify MIN/MAX remains correct. Benchmark heap usage, pages read, and latency for both selective sorted strings and free text; test repeated/concurrent scans and eviction/reload behavior. A useful first PR would pair a reproducible memory test with accurate retained-memory accounting. Compaction and cache charging address different parts of the problem and can be reviewed separately. This analysis is based on source inspection; no Doris code changes or runtime tests were performed. Breakwater-GitHub-Analysis-Slot: slot_e0cc92ac4d08 -- 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]
