GH-3394: Cache FileStatus in Footer to reduce redundant NameNode RPC calls#3395
GH-3394: Cache FileStatus in Footer to reduce redundant NameNode RPC calls#3395wangyum wants to merge 1 commit into
Conversation
|
cc @wgtmac |
|
@steveloughran What do you think of this? |
|
this saves a lot of IO against cloud storage, and 100ms per footer. it could be even better if we just relied on file length being passed down, as iceberg stores that in its metadata -if it can make its way all the way down then a lot of time and money is ultimately saved. that'd need extensions to the public API, but passing down just the length here would prepare for those future changes |
|
|
||
| public FootersCacheValue(FileStatusWrapper status, Footer footer) { | ||
| this.modificationTime = status.getModificationTime(); | ||
| this.footer = new Footer(footer.getFile(), footer.getParquetMetadata()); |
There was a problem hiding this comment.
@wangyum Should this be updated to use the new constructor and pass in footer.getFileStatus() ? I think that's needed to actually optimize the ParquetInputFormat.getSplits() call since getFooters uses the cache wrapper construct.
|
@steveloughran Your idea about passing length down is also very interesting (after codex helped me understand it). I'd be happy to pick that up as a follow-on to this when it's merged. |
Rationale for this change
When reading Parquet files from HDFS,
getFileStatus()is called twice for each file:ParquetFileReader.readAllFootersInParallel()ParquetInputFormat.getSplits()This creates redundant NameNode RPC calls. For workloads processing thousands of files, this redundancy significantly increases NameNode load and job startup time.
This PR caches
FileStatusin theFooterobject to eliminate redundant RPC calls, reducing NameNode RPC calls during Parquet file processing.What changes are included in this PR?
Footer.java: AddedFileStatusfield with backward-compatible constructorsParquetFileReader.java: PassFileStatuswhen creatingFooterobjectsParquetInputFormat.java: Reuse cachedFileStatusinstead of callingfs.getFileStatus()againTestFooterFileStatusCaching.java: New test suite with 5 tests proving RPC reductionAre these changes tested?
Yes. Added comprehensive test suite
TestFooterFileStatusCachingwith 5 test cases:Are there any user-facing changes?
No.
Closes #3394