For those wondering what happened here: Varun moved the eager read down into the Parquet file reader itself, so Parquet now buffers any file of one megabyte or less in a single request, similar to what parquet-rs (512kb prefetch) and parquet-cpp (64kb prefetch) do.
That broke CI, because our rewrite-table-path code writes manifests with an incorrect length into the manifest list. On the V4 path those manifests are Parquet, so the reader trusted the bad length and failed. It never surfaced before because every test covering that path uses HadoopInputFile, which doesn't trust the length we pass through and always goes back to the filesystem for it. Had those tests used any other FileIO (InMemoryFileIO, S3FileIO, ...) the bug would have shown up when I did the Parquet Manifests pr. The rewrite-table-path fix <https://github.com/apache/iceberg/pull/16910> has now merged and everything is green. I think this is ready to merge, but I'd encourage anyone interested to take a look. It's a quick read, and it changes every Parquet read going forward. Parquet Eager Reads <https://github.com/apache/iceberg/pull/17284> There are two things you should pay close attention to. ParquetIO and Parquet.java each special-case HadoopInputFile today with the following behaviors: *1. ParquetIO discards the length it was handed and re-fetches it from the filesystem.* *2. The Hadoop Configuration behind the HadoopInputFile is passed through to the Parquet reader, minus a few redacted properties.* Varun and I discussed (1) and decided not to preserve it on the eager path. It masks bugs and it costs an extra S3 GET. So for files less than 1MB, the special "is hadoopInputFile, file.getStatus" branch is now omitted. To preserve (2), HadoopInputFile now implements HadoopConfigurable, the check in Parquet.java was widened from HadoopInputFile to HadoopConfigurable, and Varun added a variant of EagerInputFile that carries the delegate's configuration through. Behavior (2) works exactly as before. On Tue, Aug 11, 2026 at 11:41 AM Russell Spitzer <[email protected]> wrote: > I'm good with 1. I think the link I added would be exactly where we could > put it. The code would actually be almost identical for what we would put > in FileIO but only effect Parquet. > > On Tue, Aug 11, 2026 at 11:04 AM Daniel Weeks <[email protected]> wrote: > >> I think it's fair to say that it may not benefit all reads equally >> (puffin files are a good example), but the issue isn't just confined to >> parquet (ORC would also benefit). >> >> I felt this was general enough that we wouldn't necessarily need to make >> the decision at every point of use, which I believe is roughly the same >> approach some accelerator libraries are taking. >> >> It looks like we could integrate this at one of three points: 1. The >> FileReader API, 2. The FileIO layer, 3. Directly from on the InputStream >> at the point of use. >> >> The third options seems like the tightest coupling and maybe the first is >> a better place to see if there's an obvious integration path. >> >> -Dan >> >> >> >> On Mon, Aug 10, 2026 at 7:53 AM Russell Spitzer < >> [email protected]> wrote: >> >>> I'd really like to understand the pros of putting this in FileIO a bit >>> better. >>> >>> If we want to achieve global coverage immediately, we could just jump >>> to Parquet.java >>> <https://github.com/apache/iceberg/blob/3d682a3b65/parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java#L1269-L1271> >>> instead >>> of doing >>> a manifest specific code change. That would cover all usages we care >>> about and avoid adding >>> complexity to the IO implementation for what is essentially a >>> performance fix for parquet-java. Ideally, this should be a fix in >>> parquet-java directly, >>> so keeping it contained to our Parquet reader code feels like the right >>> place for me. There's no >>> reason for us to trigger the same path for, say, a Puffin file or >>> metadata.json. >>> >>> On Wed, Aug 5, 2026 at 7:30 PM Daniel Weeks <[email protected]> wrote: >>> >>>> We almost always have the file length from metadata (we already plumbed >>>> this through to avoid additional head requests). >>>> >>>> We can infer the type from the path, but it doesn't seem like it would >>>> be necessary if Avro is a wash and parquet is faster. >>>> >>>> >>>> >>>> On Wed, Aug 5, 2026, 2:46 PM Russell Spitzer <[email protected]> >>>> wrote: >>>> >>>>> It just seems a lot more complicated to me that the decision on >>>>> whether to prefetch and catch the file would be made by the fileio and not >>>>> the tool opening the file. I wasn’t saying it would be slower, just >>>>> unnecessary. If you check the pr, Varun actually did this benchmark >>>>> already >>>>> and Avro is basically unchanged. >>>>> >>>>> Are you saying fileio should know the size of the file and its type >>>>> when opening? At the moment it really is pretty opaque about that sort of >>>>> thing. >>>>> >>>>> On Wed, Aug 5, 2026 at 1:43 PM Daniel Weeks <[email protected]> wrote: >>>>> >>>>>> I'm not convinced it's actually worse for Avro. With Avro, you >>>>>> typically read the entire file if you intend to open it at all. There's >>>>>> no >>>>>> real skipping or ranged projection happening, so I'm not convinced that a >>>>>> full file fetch would be net slower than the typical read path, which may >>>>>> require multiple incremental fetches (it might even help some of the byte >>>>>> skipping paths). >>>>>> >>>>>> That would be good to benchmark, but I know multiple implementations >>>>>> have this exact type of optimization. If we think we need file type >>>>>> specific behavior, we could incorporate that into the FileIO abstraction >>>>>> as >>>>>> well. >>>>>> >>>>>> On Wed, Aug 5, 2026 at 8:02 AM Russell Spitzer < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Yeah one of the worries here is that while this makes a lot of sense >>>>>>> for parquet, it doesn’t make any sense for Avro. We were mostly >>>>>>> targeting a >>>>>>> more conservative set of changes and then more global modifications >>>>>>> later. >>>>>>> >>>>>>> On Wed, Aug 5, 2026 at 9:41 AM Daniel Weeks <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> Sorry about the confusion, I think my phrasing wasn't quite >>>>>>>> accurate, but the point remains. >>>>>>>> >>>>>>>> The referenced PR for integrating this directly in the >>>>>>>> ManifestFiles class, which narrowly targets the metadata path, not >>>>>>>> making >>>>>>>> this a general FileIO capability, but rather a bespoke shortcircut in >>>>>>>> the >>>>>>>> metadata path. >>>>>>>> >>>>>>>> Prefetching like this would potentially benefit any file operation >>>>>>>> (particularly with small parquet files), but that's not what's >>>>>>>> proposed. >>>>>>>> >>>>>>>> I'm suggesting integrating this into the existing FileIO >>>>>>>> implementations so any InputFile::newStream would be accelerated. The >>>>>>>> difference is making it native to the FileIO as opposed to wrapping the >>>>>>>> stream after. >>>>>>>> >>>>>>>> Hopefully that clarifies, >>>>>>>> -Dan >>>>>>>> >>>>>>>> On Wed, Aug 5, 2026 at 5:13 AM Russell Spitzer < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> Could you elaborate a bit more Dan? This isn’t part of the fileio, >>>>>>>>> it’s a separate file class (much like encrypting file) which is used >>>>>>>>> by all >>>>>>>>> IO like the encryption implementation. >>>>>>>>> >>>>>>>>> On Tue, Aug 4, 2026 at 1:34 PM Daniel Weeks <[email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Hey Varun, >>>>>>>>>> >>>>>>>>>> I noticed a couple weeks ago when this went in and was a little >>>>>>>>>> confused at the time because it wasn't wired in and didn't appear to >>>>>>>>>> be >>>>>>>>>> generically useable, so I'm excited to see this follow up. >>>>>>>>>> >>>>>>>>>> I'm a little concerned based on my impression from the PRs that >>>>>>>>>> we're thinking about integrating this in the wrong way. Rather than >>>>>>>>>> inserting a specific FileIO implementation in sections of the code >>>>>>>>>> path, it >>>>>>>>>> seems this would more generically apply as a base implementation for >>>>>>>>>> any >>>>>>>>>> FileIO. >>>>>>>>>> >>>>>>>>>> I think my preference would be that if you enable eager loading, >>>>>>>>>> it would apply to any existing FileIO implementation and any access. >>>>>>>>>> Then >>>>>>>>>> it's just a matter of tuning the threshold where the implementation >>>>>>>>>> switches from eager fetching to standard/vectored IO paths. >>>>>>>>>> >>>>>>>>>> -Dan >>>>>>>>>> >>>>>>>>>> On Tue, Aug 4, 2026 at 11:27 AM vaquar khan < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >>>>>>>>>>> This is awesome and really valuable, request others to review >>>>>>>>>>> and conclude ,if needs plz perform own benchmark. >>>>>>>>>>> >>>>>>>>>>> Regards, >>>>>>>>>>> Viquar Khan >>>>>>>>>>> >>>>>>>>>>> On Mon, Aug 3, 2026, 11:15 AM Kevin Liu <[email protected]> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Thanks for the great work, this is super exciting. I've been >>>>>>>>>>>> looking at object storage optimizations lately, I'll find some >>>>>>>>>>>> time this >>>>>>>>>>>> week to take a look. >>>>>>>>>>>> >>>>>>>>>>>> On Mon, Aug 3, 2026 at 8:19 AM Varun Lakhyani < >>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> I am looking for views on keeping the one flag true by default >>>>>>>>>>>>> to use EagerInputFile in ManifestFiles read path (Major benefits >>>>>>>>>>>>> in v4 >>>>>>>>>>>>> Parquet Manifests). >>>>>>>>>>>>> We have ready to run benchmark [1] and two independent sets of >>>>>>>>>>>>> results showing similar benefits ~25 - 55% reduction in reading >>>>>>>>>>>>> Parquet >>>>>>>>>>>>> manifest depending on machine and file. >>>>>>>>>>>>> >>>>>>>>>>>>> I would appreciate the community's look and feedback on this - >>>>>>>>>>>>> PR having changes [2] and whether we can enable this path by >>>>>>>>>>>>> default. >>>>>>>>>>>>> >>>>>>>>>>>>> [1] https://github.com/varun-lakhyani/iceberg/pull/1 >>>>>>>>>>>>> [2] https://github.com/apache/iceberg/pull/17284 >>>>>>>>>>>>> >>>>>>>>>>>>> On Wed, Jul 29, 2026 at 6:29 AM Russell Spitzer < >>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> I'm not sure if anyone else has checked this out yet, but I >>>>>>>>>>>>>> think it's a really exciting improvement. We should seriously >>>>>>>>>>>>>> consider >>>>>>>>>>>>>> making this a default feature in the next release. Or >>>>>>>>>>>>>> potentially just have >>>>>>>>>>>>>> it always be on. >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Tue, Jul 28, 2026 at 4:58 PM Varun Lakhyani < >>>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> I think we can flag property to enable this as default true, >>>>>>>>>>>>>>> It would be a great improvement for v4 parquet manifest. >>>>>>>>>>>>>>> Tried to do benchmarkings as extensive as possible. >>>>>>>>>>>>>>> I would love to hear thoughts on this. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Please review the PR once and would appreciate feedback. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Mon, Jul 20, 2026 at 10:02 PM Varun Lakhyani < >>>>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Hello Everyone, >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I evaluated using EagerInputFile in v4 manifest reader path >>>>>>>>>>>>>>>> (parquet) using ManifestBenchmark and S3 as object store and >>>>>>>>>>>>>>>> EC2 machine >>>>>>>>>>>>>>>> for jmh benchmarking. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> This is to make sure we are doing a proper checklist while >>>>>>>>>>>>>>>> wiring this EagerInputFile path here, >>>>>>>>>>>>>>>> Detailed benchmarking setup and results are commented on in >>>>>>>>>>>>>>>> PR[1]. >>>>>>>>>>>>>>>> Using an EC2 machine in the same region (ap-south-1) gives >>>>>>>>>>>>>>>> *~28.4%* to *~42.7%* betterment while comparing against >>>>>>>>>>>>>>>> baseline default while benchmarking. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Please provide review and feedback. >>>>>>>>>>>>>>>> Thanks >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> [1] https://github.com/apache/iceberg/pull/17284 >>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>> Lakhyani Varun >>>>>>>>>>>>>>>> Indian Institute of Technology Roorkee >>>>>>>>>>>>>>>> Contact: +91 96246 46174 >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>
