Merge branch 'stable-3.8' * stable-3.8: 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: I52e6e7d9f435740653077f03fbef667b3dd17e8a
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();