apache / apache/datafusion

Deferred page-index load in the Parquet opener bypasses FileMetadataCache, causing repeated uncached I/O when the skip heuristic doesn't fire

Open
#23,978 1 comment 2 reactions 1 assignee Claimed by @Minghan2005 View on GitHub
performance
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Describe the bug

apache/datafusion#22857 (backported to branch-54 as #23088, released in 54.1.0) reordered the Parquet opener's state machine so the initial metadata load requests `PageIndexPolicy::Skip`, and the page index is loaded later, only if row-group statistics can't already prove the surviving row groups are fully matched.

When that later load does run, it goes through the free function `load_page_index` in `datafusion/datasource-parquet/src/opener/mod.rs`:

```rust
async fn load_page_index(
reader_metadata: ArrowReaderMetadata,
input: &mut T,
options: ArrowReaderOptions,
) -> Result {
...
let mut reader = ParquetMetaDataReader::new_with_metadata(m)
.with_page_index_policy(PageIndexPolicy::Optional);
reader.load_page_index(input).await?;
...
}
```

This calls `ParquetMetaDataReader::load_page_index`, which reads directly off the `AsyncFileReader`'s byte-range methods. It never goes through `ParquetFileReaderFactory::get_metadata` / `DFParquetMetadata::fetch_metadata`, so the result is never written back into the `FileMetadataCache` that the initial (Skip-policy) metadata load populated.

For files where the skip heuristic never fires, this makes every open of the same file pay for a fresh, uncached page-index fetch, for as many times as the file is opened (once per row-group split / partition). Before #22857, `CachedParquetFileReaderFactory` (then documented as "always loads the entire metadata, including page index, even if not required by the current query") loaded and cached both in one request. After #22857, the equivalent case (skip doesn't fire) now costs one cached footer fetch plus one uncached page-index fetch, repeated on every open.

### To Reproduce

This surfaced in Apache DataFusion Comet (apache/datafusion-comet#3978) on a TPC-DS q88-style query: three `IS NOT NULL` predicates on non-null foreign keys against `store_sales`, scanned across 10,237 files and 1,824 partitions.

Comparing DataFusion 54.0.0 (pre-#22857) to 54.1.0 (post-backport) on the same query and data, cumulative `CometNativeScan` metrics for `store_sales` moved like this (summed across all task instances in the query's physical plan):

| metric | 54.0.0 | 54.1.0 |
| --- | --- | --- |
| Wall clock time elapsed for file opening | 79.5 min | 149.8 min (+88%) |
| Wall clock time elapsed for data decompression + decoding | 57.0 min | 77.6 min (+36%) |
| Total time reading and parsing footer metadata | 79.0 min | 89.1 min (+13%) |
| Number of bytes scanned | 3.4 GiB | 3.5 GiB (+3%) |

Bytes scanned barely moved (ruling out "loading more data"), while file-opening wall clock nearly doubled: a request-count/latency signature, not a data-volume signature, consistent with a second, uncached fetch being added per open.

A minimal repro (no cluster) would be: a Parquet file with a page index, a `IS NOT NULL` predicate on a column whose row groups don't carry `null_count` statistics (or use a writer that omits it), opened twice through `CachedParquetFileReaderFactory` sharing one `FileMetadataCache`. Assert that the second open's `AsyncFileReader::get_metadata` does not re-issue a range read for the page index. On 54.1.0 (or current main) it does; it also does after the first open in the 54.0.0 case, but there the initial factory call already fetched and cached it, so nothing further happens.

### Expected behavior

Once the opener determines the page index is needed and loads it via `load_page_index`, the result should be merged back into the same `FileMetadataCache` entry that the initial (Skip-policy) load populated, so a second open of the same file for a different row-group range gets a cache hit instead of repeating the fetch. The skip optimization from #22857 should still avoid the load entirely when it can; this is about not silently losing caching in the case where it can't.

### Additional context

- apache/datafusion#22795: the original design doc for the skip optimization.
- apache/datafusion#22857 / branch-54 backport #23088: introduced the reordering and the bypass.
- apache/datafusion-comet#3978: where this was diagnosed downstream. Comet's workaround (forcing eager, always-cached page-index loading via a Comet-owned `ParquetFileReaderFactory`, giving up the skip's benefit to restore caching) is not a fix for this issue, just a way to unblock Comet while this is open upstream.
- Relevant code: `load_page_index` (module-level free function) and `RowGroupsPrunedParquetOpen::load_page_index` in `datafusion/datasource-parquet/src/opener/mod.rs`; `DFParquetMetadata::fetch_metadata` / `CachedParquetFileReaderFactory` in `datafusion/datasource-parquet/src/metadata.rs` and `reader.rs`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.