Merge branch 'stable-3.3' into stable-3.4 * stable-3.3: DRY out common functions between servlets DRY out listing of tuned files in AutoAdjustCachesIT Expose auto-adjust-cache command as Servlet Skip unnecessary auto-adjustments of caches Give more patience for cache to load in tests Remove empty char at EOL in docs Fix auto-adjust-caches example CLI in docs Use all-caps for static final strings in test Also increase the acceptance tests' heap size to 2G to allow enouch memory to run the Gerrit-based acceptance tests for cache conversion and tuning. Change-Id: Iea5144726908ef051f66ae06ec59c08cfe5a2efa
diff --git a/BUILD b/BUILD index 612cb3d..85222ca 100644 --- a/BUILD +++ b/BUILD
@@ -51,6 +51,7 @@ srcs = glob(["src/test/java/**/*IT.java"]), group = "server_cache", labels = ["server"], + vm_args = ["-Xmx2G"], deps = [ ":cache-chroniclemap__plugin", ":chroniclemap-test-lib",
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 a9f8bb7..d10ffc8 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
@@ -17,11 +17,13 @@ import com.google.common.cache.Cache; import com.google.common.flogger.FluentLogger; +import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.registration.DynamicMap; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.metrics.DisabledMetricMaker; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; -import com.google.gerrit.sshd.SshCommand; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.inject.Inject; import java.io.File; import java.io.IOException; @@ -32,23 +34,19 @@ import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang3.tuple.ImmutablePair; import org.eclipse.jgit.lib.Config; -import org.eclipse.jgit.lib.TextProgressMonitor; -import org.kohsuke.args4j.Option; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ProgressMonitor; -public class AutoAdjustCaches extends SshCommand { +public class AutoAdjustCaches { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); protected static final String CONFIG_HEADER = "__CONFIG__"; - protected static final String TUNED_INFIX = "tuned"; + protected static final String TUNED_INFIX = "_tuned_"; private final DynamicMap<Cache<?, ?>> cacheMap; private final ChronicleMapCacheConfig.Factory configFactory; private final Path cacheDir; private final AdministerCachePermission adminCachePermission; - @Option( - name = "--dry-run", - aliases = {"-d"}, - usage = "Calculate the average key and value size, but do not migrate the data.") private boolean dryRun; @Inject @@ -64,98 +62,111 @@ this.adminCachePermission = adminCachePermission; } - @Override - protected void run() throws Exception { - adminCachePermission.checkCurrentUserAllowed(e -> stderr.println(e.getLocalizedMessage())); + public boolean isDryRun() { + return dryRun; + } + + public void setDryRun(boolean dryRun) { + this.dryRun = dryRun; + } + + protected Config run(@Nullable ProgressMonitor optionalProgressMonitor) + throws AuthException, PermissionBackendException, IOException { + ProgressMonitor progressMonitor = + optionalProgressMonitor == null ? NullProgressMonitor.INSTANCE : optionalProgressMonitor; + adminCachePermission.checkCurrentUserAllowed(null); Config outputChronicleMapConfig = new Config(); Map<String, ChronicleMapCacheImpl<Object, Object>> chronicleMapCaches = getChronicleMapCaches(); - chronicleMapCaches.forEach( - (cacheName, currCache) -> { - ImmutablePair<Long, Long> avgSizes = averageSizes(cacheName, currCache.getStore()); - 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.size(), avgSizes.getKey(), avgSizes.getValue()); - return; - } + for (Map.Entry<String, ChronicleMapCacheImpl<Object, Object>> cache : + chronicleMapCaches.entrySet()) { + String cacheName = cache.getKey(); + ChronicleMapCacheImpl<Object, Object> currCache = cache.getValue(); - long averageKeySize = avgSizes.getKey(); - long averageValueSize = avgSizes.getValue(); - ChronicleMapCacheConfig newChronicleMapCacheConfig = - makeChronicleMapConfig(currCache.getConfig(), averageKeySize, averageValueSize); + { + 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.size(), avgSizes.getKey(), avgSizes.getValue()); + continue; + } - updateOutputConfig( - outputChronicleMapConfig, - cacheName, - averageKeySize, - averageValueSize, - currCache.getConfig().getMaxEntries(), - currCache.getConfig().getMaxBloatFactor()); + long averageKeySize = avgSizes.getKey(); + long averageValueSize = avgSizes.getValue(); - if (!dryRun) { - try { - ChronicleMapCacheImpl<Object, Object> newCache = - new ChronicleMapCacheImpl<>( - currCache.getCacheDefinition(), - newChronicleMapCacheConfig, - null, - new DisabledMetricMaker()); + ChronicleMapCacheConfig currCacheConfig = currCache.getConfig(); - TextProgressMonitor cacheMigrationProgress = new TextProgressMonitor(stdout); - cacheMigrationProgress.beginTask( - String.format("[%s] migrate content", cacheName), (int) currCache.size()); + if (currCacheConfig.getAverageKeySize() == averageKeySize + && currCacheConfig.getAverageValueSize() == averageValueSize) { + continue; + } - currCache - .getStore() - .forEach( - (k, v) -> { - try { - newCache.putUnchecked(k, v); - cacheMigrationProgress.update(1); - } catch (Exception e) { - logger.atWarning().withCause(e).log( - "[%s] Could not migrate entry %s -> %s", - cacheName, k.getValue(), v.getValue()); - } - }); + ChronicleMapCacheConfig newChronicleMapCacheConfig = + makeChronicleMapConfig(currCache.getConfig(), averageKeySize, averageValueSize); - } catch (IOException e) { - stderr.println(String.format("Could not create new cache %s", cacheName)); - } - } - }); + updateOutputConfig( + outputChronicleMapConfig, + cacheName, + averageKeySize, + averageValueSize, + currCache.getConfig().getMaxEntries(), + currCache.getConfig().getMaxBloatFactor()); - stdout.println(); - stdout.println("****************************"); - stdout.println("** Chronicle-map template **"); - stdout.println("****************************"); - stdout.println(); - stdout.println(CONFIG_HEADER); - stdout.println(outputChronicleMapConfig.toText()); + if (!dryRun) { + ChronicleMapCacheImpl<Object, Object> newCache = + new ChronicleMapCacheImpl<>( + currCache.getCacheDefinition(), + newChronicleMapCacheConfig, + null, + new DisabledMetricMaker()); + + progressMonitor.beginTask( + String.format("[%s] migrate content", cacheName), (int) currCache.size()); + + currCache + .getStore() + .forEach( + (k, v) -> { + try { + newCache.putUnchecked(k, v); + + progressMonitor.update(1); + } catch (Exception e) { + logger.atWarning().withCause(e).log( + "[%s] Could not migrate entry %s -> %s", + cacheName, k.getValue(), v.getValue()); + } + }); + } + } + } + + return outputChronicleMapConfig; } private ImmutablePair<Long, Long> averageSizes( - String cacheName, ConcurrentMap<KeyWrapper<Object>, TimedValue<Object>> store) { + String cacheName, + ConcurrentMap<KeyWrapper<Object>, TimedValue<Object>> store, + ProgressMonitor progressMonitor) { long kAvg = 0; long vAvg = 0; if (store.isEmpty()) return ImmutablePair.of(kAvg, vAvg); - TextProgressMonitor progress = new TextProgressMonitor(stdout); - - progress.beginTask( + 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; - progress.update(1); + progressMonitor.update(1); } - progress.endTask(); + progressMonitor.endTask(); return ImmutablePair.of(kAvg, vAvg); } @@ -186,7 +197,7 @@ private File resolveNewFile(String currentFileName) { String newFileName = String.format( - "%s_%s_%s.%s", + "%s%s%s.%s", FilenameUtils.getBaseName(currentFileName), TUNED_INFIX, System.currentTimeMillis(),
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 new file mode 100644 index 0000000..7aa240e --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesCommand.java
@@ -0,0 +1,72 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.googlesource.gerrit.modules.cache.chroniclemap; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.gerrit.sshd.SshCommand; +import com.google.inject.Inject; +import java.io.IOException; +import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.lib.TextProgressMonitor; +import org.kohsuke.args4j.Option; + +public class AutoAdjustCachesCommand extends SshCommand { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + protected static final String CONFIG_HEADER = "__CONFIG__"; + protected static final String TUNED_INFIX = "_tuned_"; + + private final AutoAdjustCaches autoAdjustCachesEngine; + + @Option( + name = "--dry-run", + aliases = {"-d"}, + usage = "Calculate the average key and value size, but do not migrate the data.") + public void setDryRun(boolean dryRun) { + autoAdjustCachesEngine.setDryRun(dryRun); + } + + @Inject + AutoAdjustCachesCommand(AutoAdjustCaches autoAdjustCachesEngine) { + this.autoAdjustCachesEngine = autoAdjustCachesEngine; + } + + @Override + protected void run() throws Exception { + try { + Config outputChronicleMapConfig = autoAdjustCachesEngine.run(new TextProgressMonitor(stdout)); + + stdout.println(); + stdout.println("**********************************"); + + if (outputChronicleMapConfig.getSections().isEmpty()) { + stdout.println("All exsting caches are already tuned: no changes needed."); + return; + } + + stdout.println("** Chronicle-map config changes **"); + stdout.println("**********************************"); + stdout.println(); + stdout.println(CONFIG_HEADER); + stdout.println(outputChronicleMapConfig.toText()); + } catch (AuthException | PermissionBackendException e) { + stderr.println(e.getLocalizedMessage()); + throw e; + } catch (IOException e) { + logger.atSevere().log("Could not create new cache", e); + stderr.println(String.format("Could not create new cache : %s", e.getLocalizedMessage())); + } + } +}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesServlet.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesServlet.java new file mode 100644 index 0000000..b217b0e --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AutoAdjustCachesServlet.java
@@ -0,0 +1,79 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.modules.cache.chroniclemap; + +import static com.googlesource.gerrit.modules.cache.chroniclemap.HttpServletOps.checkAcceptHeader; +import static com.googlesource.gerrit.modules.cache.chroniclemap.HttpServletOps.setResponse; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; +import java.io.IOException; +import java.util.Optional; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.lib.Config; + +@Singleton +public class AutoAdjustCachesServlet extends HttpServlet { + private static final long serialVersionUID = 1L; + protected static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + // This needs to be a provider so that every doPut() call is reentrant because + // uses a different auto-adjuster for the caches. + private final Provider<AutoAdjustCaches> autoAdjustCachesProvider; + + @Inject + AutoAdjustCachesServlet(Provider<AutoAdjustCaches> autoAdjustCachesProvider) { + this.autoAdjustCachesProvider = autoAdjustCachesProvider; + } + + @Override + protected void doPut(HttpServletRequest req, HttpServletResponse rsp) throws IOException { + AutoAdjustCaches autoAdjustCachesEngine = autoAdjustCachesProvider.get(); + if (!checkAcceptHeader(req, rsp)) { + return; + } + + autoAdjustCachesEngine.setDryRun( + Optional.ofNullable(req.getParameter("dry-run")) + .or(() -> Optional.ofNullable(req.getParameter("d"))) + .isPresent()); + + try { + Config outputChronicleMapConfig = autoAdjustCachesEngine.run(null); + + if (outputChronicleMapConfig.getSections().isEmpty()) { + setResponse( + rsp, + HttpServletResponse.SC_NO_CONTENT, + "All existing caches are already tuned: no changes needed."); + return; + } + + setResponse(rsp, HttpServletResponse.SC_CREATED, outputChronicleMapConfig.toText()); + } catch (AuthException | PermissionBackendException e) { + setResponse( + rsp, + HttpServletResponse.SC_FORBIDDEN, + "not permitted to administer caches : " + e.getLocalizedMessage()); + return; + } + } +}
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 ad18aa6..77d3887 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
@@ -266,8 +266,8 @@ * Associates the specified value with the specified key. This method should be used when the * {@link TimedValue} and the {@link KeyWrapper} have already been constructed elsewhere rather * than delegate their construction to this cache ({@link #put}. This is typically the case when - * the key/value are extracted from another chronicle-map cache see ({@link AutoAdjustCaches} for - * an example. + * the key/value are extracted from another chronicle-map cache see ({@link + * AutoAdjustCachesCommand} for an example. * * @param wrappedKey The wrapper for the key object * @param wrappedValue the wrapper for the value object
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 63cf659..f35f0ee 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
@@ -17,8 +17,8 @@ import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.H2_SUFFIX; import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.getStats; import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.jdbcUrl; -import static org.apache.http.HttpHeaders.ACCEPT; -import static org.eclipse.jgit.util.HttpSupport.TEXT_PLAIN; +import static com.googlesource.gerrit.modules.cache.chroniclemap.HttpServletOps.checkAcceptHeader; +import static com.googlesource.gerrit.modules.cache.chroniclemap.HttpServletOps.setResponse; import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Account; @@ -51,14 +51,12 @@ import com.google.inject.name.Named; import java.io.File; import java.io.IOException; -import java.io.PrintWriter; import java.nio.file.Files; import java.nio.file.Path; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.Timestamp; -import java.util.Arrays; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -138,11 +136,7 @@ @Override protected void doPut(HttpServletRequest req, HttpServletResponse rsp) throws IOException { - if (hasInvalidAcceptHeader(req)) { - setResponse( - rsp, - HttpServletResponse.SC_BAD_REQUEST, - "No advertised 'Accept' headers can be honoured. 'text/plain' should be provided in the request 'Accept' header."); + if (!checkAcceptHeader(req, rsp)) { return; } @@ -341,19 +335,6 @@ return typeLiteral.getRawType().getSimpleName().equals("String"); } - private void setResponse(HttpServletResponse httpResponse, int statusCode, String value) - throws IOException { - httpResponse.setContentType(TEXT_PLAIN); - httpResponse.setStatus(statusCode); - PrintWriter writer = httpResponse.getWriter(); - writer.print(value); - } - - private boolean hasInvalidAcceptHeader(HttpServletRequest req) { - return req.getHeader(ACCEPT) != null - && !Arrays.asList("text/plain", "text/*", "*/*").contains(req.getHeader(ACCEPT)); - } - private static void copyExistingCacheSettingsToConfig( Config outputConfig, ChronicleMapCacheConfig cacheConfig) { String cacheName = cacheConfig.getConfigKey();
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/HttpModule.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/HttpModule.java index afc50ca..869b623 100644 --- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/HttpModule.java +++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/HttpModule.java
@@ -46,5 +46,6 @@ } serve("/migrate").with(H2MigrationServlet.class); + serve("/auto-adjust-caches").with(AutoAdjustCachesServlet.class); } }
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/HttpServletOps.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/HttpServletOps.java new file mode 100644 index 0000000..dec758c --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/HttpServletOps.java
@@ -0,0 +1,49 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.modules.cache.chroniclemap; + +import static org.apache.http.HttpHeaders.ACCEPT; +import static org.eclipse.jgit.util.HttpSupport.TEXT_PLAIN; + +import java.io.IOException; +import java.io.PrintWriter; +import java.util.Arrays; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +class HttpServletOps { + + static boolean checkAcceptHeader(HttpServletRequest req, HttpServletResponse rsp) + throws IOException { + if (req.getHeader(ACCEPT) != null + && !Arrays.asList("text/plain", "text/*", "*/*").contains(req.getHeader(ACCEPT))) { + setResponse( + rsp, + HttpServletResponse.SC_BAD_REQUEST, + "No advertised 'Accept' headers can be honoured. 'text/plain' should be provided in the request 'Accept' header."); + return false; + } + + return true; + } + + static void setResponse(HttpServletResponse httpResponse, int statusCode, String value) + throws IOException { + httpResponse.setContentType(TEXT_PLAIN); + httpResponse.setStatus(statusCode); + PrintWriter writer = httpResponse.getWriter(); + writer.print(value); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/SSHCommandModule.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/SSHCommandModule.java index 34e4954..abe1089 100644 --- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/SSHCommandModule.java +++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/SSHCommandModule.java
@@ -37,6 +37,6 @@ factory(ChronicleMapCacheConfig.Factory.class); } command("analyze-h2-caches").to(AnalyzeH2Caches.class); - command("auto-adjust-caches").to(AutoAdjustCaches.class); + command("auto-adjust-caches").to(AutoAdjustCachesCommand.class); } }
diff --git a/src/main/resources/Documentation/tuning.md b/src/main/resources/Documentation/tuning.md index 7ed7868..feb98fc 100644 --- a/src/main/resources/Documentation/tuning.md +++ b/src/main/resources/Documentation/tuning.md
@@ -30,7 +30,7 @@ your Gerrit server will not need downtime. As follows: * Drop `cache-chroniclemap.jar` file in the `plugins/` directory. -* Wait for the pluginLoader to acknowledge and load the new plugin. You will +* Wait for the pluginLoader to acknowledge and load the new plugin. You will see an entry in the `error_log`: ``` @@ -129,11 +129,11 @@ provided for those caches, such as average key size and average value size, and you have to rely on default values. -This plugin provides an SSH command that will help you analyze the current, -suboptimal, chronicle-map caches and migrate into new ones for which a more -realistic configuration is generated based on data. +This plugin provides an SSH command and a REST-API that will help you analyze +the current, suboptimal, chronicle-map caches and migrate into new ones for +which a more realistic configuration is generated based on data. -The Gerrit/SSH command to tuning the caches requires the user to have +The tuning of the caches requires the user to have `Administrate Caches` or `Administrate Server` capabilities. * Symlink the `cache-chroniclemap.jar` file in the `plugins/` directory (from @@ -148,23 +148,29 @@ * You can now run an the tuning command: ```bash -ssh -p 29418 admin@<gerrit-server> cache-chroniclemap tune-chroniclemap-caches [--dry-run] +ssh -p 29418 admin@<gerrit-server> cache-chroniclemap auto-adjust-caches [--dry-run] ``` -* --dry-run (Optional) +* You can also use the REST-API: + +``` +PUT /plugins/cache-chroniclemap/auto-adjust-caches +``` + +* `--dry-run` or `-d` (SSH), `?dry-run` or `?d` (REST-API) optional parameter Calculate the average key and value size, but do not migrate current cache data into new files -For each chronicle-map cache (i.e. `foo_1.dat` file) in the `cache` directory, a -new one will be created (i.e. `foo_1_tuned_<timestamp>.dat`). +For each chronicle-map cache that needs tuning (i.e. `foo_1.dat` file) in +the `cache` directory, a new one will be created (i.e. `foo_1_tuned_<timestamp>.dat`). The new cache will have these characteristics: - Will have the same entries as the original cache. - Will be configured with the *actual* average key size and values calculated by looking at the content of the original cache. -An output will also be generated with the new configuration that should be put -into `gerrit.config`, should you decide to use the new caches. +An output will also be generated with the configuration changes that should +be included into `gerrit.config`, should you decide to use the new caches. An example of the output is the following:
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 a55c02a..57b0e46 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
@@ -15,22 +15,33 @@ package com.googlesource.gerrit.modules.cache.chroniclemap; import static com.google.common.truth.Truth.assertThat; -import static com.googlesource.gerrit.modules.cache.chroniclemap.AutoAdjustCaches.CONFIG_HEADER; -import static com.googlesource.gerrit.modules.cache.chroniclemap.AutoAdjustCaches.TUNED_INFIX; +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; import static com.googlesource.gerrit.modules.cache.chroniclemap.ChronicleMapCacheConfig.Defaults.maxEntriesFor; +import com.google.common.base.Joiner; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableList; import com.google.gerrit.acceptance.LightweightPluginDaemonTest; +import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.Sandboxed; import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.acceptance.UseLocalDisk; import com.google.gerrit.acceptance.UseSsh; +import com.google.gerrit.acceptance.config.GerritConfig; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.server.ModuleImpl; +import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.config.SitePaths; import com.google.inject.Inject; +import com.google.inject.name.Named; import java.io.File; +import java.nio.file.Path; import java.util.Objects; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -42,33 +53,65 @@ @UseSsh @TestPlugin( name = "cache-chroniclemap", - sshModule = "com.googlesource.gerrit.modules.cache.chroniclemap.SSHCommandModule") + sshModule = "com.googlesource.gerrit.modules.cache.chroniclemap.SSHCommandModule", + httpModule = "com.googlesource.gerrit.modules.cache.chroniclemap.HttpModule") public class AutoAdjustCachesIT extends LightweightPluginDaemonTest { - private static final String cmd = "cache-chroniclemap auto-adjust-caches"; + 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 DIFF_SUMMARY = "diff_summary"; private static final String ACCOUNTS = "accounts"; private static final String PERSISTED_PROJECTS = "persisted_projects"; + private static final String TEST_CACHE_NAME = "test_cache"; + private static final int TEST_CACHE_VERSION = 1; + private static final String TEST_CACHE_FILENAME_TUNED = + TEST_CACHE_NAME + "_" + TEST_CACHE_VERSION + AutoAdjustCaches.TUNED_INFIX; + private static final String TEST_CACHE_KEY_100_CHARS = new String(new char[100]); + 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); @Inject private SitePaths sitePaths; + @Inject + @Named(TEST_CACHE_NAME) + LoadingCache<String, String> testCache; + + @ModuleImpl(name = CacheModule.PERSISTENT_MODULE) + public static class TestPersistentCacheModule extends CacheModule { + + @Override + protected void configure() { + persist(TEST_CACHE_NAME, String.class, String.class) + .loader(TestCacheLoader.class) + .version(TEST_CACHE_VERSION); + install(new ChronicleMapCacheModule()); + } + } + + public static class TestCacheLoader extends CacheLoader<String, String> { + + @Override + public String load(String key) throws Exception { + return key; + } + } + @Override public com.google.inject.Module createModule() { - return new ChronicleMapCacheModule(); + return new TestPersistentCacheModule(); } @Test public void shouldUseDefaultsWhenCachesAreNotConfigured() throws Exception { createChange(); - String result = adminSshSession.exec(cmd); + String result = adminSshSession.exec(SSH_CMD); adminSshSession.assertSuccess(); - Config configResult = configResult(result); + Config configResult = configResult(result, CONFIG_HEADER); for (String cache : EXPECTED_CACHES) { assertThat(configResult.getLong("cache", cache, "maxEntries", 0)) @@ -82,32 +125,77 @@ public void shouldCreateNewCacheFiles() throws Exception { createChange(); - adminSshSession.exec(cmd); + adminSshSession.exec(SSH_CMD); adminSshSession.assertSuccess(); - File cacheDir = sitePaths.resolve(cfg.getString("cache", null, "directory")).toFile(); Set<String> tunedCaches = - Stream.of(Objects.requireNonNull(cacheDir.listFiles())) - .filter(file -> !file.isDirectory()) - .map(File::getName) - .filter( - n -> - n.contains(TUNED_INFIX) - && n.matches(".*(" + String.join("|", EXPECTED_CACHES) + ").*")) - .collect(Collectors.toSet()); + tunedFileNamesSet(n -> n.matches(".*(" + String.join("|", EXPECTED_CACHES) + ").*")); assertThat(tunedCaches.size()).isEqualTo(EXPECTED_CACHES.size()); } @Test - public void shouldDenyAccessToCreateNewCacheFiles() throws Exception { - userSshSession.exec(cmd); + @GerritConfig(name = "cache.test_cache.avgKeySize", value = "207") + @GerritConfig(name = "cache.test_cache.avgValueSize", value = "207") + public void shouldNotRecreateTestCacheFileWhenAlreadyTuned() throws Exception { + testCache.get(TEST_CACHE_KEY_100_CHARS); + + String tuneResult = adminSshSession.exec(SSH_CMD); + adminSshSession.assertSuccess(); + + assertThat(configResult(tuneResult, CONFIG_HEADER).getSubsections("cache")) + .doesNotContain(TEST_CACHE_NAME); + assertThat(Joiner.on('\n').join(tunedFileNamesSet(MATCH_ALL))) + .doesNotContain(TEST_CACHE_FILENAME_TUNED); + } + + @Test + public void shouldCreateTestCacheTuned() throws Exception { + testCache.get(TEST_CACHE_KEY_100_CHARS); + String tuneResult = adminSshSession.exec(SSH_CMD); + adminSshSession.assertSuccess(); + + assertThat(configResult(tuneResult, CONFIG_HEADER).getSubsections("cache")) + .contains(TEST_CACHE_NAME); + assertThat( + Joiner.on('\n').join(tunedFileNamesSet((n) -> n.contains(TEST_CACHE_FILENAME_TUNED)))) + .isNotEmpty(); + } + + @Test + public void shouldDenyAccessOverSshToCreateNewCacheFiles() throws Exception { + userSshSession.exec(SSH_CMD); userSshSession.assertFailure("not permitted"); } - private Config configResult(String result) throws ConfigInvalidException { + @Test + public void shouldDenyAccessOverRestToCreateNewCacheFiles() throws Exception { + userRestSession.put(REST_CMD).assertForbidden(); + } + + @Test + public void shouldAllowTuningOverRestForAdmin() throws Exception { + RestResponse resp = adminRestSession.put(REST_CMD); + + resp.assertCreated(); + + assertThat(configResult(resp.getEntityContent(), null).getSubsections("cache")).isNotEmpty(); + assertThat(tunedFileNamesSet(MATCH_ALL)).isNotEmpty(); + } + + private Config configResult(String result, @Nullable String configHeader) + throws ConfigInvalidException { Config configResult = new Config(); - configResult.fromText((result.split(CONFIG_HEADER))[1]); + configResult.fromText(configHeader == null ? result : result.split(configHeader)[1]); return configResult; } + + private Set<String> tunedFileNamesSet(Function<String, Boolean> fileNameFilter) { + Path cachePath = sitePaths.resolve(cfg.getString("cache", null, "directory")); + return Stream.of(Objects.requireNonNull(cachePath.toFile().listFiles())) + .filter(file -> !file.isDirectory()) + .map(File::getName) + .filter(n -> n.contains(TUNED_INFIX) && fileNameFilter.apply(n)) + .collect(Collectors.toSet()); + } }
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 93b0297..360f4cd 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
@@ -65,7 +65,7 @@ httpModule = "com.googlesource.gerrit.modules.cache.chroniclemap.HttpModule") @UseLocalDisk public class MigrateH2CachesLocalDiskIT extends LightweightPluginDaemonTest { - private final Duration LOAD_CACHE_WAIT_TIMEOUT = Duration.ofSeconds(4); + private final Duration LOAD_CACHE_WAIT_TIMEOUT = Duration.ofSeconds(60); private String ACCOUNTS_CACHE_NAME = "accounts"; private String PERSISTED_PROJECTS_CACHE_NAME = "persisted_projects"; private String MIGRATION_ENDPOINT = "/plugins/cache-chroniclemap/migrate";