Tweak cache defaults to be more reasonable

We reduce the default cache.diff.memoryLimit as the entries are now
rather large, 1024 may rip through some JVMs entire memory heap and
leave nothing left.  Since there is no way to tell Ehcache that one
entry is larger than another and force expire earlier than it wants
we sort of get stuck with needing to cap the entry count.

We also reduce the cache.openid.memoryLimit as this cache typically
only has 20 or so entries on review.source.android.com, and most of
them are likely expired.  Due to the very short TTL on the entries
the cache only needs to hold the number of authentications which we
do in a 5 minute window, and this is a fairly low figure.

We also modify the web_sessions cache to prefer an LRU expire over
an LFR expire.  This should keep help accessed sessions to be more
likely to stay in memory over being evicted when the cache gets to
a larger size.

Bug: GERRIT-266
Change-Id: I6e2e67d4e7f0bb2131005627a1e0d446d9990b60
Signed-off-by: Shawn O. Pearce <sop@google.com>

src/main/java/com/google/gerrit/server/cache/CachePool.java

@@ -114,9 +114,10 @@ public class CachePool {
       for (CacheProvider<?, ?> p : caches.values()) {
         final String name = p.getName();
         final CacheConfiguration c = newCache(name);
+        c.setMemoryStoreEvictionPolicyFromObject(toPolicy(p.evictionPolicy()));
 
         {
-          int v = c.getMaxElementsInMemory();
+          int v = p.memoryLimit();
           c.setMaxElementsInMemory(getInt(name, "memorylimit", v));
         }
 
@@ -124,13 +125,13 @@ public class CachePool {
           long idle = p.timeToIdle();
           long live = p.timeToLive();
 
-          if (idle == NamedCacheBinding.DEFAULT)
+          if (idle == NamedCacheBinding.DEFAULT_TIME)
             idle = c.getTimeToIdleSeconds();
-          if (live == NamedCacheBinding.DEFAULT)
+          if (live == NamedCacheBinding.DEFAULT_TIME)
             live = c.getTimeToLiveSeconds();
 
           idle = getSeconds(name, "maxage", idle);
-          if (live == NamedCacheBinding.INFINITE) {
+          if (live == NamedCacheBinding.INFINITE_TIME) {
             // Keep the alive period infinite, rather than expiring.
           } else {
             live = Math.max(live, idle);
@@ -143,7 +144,7 @@ public class CachePool {
         }
 
         if (p.disk() && mgr.getDiskStoreConfiguration() != null) {
-          int v = c.getMaxElementsOnDisk();
+          int v = p.diskLimit();
           c.setMaxElementsOnDisk(getInt(name, "disklimit", v));
 
           v = c.getDiskSpoolBufferSizeMB() * MB;
@@ -159,6 +160,19 @@ public class CachePool {
       return mgr;
     }
 
+    private MemoryStoreEvictionPolicy toPolicy(final EvictionPolicy policy) {
+      switch (policy) {
+        case LFU:
+          return MemoryStoreEvictionPolicy.LFU;
+
+        case LRU:
+          return MemoryStoreEvictionPolicy.LRU;
+
+        default:
+          throw new IllegalArgumentException("Unsupported " + policy);
+      }
+    }
+
     private int getInt(String name, String setting, int def) {
       return config.getInt("cache", name, setting, def);
     }

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

@@ -25,8 +25,11 @@ import java.util.concurrent.TimeUnit;
 final class CacheProvider<K, V> implements Provider<Cache<K, V>>,
     NamedCacheBinding, UnnamedCacheBinding {
   private final boolean disk;
-  private long timeToIdle = DEFAULT;
-  private long timeToLive = DEFAULT;
+  private int memoryLimit = 1024;
+  private int diskLimit = 16384;
+  private long timeToIdle = DEFAULT_TIME;
+  private long timeToLive = DEFAULT_TIME;
+  private EvictionPolicy evictionPolicy = EvictionPolicy.LFU;
   private String cacheName;
   private ProxyEhcache cache;
 
@@ -54,6 +57,14 @@ final class CacheProvider<K, V> implements Provider<Cache<K, V>>,
     return disk;
   }
 
+  int memoryLimit() {
+    return memoryLimit;
+  }
+
+  int diskLimit() {
+    return diskLimit;
+  }
+
   long timeToIdle() {
     return timeToIdle;
   }
@@ -62,6 +73,10 @@ final class CacheProvider<K, V> implements Provider<Cache<K, V>>,
     return timeToLive;
   }
 
+  EvictionPolicy evictionPolicy() {
+    return evictionPolicy;
+  }
+
   public NamedCacheBinding name(final String name) {
     if (cacheName != null) {
       throw new IllegalStateException("Cache name already set");
@@ -70,6 +85,23 @@ final class CacheProvider<K, V> implements Provider<Cache<K, V>>,
     return this;
   }
 
+  public NamedCacheBinding memoryLimit(final int objects) {
+    memoryLimit = objects;
+    return this;
+  }
+
+  public NamedCacheBinding diskLimit(final int objects) {
+    if (!disk) {
+      // TODO This should really be a compile time type error, but I'm
+      // too lazy to create the mess of permutations required to setup
+      // type safe returns for bindings in our little DSL.
+      //
+      throw new IllegalStateException("Cache is not disk based");
+    }
+    diskLimit = objects;
+    return this;
+  }
+
   public NamedCacheBinding timeToIdle(final long duration, final TimeUnit unit) {
     if (timeToIdle >= 0) {
       throw new IllegalStateException("Cache timeToIdle already set");
@@ -86,6 +118,12 @@ final class CacheProvider<K, V> implements Provider<Cache<K, V>>,
     return this;
   }
 
+  @Override
+  public NamedCacheBinding evictionPolicy(final EvictionPolicy policy) {
+    evictionPolicy = policy;
+    return this;
+  }
+
   public Cache<K, V> get() {
     if (cache == null) {
       throw new ProvisionException("Cache \"" + cacheName + "\" not available");

src/main/java/com/google/gerrit/server/cache/EvictionPolicy.java

@@ -0,0 +1,24 @@
+// Copyright (C) 2009 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+/** How entries should be evicted from the cache. */
+public enum EvictionPolicy {
+  /** Least recently used is evicted first. */
+  LRU,
+
+  /** Least frequently used is evicted first. */
+  LFU;
+}

src/main/java/com/google/gerrit/server/cache/NamedCacheBinding.java

@@ -18,12 +18,21 @@ import java.util.concurrent.TimeUnit;
 
 /** Configure a cache declared within a {@link CacheModule} instance. */
 public interface NamedCacheBinding {
-  public static final long INFINITE = 0L;
-  public static final long DEFAULT = -1L;
+  public static final long INFINITE_TIME = 0L;
+  public static final long DEFAULT_TIME = -1L;
+
+  /** Set the number of objects to cache in memory. */
+  public NamedCacheBinding memoryLimit(int objects);
+
+  /** Set the number of objects to cache in memory. */
+  public NamedCacheBinding diskLimit(int objects);
 
   /** Set the time an element lives without access before being expired. */
   public NamedCacheBinding timeToIdle(long duration, TimeUnit durationUnits);
 
   /** Set the time an element lives since creation, before being expired. */
   public NamedCacheBinding timeToLive(long duration, TimeUnit durationUnits);
+
+  /** Set the eviction policy for elements when the cache is full. */
+  public NamedCacheBinding evictionPolicy(EvictionPolicy policy);
 }

src/main/java/com/google/gerrit/server/http/WebSession.java

@@ -14,7 +14,7 @@
 
 package com.google.gerrit.server.http;
 
-import static com.google.gerrit.server.cache.NamedCacheBinding.INFINITE;
+import static com.google.gerrit.server.cache.NamedCacheBinding.INFINITE_TIME;
 import static java.util.concurrent.TimeUnit.HOURS;
 
 import com.google.gerrit.client.reviewdb.Account;
@@ -23,6 +23,7 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.cache.Cache;
 import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.cache.EvictionPolicy;
 import com.google.gerrit.server.http.WebSessionManager.Key;
 import com.google.gerrit.server.http.WebSessionManager.Val;
 import com.google.inject.Inject;
@@ -45,7 +46,12 @@ public final class WebSession {
         final String cacheName = WebSessionManager.CACHE_NAME;
         final TypeLiteral<Cache<Key, Val>> type =
             new TypeLiteral<Cache<Key, Val>>() {};
-        disk(type, cacheName).timeToIdle(12, HOURS).timeToLive(INFINITE, HOURS);
+        disk(type, cacheName) //
+            .memoryLimit(1024) // reasonable default for many sites
+            .timeToIdle(12, HOURS) // expire sessions if they are inactive
+            .timeToLive(INFINITE_TIME, HOURS) // never expire a live session
+            .evictionPolicy(EvictionPolicy.LRU) // keep most recently used
+        ;
         bind(WebSessionManager.class);
         bind(WebSession.class).in(RequestScoped.class);
       }

src/main/java/com/google/gerrit/server/openid/OpenIdModule.java

@@ -14,6 +14,8 @@
 
 package com.google.gerrit.server.openid;
 
+import static java.util.concurrent.TimeUnit.MINUTES;
+
 import com.google.gerrit.server.cache.Cache;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.gerrit.server.http.RpcServletModule;
@@ -22,18 +24,22 @@ import com.google.inject.TypeLiteral;
 import com.google.inject.servlet.ServletModule;
 
 import java.util.List;
-import static java.util.concurrent.TimeUnit.MINUTES;
 
 /** Servlets and RPC support related to OpenID authentication. */
 public class OpenIdModule extends ServletModule {
   @Override
   protected void configureServlets() {
     install(new CacheModule() {
+      @SuppressWarnings("unchecked")
       @Override
       protected void configure() {
         final TypeLiteral<Cache<String, List>> type =
             new TypeLiteral<Cache<String, List>>() {};
-        core(type, "openid").timeToIdle(5, MINUTES).timeToLive(5, MINUTES);
+        core(type, "openid") //
+            .timeToIdle(5, MINUTES) // don't cache too long, might be stale
+            .timeToLive(5, MINUTES) //
+            .memoryLimit(64) // short TTL means we won't have many entries
+        ;
       }
     });
 

src/main/java/com/google/gerrit/server/patch/PatchListCache.java

@@ -23,6 +23,7 @@ import com.google.gerrit.client.reviewdb.Project;
 import com.google.gerrit.server.GerritServer;
 import com.google.gerrit.server.cache.Cache;
 import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.cache.EvictionPolicy;
 import com.google.gerrit.server.cache.SelfPopulatingCache;
 import com.google.inject.Inject;
 import com.google.inject.Module;
@@ -55,7 +56,10 @@ public class PatchListCache {
       protected void configure() {
         final TypeLiteral<Cache<PatchListKey, PatchList>> type =
             new TypeLiteral<Cache<PatchListKey, PatchList>>() {};
-        disk(type, CACHE_NAME);
+        disk(type, CACHE_NAME) //
+            .memoryLimit(128) // very large items, cache only a few
+            .evictionPolicy(EvictionPolicy.LRU) // prefer most recent
+        ;
         bind(PatchListCache.class);
       }
     };