Delegate to memory cache factory for non persistent caches
Restore the same contract compared to H2 based persistent caches: if
diskLimit is zero or negative, the cache is considered to be in memory
only and currently installed memory cache factory should be used.
Feature: Issue 13941
Change-Id: If911621de8ca9676d29850e449f14c684f73710e
diff --git a/BUILD b/BUILD
index 1ffe0a7..e6869e1 100644
--- a/BUILD
+++ b/BUILD
@@ -40,6 +40,7 @@
deps = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
":cache-chroniclemap__plugin",
"@chronicle-bytes//jar",
+ ":chroniclemap-test-lib",
],
)
@@ -49,5 +50,13 @@
labels = ["server"],
deps = [
":cache-chroniclemap__plugin",
+ ":chroniclemap-test-lib",
],
)
+
+java_library(
+ name = "chroniclemap-test-lib",
+ testonly = True,
+ srcs = ["src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/TestPersistentCacheDef.java"],
+ deps = PLUGIN_DEPS,
+)
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheFactory.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheFactory.java
index 174ba03..fafcd42 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheFactory.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheFactory.java
@@ -22,39 +22,54 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.cache.CacheBackend;
+import com.google.gerrit.server.cache.MemoryCacheFactory;
import com.google.gerrit.server.cache.PersistentCacheDef;
import com.google.gerrit.server.cache.PersistentCacheFactory;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.logging.LoggingContextAwareScheduledExecutorService;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.lib.Config;
@Singleton
class ChronicleMapCacheFactory implements PersistentCacheFactory, LifecycleListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final MemoryCacheFactory memCacheFactory;
+ private final Config config;
private final ChronicleMapCacheConfig.Factory configFactory;
private final MetricMaker metricMaker;
private final DynamicMap<Cache<?, ?>> cacheMap;
private final List<ChronicleMapCacheImpl<?, ?>> caches;
private final ScheduledExecutorService cleanup;
+ private final Path cacheDir;
@Inject
ChronicleMapCacheFactory(
+ MemoryCacheFactory memCacheFactory,
+ @GerritServerConfig Config cfg,
+ SitePaths site,
ChronicleMapCacheConfig.Factory configFactory,
DynamicMap<Cache<?, ?>> cacheMap,
MetricMaker metricMaker) {
+ this.memCacheFactory = memCacheFactory;
+ this.config = cfg;
this.configFactory = configFactory;
this.metricMaker = metricMaker;
this.caches = new LinkedList<>();
+ this.cacheDir = getCacheDir(site, cfg.getString("cache", null, "directory"));
this.cacheMap = cacheMap;
this.cleanup =
new LoggingContextAwareScheduledExecutorService(
@@ -66,9 +81,11 @@
.build()));
}
- @SuppressWarnings({"unchecked"})
@Override
public <K, V> Cache<K, V> build(PersistentCacheDef<K, V> in, CacheBackend backend) {
+ if (isInMemoryCache(in)) {
+ return memCacheFactory.build(in, backend);
+ }
ChronicleMapCacheConfig config =
configFactory.create(
in.name(),
@@ -77,7 +94,7 @@
in.expireAfterWrite(),
in.refreshAfterWrite(),
in.version());
- ChronicleMapCacheImpl<K, V> cache = null;
+ ChronicleMapCacheImpl<K, V> cache;
try {
cache = new ChronicleMapCacheImpl<>(in, config, null, metricMaker);
} catch (IOException e) {
@@ -89,10 +106,12 @@
return cache;
}
- @SuppressWarnings("unchecked")
@Override
public <K, V> LoadingCache<K, V> build(
PersistentCacheDef<K, V> in, CacheLoader<K, V> loader, CacheBackend backend) {
+ if (isInMemoryCache(in)) {
+ return memCacheFactory.build(in, loader, backend);
+ }
ChronicleMapCacheConfig config =
configFactory.create(
in.name(),
@@ -101,7 +120,7 @@
in.expireAfterWrite(),
in.refreshAfterWrite(),
in.version());
- ChronicleMapCacheImpl<K, V> cache = null;
+ ChronicleMapCacheImpl<K, V> cache;
try {
cache = new ChronicleMapCacheImpl<>(in, config, loader, metricMaker);
} catch (IOException e) {
@@ -125,6 +144,11 @@
}
}
+ private <K, V> boolean isInMemoryCache(PersistentCacheDef<K, V> in) {
+ return cacheDir == null
+ || config.getLong("cache", in.configKey(), "diskLimit", in.diskLimit()) <= 0;
+ }
+
@Override
public void start() {
for (ChronicleMapCacheImpl<?, ?> cache : caches) {
@@ -136,4 +160,25 @@
public void stop() {
cleanup.shutdownNow();
}
+
+ private static Path getCacheDir(SitePaths site, String name) {
+ if (name == null) {
+ return null;
+ }
+ Path loc = site.resolve(name);
+ if (!Files.exists(loc)) {
+ try {
+ Files.createDirectories(loc);
+ } catch (IOException e) {
+ logger.atWarning().log("Can't create disk cache: %s", loc.toAbsolutePath());
+ return null;
+ }
+ }
+ if (!Files.isWritable(loc)) {
+ logger.atWarning().log("Can't write to disk cache: %s", loc.toAbsolutePath());
+ return null;
+ }
+ logger.atInfo().log("Enabling disk cache %s", loc.toAbsolutePath());
+ return loc;
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java
index f3f6483..2a21121 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java
@@ -84,17 +84,12 @@
mapBuilder.maxBloatFactor(config.getMaxBloatFactor());
- if (config.getPersistedFile() == null || config.getDiskLimit() < 0) {
- store = mapBuilder.create();
- } else {
- logger.atWarning().log(
- "disk storage is enabled for '%s', however chronicle-map "
- + "cannot honour the diskLimit of %s bytes, since the file size "
- + "is pre-allocated rather than being a function of the number"
- + "of entries in the cache",
- def.name(), def.diskLimit());
- store = mapBuilder.createOrRecoverPersistedTo(config.getPersistedFile());
- }
+ logger.atWarning().log(
+ "chronicle-map cannot honour the diskLimit of %s bytes for the %s "
+ + "cache, since the file size is pre-allocated rather than being "
+ + "a function of the number of entries in the cache",
+ def.diskLimit(), def.name());
+ store = mapBuilder.createOrRecoverPersistedTo(config.getPersistedFile());
logger.atInfo().log(
"Initialized '%s'|version: %s|avgKeySize: %s bytes|avgValueSize:"
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheIT.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheIT.java
index 69f0845..df6fc38 100644
--- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheIT.java
+++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheIT.java
@@ -16,15 +16,19 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
+import com.google.common.cache.Cache;
import com.google.common.truth.Truth8;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.accounts.AccountInput;
+import com.google.gerrit.server.cache.CacheBackend;
import com.google.gerrit.server.cache.PersistentCacheFactory;
import com.google.inject.Inject;
import org.junit.Test;
+@UseLocalDisk
public class ChronicleMapCacheIT extends AbstractDaemonTest {
@Inject PersistentCacheFactory persistentCacheFactory;
@@ -40,6 +44,25 @@
}
@Test
+ public void shouldBuildInMemoryCacheWhenDiskLimitIsNegative() {
+ final int negativeDiskLimit = -1;
+ final Cache<String, String> cache =
+ persistentCacheFactory.build(
+ new TestPersistentCacheDef("foo", negativeDiskLimit), CacheBackend.CAFFEINE);
+
+ assertThat(cache.getClass().getSimpleName()).isEqualTo("CaffeinatedGuavaCache");
+ }
+
+ @Test
+ public void shouldBuildInMemoryCacheWhenDiskLimitIsPositive() {
+ final int positiveDiskLimit = 1024;
+ assertThat(
+ persistentCacheFactory.build(
+ new TestPersistentCacheDef("foo", positiveDiskLimit), CacheBackend.CAFFEINE))
+ .isInstanceOf(ChronicleMapCacheImpl.class);
+ }
+
+ @Test
public void shouldCacheNewProject() throws Exception {
String newProjectName = name("newProject");
RestResponse r = adminRestSession.put("/projects/" + newProjectName);
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java
index e8e6236..844212a 100644
--- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java
+++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java
@@ -19,22 +19,17 @@
import com.codahale.metrics.Gauge;
import com.codahale.metrics.MetricRegistry;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.Weigher;
import com.google.gerrit.acceptance.WaitUtil;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker;
-import com.google.gerrit.server.cache.PersistentCacheDef;
-import com.google.gerrit.server.cache.serialize.CacheSerializer;
import com.google.gerrit.server.cache.serialize.StringCacheSerializer;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
-import com.google.inject.TypeLiteral;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.file.Files;
@@ -67,6 +62,9 @@
new FileBasedConfig(
sitePaths.resolve("etc").resolve("gerrit.config").toFile(), FS.DETECTED);
gerritConfig.load();
+ String cacheDirectory = "cache-dir";
+ gerritConfig.setString("cache", null, "directory", cacheDirectory);
+ gerritConfig.save();
setupMetrics();
}
@@ -547,89 +545,4 @@
assertWithMessage(name).that(gauge).isNotNull();
return gauge;
}
-
- public static class TestPersistentCacheDef implements PersistentCacheDef<String, String> {
-
- private final String loadedValue;
-
- TestPersistentCacheDef(String loadedValue) {
-
- this.loadedValue = loadedValue;
- }
-
- @Override
- public long diskLimit() {
- return 0;
- }
-
- @Override
- public int version() {
- return 0;
- }
-
- @Override
- public CacheSerializer<String> keySerializer() {
- return StringCacheSerializer.INSTANCE;
- }
-
- @Override
- public CacheSerializer<String> valueSerializer() {
- return StringCacheSerializer.INSTANCE;
- }
-
- @Override
- public String name() {
- return loadedValue;
- }
-
- @Override
- public String configKey() {
- return name();
- }
-
- @Override
- public TypeLiteral<String> keyType() {
- return new TypeLiteral<String>() {};
- }
-
- @Override
- public TypeLiteral<String> valueType() {
- return new TypeLiteral<String>() {};
- }
-
- @Override
- public long maximumWeight() {
- return 0;
- }
-
- @Override
- public Duration expireAfterWrite() {
- return Duration.ZERO;
- }
-
- @Override
- public Duration expireFromMemoryAfterAccess() {
- return Duration.ZERO;
- }
-
- @Override
- public Duration refreshAfterWrite() {
- return Duration.ZERO;
- }
-
- @Override
- public Weigher<String, String> weigher() {
- return (s, s2) -> 0;
- }
-
- @Override
- public CacheLoader<String, String> loader() {
- return new CacheLoader<String, String>() {
- @Override
- public String load(String s) {
- return loadedValue != null ? loadedValue : UUID.randomUUID().toString();
- }
- };
- }
- }
}
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/TestPersistentCacheDef.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/TestPersistentCacheDef.java
new file mode 100644
index 0000000..b81baec
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/TestPersistentCacheDef.java
@@ -0,0 +1,136 @@
+// Copyright (C) 2020 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.googlesource.gerrit.modules.cache.chroniclemap;
+
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.Weigher;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.server.cache.PersistentCacheDef;
+import com.google.gerrit.server.cache.serialize.CacheSerializer;
+import com.google.gerrit.server.cache.serialize.StringCacheSerializer;
+import com.google.inject.TypeLiteral;
+import java.time.Duration;
+import java.util.UUID;
+
+public class TestPersistentCacheDef implements PersistentCacheDef<String, String> {
+
+ private static final Integer DEFAULT_DISK_LIMIT = 1024;
+
+ private final String loadedValue;
+ private final Duration expireAfterWrite;
+ private final Duration refreshAfterWrite;
+ private final Integer diskLimit;
+
+ public TestPersistentCacheDef(
+ String loadedValue,
+ @Nullable Duration expireAfterWrite,
+ @Nullable Duration refreshAfterWrite) {
+
+ this.loadedValue = loadedValue;
+ this.expireAfterWrite = expireAfterWrite;
+ this.refreshAfterWrite = refreshAfterWrite;
+ this.diskLimit = DEFAULT_DISK_LIMIT;
+ }
+
+ public TestPersistentCacheDef(String loadedValue, Integer diskLimit) {
+
+ this.loadedValue = loadedValue;
+ this.expireAfterWrite = null;
+ this.refreshAfterWrite = null;
+ this.diskLimit = diskLimit;
+ }
+
+ public TestPersistentCacheDef(String loadedValue) {
+
+ this.loadedValue = loadedValue;
+ this.expireAfterWrite = Duration.ZERO;
+ this.refreshAfterWrite = Duration.ZERO;
+ this.diskLimit = DEFAULT_DISK_LIMIT;
+ }
+
+ @Override
+ public long diskLimit() {
+ return diskLimit;
+ }
+
+ @Override
+ public int version() {
+ return 0;
+ }
+
+ @Override
+ public CacheSerializer<String> keySerializer() {
+ return StringCacheSerializer.INSTANCE;
+ }
+
+ @Override
+ public CacheSerializer<String> valueSerializer() {
+ return StringCacheSerializer.INSTANCE;
+ }
+
+ @Override
+ public String name() {
+ return loadedValue;
+ }
+
+ @Override
+ public String configKey() {
+ return name();
+ }
+
+ @Override
+ public TypeLiteral<String> keyType() {
+ return new TypeLiteral<String>() {};
+ }
+
+ @Override
+ public TypeLiteral<String> valueType() {
+ return new TypeLiteral<String>() {};
+ }
+
+ @Override
+ public long maximumWeight() {
+ return 0;
+ }
+
+ @Override
+ public Duration expireAfterWrite() {
+ return expireAfterWrite;
+ }
+
+ @Override
+ public Duration expireFromMemoryAfterAccess() {
+ return Duration.ZERO;
+ }
+
+ @Override
+ public Duration refreshAfterWrite() {
+ return refreshAfterWrite;
+ }
+
+ @Override
+ public Weigher<String, String> weigher() {
+ return (s, s2) -> 0;
+ }
+
+ @Override
+ public CacheLoader<String, String> loader() {
+ return new CacheLoader<String, String>() {
+ @Override
+ public String load(String s) {
+ return loadedValue != null ? loadedValue : UUID.randomUUID().toString();
+ }
+ };
+ }
+}
