peterphitran opened a new pull request, #18238: URL: https://github.com/apache/iceberg/pull/18238
## Summary Two Parquet read settings could be passed into `Parquet.ReadBuilder`, but neither actually worked - the value was silently thrown away, so there was no way to bound how much memory a single read could allocate: - `parquet.read.allocation.size` - caps the size of a single buffer Parquet allocates while reading. - `parquet.hadoop.vectored.io.enabled` - was hardcoded on with no way to turn it off, even though vectored reads ignore the allocation cap entirely. **Why it didn't work:** `set(key, value)` handed the value to Parquet's read-options builder *after* that builder was already built. Parquet only reads those two settings once, while building itself - anything set afterward is silently ignored. **The fix:** hand the settings over *before* the builder is built, instead of after. No new logic for interpreting the settings - Parquet already knows how to read them correctly, it just needed the value at the right time. This also fixes the same gap for callers that only reach Iceberg through the generic `org.apache.iceberg.formats.ReadBuilder` interface (e.g. Beam, via `FormatModelRegistry`) - `.set(key, value)` is the only channel available to them, and it now actually works. Everything else passed to `set()` is unaffected. Related: #16600 reports the same underlying defect (hardcoded vectored I/O causing production OOMs on `S3FileIO`), and #16614 attempted a fix but stalled and was closed for inactivity. This PR only addresses the `iceberg-parquet` layer both of those also touched - it does not include the Spark-side plumbing #16614 also carried (wiring the setting through `SparkReadConf`/reader classes so it's actually reachable from Spark), which is a separate, larger effort out of scope here. ## Test plan New tests spy on the actual buffer sizes Parquet requests while reading, to prove the setting is really applied - not just that reads succeed: - Setting the size bounds every read to that size. - Setting it through the generic string-based call works the same way. - Not setting it at all leaves reads unbounded - proving the above checks aren't vacuous. - The same holds when the setting comes purely from a Hadoop file's own ambient configuration (e.g. a Spark-level Hadoop setting), not through any new API call - this exercises a second, previously-untested code path, using a small instrumented Hadoop filesystem built for this test since the usual spying technique doesn't observe Hadoop-backed reads. Existing Parquet test suites re-run clean: 85 tests total, 0 failures. -- 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]
