TEST prefetching Row Groups using next_reader API in parquet-rs #18391
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🤖 |
alamb
commented
Oct 30, 2025
| { | ||
| 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? |
Dandandan
reviewed
Oct 31, 2025
| 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! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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?