You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "zhuqi-lucas (via GitHub)" <gi...@apache.org> on 2025/10/17 07:23:21 UTC
[PR] Support eager loading page index parquet metadata [datafusion]
zhuqi-lucas opened a new pull request, #18112:
URL: https://github.com/apache/datafusion/pull/18112
## Which issue does this PR close?
- Closes [#18111](https://github.com/apache/datafusion/issues/18111)
## Rationale for this change
Currently loading metadata will split to two parts, if we enable page index. It will cause heavy load for s3 request when we store parquet file in s3 and without local cache.
This PR add support to eager loading page index parquet metadata, combined two loading to one which is the original datafusion implementation.
## What changes are included in this PR?
Currently loading metadata will split to two parts, if we enable page index. It will cause heavy load for s3 request when we store parquet file in s3 and without local cache.
This PR add support to eager loading page index parquet metadata, combined two loading to one which is the original datafusion implementation.
## Are these changes tested?
Yes
## Are there any user-facing changes?
No
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3418449743
> > I am not sure if with_prefetch_hint can make the eager loading page index parquet metadata to happen in one request...
>
> I agree today that is not how the code works. However, I think it is technically possible and likely not super hard to implement. -- not sure if it is best done in DataFusion or arrow-rs
Great, this will be a further improvement i agree, i will look into!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3418443216
> I am not sure if with_prefetch_hint can make the eager loading page index parquet metadata to happen in one request...
I agree today that is not how the code works. However, I think it is technically possible and likely not super hard to implement. -- not sure if it is best done in DataFusion or arrow-rs
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3418422210
It seems always request 2 times to s3 at lease if we enable page index, i will investigate more:
```rust
/// Attempts to (asynchronously) parse the footer metadata (and optionally page indexes)
/// given a [`MetadataFetch`].
///
/// See [`Self::with_prefetch_hint`] for a discussion of how to reduce the number of fetches
/// performed by this function.
#[cfg(all(feature = "async", feature = "arrow"))]
pub async fn try_load<F: MetadataFetch>(&mut self, mut fetch: F, file_size: u64) -> Result<()> {
let (metadata, remainder) = self.load_metadata(&mut fetch, file_size).await?;
self.metadata = Some(metadata);
// we can return if page indexes aren't requested
if self.column_index == PageIndexPolicy::Skip && self.offset_index == PageIndexPolicy::Skip
{
return Ok(());
}
self.load_page_index_with_remainder(fetch, remainder).await
}
```
I am not sure if with_prefetch_hint can make the eager loading page index parquet metadata to happen in one 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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3418444164
I taken the task https://github.com/apache/datafusion/issues/18118, at least we can reduce one.
> The newly added Push metadata decoder likely makes this easier to implement:
>
> https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataPushDecoder.html
> (as it will tell you what ranges are needed)
Cool, i will look into this.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3451418632
I think this PR is not needed after we close
- https://github.com/apache/datafusion/issues/18118
Specifically, the tests added in the following PR show that with a sufficient prefetch_hint size, only 1 object store request will be performed:
- https://github.com/apache/datafusion/pull/18160
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3451544454
> I think this PR is not needed after we close
>
> * [Reduce number of object store requests when reading parquet files by default (set `metadata_size_hint`) #18118](https://github.com/apache/datafusion/issues/18118)
>
> Specifically, the tests added in the following PR show that with a sufficient prefetch_hint size, only 1 object store request will be performed:
>
> * [Change default prefetch_hint to 512Kb to reduce number of object store requests when reading parquet files #18160](https://github.com/apache/datafusion/pull/18160)
Thank you @alamb , closing this PR now.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3414623178
<img width="1361" height="571" alt="image" src="https://github.com/user-attachments/assets/59b1c7bd-bb79-428d-a6ad-158a6f737fc7" />
Thank you @alamb , the hits file here does not have page index.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3418429112
More details, it seems datafusion default will not add metadata_size_hint, we can try to add and testing it.
```rust
/// (reading) If specified, the parquet reader will try and fetch the last `size_hint`
/// bytes of the parquet file optimistically. If not specified, two reads are required:
/// One read to fetch the 8-byte parquet footer and
/// another to fetch the metadata length encoded in the footer
pub metadata_size_hint: Option<usize>, default = None
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3418442061
> > More details, it seems datafusion default will not add metadata_size_hint, we can try to add and testing it.
>
> I filed a potentially related ticket here:
>
> * [Reduce number of object store requests when reading parquet files by default (set `metadata_size_hint`) #18118](https://github.com/apache/datafusion/issues/18118)
Got it, thank you @alamb!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3425959142
> > This PR is still needed, because currently we split two part of loading metadata if we enable page index, and the second one loading page index will not use reminder, so even we setting prefetch_hint now, we can't reduce the page index request without this PR.
>
> As I understand it, your goal is to reduce the number of object store requests when loading parquet metadata (which is a good goal 👍 )
>
> However, I am not sure this PR achieve this goal-- instead what i think this PR does is change **when** the requests are made to object store to a bit earlier in the processing pipeline, but not the actual number of them.
>
> I think the best way to ensure we minimize the object store requests is:
>
> 1. Use `prefetch_hint` to make a single request and read a portion of the end of the file
> 2. Try and parse both the metadata and page index from the result (if the prefetch was bit enough it will have both structures)
>
> If prefetch fetches enough bytes, this strategy will result in a single object store requests to read all required metadata
>
> I realize setting prefetch today may not actually also parse the page index, but I think that is what we should be working towards (rather than adding another flag, unless there is some need I am missing)
>
> i personally suggest starting with an end to end type test (perhaps in https://github.com/apache/datafusion/blob/main/datafusion/core/tests/parquet_config.rs) that illustrates what is happening:
>
> 1. Runs a SQL query from a parquet file
> 2. Uses an instrumented object store (e.g. something like https://github.com/apache/datafusion/blob/f363e382661a4f45dad2912e9988f1703e46939b/datafusion/core/src/datasource/file_format/parquet.rs#L304-L303 or https://github.com/apache/datafusion/blob/93f136c06dcb6d4cb362110ae5a4b2b3b8571bb7/datafusion-cli/src/object_storage/instrumented.rs#L253-L252) to verify what requests are made
>
> Then we can configure various prefetch settings and ensure that only the expected number of requests are made
Thank you @alamb, this is good suggestion. I will try to do this!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3414614877
Actually, I forgot that https://datasets.clickhouse.com/hits_compatible/athena_partitioned/hits_1.parquet does not have page indexes. I rewrote this file so that it does:
```sql
copy 'hits_1.parquet' to 'hits_1_index.parquet';
```
I was having trouble getting the trace thing to work
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas closed pull request #18112: Support eager loading page index parquet metadata
URL: https://github.com/apache/datafusion/pull/18112
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3425800587
> This PR is still needed, because currently we split two part of loading metadata if we enable page index, and the second one loading page index will not use reminder, so even we setting prefetch_hint now, we can't reduce the page index request without this PR.
As I understand it, your goal is to reduce the number of object store requests when loading parquet metadata (which is a good goal 👍 )
However, I am not sure this PR achieve this goal-- instead what i think this PR does is change **when** the requests are made to object store to a bit earlier in the processing pipeline, but not the actual number of them.
I think the best way to ensure we minimize the object store requests is:
1. Use `prefetch_hint` to make a single request and read a portion of the end of the file
2. Try and parse both the metadata and page index from the result (if the prefetch was bit enough it will have both structures)
If prefetch fetches enough bytes, this strategy will result in a single object store requests to read all required metadata
I realize setting prefetch today may not actually also parse the page index, but I think that is what we should be working towards (rather than adding another flag, unless there is some need I am missing)
i personally suggest starting with an end to end type test (perhaps in https://github.com/apache/datafusion/blob/main/datafusion/core/tests/parquet_config.rs) that illustrates what is happening:
1. Runs a SQL query from a parquet file
2. Uses an instrumented object store (e.g. something like https://github.com/apache/datafusion/blob/f363e382661a4f45dad2912e9988f1703e46939b/datafusion/core/src/datasource/file_format/parquet.rs#L304-L303 or https://github.com/apache/datafusion/blob/93f136c06dcb6d4cb362110ae5a4b2b3b8571bb7/datafusion-cli/src/object_storage/instrumented.rs#L253-L252) to verify what requests are made
Then we can configure various prefetch settings and ensure that only the expected number of requests are made
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3419760426
This PR is still needed, because currently we split two part of page index, and the second one will not use reminder, so even we setting prefetch_hint now, we can't reduce the page index request without this PR.
https://github.com/apache/datafusion/blob/f199b000861360aca01d4f1b9104bf73e9d831cc/datafusion/datasource-parquet/src/opener.rs#L705
```rust
/// Asynchronously fetch the page index structures when a [`ParquetMetaData`] has already
/// been obtained. See [`Self::new_with_metadata()`].
#[cfg(all(feature = "async", feature = "arrow"))]
pub async fn load_page_index<F: MetadataFetch>(&mut self, fetch: F) -> Result<()> {
self.load_page_index_with_remainder(fetch, None).await
}
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3444706999
Ok, I have now created a test harness that I think we can use to write a test that shows the effect of this change on access patterns:
- https://github.com/apache/datafusion/pull/18229
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3418442831
From what I can tell, to read ParquetMetadata with the default configuration of LIstingTable DataFusion will issue 3 object store requests:
* 8 bytes for the footer (which has the length of the metadata)
* N bytes for the metadata (has offsets to the page index, but not the page index structures itself)
* M bytes for the page index structures (which are typically right before the metadata in the file, but not required to be)
The first 8 byte request could be avoiding by changing the default prefetch_hint aka https://github.com/apache/datafusion/issues/18118
I think you could potentially avoiding the third request if you extended the prefetch_hint code to use the page index if it was fetched in the initial request
So the flow would be DataFusion makes an initial request for the last `prefetch_hint` bytes in the file. If that happens to contain enough bytes for metadata and page index no more requests would be made. If additional data was needed additional requests would be made
The newly added Push metadata decoder likely makes this easier to implement:
- https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataPushDecoder.html
(as it will tell you what ranges are needed)
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support eager loading page index parquet metadata [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #18112:
URL: https://github.com/apache/datafusion/pull/18112#issuecomment-3418439165
> More details, it seems datafusion default will not add metadata_size_hint, we can try to add and testing it.
I filed a potentially related ticket here:
- https://github.com/apache/datafusion/issues/18118
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org