Add manifest file caching support for Iceberg Hive Metastore catalog

[hackeryang]

Description

Iceberg library has a manifest caching feature contributed by Cloudera:
https://blog.cloudera.com/12-times-faster-query-planning-with-iceberg-manifest-caching-in-impala/

PrestoDB also has similar configurations to turn on this feature: prestodb/presto#21399

Iceberg manifest caching can accelerate query planning.

Additional context and related issues

https://prestodb.io/docs/current/connector/iceberg.html#manifest-file-caching

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
(x) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Add manifest file caching support for Iceberg Hive Metastore catalog. ({issue}`22376`)

[findepi]

this is configuring org.apache.iceberg.ManifestFiles#CONTENT_CACHES, right?
the cache is keyed by FileIO and our ForwardingFileIo is typically short-lived. how effective is this caching?

cc @alexjo2144 @electrum

[hackeryang]

Yes, in one of our production environments, a big sql which was planned for 12s has been reduced to 8s~

In our internal implementation, we also used a Caffeine cache with the key CatalogType and the value FileIo to make a ForwardingFileIo live longer, i can also add relevant codes to our PR

[findepi]

the 12s -> 8s improvement -- was it with or without fileio caching?

what would the numbers be if we don't do this PR and instead enable filesystem caching that we already have in trino?

[hackeryang]

the 12s -> 8s improvement -- was it with or without fileio caching?

what would the numbers be if we don't do this PR and instead enable filesystem caching that we already have in trino?

@findepi the 12s -> 8s improvement was with fileio/manifest caching.

Sorry our newest production environment is 423, so i couldn't test the performance about filesystem caching, but i saw that PrestoDB have manifest caching and filesystem caching both, so i thought that they may not conflict with each other~ The metadata caching in memory is also faster than metadata caching in disks in most cases.

Thank you for your advice, I have modified some codes accordingly, please review again when you have time~ The CICD error seems not related to our PR

[findepi]

@findepi the 12s -> 8s improvement was with fileio/manifest caching.

thanks!

before merging this PR, we should understand what's the level of improvement that it brings (in isolation).

cc @sopel39 @raunaqmorarka

[findinpath]

@hackeryang pls try adding a test into io.trino.plugin.iceberg.TestIcebergFileOperations

I had taken a stint on integrating this functionality into Iceberg as well some time ago in #20862 and the limitation on which I've stumbled was

At the time of this writing, the main limitation in Trino to use this feature is that each time a storage operation is done, a new ForwardingFileIo instance is being created, while the CONTENT_CACHES static field from Iceberg awaits the same FileIO instance to retrieve a certain content cache.

I'm hopeful though that I missed something in my original PR and that your feature is effective for making use of the manifest cache.

[hackeryang]

@hackeryang pls try adding a test into io.trino.plugin.iceberg.TestIcebergFileOperations

I had taken a stint on integrating this functionality into Iceberg as well some time ago in #20862 and the limitation on which I've stumbled was

At the time of this writing, the main limitation in Trino to use this feature is that each time a storage operation is done, a new ForwardingFileIo instance is being created, while the CONTENT_CACHES static field from Iceberg awaits the same FileIO instance to retrieve a certain content cache.

I'm hopeful though that I missed something in my original PR and that your feature is effective for making use of the manifest cache.

We used a Caffeine cache in HiveMetastoreTableOperationsProvider with the key CatalogType and the value FileIO to reduce the frequency of temporary creations of fileio~

Sorry i haven't figure out good logic and APIs to write the ut yet, i will try to add one if i have a better idea, i saw that PrestoDB didn't add it

[lozbrown]

Will there be ways to flush this cache (specific to a table) in the same way the metadata cache can be via SQL

This allows you to have long cache times and still have data available as soon as it ingests by flushing the cache for tables after cache.

its something that's currently missing from the file status cache

[github-actions]

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

[mosabua]

fyi @cwsteinbach .. maybe you can help here.

[raunaqmorarka]

I have another PR adding iceberg metadata caching through a different approach #22739
That PR avoids depending on iceberg's manifest file caching and just implements the caching within Trino. I had attempted using iceberg manifest file caching as well, it forced changes around lifecycle of FileIO object because of the caching being tied to FileIO instance in iceberg. It's also harder to re-use across queries as we'd have to retain FileIO objects across queries to make that work and that may not be appropriate as each of those objects may have different ConnectorIdentity per query. Implementing it in Trino also makes it possible to potentially reuse in delta connector.

[mosabua]

That sounds great @raunaqmorarka - can we conclude that we should therefore close this PR and help with reviews and input on yours?

[raunaqmorarka]

That sounds great @raunaqmorarka - can we conclude that we should therefore close this PR and help with reviews and input on yours?

yup, i think so.
@hackeryang please try out #22739 and let me know if you run into any problems with that.

[mosabua]

Closing as discussed in the previous comments since the approach in this PR is inferior to the linked PR from @raunaqmorarka - we will continue to collaborate their and would love to get review and testing feedback.