Persist caches keys index at configurable pace
Introduce 'cache.persistIndexEvery' parameter (by default '15m') that
specifies how often indexes should be persisted to a disk.
Notes:
* in order to avoid race condition between evict and persist operations
the latter is performed after the former is finished
* considering the above the smallest duration between persist operations
is 30s amd smaller values of 'persistIndexEvery' will be rounded up to
30s
* higher values are rounded down (if needed) to the closest multiple of
30s
* persist operation is additionally called on Gerrit exit - regardless
of the configured duration but it will be skipped on a particular
index level if there is one already running (as a result of schedule)
* persist operation is scheduled in a dedicated thread
'ChronicleMap-Index-0'
Example when 'cache.persistIndexEvery = 2m' it means that persist
operation will be performed after every 4th eviction is finished.
And additionally on Gerrit exit provided that there is no one in
progress.
Bug: Issue 15121
Change-Id: Ib6ea9c0e2819bf6d93ab4654db9e8a2abc8b5b20
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCaches.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCaches.java
index 6b2b546..e7e1415 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCaches.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCaches.java
@@ -212,7 +212,8 @@
newMaxEntries,
averageKeySize,
averageValueSize,
- currentChronicleMapConfig.getMaxBloatFactor());
+ currentChronicleMapConfig.getMaxBloatFactor(),
+ currentChronicleMapConfig.getPersistIndexEvery());
}
private long newMaxEntries(ChronicleMapCacheImpl<Object, Object> currentCache) {
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndex.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndex.java
index 9e56321..c9aff80 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndex.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndex.java
@@ -41,6 +41,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
class CacheKeysIndex<T> {
@@ -178,6 +179,7 @@
private final String name;
private final File indexFile;
private final File tempIndexFile;
+ private final AtomicBoolean persistInProgress;
CacheKeysIndex(MetricMaker metricMaker, String name, File indexFile, boolean cacheFileExists) {
this.keys = Collections.synchronizedSet(new LinkedHashSet<>());
@@ -185,6 +187,7 @@
this.name = name;
this.indexFile = indexFile;
this.tempIndexFile = tempIndexFile(indexFile);
+ this.persistInProgress = new AtomicBoolean(false);
restore(cacheFileExists);
}
@@ -251,6 +254,14 @@
}
void persist() {
+ if (!persistInProgress.compareAndSet(false, true)) {
+ logger.atWarning().log(
+ "Persist cache keys index %s to %s file is already in progress. This persist request was"
+ + " skipped.",
+ name, indexFile);
+ return;
+ }
+
logger.atInfo().log("Persisting cache keys index %s to %s file", name, indexFile);
try (Timer0.Context timer = metrics.persistLatency.start()) {
ArrayList<TimedKey> toPersist;
@@ -277,6 +288,8 @@
logger.atSevere().withCause(e).log("Persisting cache keys index %s failed", name);
metrics.persistFailures.increment();
}
+ } finally {
+ persistInProgress.set(false);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheConfig.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheConfig.java
index 2b81078..3c7f380 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheConfig.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheConfig.java
@@ -13,9 +13,11 @@
// limitations under the License.
package com.googlesource.gerrit.modules.cache.chroniclemap;
+import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheFactory.PRUNE_DELAY;
import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.collect.ImmutableMap;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -29,6 +31,8 @@
import org.eclipse.jgit.lib.Config;
public class ChronicleMapCacheConfig {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final File cacheFile;
private final boolean cacheFileExists;
private final File indexFile;
@@ -40,6 +44,8 @@
private final int maxBloatFactor;
private final int percentageFreeSpaceEvictionThreshold;
private final String configKey;
+ private final Duration persistIndexEvery;
+ private final long persistIndexEveryNthPrune;
public interface Factory {
ChronicleMapCacheConfig create(
@@ -56,7 +62,8 @@
@Assisted("maxEntries") long maxEntries,
@Assisted("avgKeySize") long avgKeySize,
@Assisted("avgValueSize") long avgValueSize,
- @Assisted("maxBloatFactor") int maxBloatFactor);
+ @Assisted("maxBloatFactor") int maxBloatFactor,
+ @Assisted("PersistIndexEvery") Duration persistIndexEver);
}
@AssistedInject
@@ -76,7 +83,14 @@
cfg.getLong("cache", configKey, "maxEntries", Defaults.maxEntriesFor(configKey)),
cfg.getLong("cache", configKey, "avgKeySize", Defaults.averageKeySizeFor(configKey)),
cfg.getLong("cache", configKey, "avgValueSize", Defaults.avgValueSizeFor(configKey)),
- cfg.getInt("cache", configKey, "maxBloatFactor", Defaults.maxBloatFactorFor(configKey)));
+ cfg.getInt("cache", configKey, "maxBloatFactor", Defaults.maxBloatFactorFor(configKey)),
+ Duration.ofSeconds(
+ cfg.getTimeUnit(
+ "cache",
+ null,
+ "persistIndexEvery",
+ Defaults.persistIndexEvery().toSeconds(),
+ SECONDS)));
}
@AssistedInject
@@ -89,7 +103,8 @@
@Assisted("maxEntries") long maxEntries,
@Assisted("avgKeySize") long avgKeySize,
@Assisted("avgValueSize") long avgValueSize,
- @Assisted("maxBloatFactor") int maxBloatFactor) {
+ @Assisted("maxBloatFactor") int maxBloatFactor,
+ @Assisted("PersistIndexEvery") Duration persistIndexEvery) {
this.cacheFile = cacheFile;
this.cacheFileExists = cacheFile.exists();
this.indexFile = resolveIndexFile(cacheFile);
@@ -120,6 +135,23 @@
configKey,
"percentageFreeSpaceEvictionThreshold",
Defaults.percentageFreeSpaceEvictionThreshold());
+
+ long persistIndexEverySeconds = persistIndexEvery.getSeconds();
+ if (persistIndexEverySeconds < PRUNE_DELAY) {
+ logger.atWarning().log(
+ "Configured 'persistIndexEvery' duration [%ds] is lower than minimal threshold [%ds]."
+ + " Minimal threshold will be used.",
+ persistIndexEverySeconds, PRUNE_DELAY);
+ persistIndexEverySeconds = PRUNE_DELAY;
+ } else if (persistIndexEverySeconds % PRUNE_DELAY != 0L) {
+ logger.atWarning().log(
+ "Configured 'persistIndexEvery' duration [%ds] will be rounded down to a multiple of"
+ + " [%ds]. [%ds] is the minimal 'persistIndexEvery' resolution.",
+ persistIndexEverySeconds, PRUNE_DELAY, PRUNE_DELAY);
+ persistIndexEverySeconds = (persistIndexEverySeconds / PRUNE_DELAY) * PRUNE_DELAY;
+ }
+ this.persistIndexEvery = Duration.ofSeconds(persistIndexEverySeconds);
+ this.persistIndexEveryNthPrune = persistIndexEverySeconds / PRUNE_DELAY;
}
public int getPercentageFreeSpaceEvictionThreshold() {
@@ -134,6 +166,14 @@
return refreshAfterWrite;
}
+ public Duration getPersistIndexEvery() {
+ return persistIndexEvery;
+ }
+
+ public long getPersistIndexEveryNthPrune() {
+ return persistIndexEveryNthPrune;
+ }
+
public long getMaxEntries() {
return maxEntries;
}
@@ -188,6 +228,8 @@
public static final int DEFAULT_PERCENTAGE_FREE_SPACE_EVICTION_THRESHOLD = 90;
+ public static final Duration DEFAULT_PERSIST_INDEX_EVERY = Duration.ofMinutes(15);
+
private static final ImmutableMap<String, DefaultConfig> defaultMap =
new ImmutableMap.Builder<String, DefaultConfig>()
.put("web_sessions", DefaultConfig.create(45, 221, 1000, 1))
@@ -232,5 +274,9 @@
public static int percentageFreeSpaceEvictionThreshold() {
return DEFAULT_PERCENTAGE_FREE_SPACE_EVICTION_THRESHOLD;
}
+
+ public static Duration persistIndexEvery() {
+ return DEFAULT_PERSIST_INDEX_EVERY;
+ }
}
}
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 1161699..e4474b0 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
@@ -46,6 +46,8 @@
@Singleton
class ChronicleMapCacheFactory extends PersistentCacheBaseFactory implements LifecycleListener {
+ static final long PRUNE_DELAY = 30;
+
private final ChronicleMapCacheConfig.Factory configFactory;
private final MetricMaker metricMaker;
private final DynamicMap<Cache<?, ?>> cacheMap;
@@ -53,6 +55,7 @@
private final ScheduledExecutorService cleanup;
private final LoggingContextAwareExecutorService storePersistenceExecutor;
+ private final LoggingContextAwareExecutorService indexPersistenceExecutor;
@Inject
ChronicleMapCacheFactory(
@@ -79,6 +82,10 @@
new LoggingContextAwareExecutorService(
Executors.newFixedThreadPool(
1, new ThreadFactoryBuilder().setNameFormat("ChronicleMap-Store-%d").build()));
+ this.indexPersistenceExecutor =
+ new LoggingContextAwareExecutorService(
+ Executors.newFixedThreadPool(
+ 1, new ThreadFactoryBuilder().setNameFormat("ChronicleMap-Index-%d").build()));
}
@Override
@@ -121,7 +128,8 @@
config,
metricMaker,
memLoader,
- new InMemoryCacheLoadingFromStoreImpl<>(mem, false));
+ new InMemoryCacheLoadingFromStoreImpl<>(mem, false),
+ indexPersistenceExecutor);
} catch (IOException e) {
throw new UncheckedIOException(e);
@@ -173,7 +181,8 @@
config,
metricMaker,
memLoader,
- new InMemoryCacheLoadingFromStoreImpl<>(mem, true));
+ new InMemoryCacheLoadingFromStoreImpl<>(mem, true),
+ indexPersistenceExecutor);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
@@ -198,13 +207,19 @@
@Override
public void start() {
for (ChronicleMapCacheImpl<?, ?> cache : caches) {
- cleanup.scheduleWithFixedDelay(cache::prune, 30, 30, TimeUnit.SECONDS);
+ cleanup.scheduleWithFixedDelay(cache::prune, PRUNE_DELAY, PRUNE_DELAY, TimeUnit.SECONDS);
}
}
@Override
public void stop() {
cleanup.shutdownNow();
+ for (ChronicleMapCacheImpl<?, ?> cache : caches) {
+ cache.close();
+ }
+ caches.clear();
+ storePersistenceExecutor.shutdown();
+ indexPersistenceExecutor.shutdown();
}
public static File fileName(Path cacheDir, String name, Integer version) {
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 8f33271..b9d5c29 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
@@ -28,6 +28,7 @@
import java.time.Instant;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executor;
import java.util.concurrent.atomic.LongAdder;
import net.openhft.chronicle.map.ChronicleMap;
import net.openhft.chronicle.map.ChronicleMapBuilder;
@@ -48,6 +49,8 @@
private final PersistentCacheDef<K, V> cacheDefinition;
private final ChronicleMapCacheLoader<K, V> memLoader;
private final InMemoryCache<K, V> mem;
+ private final Executor indexPersistenceExecutor;
+ private long pruneCount;
ChronicleMapCacheImpl(PersistentCacheDef<K, V> def, ChronicleMapCacheConfig config)
throws IOException {
@@ -63,6 +66,7 @@
new ChronicleMapCacheLoader<>(
MoreExecutors.directExecutor(), store, config.getExpireAfterWrite());
this.mem = memLoader.asInMemoryCacheBypass();
+ this.indexPersistenceExecutor = MoreExecutors.directExecutor();
}
ChronicleMapCacheImpl(
@@ -70,7 +74,8 @@
ChronicleMapCacheConfig config,
MetricMaker metricMaker,
ChronicleMapCacheLoader<K, V> memLoader,
- InMemoryCache<K, V> mem) {
+ InMemoryCache<K, V> mem,
+ Executor indexPersistenceExecutor) {
this.cacheDefinition = def;
this.config = config;
@@ -80,6 +85,7 @@
this.memLoader = memLoader;
this.mem = mem;
this.store = memLoader.getStore();
+ this.indexPersistenceExecutor = indexPersistenceExecutor;
}
@SuppressWarnings({"unchecked", "cast", "rawtypes"})
@@ -121,7 +127,7 @@
logger.atInfo().log(
"Initialized '%s'|version: %s|avgKeySize: %s bytes|avgValueSize:"
+ " %s bytes|entries: %s|maxBloatFactor: %s|remainingAutoResizes:"
- + " %s|percentageFreeSpace: %s",
+ + " %s|percentageFreeSpace: %s|persistIndexEvery: %s",
def.name(),
def.version(),
mapBuilder.constantlySizedKeys() ? "CONSTANT" : config.getAverageKeySize(),
@@ -129,7 +135,8 @@
config.getMaxEntries(),
config.getMaxBloatFactor(),
store.remainingAutoResizes(),
- store.percentageFreeSpace());
+ store.percentageFreeSpace(),
+ config.getPersistIndexEvery());
return new ChronicleMapStore<>(store, config, metricMaker);
}
@@ -274,7 +281,9 @@
evictColdEntries();
}
- keysIndex.persist();
+ if (++pruneCount % config.getPersistIndexEveryNthPrune() == 0L) {
+ indexPersistenceExecutor.execute(keysIndex::persist);
+ }
}
private boolean needsRefresh(long created) {
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/H2MigrationServlet.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/H2MigrationServlet.java
index f96786d..c3959e9 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/H2MigrationServlet.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/H2MigrationServlet.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.modules.cache.chroniclemap;
+import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheConfig.Defaults.persistIndexEvery;
import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.H2_SUFFIX;
import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.getStats;
import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.jdbcUrl;
@@ -295,7 +296,8 @@
stats.size() * sizeMultiplier,
stats.avgKeySize(),
stats.avgValueSize(),
- maxBloatFactor);
+ maxBloatFactor,
+ persistIndexEvery());
}
private void doMigrate(
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 0cacd22..f554b6b 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -90,6 +90,26 @@
Default: *90*
+* `cache.persistIndexEvery`
+: Duration (in seconds if not specified) between caches keys index persist
+operations.
+
+Note that in order to avoid race condition between evict and persist operations
+the latter is performed after the former is finished. Therefore the lowest
+persist resolution is 30s. Smaller values will be rounded up to 30s whereas
+higher values will be rounded down (if needed) to the closest multiple of 30s.
+From practical point of view it means that persist operation will be performed
+after every n-th evict operation is finished.
+For instance if `cache.persistIndexEvery = 2m` then persist will be called
+after every 4th eviction is finished.
+
+Notes:
+* regardless of `cache.persistIndexEvery` persist will be called on
+ Gerrit termination unless there is one already in progress.
+* persist operation is scheduled in a dedicated thread ('ChronicleMap-Index-0')
+
+Default: *15m*
+
### Defaults
Unless overridden by configuration, sensible default values are be provided for
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheConfigTest.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheConfigTest.java
index 61cacca..7f705f3 100644
--- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheConfigTest.java
@@ -19,6 +19,8 @@
import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheConfig.Defaults.DEFAULT_MAX_BLOAT_FACTOR;
import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheConfig.Defaults.DEFAULT_MAX_ENTRIES;
import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheConfig.Defaults.DEFAULT_PERCENTAGE_FREE_SPACE_EVICTION_THRESHOLD;
+import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheConfig.Defaults.DEFAULT_PERSIST_INDEX_EVERY;
+import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheFactory.PRUNE_DELAY;
import com.google.gerrit.server.config.SitePaths;
import java.io.File;
@@ -188,6 +190,32 @@
.isEqualTo(DEFAULT_PERCENTAGE_FREE_SPACE_EVICTION_THRESHOLD);
}
+ @Test
+ public void shouldProvideDefaultIndexPersistEveryValuesWhenNotConfigured() {
+ ChronicleMapCacheConfig configUnderTest = configUnderTest(gerritConfig);
+ assertThat(configUnderTest.getPersistIndexEvery()).isEqualTo(DEFAULT_PERSIST_INDEX_EVERY);
+ assertThat(configUnderTest.getPersistIndexEveryNthPrune()).isEqualTo(30L);
+ }
+
+ @Test
+ public void shouldPersistIndexEveryBePruneDelayWhenPersistIndexEveryIsLowerThanPruneDelay() {
+ gerritConfig.setString(
+ "cache", null, "persistIndexEvery", String.format("%ds", PRUNE_DELAY - 1L));
+ ChronicleMapCacheConfig configUnderTest = configUnderTest(gerritConfig);
+ assertThat(configUnderTest.getPersistIndexEvery()).isEqualTo(Duration.ofSeconds(PRUNE_DELAY));
+ assertThat(configUnderTest.getPersistIndexEveryNthPrune()).isEqualTo(1L);
+ }
+
+ @Test
+ public void shouldPersistIndexEveryBeRoundedDownToAMultiplyOfPruneDelay() {
+ gerritConfig.setString(
+ "cache", null, "persistIndexEvery", String.format("%ds", 2L * PRUNE_DELAY + 1L));
+ ChronicleMapCacheConfig configUnderTest = configUnderTest(gerritConfig);
+ assertThat(configUnderTest.getPersistIndexEvery())
+ .isEqualTo(Duration.ofSeconds(2L * PRUNE_DELAY));
+ assertThat(configUnderTest.getPersistIndexEveryNthPrune()).isEqualTo(2L);
+ }
+
private ChronicleMapCacheConfig configUnderTest(StoredConfig gerritConfig) {
File cacheFile =
ChronicleMapCacheFactory.fileName(
