deepthi912 opened a new pull request, #19430:
URL: https://github.com/apache/pinot/pull/19430
## Summary
`OffHeapSingleTreeBuilder.sortAndAggregateSegmentRecords` fills a
direct/mmap buffer with every row's dim dict-ids in a sequential pass, sorts
docIds by dim tuple, then — today — **closes the buffer before returning the
aggregation iterator, and the iterator re-reads the same dim values from the
segment forward-index** via `getSegmentRecord(sortedDocIds[i])`. After the
sort, those docIds are random with respect to Parquet page layout, so every
re-read triggers a full page-context miss: Snappy decompress + RLE decode +
fresh `String[]` / `int[]` materialize, for one value consumed.
For external-table segments backed by `parquet_page_index.bin` + on-demand
parquet page decode, this dominates preprocess wall clock. On a real cluster
today, a 3.3M-row segment spends **~24 minutes** in this method; a JFR taken
during a live build shows 77% of samples in
`RunLengthBitPackingHybridDecoder.readInt` under the following stack:
```
OffHeapSingleTreeBuilder$1.next
-> BaseSingleTreeBuilder.getSegmentRecord
-> BaseSingleTreeBuilder.getSegmentRecordDimensions
-> PinotSegmentColumnReader.getDictId
-> ParquetForwardIndexReaderV6.getString
-> ensureContextForDocId (page-context miss)
-> DeserializedParquetPageData.deserializeData
-> decodeDictStringPage
-> RunLengthBitPackingHybridDecoder.readInt
```
## Fix
Extend the buffer's lifetime to iterator exhaustion, and add a
`BaseSingleTreeBuilder#getSegmentRecordWithBufferDims(int, PinotDataBuffer)`
helper that reads dim dict-ids back from the buffer (already in memory) instead
of re-fetching from the segment. Metrics still read from the segment.
For a star-tree with N dims and M non-COUNT metrics, random column reads per
aggregated row drop from N+M to M — ~67% fewer random reads for a common (N=4,
M=2) config.
Buffer ownership transfers to the iterator: it calls
`closeSegmentRecordBuffer` on the terminal `next()`. If an exception is thrown
before the iterator is returned (e.g. during fill or sort), the outer `finally`
still cleans up via the same helper, so no leak.
## What is NOT changed
- `OnHeapSingleTreeBuilder` — its own scan is already sequential, no change.
- `BaseSingleTreeBuilder#getSegmentRecord(int)` — kept intact for other
callers (tests, subclasses).
- Public/SPI API, config, on-disk formats — nothing changes.
- Star-tree records emitted by the iterator — same content, same order, same
aggregation semantics.
## Test plan
- [ ] Existing `pinot-segment-local` module tests (`./mvnw -pl
pinot-segment-local test`) pass.
- [ ] New targeted unit test asserting
`getSegmentRecordWithBufferDims(docId, buffer)` produces a `Record` equal to
`getSegmentRecord(docId)` for the same docId, over a hand-built segment
covering all dim dict-id combinations.
- [ ] Integration test that exercises `MultipleTreesBuilder.build()`
end-to-end on the OffHeap path and asserts the resulting star-tree matches a
snapshot from before this change.
- [ ] Manual benchmark: rebuild a 3.3M-row external-table segment on a
StarTree Cloud dev cluster and confirm preprocess wall-clock drops from ~24 min
to ~8–14 min per segment.
## Measurement context
The 24-minute-per-segment baseline is measured on a StarTree Cloud dev
cluster with `uc_tvi_insights_v3_OFFLINE` (external table, parquet-backed, one
row group per file, 30 leaf columns, ~1500 pages per 6-col Sub-phase-C walk).
JFR + `DeserCacheMiss=482M` counter attached to the ticket confirm this is
CPU-bound decode, not S3/IO. Full JFR + call-stack analysis available on
request.
--
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]
