Merge branch 'stable-3.5' into stable-3.6
* stable-3.5:
Allow specify either avgKeySize or avgValueSize as command parameters
Fix avg value and key size calculation when auto-adjusting caches
Apply Flogger fixes from Ia4e5a3c513
Change-Id: Ic4238ee6137910f946ae8f0b669aa68d11482bdb
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 a07d8c4..a851c58 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
@@ -15,6 +15,7 @@
import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheFactory.getCacheDir;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.Cache;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
@@ -57,6 +58,8 @@
private boolean dryRun;
private boolean adjustCachesOnDefaults;
+ private Optional<Long> optionalAvgKeySize = Optional.empty();
+ private Optional<Long> optionalAvgValueSize = Optional.empty();
private Optional<Long> optionalMaxEntries = Optional.empty();
private Set<String> cacheNames = new HashSet<>();
@@ -95,10 +98,26 @@
return optionalMaxEntries;
}
+ public Optional<Long> getOptionalAvgKeySize() {
+ return optionalAvgKeySize;
+ }
+
+ public Optional<Long> getOptionalAvgValueSize() {
+ return optionalAvgValueSize;
+ }
+
public void setOptionalMaxEntries(Optional<Long> maxEntries) {
this.optionalMaxEntries = maxEntries;
}
+ public void setAvgKeySize(Optional<Long> avgKeySize) {
+ this.optionalAvgKeySize = avgKeySize;
+ }
+
+ public void setAvgValueSize(Optional<Long> avgValueSize) {
+ this.optionalAvgValueSize = avgValueSize;
+ }
+
public void addCacheNames(List<String> cacheNames) {
this.cacheNames.addAll(cacheNames);
}
@@ -119,36 +138,43 @@
ChronicleMapCacheImpl<Object, Object> currCache = cache.getValue();
{
- ImmutablePair<Long, Long> avgSizes =
- averageSizes(cacheName, currCache.getStore(), progressMonitor);
- if (!(avgSizes.getKey() > 0) || !(avgSizes.getValue() > 0)) {
- logger.atWarning().log(
- "Cache [%s] has %s entries, but average of (key: %d, value: %d). Skipping.",
- cacheName, currCache.diskStats().size(), avgSizes.getKey(), avgSizes.getValue());
- continue;
- }
+ long newKeySize;
+ long newValueSize;
- long averageKeySize = avgSizes.getKey();
- long averageValueSize = avgSizes.getValue();
+ if (optionalAvgKeySize.isPresent() && optionalAvgValueSize.isPresent()) {
+ newKeySize = optionalAvgKeySize.get();
+ newValueSize = optionalAvgValueSize.get();
+ } else {
+ ImmutablePair<Long, Long> avgSizes =
+ averageSizes(cacheName, currCache.getStore(), progressMonitor);
+ if (!(avgSizes.getKey() > 0) || !(avgSizes.getValue() > 0)) {
+ logger.atWarning().log(
+ "Cache [%s] has %s entries, but average of (key: %d, value: %d). Skipping.",
+ cacheName, currCache.diskStats().size(), avgSizes.getKey(), avgSizes.getValue());
+ continue;
+ }
+
+ newKeySize = getOptionalAvgKeySize().orElseGet(avgSizes::getKey);
+ newValueSize = getOptionalAvgValueSize().orElseGet(avgSizes::getValue);
+ }
ChronicleMapCacheConfig currCacheConfig = currCache.getConfig();
long newMaxEntries = newMaxEntries(currCache);
- if (currCacheConfig.getAverageKeySize() == averageKeySize
- && currCacheConfig.getAverageValueSize() == averageValueSize
+ if (currCacheConfig.getAverageKeySize() == newKeySize
+ && currCacheConfig.getAverageValueSize() == newValueSize
&& currCacheConfig.getMaxEntries() == newMaxEntries) {
continue;
}
ChronicleMapCacheConfig newChronicleMapCacheConfig =
- makeChronicleMapConfig(
- currCache.getConfig(), newMaxEntries, averageKeySize, averageValueSize);
+ makeChronicleMapConfig(currCache.getConfig(), newMaxEntries, newKeySize, newValueSize);
updateOutputConfig(
outputChronicleMapConfig,
cacheName,
- averageKeySize,
- averageValueSize,
+ newKeySize,
+ newValueSize,
newMaxEntries,
currCache.getConfig().getMaxBloatFactor());
@@ -185,29 +211,31 @@
String cacheName,
ConcurrentMap<KeyWrapper<Object>, TimedValue<Object>> store,
ProgressMonitor progressMonitor) {
- long kAvg = 0;
- long vAvg = 0;
+ long kTotal = 0;
+ long vTotal = 0;
- if (store.isEmpty()) return ImmutablePair.of(kAvg, vAvg);
+ if (store.isEmpty()) return ImmutablePair.of(kTotal, vTotal);
progressMonitor.beginTask(
String.format("[%s] calculate average key/value size", cacheName), store.size());
- int i = 1;
for (Map.Entry<KeyWrapper<Object>, TimedValue<Object>> entry : store.entrySet()) {
- kAvg = kAvg + (serializedKeyLength(cacheName, entry.getKey()) - kAvg) / i;
- vAvg = vAvg + (serializedValueLength(cacheName, entry.getValue()) - vAvg) / i;
+ kTotal += serializedKeyLength(cacheName, entry.getKey());
+ vTotal += serializedValueLength(cacheName, entry.getValue());
progressMonitor.update(1);
}
progressMonitor.endTask();
- return ImmutablePair.of(kAvg, vAvg);
+ long numCacheEntries = store.entrySet().size();
+ return ImmutablePair.of(kTotal / numCacheEntries, vTotal / numCacheEntries);
}
- private static int serializedKeyLength(String cacheName, KeyWrapper<Object> keyWrapper) {
+ @VisibleForTesting
+ static int serializedKeyLength(String cacheName, KeyWrapper<Object> keyWrapper) {
return CacheSerializers.getKeySerializer(cacheName).serialize(keyWrapper.getValue()).length;
}
- private static int serializedValueLength(String cacheName, TimedValue<Object> timedValue) {
+ @VisibleForTesting
+ static int serializedValueLength(String cacheName, TimedValue<Object> timedValue) {
return CacheSerializers.getValueSerializer(cacheName).serialize(timedValue.getValue()).length;
}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesCommand.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesCommand.java
index 9e7efd3..b49727b 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesCommand.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesCommand.java
@@ -57,6 +57,22 @@
autoAdjustCachesEngine.setAdjustCachesOnDefaults(adjustCachesOnDefaults);
}
+ @Option(
+ name = "--avg-key-size",
+ aliases = {"-k"},
+ usage = "Set avg key size.")
+ public void setAvgKeySize(long avgKeySize) {
+ autoAdjustCachesEngine.setAvgKeySize(Optional.of(avgKeySize));
+ }
+
+ @Option(
+ name = "--avg-value-size",
+ aliases = {"-v"},
+ usage = "Set avg value size")
+ public void setAvgValueSize(long avgValueSize) {
+ autoAdjustCachesEngine.setAvgValueSize(Optional.of(avgValueSize));
+ }
+
@Argument(
index = 0,
required = false,
diff --git a/src/main/resources/Documentation/tuning.md b/src/main/resources/Documentation/tuning.md
index 7abb4d0..30b9162 100644
--- a/src/main/resources/Documentation/tuning.md
+++ b/src/main/resources/Documentation/tuning.md
@@ -180,6 +180,18 @@
to increase the size of a particular cache only you should be using this
together with the `cache-name` parameter.
+* `--avg-key-size` or `-k` (SSH), `?avg-key-size` or `?k` (REST-API) optional
+ parameter
+
+It's possible to specify a pre-defined key size in case the user wants to set
+a different value from the one automatically calculated from the auto-adjust tool.
+
+* `--avg-value-size` or `-v` (SSH), `?avg-value-size` or `?v` (REST-API)
+ optional parameter
+
+It's possible to specify a pre-defined value size in case the user wants to set
+a different value from the one automatically calculated from the auto-adjust tool.
+
* `--adjust-caches-on-defaults` or `-a` (SSH), `?adjust-caches-on-defaults` or `?a` (REST-API)
optional parameter
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 5e4fe22..c030bb3 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
@@ -17,6 +17,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.googlesource.gerrit.modules.cache.chroniclemap.AutoAdjustCaches.MAX_ENTRIES_MULTIPLIER;
import static com.googlesource.gerrit.modules.cache.chroniclemap.AutoAdjustCaches.PERCENTAGE_SIZE_INCREASE_THRESHOLD;
+import static com.googlesource.gerrit.modules.cache.chroniclemap.AutoAdjustCaches.serializedKeyLength;
+import static com.googlesource.gerrit.modules.cache.chroniclemap.AutoAdjustCaches.serializedValueLength;
import static com.googlesource.gerrit.modules.cache.chroniclemap.AutoAdjustCachesCommand.CONFIG_HEADER;
import static com.googlesource.gerrit.modules.cache.chroniclemap.AutoAdjustCachesCommand.TUNED_INFIX;
import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheConfig.Defaults.maxBloatFactorFor;
@@ -134,13 +136,20 @@
@Test
@GerritConfig(name = "cache.test_cache.maxEntries", value = "10")
@GerritConfig(name = "cache.test_cache.maxBloatFactor", value = "1")
- public void shouldIncreaseCacheSizeWhenIsGettingFull() throws Exception {
+ public void shouldCorrectlyIncreaseCacheSizeWhenIsGettingFull() throws Exception {
ChronicleMapCacheImpl<String, String> chronicleMapCache =
(ChronicleMapCacheImpl<String, String>) testCache;
+ long elemsAdded = 0;
+ long totalKeySize = 0;
+ long totalValueSize = 0;
while (chronicleMapCache.percentageUsedAutoResizes() < PERCENTAGE_SIZE_INCREASE_THRESHOLD) {
- String aString = UUID.randomUUID().toString();
- testCache.put(aString, aString);
+ String key = UUID.randomUUID() + "someExtraValue";
+ String value = UUID.randomUUID().toString();
+ elemsAdded += 1;
+ totalKeySize += serializedKeyLength(TEST_CACHE_NAME, new KeyWrapper<>(key));
+ totalValueSize += serializedValueLength(TEST_CACHE_NAME, new TimedValue<>(value));
+ testCache.put(key, value);
}
String tuneResult = adminSshSession.exec(SSH_CMD + " " + TEST_CACHE_NAME);
@@ -150,6 +159,10 @@
assertThat(tunedConfig.getSubsections("cache")).contains(TEST_CACHE_NAME);
assertThat(tunedConfig.getLong("cache", TEST_CACHE_NAME, "maxEntries", 0))
.isEqualTo(chronicleMapCache.getConfig().getMaxEntries() * MAX_ENTRIES_MULTIPLIER);
+ assertThat(tunedConfig.getLong("cache", TEST_CACHE_NAME, "avgKeySize", 0))
+ .isEqualTo(totalKeySize / elemsAdded);
+ assertThat(tunedConfig.getLong("cache", TEST_CACHE_NAME, "avgValueSize", 0))
+ .isEqualTo(totalValueSize / elemsAdded);
}
@Test
@@ -169,6 +182,39 @@
}
@Test
+ public void shouldHonourAvgKeySizeParameter() throws Exception {
+ createChange();
+ Long wantedAvgKeySize = 50L;
+
+ String result =
+ adminSshSession.exec(String.format("%s --avg-key-size %s", SSH_CMD, wantedAvgKeySize));
+
+ adminSshSession.assertSuccess();
+ Config configResult = configResult(result, CONFIG_HEADER);
+
+ for (String cache : EXPECTED_CACHES) {
+ assertThat(configResult.getLong("cache", cache, "avgKeySize", 0)).isEqualTo(wantedAvgKeySize);
+ }
+ }
+
+ @Test
+ public void shouldHonourAvgValueSizeParameter() throws Exception {
+ createChange();
+ Long wantedAvgValueSize = 100L;
+
+ String result =
+ adminSshSession.exec(String.format("%s --avg-value-size %s", SSH_CMD, wantedAvgValueSize));
+
+ adminSshSession.assertSuccess();
+ Config configResult = configResult(result, CONFIG_HEADER);
+
+ for (String cache : EXPECTED_CACHES) {
+ assertThat(configResult.getLong("cache", cache, "avgValueSize", 0))
+ .isEqualTo(wantedAvgValueSize);
+ }
+ }
+
+ @Test
public void shouldCreateNewCacheFiles() throws Exception {
createChange();
