Prefetch Row Groups using next_reader API in parquet-rs#18391
Prefetch Row Groups using next_reader API in parquet-rs#18391alamb wants to merge 3 commits intoapache:mainfrom
next_reader API in parquet-rs#18391Conversation
|
🤖 |
| { | ||
| pub fn new(stream: ParquetRecordBatchStream<T>) -> Self { | ||
| Self { | ||
| state: EagerPrefetchState::next_row_group(stream), |
There was a problem hiding this comment.
in real code, this shouldn't start until the first batch is requested I suspect
|
🤖: Benchmark completed Details
|
|
🤖 |
|
🤖: Benchmark completed Details
|
|
🤖 |
|
🤖: Benchmark completed Details
|
|
🤖 |
|
🤖: Benchmark completed Details
|
|
The benchmark results actually look fairly promising. I'll try and confirm these results manually and then polish this PR up and mark it ready for review |
|
Might as well run some tests agains s3 / object store to see if performance improvement is even better? |
| self.state = EagerPrefetchState::Done; | ||
| } else { | ||
| // immediately start reading the next row group | ||
| self.state = EagerPrefetchState::next_row_group(stream); |
There was a problem hiding this comment.
Might even make this configurable (i.e. prefetch >=2 row groups?)
There was a problem hiding this comment.
good call -- I will look into that. I am not sure buffering more than 1 row group will really help (if you have more than one reader buffered it means the rest of the plan can't consume the data fast enough (and we need to apply backpressure)
|
I filed #18391 to track this idea more |
|
Cool! |
e773e91 to
d791922
Compare
4f1deda to
9fcad0f
Compare
next_reader API in parquet-rs
9fcad0f to
992b9e7
Compare
Partially written by Codex
992b9e7 to
e4f3634
Compare
|
🤖 |
|
🤖: Benchmark completed Details
|
|
🤖 |
|
🤖: Benchmark completed Details
|
|
🤖 |
|
🤖: Benchmark completed Details
|
|
🤖 |
|
🤖: Benchmark completed Details
|
|
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days. |
Which issue does this PR close?
Rationale for this change
While messing arund with the push decoder in
ParquetRecordBatchStreamin terms of the PushDecoder arrow-rs#8159I stumbled upon the
ParquetRecordBatchStream::next_row_groupAPI added by @Xuanwo upstream in parquet:This API allows starting the IO work for the next reader while processing the current reader which should
allow better overlapping of IO and CPU work.
I figured I would give it a quick test in DataFusion to see if it actually helps.
What changes are included in this PR?
Add a prefetch stream that uses
ParquetRecordBatchStream::next_row_groupNote: this will buffer more memory (potentially has an extra row group fetched in practice)
Are these changes tested?
Are there any user-facing changes?