gaborkaszab commented on code in PR #14440:
URL: https://github.com/apache/iceberg/pull/14440#discussion_r2631151226
##########
core/src/main/java/org/apache/iceberg/CachingCatalog.java:
##########
@@ -104,18 +160,27 @@ public void onRemoval(TableIdentifier tableIdentifier,
Table table, RemovalCause
}
private Cache<TableIdentifier, Table> createTableCache(Ticker ticker) {
- Caffeine<Object, Object> cacheBuilder = Caffeine.newBuilder().softValues();
-
- if (expirationIntervalMillis > 0) {
- return cacheBuilder
- .removalListener(new MetadataTableInvalidatingRemovalListener())
- .executor(Runnable::run) // Makes the callbacks to removal listener
synchronous
- .expireAfterAccess(Duration.ofMillis(expirationIntervalMillis))
- .ticker(ticker)
- .build();
+
+ if (expirationIntervalMillis <= 0) {
+ return Caffeine.newBuilder().softValues().build();
}
- return cacheBuilder.build();
+ Caffeine<TableIdentifier, Table> cacheBuilder =
+ Caffeine.newBuilder()
+ .softValues()
+ .removalListener(new MetadataTableInvalidatingRemovalListener())
+ .executor(Runnable::run) // Makes the callbacks to removal
listener synchronous
+ .ticker(ticker);
+
+ switch (cacheExpirationPolicy) {
+ case EXPIRE_AFTER_WRITE:
+ return
cacheBuilder.expireAfterWrite(Duration.ofMillis(expirationIntervalMillis)).build();
+ case EXPIRE_AFTER_ACCESS:
+ return
cacheBuilder.expireAfterAccess(Duration.ofMillis(expirationIntervalMillis)).build();
Review Comment:
Giving this a second thought I think using both could make sense. We could
cover the following setup:
1) Have a "longer" ExpireAfterWrite that is aligned with the credential
expiration time for the table obj (e.g. half of the credential expiration).
This guarantees that the table won't be kept in cache so long that the
credentials expire in the meantime.
2) Have a "shorter" ExpireAfterAccess together with the above so that when
there is no access for the table we could evict it sooner.
I'm wondering if `softValues()` is also something we want to tune, @findepi
. E.g. now if the engine holds a table object only as long as aa query (or
maybe query planning) lasts then if we don't have overlapping queries for the
same table, the above setup won't do much. The table will be evicted from cache
between two queries regardless of the above setups.
--
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]
