Support expiring from in-memory caches after last access

Use the existing CacheBuilder#expireAfterAccess method for in-memory
caches.

This configuration is not respected for persistent H2 caches. There is
already an "accessed" column, so in theory we could compare the current
time to the value in this column. However, this has the problem that the
column is not updated on every lookup against the outer memory cache
which wraps the H2 cache; it's only updated when the value isn't in the
memory cache and needs to be read into the memory cache from storage. As
a result, the most frequently accessed elements in the in-memory cache
would be the first elements to expire from the persistent cache.

Rather than allowing this perverse behavior, just ignore
expireAfterAccess in the H2 implementation, and document it properly.
Elements may still get pruned from the cache when it exceeds the
configured size. (The pruning code actually uses the accessed column
and thus may be subject to the perverse behavior we're trying to avoid,
but fixing it is beyond the scope of this change.)

It would be theoretically possible to update the accessed timestamp in
the H2 database on every read from the memory cache, but cache reads are
extremely frequent and this would likely put too much write load on the
H2 database.

Change-Id: I881df14bcabde1376145eed815f69d7493751574

java/com/google/gerrit/server/cache/CacheBinding.java

@@ -23,9 +23,12 @@ public interface CacheBinding<K, V> {
   /** Set the total size of the cache. */
   CacheBinding<K, V> maximumWeight(long weight);
 
-  /** Set the time an element lives before being expired. */
+  /** Set the time an element lives after last write before being expired. */
   CacheBinding<K, V> expireAfterWrite(Duration duration);
 
+  /** Set the time an element lives after last access before being expired. */
+  CacheBinding<K, V> expireFromMemoryAfterAccess(Duration duration);
+
   /** Populate the cache with items from the CacheLoader. */
   CacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> clazz);
 

java/com/google/gerrit/server/cache/CacheDef.java

@@ -47,6 +47,9 @@ public interface CacheDef<K, V> {
   @Nullable
   Duration expireAfterWrite();
 
+  @Nullable
+  Duration expireFromMemoryAfterAccess();
+
   @Nullable
   Weigher<K, V> weigher();
 

java/com/google/gerrit/server/cache/CacheProvider.java

@@ -36,6 +36,7 @@ class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>,
   private String configKey;
   private long maximumWeight;
   private Duration expireAfterWrite;
+  private Duration expireFromMemoryAfterAccess;
   private Provider<CacheLoader<K, V>> loader;
   private Provider<Weigher<K, V>> weigher;
 
@@ -74,6 +75,13 @@ class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>,
     return this;
   }
 
+  @Override
+  public CacheBinding<K, V> expireFromMemoryAfterAccess(Duration duration) {
+    checkNotFrozen();
+    expireFromMemoryAfterAccess = duration;
+    return this;
+  }
+
   @Override
   public CacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> impl) {
     checkNotFrozen();
@@ -129,6 +137,12 @@ class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>,
     return expireAfterWrite;
   }
 
+  @Override
+  @Nullable
+  public Duration expireFromMemoryAfterAccess() {
+    return expireFromMemoryAfterAccess;
+  }
+
   @Override
   @Nullable
   public Weigher<K, V> weigher() {

java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java

@@ -36,6 +36,12 @@ class H2CacheDefProxy<K, V> implements PersistentCacheDef<K, V> {
     return source.expireAfterWrite();
   }
 
+  @Override
+  @Nullable
+  public Duration expireFromMemoryAfterAccess() {
+    return source.expireFromMemoryAfterAccess();
+  }
+
   @SuppressWarnings("unchecked")
   @Override
   public Weigher<K, V> weigher() {

java/com/google/gerrit/server/cache/mem/BUILD

@@ -3,6 +3,7 @@ java_library(
     srcs = glob(["*.java"]),
     visibility = ["//visibility:public"],
     deps = [
+        "//java/com/google/gerrit/common:annotations",
         "//java/com/google/gerrit/extensions:api",
         "//java/com/google/gerrit/server",
         "//lib:guava",

java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java

@@ -14,12 +14,16 @@
 
 package com.google.gerrit.server.cache.mem;
 
+import static java.util.concurrent.TimeUnit.NANOSECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+
 import com.google.common.base.Strings;
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.cache.Weigher;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.server.cache.CacheDef;
 import com.google.gerrit.server.cache.ForwardingRemovalListener;
 import com.google.gerrit.server.cache.MemoryCacheFactory;
@@ -27,7 +31,6 @@ import com.google.gerrit.server.config.ConfigUtil;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
 import java.time.Duration;
-import java.util.concurrent.TimeUnit;
 import org.eclipse.jgit.lib.Config;
 
 class DefaultMemoryCacheFactory implements MemoryCacheFactory {
@@ -67,24 +70,38 @@ class DefaultMemoryCacheFactory implements MemoryCacheFactory {
     }
     builder.weigher(weigher);
 
-    Duration age = def.expireAfterWrite();
+    Duration expireAfterWrite = def.expireAfterWrite();
     if (has(def.configKey(), "maxAge")) {
       builder.expireAfterWrite(
+          ConfigUtil.getTimeUnit(
+              cfg, "cache", def.configKey(), "maxAge", toSeconds(expireAfterWrite), SECONDS),
+          SECONDS);
+    } else if (expireAfterWrite != null) {
+      builder.expireAfterWrite(expireAfterWrite.toNanos(), NANOSECONDS);
+    }
+
+    Duration expireAfterAccess = def.expireFromMemoryAfterAccess();
+    if (has(def.configKey(), "expireFromMemoryAfterAccess")) {
+      builder.expireAfterAccess(
           ConfigUtil.getTimeUnit(
               cfg,
               "cache",
               def.configKey(),
-              "maxAge",
-              age != null ? age.getSeconds() : 0,
-              TimeUnit.SECONDS),
-          TimeUnit.SECONDS);
-    } else if (age != null) {
-      builder.expireAfterWrite(age.toNanos(), TimeUnit.NANOSECONDS);
+              "expireFromMemoryAfterAccess",
+              toSeconds(expireAfterAccess),
+              SECONDS),
+          SECONDS);
+    } else if (expireAfterAccess != null) {
+      builder.expireAfterAccess(expireAfterAccess.toNanos(), NANOSECONDS);
     }
 
     return builder;
   }
 
+  private static long toSeconds(@Nullable Duration duration) {
+    return duration != null ? duration.getSeconds() : 0;
+  }
+
   private boolean has(String name, String var) {
     return !Strings.isNullOrEmpty(cfg.getString("cache", name, var));
   }