swyxio opened a new pull request, #16541:
URL: https://github.com/apache/pinot/pull/16541
- I’m going to locate the driver and how it currently builds segments, then
switch it to a column-major flow and add logs so we can confirm the path used
at runtime.
What I did
- Refactored `SegmentIndexCreationDriverImpl.build()` to default to
column-major when possible, falling back to row-major only if necessary.
- Added minimal getters to avoid redundant data loads:
- `PinotSegmentRecordReader.getIndexSegment()` to expose the underlying
segment.
- `RealtimeSegmentSegmentCreationDataSource.getMutableSegment()` to expose
the segment for column-major use.
- Ensured we don’t re-read rows for building after stats collection;
column-major only reads per-column values to build indexes.
- Added log messages to confirm which path is used at runtime.
Files changed
-
`pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentIndexCreationDriverImpl.java`
-
`pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/readers/PinotSegmentRecordReader.java`
-
`pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/converter/stats/RealtimeSegmentSegmentCreationDataSource.java`
Why this fixes it
- Most likely causes we wanted to address:
- Double reading of data: once for statistics, again for row-by-row
indexing.
- Inefficiency from row-major path when the source already supports column
access (Pinot segments, realtime mutable segments).
- The new flow:
- Always collects stats first (required for index sizing), but:
- Realtime stats reuse dictionaries/forward indexes (not a row scan).
- Immutable Pinot segment input may do a row scan for stats, but the
build phase no longer does a second row scan; it builds by columns.
- The build phase uses `indexColumn(...)` per column, avoiding row
re-reads and aligning with column-major.
How column-major is selected
- If `_recordReader` is a `PinotSegmentRecordReader`, we:
- Use its `getSortedDocIds()` (if any).
- Initialize creators and call `indexColumn(col, sortedDocIds,
pinotReader.getIndexSegment())` per column.
- If the data source is `RealtimeSegmentSegmentCreationDataSource` (realtime
conversion), we:
- Use its `getMutableSegment()` directly for column-major indexing.
- Otherwise, we fall back to the existing row-major loop.
Added logs
- “Start building Index by column (column-major)” to confirm we’re on the
column-major path.
- “Start building IndexCreator (row-major fallback)!” for the fallback path.
Validation
- Ran a linter pass on the edited files; no errors.
Notes on avoiding double reads for statistics
- Realtime path: stats come from dictionary/forward index; no row re-scan.
- Pinot segment input: stats are gathered once via the
`RecordReaderSegmentCreationDataSource` (row scan), then build is column-major,
so no second row scan.
- Implemented column-major build in `SegmentIndexCreationDriverImpl.build()`
with a row-major fallback for non-Pinot readers.
- Added `PinotSegmentRecordReader.getIndexSegment()` and
`RealtimeSegmentSegmentCreationDataSource.getMutableSegment()` to avoid extra
loads and enable column-major indexing.
- Added clear log messages to validate which path is used.
- Avoids double row reads: only one row scan for stats (or none for
realtime) and then column reads for building.
--
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]
