Merge branch 'stable-3.4' into stable-3.5
* stable-3.4:
Fix Flogger formatting of exception in AutoAdjustCachesCommand
Extract ChronicleMapStoreMetrics to its own class file
Ignore the BytesStore raw type
Emit metric to track number of put failures
Move tryPut to ChronicleMapStore
Move store related metrics to ChronicleMapStore
Introduce ChronicleMapStore
Remove redundant store parameter from ChronicleMapCacheImpl
Do not throw when failing to store into chronicle-map
Expose max auto resizes metric
Allow to specify number of entries in auto-adjust-caches
Fix link to auto-adjust caches in tuning docs
Add acceptance test for REST auto-tuning for single cache
Allow to restrict the caches to auto-tune
Increase the memory used in acceptance tests
build.md: Use correct path for external_plugin_deps.bzl
Replace diff cache IT testing with gerrit_file_diff and
git_file_diff caches.
Change-Id: I955a75c72dd61c4b36a2a96b32aaa73eacec3762
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 1235e53..79e4b38 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
@@ -185,7 +185,8 @@
.put("web_sessions", DefaultConfig.create(45, 221, 1000, 1))
.put("change_notes", DefaultConfig.create(36, 10240, 1000, 3))
.put("accounts", DefaultConfig.create(30, 256, 1000, 1))
- .put("diff", DefaultConfig.create(98, 10240, 1000, 2))
+ .put("gerrit_file_diff", DefaultConfig.create(98, 10240, 1000, 2))
+ .put("git_file_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 f96786d..c74594f 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
@@ -41,8 +41,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;
@@ -103,7 +105,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,
@@ -125,7 +129,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 71cb86e..776ee45 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) {
