Persist cache keys index to a file Persist cache keys index after every prune operation (temporarily) and additionally on cache close operation. Notes: * it is persisted to '[cache_file_name].index.tmp' file and once operation is finished renamed to '[cache_file_name].index' so that in case when Gerrit is abruptly closed the index file is not corrupted Bug: Issue 15121 Change-Id: Ib2013cede2be758148a7031ff6942f5c2b035caf
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 948aaee..6b2b546 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
@@ -206,7 +206,7 @@ return configFactory.createWithValues( currentChronicleMapConfig.getConfigKey(), - resolveNewFile(currentChronicleMapConfig.getPersistedFile().getName()), + resolveNewFile(currentChronicleMapConfig.getCacheFile().getName()), currentChronicleMapConfig.getExpireAfterWrite(), currentChronicleMapConfig.getRefreshAfterWrite(), newMaxEntries,
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 a6f608d..430a95e 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
@@ -17,11 +17,24 @@ import static java.util.stream.Collectors.toUnmodifiableSet; import com.google.common.annotations.VisibleForTesting; +import com.google.common.flogger.FluentLogger; import com.google.errorprone.annotations.CompatibleWith; import com.google.gerrit.metrics.Description; import com.google.gerrit.metrics.Description.Units; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.metrics.Timer0; +import com.google.gerrit.server.cache.serialize.CacheSerializer; +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.DataInput; +import java.io.DataInputStream; +import java.io.DataOutput; +import java.io.DataOutputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashSet; import java.util.Objects; @@ -117,12 +130,21 @@ } } + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private final Set<TimedKey> keys; private final Metrics metrics; + private final String name; + private final File indexFile; + private final File tempIndexFile; - CacheKeysIndex(MetricMaker metricMaker, String name) { + CacheKeysIndex(MetricMaker metricMaker, String name, File indexFile, boolean cacheFileExists) { this.keys = Collections.synchronizedSet(new LinkedHashSet<>()); this.metrics = new Metrics(metricMaker, name); + this.name = name; + this.indexFile = indexFile; + this.tempIndexFile = tempIndexFile(indexFile); + restore(cacheFileExists); } @SuppressWarnings("unchecked") @@ -186,4 +208,90 @@ Set<TimedKey> keys() { return keys; } + + void persist() { + logger.atInfo().log("Persisting cache keys index %s to %s file", name, indexFile); + ArrayList<TimedKey> toPersist; + synchronized (keys) { + toPersist = new ArrayList<>(keys); + } + CacheSerializer<T> serializer = CacheSerializers.getKeySerializer(name); + try (DataOutputStream dos = + new DataOutputStream(new BufferedOutputStream(new FileOutputStream(tempIndexFile)))) { + for (TimedKey key : toPersist) { + writeKey(serializer, dos, key); + } + dos.flush(); + indexFile.delete(); + if (!tempIndexFile.renameTo(indexFile)) { + logger.atSevere().log( + "Renaming temporary index file %s to %s was not successful. Persisting cache key index failed.", + tempIndexFile, indexFile); + return; + } + logger.atInfo().log("Cache keys index %s was persisted to %s file", name, indexFile); + } catch (Exception e) { + logger.atSevere().withCause(e).log("Persisting cache keys index %s failed", name); + } + } + + void restore(boolean cacheFileExists) { + try { + if (tempIndexFile.isFile()) { + logger.atWarning().log( + "Gerrit was not closed properly as index persist operation was not finished: temporary" + + " index storage file %s exists for %s cache.", + tempIndexFile, name); + if (!tempIndexFile.delete()) { + logger.atSevere().log( + "Cannot delete the temporary index storage file %s.", tempIndexFile); + return; + } + } + + if (!indexFile.isFile() || !indexFile.canRead()) { + if (cacheFileExists) { + logger.atWarning().log( + "Restoring cache keys index %s not possible. File %s doesn't exist or cannot be read." + + " Existing persisted entries will be pruned only when they are accessed after" + + " Gerrit start.", + name, indexFile.getPath()); + } + return; + } + + logger.atInfo().log("Restoring cache keys index %s from %s file", name, indexFile); + CacheSerializer<T> serializer = CacheSerializers.getKeySerializer(name); + try (DataInputStream dis = + new DataInputStream(new BufferedInputStream(new FileInputStream(indexFile)))) { + while (dis.available() > 0) { + keys.add(readKey(serializer, dis)); + } + logger.atInfo().log("Cache keys index %s was restored from %s file", name, indexFile); + } + } catch (Exception e) { + logger.atSevere().withCause(e).log("Restoring cache keys index %s failed", name); + } + } + + static final File tempIndexFile(File indexFile) { + return new File(String.format("%s.tmp", indexFile.getPath())); + } + + private void writeKey(CacheSerializer<T> serializer, DataOutput out, TimedKey key) + throws IOException { + byte[] serializeKey = serializer.serialize(key.getKey()); + out.writeLong(key.getCreated()); + out.writeInt(serializeKey.length); + out.write(serializeKey); + } + + private TimedKey readKey(CacheSerializer<T> serializer, DataInput in) throws IOException { + long created = in.readLong(); + int keySize = in.readInt(); + byte[] serializedKey = new byte[keySize]; + in.readFully(serializedKey); + T key = serializer.deserialize(serializedKey); + return new TimedKey(key, created); + } }
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 7011f65..2b81078 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
@@ -22,12 +22,16 @@ import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; import java.io.File; +import java.nio.file.Path; import java.time.Duration; import java.util.Optional; +import org.apache.commons.io.FilenameUtils; import org.eclipse.jgit.lib.Config; public class ChronicleMapCacheConfig { - private final File persistedFile; + private final File cacheFile; + private final boolean cacheFileExists; + private final File indexFile; private final long maxEntries; private final long averageKeySize; private final long averageValueSize; @@ -40,13 +44,13 @@ public interface Factory { ChronicleMapCacheConfig create( @Assisted("ConfigKey") String configKey, - @Assisted File persistedFile, + @Assisted File cacheFile, @Nullable @Assisted("ExpireAfterWrite") Duration expireAfterWrite, @Nullable @Assisted("RefreshAfterWrite") Duration refreshAfterWrite); ChronicleMapCacheConfig createWithValues( @Assisted("ConfigKey") String configKey, - @Assisted File persistedFile, + @Assisted File cacheFile, @Nullable @Assisted("ExpireAfterWrite") Duration expireAfterWrite, @Nullable @Assisted("RefreshAfterWrite") Duration refreshAfterWrite, @Assisted("maxEntries") long maxEntries, @@ -59,14 +63,14 @@ ChronicleMapCacheConfig( @GerritServerConfig Config cfg, @Assisted("ConfigKey") String configKey, - @Assisted File persistedFile, + @Assisted File cacheFile, @Nullable @Assisted("ExpireAfterWrite") Duration expireAfterWrite, @Nullable @Assisted("RefreshAfterWrite") Duration refreshAfterWrite) { this( cfg, configKey, - persistedFile, + cacheFile, expireAfterWrite, refreshAfterWrite, cfg.getLong("cache", configKey, "maxEntries", Defaults.maxEntriesFor(configKey)), @@ -79,14 +83,16 @@ ChronicleMapCacheConfig( @GerritServerConfig Config cfg, @Assisted("ConfigKey") String configKey, - @Assisted File persistedFile, + @Assisted File cacheFile, @Nullable @Assisted("ExpireAfterWrite") Duration expireAfterWrite, @Nullable @Assisted("RefreshAfterWrite") Duration refreshAfterWrite, @Assisted("maxEntries") long maxEntries, @Assisted("avgKeySize") long avgKeySize, @Assisted("avgValueSize") long avgValueSize, @Assisted("maxBloatFactor") int maxBloatFactor) { - this.persistedFile = persistedFile; + this.cacheFile = cacheFile; + this.cacheFileExists = cacheFile.exists(); + this.indexFile = resolveIndexFile(cacheFile); this.configKey = configKey; this.maxEntries = maxEntries; @@ -132,8 +138,16 @@ return maxEntries; } - public File getPersistedFile() { - return persistedFile; + public File getCacheFile() { + return cacheFile; + } + + public boolean getCacheFileExists() { + return cacheFileExists; + } + + public File getIndexFile() { + return indexFile; } public long getAverageKeySize() { @@ -152,6 +166,13 @@ return configKey; } + private static File resolveIndexFile(File persistedCacheFile) { + String cacheFileName = persistedCacheFile.getName(); + String indexFileName = String.format("%s.index", FilenameUtils.getBaseName(cacheFileName)); + + return Path.of(persistedCacheFile.getParent()).resolve(indexFileName).toFile(); + } + private static long toSeconds(@Nullable Duration duration) { return duration != null ? duration.getSeconds() : 0; }
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 3740959..8f33271 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
@@ -55,7 +55,9 @@ this.cacheDefinition = def; this.config = config; - this.keysIndex = new CacheKeysIndex<>(metricMaker, def.name()); + this.keysIndex = + new CacheKeysIndex<>( + metricMaker, def.name(), config.getIndexFile(), config.getCacheFileExists()); this.store = createOrRecoverStore(def, config, metricMaker); this.memLoader = new ChronicleMapCacheLoader<>( @@ -72,7 +74,9 @@ this.cacheDefinition = def; this.config = config; - this.keysIndex = new CacheKeysIndex<>(metricMaker, def.name()); + this.keysIndex = + new CacheKeysIndex<>( + metricMaker, def.name(), config.getIndexFile(), config.getCacheFileExists()); this.memLoader = memLoader; this.mem = mem; this.store = memLoader.getStore(); @@ -112,7 +116,7 @@ + "a function of the number of entries in the cache", def.diskLimit(), def.name()); ChronicleMap<KeyWrapper<K>, TimedValue<V>> store = - mapBuilder.createOrRecoverPersistedTo(config.getPersistedFile()); + mapBuilder.createOrRecoverPersistedTo(config.getCacheFile()); logger.atInfo().log( "Initialized '%s'|version: %s|avgKeySize: %s bytes|avgValueSize:" @@ -269,6 +273,8 @@ if (runningOutOfFreeSpace()) { evictColdEntries(); } + + keysIndex.persist(); } private boolean needsRefresh(long created) { @@ -321,7 +327,7 @@ public DiskStats diskStats() { return new DiskStats( store.longSize(), - config.getPersistedFile().length(), + config.getCacheFile().length(), hitCount.longValue(), missCount.longValue()); } @@ -332,6 +338,7 @@ public void close() { store.close(); + keysIndex.persist(); } public double percentageUsedAutoResizes() {
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndexTest.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndexTest.java index 27c9e76..c8a8b66 100644 --- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndexTest.java +++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndexTest.java
@@ -15,23 +15,35 @@ package com.googlesource.gerrit.modules.cache.chroniclemap; import static com.google.common.truth.Truth.assertThat; +import static com.googlesource.gerrit.modules.cache.chroniclemap.CacheKeysIndex.tempIndexFile; import static java.util.stream.Collectors.toList; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import com.google.gerrit.metrics.DisabledMetricMaker; +import com.google.gerrit.server.cache.serialize.StringCacheSerializer; +import java.io.File; +import java.io.IOException; import java.util.List; import java.util.function.Consumer; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; public class CacheKeysIndexTest { + private static final String CACHE_NAME = "test-cache"; + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + private CacheKeysIndex<String> index; + private File indexFile; @Before - public void setup() { - index = new CacheKeysIndex<>(new DisabledMetricMaker(), "test-cache"); + public void setup() throws IOException { + CacheSerializers.registerCacheKeySerializer(CACHE_NAME, StringCacheSerializer.INSTANCE); + indexFile = temporaryFolder.newFolder().toPath().resolve("cache.index").toFile(); + index = new CacheKeysIndex<>(new DisabledMetricMaker(), CACHE_NAME, indexFile, false); } @Test @@ -103,6 +115,35 @@ assertThat(keys(index)).containsExactly("newer"); } + @Test + public void persist_shouldPersistAndRestoreKeys() { + index.add("older", 1L); + index.add("newer", 1L); + assertThat(indexFile.exists()).isFalse(); + + index.persist(); + assertThat(indexFile.isFile()).isTrue(); + assertThat(indexFile.canRead()).isTrue(); + + index.clear(); + assertThat(keys(index)).isEmpty(); + + index.restore(true); + assertThat(keys(index)).containsExactly("newer", "older"); + } + + @Test + public void restore_shouldDeleteExistingTemporaryIndexStorageFileDuringRestore() + throws IOException { + File indexTempFile = tempIndexFile(indexFile); + indexTempFile.createNewFile(); + assertThat(indexTempFile.isFile()).isTrue(); + + index.restore(true); + + assertThat(indexTempFile.exists()).isFalse(); + } + private static List<String> keys(CacheKeysIndex<String> index) { return index.keys().stream().map(CacheKeysIndex.TimedKey::getKey).collect(toList()); }
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 08f5977..61cacca 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
@@ -59,10 +59,10 @@ } @Test - public void shouldProvidePersistedFile() throws Exception { + public void shouldProvideCacheFile() throws Exception { assertThat( configUnderTest(gerritConfig) - .getPersistedFile() + .getCacheFile() .toPath() .getParent() .toRealPath() @@ -71,6 +71,19 @@ } @Test + public void shouldProvideIndexFileThatIsRelatedToCacheFile() { + ChronicleMapCacheConfig config = configUnderTest(gerritConfig); + File cacheFile = config.getCacheFile(); + File indexFile = config.getIndexFile(); + + assertThat(indexFile.getParentFile()).isEqualTo(cacheFile.getParentFile()); + String cacheFileName = cacheFile.getName(); + assertThat(indexFile.getName()) + .isEqualTo( + String.format("%s.index", cacheFileName.substring(0, cacheFileName.indexOf(".dat")))); + } + + @Test public void shouldProvideConfiguredMaxEntriesWhenDefined() throws Exception { long maxEntries = 10; gerritConfig.setLong("cache", cacheKey, "maxEntries", maxEntries); @@ -176,7 +189,7 @@ } private ChronicleMapCacheConfig configUnderTest(StoredConfig gerritConfig) { - File persistentFile = + File cacheFile = ChronicleMapCacheFactory.fileName( sitePaths.site_path.resolve(cacheDirectory), cacheName, version); sitePaths @@ -185,6 +198,6 @@ .toFile(); return new ChronicleMapCacheConfig( - gerritConfig, cacheKey, persistentFile, expireAfterWrite, refreshAfterWrite); + gerritConfig, cacheKey, cacheFile, expireAfterWrite, refreshAfterWrite); } }