diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 7ed0e17b76..4bfd2a45aa 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -753,6 +753,7 @@ Default is 1024 for most caches, except: * `"diff"`: default is `10m` (10 MiB of memory) * `"diff_intraline"`: default is `10m` (10 MiB of memory) * `"diff_summary"`: default is `10m` (10 MiB of memory) +* `"external_ids_map"`: default is `2` and should not be changed * `"groups"`: default is unlimited * `"groups_byname"`: default is unlimited * `"groups_byuuid"`: default is unlimited @@ -762,6 +763,16 @@ Default is 1024 for most caches, except: If set to 0 the cache is disabled. Entries are removed immediately after being stored by the cache. This is primarily useful for testing. +[[cache.name.expireFromMemoryAfterAccess]]cache..expireFromMemoryAfterAccess:: ++ +Time after last access to automatically expire entries from an in-memory +cache. If 0 or not specified, entries are never expired in this manner. +Values may use unit suffixes as in link:#cache.name.maxAge[maxAge]. ++ +This option only applies to in-memory caches; persistent cache values are +not expired in this manner, and are only pruned via +link:#cache.name.diskLimit[diskLimit]. + [[cache.name.diskLimit]]cache..diskLimit:: + Total size in bytes of the keys and values stored on disk. Caches that @@ -844,6 +855,16 @@ Ideally, disk limit of this cache is large enough to cover all changes. This should significantly speed up change reindexing, especially full offline reindexing. +cache `"external_ids_map"`:: ++ +A singleton cache whose sole entry is a map of the parsed representation +of link:config-accounts.html#external-ids[all current external IDs]. The +cache may temporarily contain 2 entries, but the second one is promptly +expired. ++ +It is not recommended to change the attributes of this cache away from +the defaults. + cache `"git_tags"`:: + If branch or reference level READ access controls are used, this diff --git a/java/com/google/gerrit/common/TimeUtil.java b/java/com/google/gerrit/common/TimeUtil.java index 7f53f84bd0..e42eb09fc0 100644 --- a/java/com/google/gerrit/common/TimeUtil.java +++ b/java/com/google/gerrit/common/TimeUtil.java @@ -17,6 +17,7 @@ package com.google.gerrit.common; import com.google.common.annotations.GwtIncompatible; import com.google.common.annotations.VisibleForTesting; import java.sql.Timestamp; +import java.time.Instant; import java.util.function.LongSupplier; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.storage.file.FileBasedConfig; @@ -36,6 +37,10 @@ public class TimeUtil { return currentMillisSupplier.getAsLong(); } + public static Instant now() { + return Instant.ofEpochMilli(nowMs()); + } + public static Timestamp nowTs() { return new Timestamp(nowMs()); } diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java index 739726ed88..c1a75bb31e 100644 --- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java +++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java @@ -44,10 +44,10 @@ import com.google.inject.Singleton; import com.google.inject.TypeLiteral; import com.google.inject.name.Named; import java.io.IOException; +import java.time.Duration; import java.util.Collections; import java.util.HashSet; import java.util.Set; -import java.util.concurrent.TimeUnit; import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; @@ -119,7 +119,7 @@ public class GitOverHttpServlet extends GitServlet { protected void configure() { cache(ID_CACHE, AdvertisedObjectsCacheKey.class, new TypeLiteral>() {}) .maximumWeight(4096) - .expireAfterWrite(10, TimeUnit.MINUTES); + .expireAfterWrite(Duration.ofMinutes(10)); } }); } diff --git a/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java b/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java index c466290b28..caced27a79 100644 --- a/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java +++ b/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java @@ -14,8 +14,6 @@ package com.google.gerrit.httpd; -import static java.util.concurrent.TimeUnit.MINUTES; - import com.google.common.cache.Cache; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.registration.DynamicItem; @@ -30,6 +28,7 @@ import com.google.inject.Provider; import com.google.inject.assistedinject.FactoryModuleBuilder; import com.google.inject.name.Named; import com.google.inject.servlet.RequestScoped; +import java.time.Duration; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -41,10 +40,8 @@ public class H2CacheBasedWebSession extends CacheBasedWebSession { protected void configure() { persist(WebSessionManager.CACHE_NAME, String.class, Val.class) .maximumWeight(1024) // reasonable default for many sites - .expireAfterWrite( - CacheBasedWebSession.MAX_AGE_MINUTES, - MINUTES) // expire sessions if they are inactive - ; + // expire sessions if they are inactive + .expireAfterWrite(Duration.ofMinutes(CacheBasedWebSession.MAX_AGE_MINUTES)); install(new FactoryModuleBuilder().build(WebSessionManagerFactory.class)); DynamicItem.itemOf(binder(), WebSession.class); DynamicItem.bind(binder(), WebSession.class) diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java index c23dd99b79..9d2cbaec62 100644 --- a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java +++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java @@ -16,7 +16,6 @@ package com.google.gerrit.server.account.externalids; import com.google.auto.value.AutoValue; import com.google.common.base.Strings; -import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableSetMultimap; @@ -29,6 +28,7 @@ import com.google.common.flogger.FluentLogger; import com.google.gerrit.reviewdb.client.Account; import com.google.inject.Inject; import com.google.inject.Singleton; +import com.google.inject.name.Named; import java.io.IOException; import java.util.Collection; import java.util.Set; @@ -43,31 +43,17 @@ import org.eclipse.jgit.lib.ObjectId; class ExternalIdCacheImpl implements ExternalIdCache { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + public static final String CACHE_NAME = "external_ids_map"; + private final LoadingCache extIdsByAccount; private final ExternalIdReader externalIdReader; private final Lock lock; @Inject - ExternalIdCacheImpl(ExternalIdReader externalIdReader) { - this.extIdsByAccount = - CacheBuilder.newBuilder() - // The cached data is potentially pretty large and we are always only interested - // in the latest value, hence the maximum cache size is set to 1. - // This can lead to extra cache loads in case of the following race: - // 1. thread 1 reads the notes ref at revision A - // 2. thread 2 updates the notes ref to revision B and stores the derived value - // for B in the cache - // 3. thread 1 attempts to read the data for revision A from the cache, and misses - // 4. later threads attempt to read at B - // In this race unneeded reloads are done in step 3 (reload from revision A) and - // step 4 (reload from revision B, because the value for revision B was lost when the - // reload from revision A was done, since the cache can hold only one entry). - // These reloads could be avoided by increasing the cache size to 2. However the race - // window between reading the ref and looking it up in the cache is small so that - // it's rare that this race happens. Therefore it's not worth to double the memory - // usage of this cache, just to avoid this. - .maximumSize(1) - .build(new Loader(externalIdReader)); + ExternalIdCacheImpl( + @Named(CACHE_NAME) LoadingCache extIdsByAccount, + ExternalIdReader externalIdReader) { + this.extIdsByAccount = extIdsByAccount; this.externalIdReader = externalIdReader; this.lock = new ReentrantLock(true /* fair */); } @@ -159,9 +145,10 @@ class ExternalIdCacheImpl implements ExternalIdCache { } } - private static class Loader extends CacheLoader { + static class Loader extends CacheLoader { private final ExternalIdReader externalIdReader; + @Inject Loader(ExternalIdReader externalIdReader) { this.externalIdReader = externalIdReader; } diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java b/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java index 8c9714474c..7f5a97f282 100644 --- a/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java +++ b/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java @@ -14,11 +14,27 @@ package com.google.gerrit.server.account.externalids; -import com.google.inject.AbstractModule; +import com.google.gerrit.server.account.externalids.ExternalIdCacheImpl.AllExternalIds; +import com.google.gerrit.server.account.externalids.ExternalIdCacheImpl.Loader; +import com.google.gerrit.server.cache.CacheModule; +import com.google.inject.TypeLiteral; +import java.time.Duration; +import org.eclipse.jgit.lib.ObjectId; -public class ExternalIdModule extends AbstractModule { +public class ExternalIdModule extends CacheModule { @Override protected void configure() { + cache(ExternalIdCacheImpl.CACHE_NAME, ObjectId.class, new TypeLiteral() {}) + // The cached data is potentially pretty large and we are always only interested + // in the latest value. However, due to a race condition, it is possible for different + // threads to observe different values of the meta ref, and hence request different keys + // from the cache. Extend the cache size by 1 to cover this case, but expire the extra + // object after a short period of time, since it may be a potentially large amount of + // memory. + .maximumWeight(2) + .expireFromMemoryAfterAccess(Duration.ofMinutes(1)) + .loader(Loader.class); + bind(ExternalIdCacheImpl.class); bind(ExternalIdCache.class).to(ExternalIdCacheImpl.class); } diff --git a/java/com/google/gerrit/server/auth/ldap/LdapModule.java b/java/com/google/gerrit/server/auth/ldap/LdapModule.java index 05228b4168..3fbf04996f 100644 --- a/java/com/google/gerrit/server/auth/ldap/LdapModule.java +++ b/java/com/google/gerrit/server/auth/ldap/LdapModule.java @@ -14,8 +14,6 @@ package com.google.gerrit.server.auth.ldap; -import static java.util.concurrent.TimeUnit.HOURS; - import com.google.common.collect.ImmutableSet; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.reviewdb.client.Account; @@ -25,6 +23,7 @@ import com.google.gerrit.server.account.Realm; import com.google.gerrit.server.cache.CacheModule; import com.google.inject.Scopes; import com.google.inject.TypeLiteral; +import java.time.Duration; import java.util.Optional; import java.util.Set; @@ -37,18 +36,18 @@ public class LdapModule extends CacheModule { @Override protected void configure() { cache(GROUP_CACHE, String.class, new TypeLiteral>() {}) - .expireAfterWrite(1, HOURS) + .expireAfterWrite(Duration.ofHours(1)) .loader(LdapRealm.MemberLoader.class); cache(USERNAME_CACHE, String.class, new TypeLiteral>() {}) .loader(LdapRealm.UserLoader.class); cache(GROUP_EXIST_CACHE, String.class, new TypeLiteral() {}) - .expireAfterWrite(1, HOURS) + .expireAfterWrite(Duration.ofHours(1)) .loader(LdapRealm.ExistenceLoader.class); cache(PARENT_GROUPS_CACHE, String.class, new TypeLiteral>() {}) - .expireAfterWrite(1, HOURS); + .expireAfterWrite(Duration.ofHours(1)); bind(Helper.class); bind(Realm.class).to(LdapRealm.class).in(Scopes.SINGLETON); diff --git a/java/com/google/gerrit/server/cache/CacheBinding.java b/java/com/google/gerrit/server/cache/CacheBinding.java index 1c0d7effcd..9d90d073d5 100644 --- a/java/com/google/gerrit/server/cache/CacheBinding.java +++ b/java/com/google/gerrit/server/cache/CacheBinding.java @@ -16,15 +16,18 @@ package com.google.gerrit.server.cache; import com.google.common.cache.CacheLoader; import com.google.common.cache.Weigher; -import java.util.concurrent.TimeUnit; +import java.time.Duration; /** Configure a cache declared within a {@link CacheModule} instance. */ public interface CacheBinding { /** Set the total size of the cache. */ CacheBinding maximumWeight(long weight); - /** Set the time an element lives before being expired. */ - CacheBinding expireAfterWrite(long duration, TimeUnit durationUnits); + /** Set the time an element lives after last write before being expired. */ + CacheBinding expireAfterWrite(Duration duration); + + /** Set the time an element lives after last access before being expired. */ + CacheBinding expireFromMemoryAfterAccess(Duration duration); /** Populate the cache with items from the CacheLoader. */ CacheBinding loader(Class> clazz); diff --git a/java/com/google/gerrit/server/cache/CacheDef.java b/java/com/google/gerrit/server/cache/CacheDef.java index adb75857fe..d0c633e6fc 100644 --- a/java/com/google/gerrit/server/cache/CacheDef.java +++ b/java/com/google/gerrit/server/cache/CacheDef.java @@ -18,7 +18,7 @@ import com.google.common.cache.CacheLoader; import com.google.common.cache.Weigher; import com.google.gerrit.common.Nullable; import com.google.inject.TypeLiteral; -import java.util.concurrent.TimeUnit; +import java.time.Duration; public interface CacheDef { /** @@ -45,7 +45,10 @@ public interface CacheDef { long maximumWeight(); @Nullable - Long expireAfterWrite(TimeUnit unit); + Duration expireAfterWrite(); + + @Nullable + Duration expireFromMemoryAfterAccess(); @Nullable Weigher weigher(); diff --git a/java/com/google/gerrit/server/cache/CacheProvider.java b/java/com/google/gerrit/server/cache/CacheProvider.java index ce94bfa2a2..2bd570e434 100644 --- a/java/com/google/gerrit/server/cache/CacheProvider.java +++ b/java/com/google/gerrit/server/cache/CacheProvider.java @@ -16,7 +16,6 @@ package com.google.gerrit.server.cache; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import static java.util.concurrent.TimeUnit.SECONDS; import com.google.common.base.Strings; import com.google.common.cache.Cache; @@ -27,7 +26,7 @@ import com.google.gerrit.extensions.annotations.PluginName; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.TypeLiteral; -import java.util.concurrent.TimeUnit; +import java.time.Duration; class CacheProvider implements Provider>, CacheBinding, CacheDef { private final CacheModule module; @@ -36,7 +35,8 @@ class CacheProvider implements Provider>, CacheBinding, private final TypeLiteral valType; private String configKey; private long maximumWeight; - private Long expireAfterWrite; + private Duration expireAfterWrite; + private Duration expireFromMemoryAfterAccess; private Provider> loader; private Provider> weigher; @@ -69,9 +69,16 @@ class CacheProvider implements Provider>, CacheBinding, } @Override - public CacheBinding expireAfterWrite(long duration, TimeUnit unit) { + public CacheBinding expireAfterWrite(Duration duration) { checkNotFrozen(); - expireAfterWrite = SECONDS.convert(duration, unit); + expireAfterWrite = duration; + return this; + } + + @Override + public CacheBinding expireFromMemoryAfterAccess(Duration duration) { + checkNotFrozen(); + expireFromMemoryAfterAccess = duration; return this; } @@ -126,8 +133,14 @@ class CacheProvider implements Provider>, CacheBinding, @Override @Nullable - public Long expireAfterWrite(TimeUnit unit) { - return expireAfterWrite != null ? unit.convert(expireAfterWrite, SECONDS) : null; + public Duration expireAfterWrite() { + return expireAfterWrite; + } + + @Override + @Nullable + public Duration expireFromMemoryAfterAccess() { + return expireFromMemoryAfterAccess; } @Override diff --git a/java/com/google/gerrit/server/cache/PersistentCacheBinding.java b/java/com/google/gerrit/server/cache/PersistentCacheBinding.java index 794d3bbeee..0239ea2f1f 100644 --- a/java/com/google/gerrit/server/cache/PersistentCacheBinding.java +++ b/java/com/google/gerrit/server/cache/PersistentCacheBinding.java @@ -16,7 +16,7 @@ package com.google.gerrit.server.cache; import com.google.common.cache.CacheLoader; import com.google.common.cache.Weigher; -import java.util.concurrent.TimeUnit; +import java.time.Duration; /** Configure a persistent cache declared within a {@link CacheModule} instance. */ public interface PersistentCacheBinding extends CacheBinding { @@ -24,7 +24,7 @@ public interface PersistentCacheBinding extends CacheBinding { PersistentCacheBinding maximumWeight(long weight); @Override - PersistentCacheBinding expireAfterWrite(long duration, TimeUnit durationUnits); + PersistentCacheBinding expireAfterWrite(Duration duration); @Override PersistentCacheBinding loader(Class> clazz); diff --git a/java/com/google/gerrit/server/cache/PersistentCacheProvider.java b/java/com/google/gerrit/server/cache/PersistentCacheProvider.java index 46a9e61ef3..2db9e56f8c 100644 --- a/java/com/google/gerrit/server/cache/PersistentCacheProvider.java +++ b/java/com/google/gerrit/server/cache/PersistentCacheProvider.java @@ -24,7 +24,7 @@ import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.TypeLiteral; import java.io.Serializable; -import java.util.concurrent.TimeUnit; +import java.time.Duration; class PersistentCacheProvider extends CacheProvider implements Provider>, PersistentCacheBinding, PersistentCacheDef { @@ -53,8 +53,8 @@ class PersistentCacheProvider extends CacheProvider } @Override - public PersistentCacheBinding expireAfterWrite(long duration, TimeUnit durationUnits) { - return (PersistentCacheBinding) super.expireAfterWrite(duration, durationUnits); + public PersistentCacheBinding expireAfterWrite(Duration duration) { + return (PersistentCacheBinding) super.expireAfterWrite(duration); } @Override diff --git a/java/com/google/gerrit/server/cache/h2/BUILD b/java/com/google/gerrit/server/cache/h2/BUILD index 96eba1c9ed..fc57a11796 100644 --- a/java/com/google/gerrit/server/cache/h2/BUILD +++ b/java/com/google/gerrit/server/cache/h2/BUILD @@ -3,6 +3,7 @@ java_library( srcs = glob(["**/*.java"]), visibility = ["//visibility:public"], deps = [ + "//java/com/google/gerrit/common:annotations", "//java/com/google/gerrit/common:server", "//java/com/google/gerrit/extensions:api", "//java/com/google/gerrit/lifecycle", diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java b/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java index 7b3da31c0a..78de67dd02 100644 --- a/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java +++ b/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java @@ -16,11 +16,12 @@ package com.google.gerrit.server.cache.h2; import com.google.common.cache.CacheLoader; import com.google.common.cache.Weigher; +import com.google.gerrit.common.Nullable; import com.google.gerrit.server.cache.CacheSerializer; import com.google.gerrit.server.cache.PersistentCacheDef; import com.google.gerrit.server.cache.h2.H2CacheImpl.ValueHolder; import com.google.inject.TypeLiteral; -import java.util.concurrent.TimeUnit; +import java.time.Duration; class H2CacheDefProxy implements PersistentCacheDef { private final PersistentCacheDef source; @@ -30,8 +31,15 @@ class H2CacheDefProxy implements PersistentCacheDef { } @Override - public Long expireAfterWrite(TimeUnit unit) { - return source.expireAfterWrite(unit); + @Nullable + public Duration expireAfterWrite() { + return source.expireAfterWrite(); + } + + @Override + @Nullable + public Duration expireFromMemoryAfterAccess() { + return source.expireFromMemoryAfterAccess(); } @SuppressWarnings("unchecked") diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java index a1cc1c2690..9abccbc30f 100644 --- a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java +++ b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java @@ -215,7 +215,6 @@ class H2CacheFactory implements PersistentCacheFactory, LifecycleListener { if (h2AutoServer) { url.append(";AUTO_SERVER=TRUE"); } - Long expireAfterWrite = def.expireAfterWrite(TimeUnit.SECONDS); return new SqlStore<>( url.toString(), def.keyType(), @@ -223,6 +222,6 @@ class H2CacheFactory implements PersistentCacheFactory, LifecycleListener { def.valueSerializer(), def.version(), maxSize, - expireAfterWrite == null ? 0 : expireAfterWrite.longValue()); + def.expireAfterWrite()); } } diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java index 1c87692afd..d7baee2f97 100644 --- a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java +++ b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java @@ -23,6 +23,7 @@ import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import com.google.common.hash.BloomFilter; +import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.server.cache.CacheSerializer; import com.google.gerrit.server.cache.PersistentCache; @@ -35,6 +36,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.sql.Timestamp; +import java.time.Duration; import java.util.Calendar; import java.util.Map; import java.util.concurrent.ArrayBlockingQueue; @@ -254,7 +256,7 @@ public class H2CacheImpl extends AbstractLoadingCache implements Per private final CacheSerializer valueSerializer; private final int version; private final long maxSize; - private final long expireAfterWrite; + @Nullable private final Duration expireAfterWrite; private final BlockingQueue handles; private final AtomicLong hitCount = new AtomicLong(); private final AtomicLong missCount = new AtomicLong(); @@ -268,7 +270,7 @@ public class H2CacheImpl extends AbstractLoadingCache implements Per CacheSerializer valueSerializer, int version, long maxSize, - long expireAfterWrite) { + @Nullable Duration expireAfterWrite) { this.url = jdbcUrl; this.keyType = createKeyType(keyType, keySerializer); this.valueSerializer = valueSerializer; @@ -421,11 +423,11 @@ public class H2CacheImpl extends AbstractLoadingCache implements Per } private boolean expired(Timestamp created) { - if (expireAfterWrite == 0) { + if (expireAfterWrite == null) { return false; } - long age = TimeUtil.nowMs() - created.getTime(); - return 1000 * expireAfterWrite < age; + Duration age = Duration.between(created.toInstant(), TimeUtil.now()); + return age.compareTo(expireAfterWrite) > 0; } private void touch(SqlHandle c, K key) throws IOException, SQLException { diff --git a/java/com/google/gerrit/server/cache/mem/BUILD b/java/com/google/gerrit/server/cache/mem/BUILD index ef297f1cdb..4106714b3d 100644 --- a/java/com/google/gerrit/server/cache/mem/BUILD +++ b/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", diff --git a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java index f16912a05b..ad1d396668 100644 --- a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java +++ b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java @@ -14,19 +14,23 @@ 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; import com.google.gerrit.server.config.ConfigUtil; import com.google.gerrit.server.config.GerritServerConfig; import com.google.inject.Inject; -import java.util.concurrent.TimeUnit; +import java.time.Duration; import org.eclipse.jgit.lib.Config; class DefaultMemoryCacheFactory implements MemoryCacheFactory { @@ -66,19 +70,38 @@ class DefaultMemoryCacheFactory implements MemoryCacheFactory { } builder.weigher(weigher); - Long age = def.expireAfterWrite(TimeUnit.SECONDS); + Duration expireAfterWrite = def.expireAfterWrite(); if (has(def.configKey(), "maxAge")) { builder.expireAfterWrite( ConfigUtil.getTimeUnit( - cfg, "cache", def.configKey(), "maxAge", age != null ? age : 0, TimeUnit.SECONDS), - TimeUnit.SECONDS); - } else if (age != null) { - builder.expireAfterWrite(age, TimeUnit.SECONDS); + 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(), + "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)); } diff --git a/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java b/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java index 32d0d27e3b..4180192e6e 100644 --- a/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java +++ b/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java @@ -47,7 +47,7 @@ public class H2CacheTest { StringSerializer.INSTANCE, version, 1 << 20, - 0); + null); return new H2CacheImpl<>(MoreExecutors.directExecutor(), store, KEY_TYPE, mem); }