kbendick commented on a change in pull request #3543:
URL: https://github.com/apache/iceberg/pull/3543#discussion_r767343144



##########
File path: core/src/main/java/org/apache/iceberg/CachingCatalog.java
##########
@@ -29,24 +34,99 @@
 import org.apache.iceberg.catalog.Namespace;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
-
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Class that wraps an Iceberg Catalog to cache tables.
+ * <p>
+ * When {@code expirationIntervalMillis} is positive, the cache will expire 
tables after
+ * the given interval provided they have not been accessed via the catalog in 
that time.
+ * Each lookup of the table via the catalog will restart the expiration time.
+ * <p>
+ * If the duration is negative / -1, cache-entry expiration is disabled.
+ * <p>
+ * If the duration is zero, caching should be turned off. If a value of zero 
gets passed to
+ * this {@link Catalog} wrapper, that is a bug.
+ */
 public class CachingCatalog implements Catalog {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(CachingCatalog.class);
+  private static final RemovalListener<TableIdentifier, Table> 
identLoggingRemovalListener =
+      (key, value, cause) -> LOG.debug("Evicted {} from the table cache ({})", 
key, cause);
+
   public static Catalog wrap(Catalog catalog) {
-    return wrap(catalog, true);
+    return wrap(catalog, 
CatalogProperties.TABLE_CACHE_EXPIRATION_INTERVAL_MS_OFF);
+  }
+
+  public static Catalog wrap(Catalog catalog, long expirationIntervalMillis) {
+    return wrap(catalog, true, expirationIntervalMillis);
   }
 
-  public static Catalog wrap(Catalog catalog, boolean caseSensitive) {
-    return new CachingCatalog(catalog, caseSensitive);
+  public static Catalog wrap(Catalog catalog, boolean caseSensitive, long 
expirationIntervalMillis) {
+    return new CachingCatalog(catalog, caseSensitive, 
expirationIntervalMillis);
   }
 
-  private final Cache<TableIdentifier, Table> tableCache = 
Caffeine.newBuilder().softValues().build();
   private final Catalog catalog;
   private final boolean caseSensitive;
+  private final boolean expirationEnabled;
+  @SuppressWarnings("checkstyle:VisibilityModifier")
+  protected final long expirationIntervalMillis;
+  @SuppressWarnings("checkstyle:VisibilityModifier")
+  protected final Cache<TableIdentifier, Table> tableCache;
+
+  private CachingCatalog(Catalog catalog, boolean caseSensitive, long 
expirationIntervalMillis) {
+    this(catalog, caseSensitive, expirationIntervalMillis, 
Ticker.systemTicker());
+  }
 
-  private CachingCatalog(Catalog catalog, boolean caseSensitive) {
+  @SuppressWarnings("checkstyle:VisibilityModifier")
+  protected CachingCatalog(Catalog catalog, boolean caseSensitive, long 
expirationIntervalMillis, Ticker ticker) {
+    Preconditions.checkArgument(expirationIntervalMillis != 0,
+        "When %s is set to 0, the catalog cache should be disabled. This 
indicates a bug.",
+        CatalogProperties.TABLE_CACHE_EXPIRATION_INTERVAL_MS);
     this.catalog = catalog;
     this.caseSensitive = caseSensitive;
+    this.expirationIntervalMillis = expirationIntervalMillis;
+    this.expirationEnabled = expirationIntervalMillis > 0;
+    this.tableCache = createTableCache(ticker);
+  }
+
+  // Class called when an item is removed from or written to the cache, to 
allow for callbacks.
+  // Only used when cache expiration is enabled.
+  class MetadataTableInvalidatingCacheWriter implements 
CacheWriter<TableIdentifier, Table> {

Review comment:
       If it's static, it can't access the `tableCache` variable, which it does 
now that the `onTableExpiration` function's logic has been moved in. E.g. we 
need to be able to `tableCache.invalidateAll` on any metadata tables.




-- 
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]

Reply via email to