gtrettenero opened a new pull request, #18724: URL: https://github.com/apache/iceberg/pull/18724
## Summary Parquet reads the footer by seeking to the end of the file for the footer length and magic, then seeking backward to read the footer (`ParquetFileReader.readFooter`). For `FileIO` streams backed by object stores this is two sequential GET requests per file. For `S3InputStream`, the backward seek also aborts the first, partially read HTTP stream, so the second request opens a new connection and pays a new TLS handshake. This change reads the footer with a single `RangeReadable.readTail` call when the stream supports it, which covers S3, GCS, and ADLS: - Read the last 64 KiB of the file, or the whole file if it is smaller. 64 KiB matches the default footer read size of the Arrow C++ Parquet reader. - If the footer is larger than that, issue one more `readTail` of exactly `footer length + 8` bytes. - Parse the footer with Parquet's own public `ParquetFileReader.readFooter(InputFile, ParquetReadOptions, SeekableInputStream)` over an in-memory stream of those bytes. Parquet still validates length, magic, and footer position, applies the read options' metadata filter (row-group range), and handles encrypted footers. - Open the reader with the existing `ParquetFileReader(InputFile, ParquetMetadata, ParquetReadOptions, SeekableInputStream)` constructor. `ParquetFileReader(InputFile, ParquetReadOptions, SeekableInputStream)` already does exactly this internally (`this(file, readFooter(file, options, f, true), options, f)`); only the source of the footer bytes changes. - Streams that are not `RangeReadable` keep the current path unchanged: Hadoop streams, `AesGcmInputStream`, and in-memory streams. Because `ReadBuilder` always enables vectored IO, and `ParquetRangeReadableInputStreamAdapter` serves `readVectored` with positioned reads, the sequential stream is no longer used at all when reading through `ReadConf`. Each file now costs one tail request plus row-group range requests, instead of two sequential footer requests plus an aborted connection plus the row-group requests. This complements #17284 (eager fetch of files ≤ 1 MiB) for files above the eager-fetch threshold. Below the threshold the stream is the in-memory `EagerInputStream`, so the tail read costs no extra request. ## Motivation We found this while profiling a Spark `rewrite_data_files` workload on S3 that reads many small and medium files. In sampled executor thread dumps, task threads spent a large share of wall time blocked in `ReadConf.newReader` → `ParquetFileReader.readFooter` → `S3InputStream.openStream`. The time split between the two footer requests, and there were TLS handshake frames from the reopened connection. The cost is per file, so it dominates when files are small and numerous. ## Testing New `TestParquetFooterRead`: - A small footer is read with a single tail read and no seek-and-read on the stream. - A footer larger than 64 KiB (2,000-column file) uses exactly two tail reads: 64 KiB, then `footer length + 8`. - An end-to-end `Parquet.read` of a file above the eager-fetch threshold makes no sequential reads; row groups use positioned reads. - Streams without range reads fall back to the existing path. - A non-Parquet file still fails with Parquet's `is not a Parquet file` error, and the stream is closed. Existing tests: - `./gradlew :iceberg-parquet:check`: 784 tests, plus checkstyle, error-prone, and spotless. - `:iceberg-data:test --tests '*Parquet*'`: 146 tests. ## Notes for reviewers - The 64 KiB read size is a constant, not a read property. I'm happy to make it configurable if preferred. - The change relies on each `RangeReadable.readTail` returning exactly the requested bytes when the request is no larger than the file. A short read raises `EOFException`. Like the existing path and `EagerInputFile`, it trusts `InputFile.getLength()`, which may come from manifest metadata. - Follow-up: `ParquetUtil.fileMetrics`, which reads only the footer (used by `TableMigrationUtil`), could use `ParquetIO.open` too. I kept this PR focused on the read path. AI disclosure: this change was drafted with AI assistance (Claude Code). That covered the analysis of the read path, the implementation, and the test scaffolding. The areas above are the ones that most deserve careful review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
