Merge branch 'stable-3.4' into stable-3.5 * stable-3.4: Provide default configuration for Gerrit persistent caches Introduce metric for caches that fall back to default config Add test that detects persistent caches without defaults Remove metrics when cache gets closed Use static, per-thread buffers for (de)serialization Add read/write metrics to TimedValueMarshaller ChronicleMapCacheIT: remove Truth8 qualifier from assertThat *Marshaller: get CacheSerilizer only once Persist caches keys index at configurable pace Add restore/persist operations related metrics Persist cache keys index to a file Add cache keys index metrics Avoid full cache scanning for pruning Change-Id: Iab85e600cfdcb9709484d6f694a0e3639ac9f6b0
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 01ba278..e2c5afe 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
@@ -264,11 +264,11 @@ static final ImmutableMap<String, DefaultConfig> defaultMap = new ImmutableMap.Builder<String, DefaultConfig>() .put("accounts", DefaultConfig.create(30, 256, 1000, 1)) + .put("approvals", DefaultConfig.create(103, 365, 1000, 3)) .put("change_kind", DefaultConfig.create(59, 26, 1000, 1)) .put("change_notes", DefaultConfig.create(36, 10240, 1000, 3)) .put("comment_context", DefaultConfig.create(80, 662, 2000, 3)) .put("conflicts", DefaultConfig.create(70, 16, 1000, 1)) - .put("diff", DefaultConfig.create(98, 10240, 1000, 2)) .put("diff_intraline", DefaultConfig.create(512, 2048, 1000, 2)) .put("diff_summary", DefaultConfig.create(128, 2048, 1000, 1)) .put("external_ids_map", DefaultConfig.create(128, 204800, 2, 1))
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 c3959e9..287fe72 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
@@ -42,8 +42,10 @@ import com.google.gerrit.server.patch.DiffSummaryKey; import com.google.gerrit.server.patch.IntraLineDiff; import com.google.gerrit.server.patch.IntraLineDiffKey; -import com.google.gerrit.server.patch.PatchList; -import com.google.gerrit.server.patch.PatchListKey; +import com.google.gerrit.server.patch.filediff.FileDiffCacheKey; +import com.google.gerrit.server.patch.filediff.FileDiffOutput; +import com.google.gerrit.server.patch.gitfilediff.GitFileDiff; +import com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheKey; import com.google.gerrit.server.query.change.ConflictKey; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -104,7 +106,9 @@ @Named("git_tags") PersistentCacheDef<String, TagSetHolder> gitTagsCacheDef, @Named("change_notes") PersistentCacheDef<ChangeNotesCache.Key, ChangeNotesState> changeNotesCacheDef, - @Named("diff") PersistentCacheDef<PatchListKey, PatchList> diffCacheDef, + @Named("gerrit_file_diff") + PersistentCacheDef<FileDiffCacheKey, FileDiffOutput> gerritFileDiffDef, + @Named("git_file_diff") PersistentCacheDef<GitFileDiffCacheKey, GitFileDiff> gitFileDiffDef, @Named("diff_intraline") PersistentCacheDef<IntraLineDiffKey, IntraLineDiff> diffIntraLineCacheDef, @Named("diff_summary") PersistentCacheDef<DiffSummaryKey, DiffSummary> diffSummaryCacheDef, @@ -126,7 +130,8 @@ pureRevertCacheDef, gitTagsCacheDef, changeNotesCacheDef, - diffCacheDef, + gerritFileDiffDef, + gitFileDiffDef, diffIntraLineCacheDef, diffSummaryCacheDef, persistedProjectsCacheDef,
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryCacheLoadingFromStoreImpl.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryCacheLoadingFromStoreImpl.java index 96b75da..ea11bba 100644 --- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryCacheLoadingFromStoreImpl.java +++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryCacheLoadingFromStoreImpl.java
@@ -14,7 +14,6 @@ package com.googlesource.gerrit.modules.cache.chroniclemap; -import com.google.common.cache.Cache; import com.google.common.cache.CacheStats; import com.google.common.cache.LoadingCache; import com.google.gerrit.common.Nullable; @@ -22,7 +21,7 @@ import java.util.concurrent.ExecutionException; class InMemoryCacheLoadingFromStoreImpl<K, V> implements InMemoryCache<K, V> { - private final Cache<K, TimedValue<V>> loadingFromStoreCache; + private final LoadingCache<K, TimedValue<V>> loadingFromStoreCache; private final boolean loadingFromSource; /** @@ -59,15 +58,15 @@ @Override public TimedValue<V> get(K key) throws ExecutionException { - if (loadingFromSource) { - return ((LoadingCache<K, TimedValue<V>>) loadingFromStoreCache).get(key); - } - TimedValue<V> cachedValue = getIfPresent(key); if (cachedValue != null) { return cachedValue; } + if (loadingFromSource) { + return loadingFromStoreCache.get(key); + } + throw new UnsupportedOperationException( String.format("Could not load value for %s without any loader", key)); } @@ -75,7 +74,7 @@ @Override public void refresh(K key) { if (loadingFromSource) { - ((LoadingCache<K, TimedValue<V>>) loadingFromStoreCache).refresh(key); + loadingFromStoreCache.refresh(key); } }
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2CachesIT.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2CachesIT.java index 482b9da..c02d581 100644 --- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2CachesIT.java +++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2CachesIT.java
@@ -49,7 +49,8 @@ String result = adminSshSession.exec(cmd); adminSshSession.assertSuccess(); - assertThat(result).contains("[cache \"diff\"]\n" + "\tmaxEntries = 1\n"); + assertThat(result).contains("[cache \"git_file_diff\"]\n" + "\tmaxEntries = 1\n"); + assertThat(result).contains("[cache \"gerrit_file_diff\"]\n" + "\tmaxEntries = 2\n"); assertThat(result).contains("[cache \"accounts\"]\n" + "\tmaxEntries = 4\n"); assertThat(result).contains("[cache \"diff_summary\"]\n" + "\tmaxEntries = 1\n"); assertThat(result).contains("[cache \"persisted_projects\"]\n" + "\tmaxEntries = 3\n"); @@ -68,7 +69,8 @@ "WARN: Cache diff_intraline is empty, skipping.", "WARN: Cache change_kind is empty, skipping.", "WARN: Cache diff_summary is empty, skipping.", - "WARN: Cache diff is empty, skipping.", + "WARN: Cache gerrit_file_diff is empty, skipping.", + "WARN: Cache git_file_diff is empty, skipping.", "WARN: Cache pure_revert is empty, skipping.", "WARN: Cache git_tags is empty, skipping."); String result = adminSshSession.exec(cmd); @@ -87,7 +89,8 @@ "WARN: Cache diff_intraline is empty, skipping.", "WARN: Cache change_kind is empty, skipping.", "WARN: Cache diff_summary is empty, skipping.", - "WARN: Cache diff is empty, skipping.", + "WARN: Cache gerrit_file_diff is empty, skipping.", + "WARN: Cache git_file_diff is empty, skipping.", "WARN: Cache pure_revert is empty, skipping.", "WARN: Cache git_tags is empty, skipping."); String result = adminSshSession.exec(cmd);
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesIT.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesIT.java index 39e832a..5e4fe22 100644 --- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesIT.java +++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesIT.java
@@ -62,7 +62,8 @@ private static final String SSH_CMD = "cache-chroniclemap auto-adjust-caches"; private static final String REST_CMD = "/plugins/cache-chroniclemap/auto-adjust-caches"; private static final String GROUPS_BYUUID_PERSISTED = "groups_byuuid_persisted"; - private static final String DIFF = "diff"; + private static final String GERRIT_FILE_DIFF = "gerrit_file_diff"; + private static final String GIT_FILE_DIFF = "git_file_diff"; private static final String DIFF_SUMMARY = "diff_summary"; private static final String ACCOUNTS = "accounts"; private static final String PERSISTED_PROJECTS = "persisted_projects"; @@ -74,7 +75,13 @@ private static final Function<String, Boolean> MATCH_ALL = (n) -> true; private static final ImmutableList<String> EXPECTED_CACHES = - ImmutableList.of(GROUPS_BYUUID_PERSISTED, DIFF, DIFF_SUMMARY, ACCOUNTS, PERSISTED_PROJECTS); + ImmutableList.of( + GROUPS_BYUUID_PERSISTED, + GERRIT_FILE_DIFF, + GIT_FILE_DIFF, + DIFF_SUMMARY, + ACCOUNTS, + PERSISTED_PROJECTS); @Inject private SitePaths sitePaths; @@ -175,10 +182,23 @@ } @Test - public void shouldCreateNewCacheFileForSingleDiffCache() throws Exception { + public void shouldCreateNewCacheFileForSingleGitFileDiffCache() throws Exception { createChange(); - adminSshSession.exec(SSH_CMD + " " + DIFF); + adminSshSession.exec(SSH_CMD + " " + GIT_FILE_DIFF); + + adminSshSession.assertSuccess(); + Set<String> tunedCaches = + tunedFileNamesSet(n -> n.matches(".*" + AutoAdjustCaches.TUNED_INFIX + ".*")); + + assertThat(tunedCaches.size()).isEqualTo(1); + } + + @Test + public void shouldCreateNewCacheFileForSingleGerritFileDiffCache() throws Exception { + createChange(); + + adminSshSession.exec(SSH_CMD + " " + GERRIT_FILE_DIFF); adminSshSession.assertSuccess(); Set<String> tunedCaches = @@ -256,10 +276,23 @@ } @Test - public void shouldAllowTuningOfSingleDiffCacheOverRestForAdmin() throws Exception { + public void shouldAllowTuningOfSingleGitFileDiffCacheOverRestForAdmin() throws Exception { createChange(); - RestResponse resp = adminRestSession.put(REST_CMD + "?CACHE_NAME=" + DIFF); + RestResponse resp = adminRestSession.put(REST_CMD + "?CACHE_NAME=" + GIT_FILE_DIFF); + + resp.assertCreated(); + + assertThat(configResult(resp.getEntityContent(), null).getSubsections("cache")).isNotEmpty(); + assertThat(tunedFileNamesSet(n -> n.matches(".*" + AutoAdjustCaches.TUNED_INFIX + ".*"))) + .hasSize(1); + } + + @Test + public void shouldAllowTuningOfSingleGerritFileDiffCacheOverRestForAdmin() throws Exception { + createChange(); + + RestResponse resp = adminRestSession.put(REST_CMD + "?CACHE_NAME=" + GERRIT_FILE_DIFF); resp.assertCreated();
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesInMemoryIT.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesInMemoryIT.java index dbca0df..3561051 100644 --- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesInMemoryIT.java +++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesInMemoryIT.java
@@ -65,6 +65,6 @@ private RestResponse runMigrationWithAcceptHeader(RestSession restSession, String acceptHeader) throws IOException { - return restSession.putWithHeader(MIGRATION_ENDPOINT, new BasicHeader(ACCEPT, acceptHeader)); + return restSession.putWithHeaders(MIGRATION_ENDPOINT, new BasicHeader(ACCEPT, acceptHeader)); } }
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesLocalDiskIT.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesLocalDiskIT.java index 77ae477..57de6cd 100644 --- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesLocalDiskIT.java +++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/MigrateH2CachesLocalDiskIT.java
@@ -288,7 +288,7 @@ private RestResponse runMigrationWithAcceptHeader(RestSession restSession, String acceptHeader) throws IOException { - return restSession.putWithHeader(MIGRATION_ENDPOINT, new BasicHeader(ACCEPT, acceptHeader)); + return restSession.putWithHeaders(MIGRATION_ENDPOINT, new BasicHeader(ACCEPT, acceptHeader)); } private <T> T findClassBoundWithName(Class<T> clazz, String named) {