Uh oh!
There was an error while loading. Please reload this page.
GH-3697: Reuse hadoop file status and footer in ParquetRecordReader - #3698
GH-3697: Reuse hadoop file status and footer in ParquetRecordReader#3698steveloughran wants to merge 3 commits into
Conversation
…ader - Ignore InputFile field in ShowFooterCommand - Create ParquetFileReader with InputFile instead of HadoopInputFile Was PARQUET-2415 Contains contributions by Claude.ai (rebasing; junit5/assertj migration) Contributed by wankunde
verify that the metadata isn't included in json or writable marshalling.
wankunde
commented
Jul 31, 2026
AFAK, this changed has been ported into several cloud products. |
There was a problem hiding this comment.
These review comments are generated by Codex. I've just checked them and all of these seem non-blocking to me. Let me know what you think @steveloughran@wankunde.
| * @param footer footer of the file this split refers to | ||
| */ | ||
| public void setFooter(ParquetMetadata footer) { | ||
| this.footer = footer; |
There was a problem hiding this comment.
footer is omitted from write(), but readFields() never clears it. Hadoop may reuse Writable instances, so a split can retain the footer from another file and ParquetRecordReader may open the wrong InputFile. Please clear footer in readFields() and cover object reuse in the round-trip test.
There was a problem hiding this comment.
Good one. Needs fix.
| : HadoopInputFile.fromPath(path, configuration); | ||
| ParquetReadOptions options = optionsBuilder.build(); | ||
| ParquetFileReader reader = footer != null | ||
| ? ParquetFileReader.open(inputFile, footer, options, inputFile.newStream()) |
There was a problem hiding this comment.
Readers opened from the same encrypted footer share its mutable InternalFileDecryptor and JCE Cipher state, so concurrent splits in one JVM can race. Please use reader-local decryption state or skip footer reuse for encrypted files, and add a concurrent encrypted test.
| * Footer of the file, if the split was built by a caller which had already read it. | ||
| * Not written by {@link #write(DataOutput)}, so it is only visible within the JVM which set it. | ||
| */ | ||
| @JsonIgnore |
There was a problem hiding this comment.
This JsonIgnore is relocated in the published jar and is invisible to an external ObjectMapper, as the new CLI mix-in notes. The current test runs before shading. Please use a serialization-neutral exclusion and test the shaded artifact; ParquetMetadata.inputFile has the same issue.
steveloughran
commented
Aug 2, 2026
I will try to fix but can't set a timeline. If you haven't heard I left cloudera on Friday, I do want to wrap up the recent PRs but I need to set up my mac mini as a dev box first, with the pi5 rebuilt as the docker server. That's where the hadoop 3.4.3 jars on maven central came from, FWIW |
Contains contributions by Claude.ai (rebasing; junit5/assertj migration)
Contributed by @wankunde; made current by claude.
Rationale for this change
HTTP HEAD is expensive, slow and comes out of your quota of a few thousand reads/second
What changes are included in this PR?
The InputFile used when reading a file footer is cached in the input split, so all splits read in the same process only will use that and any cached FileStatus when opening the file. This saves a HEAD against s3, azure and gcs on every split read within that process.
Are these changes tested?
new test, otherwise all internal.
Are there any user-facing changes?
no
Closes#3697