[GitHub] [iceberg] danielcweeks commented on a diff in pull request #4518: core: Provide mechanism to cache manifest file content

[GitBox <gi...@apache.org>]

danielcweeks commented on code in PR #4518:
URL: https://github.com/apache/iceberg/pull/4518#discussion_r961815570


##########
core/src/main/java/org/apache/iceberg/ManifestFiles.java:
##########
@@ -43,6 +47,30 @@ private ManifestFiles() {}
               GenericManifestFile.class.getName(),
               ManifestFile.PARTITION_SUMMARY_TYPE,
               GenericPartitionFieldSummary.class.getName()));
+  private static final int CONTENT_CACHES_EXPIRATION_INTERVAL_SECOND = 60 * 60; // 1 hour

Review Comment:
   It seems like if the content cache lives for only 60s and this cache expires after an hour, we're holding on to a lot of FileIO handles for much longer than necessary.
   
   Is there a reason why this need to be so long?  Also it doesn't look like we have an upper bound on this cache, which could be problematic due to how much manifest IO activity can happen during any planning phase. 



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org