andygrove opened a new pull request, #6923: URL: https://github.com/apache/datafusion-comet/pull/6923
## Which issue does this PR close? Closes #6912. ## Rationale for this change After the native Iceberg writer finishes a task, `IcebergReflection.rebuildDataFilesWithJavaMetrics` re-reads every data file's footer so that the committed metrics come from iceberg-java's `ParquetUtil.footerMetrics`. It opened each file with `ParquetFileReader.open(InputFile)`. On parquet-mr 1.14 and later, that call builds its read options from a new Hadoop `Configuration` and reads them back, which parses Hadoop's default XML resources every time: once per data file. That covers every Iceberg version Comet supports except 1.5.2 (Spark 3.4), which bundles parquet-mr 1.13.1. Profiling #6910's `CometIcebergEndToEndBenchmark` put most of the metrics rebuild in `Configuration.loadResources`, and a fanout write pays it for every file it writes. ## What changes are included in this PR? - `readParquetFooter` opens the file with `ParquetFileReader.open(InputFile, ParquetReadOptions)`, passing parquet-mr's default read options built from a `PlainParquetConfiguration`, so no Hadoop `Configuration` is created. The classes are found next to the possibly relocated `ParquetFileReader`, as the existing reflection already does. - On parquet-mr without `ParquetConfiguration` (1.13.1), the footer is still read through `open(InputFile)`, whose defaults are built without Hadoop. - The footer read no longer picks up `parquet.*` keys from an executor-side `core-site.xml`. Read options cover filtering, decryption, allocation, and how string statistics from old parquet-mr writers are read, none of which applies to a footer the native writer has just written. ## How are these changes tested? - A new `IcebergReflectionSuite` test checks that the footer read options carry a `PlainParquetConfiguration` where parquet-mr has one, and that only parquet-mr without `ParquetConfiguration` falls back. It passes on the default profile (Iceberg 1.11, parquet-mr 1.17.1) and on Spark 3.4 (Iceberg 1.5.2, parquet-mr 1.13.1). - `CometIcebergWriteActionSuite` passes. Its metrics-parity tests compare the rebuilt metrics with iceberg-java's writer. - Before and after, with the primitives corpus from #6910 (2M rows, fully native write, `local[5]` on an Apple M3 Ultra, async-profiler attached to both runs): | Target table | Files written | Statement time | Metrics rebuild, share of all CPU | | --- | --- | --- | --- | | Unpartitioned | 10 | within noise (530 ms, then 554 ms) | 3.1% to 0.6% | | Fanout over `bucket(32, id)` | 320 | 1899 ms to 1584 ms (17% faster) | 31.6% to 2.6% | In the fanout case, the write stage's task CPU drops from 3.2 s to 1.7 s per statement, about 4.7 ms per file written. The change is version-sensitive (parquet-mr differs per Iceberg version), so this PR carries `run-iceberg-tests` and `run-all-spark-profiles`. -- 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]
