real-mj-song commented on PR #18638:
URL: https://github.com/apache/pinot/pull/18638#issuecomment-4705032699

   @siddharthteotia 
   https://github.com/apache/pinot/pull/18638#issuecomment-4667887925
   Out of scope intentionally. Within this PR, I'm trying to contain changes 
for the Arrow plugin only `pinot-plugins/pinot-input-format/pinot-arrow` 
although I ended up touching `pinot-segment-local` a little as well to add 
null/type normalization. The reader side change is done here so the builder 
side change will be a follow-up PR touching:
   * Stats pass — `ColumnarSegmentPreIndexStatsContainer`
   * Index pass — `SegmentColumnarIndexCreator`
   
   https://github.com/apache/pinot/pull/18638#issuecomment-4671734661
   Thanks. Yeah that's definitely a concern. I refactored the file factory so 
that it reads one record batch at a time so that peak resident memory is 
therefore the largest record batch, the same model as the existing [row-major 
`ArrowRecordReader`](https://github.com/apache/pinot/blob/master/pinot-plugins/pinot-input-format/pinot-arrow/src/main/java/org/apache/pinot/plugin/inputformat/arrow/ArrowRecordReader.java#L76-L89).
 The multi-batch path is verified by an end-to-end build that asserts per-doc 
value equivalence, isSorted, and inverted-index parity against the row-major 
build across batch boundaries, plus a test that consumes a file far larger than 
the allocator limit (every batch fits, the whole file doesn't).
   
   > Have we tested this in our production environment with PBNJ to exercise 
end to end build path successfully ?
   
   The integration test added does validate the e2e flow but it hasn't tested 
with the scale of linkedin prod tables. There needs some wiring to do on PBNJ 
side to test/use this builder. I do plan to benchmark locally.


-- 
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]

Reply via email to