Merge branch 'stable-3.3' into stable-3.4
* stable-3.3:
Use existing pre-configured cache config for H2 migration
Allow migrating H2 to ChronicleMap from non-admin
Remove unused fields and imports
Add serialVersionUID in inner anonymous class
Fix references to put method in JavaDoc
Suppress unchecked, cast and rawtypes warnings
Remove unused variable and method in test
Assert return value when creating project in test
Remove unneeded else condition
Remove duplicate fields exposed by AbstractDaemonTest
Change-Id: Ibe25be7ffe2976fdadd08f4cd38a9b52e442aa0b
diff --git a/BUILD b/BUILD
index 7029515..612cb3d 100644
--- a/BUILD
+++ b/BUILD
@@ -11,6 +11,7 @@
name = "cache-chroniclemap",
srcs = glob(["src/main/java/**/*.java"]),
manifest_entries = [
+ "Gerrit-Module: com.googlesource.gerrit.modules.cache.chroniclemap.CapabilityModule",
"Gerrit-SshModule: com.googlesource.gerrit.modules.cache.chroniclemap.SSHCommandModule",
"Gerrit-HttpModule: com.googlesource.gerrit.modules.cache.chroniclemap.HttpModule",
],
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AdministerCachePermission.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AdministerCachePermission.java
new file mode 100644
index 0000000..8cc06d5
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AdministerCachePermission.java
@@ -0,0 +1,63 @@
+// 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.collect.ImmutableSet;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.api.access.PluginPermission;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.permissions.GlobalPermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.inject.Inject;
+import java.util.function.Consumer;
+
+class AdministerCachePermission {
+ private final PermissionBackend permissionBackend;
+ private final String pluginName;
+
+ @Inject
+ AdministerCachePermission(PermissionBackend permissionBackend, @PluginName String pluginName) {
+ this.permissionBackend = permissionBackend;
+ this.pluginName = pluginName;
+ }
+
+ boolean isCurrentUserAllowed() {
+ try {
+ checkCurrentUserAllowed(null);
+ return true;
+ } catch (AuthException | PermissionBackendException e) {
+ return false;
+ }
+ }
+
+ void checkCurrentUserAllowed(@Nullable Consumer<Exception> failureFunction)
+ throws AuthException, PermissionBackendException {
+ try {
+ permissionBackend
+ .currentUser()
+ .checkAny(
+ ImmutableSet.of(
+ GlobalPermission.ADMINISTRATE_SERVER,
+ new PluginPermission(pluginName, AdministerCachesCapability.ID)));
+ } catch (AuthException | PermissionBackendException e) {
+ if (failureFunction != null) {
+ failureFunction.accept(e);
+ }
+ throw e;
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AdministerCachesCapability.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AdministerCachesCapability.java
new file mode 100644
index 0000000..3d818b0
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AdministerCachesCapability.java
@@ -0,0 +1,26 @@
+// 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.gerrit.extensions.config.CapabilityDefinition;
+
+public class AdministerCachesCapability extends CapabilityDefinition {
+ static final String ID = "administerCaches";
+
+ @Override
+ public String getDescription() {
+ return "Administer Caches";
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2Caches.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2Caches.java
index 6f501a9..0ec4d2e 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2Caches.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/AnalyzeH2Caches.java
@@ -36,15 +36,22 @@
private final Config gerritConfig;
private final SitePaths site;
+ private final AdministerCachePermission adminCachePermission;
@Inject
- AnalyzeH2Caches(@GerritServerConfig Config cfg, SitePaths site) {
+ AnalyzeH2Caches(
+ @GerritServerConfig Config cfg,
+ SitePaths site,
+ AdministerCachePermission adminCachePermission) {
this.gerritConfig = cfg;
this.site = site;
+ this.adminCachePermission = adminCachePermission;
}
@Override
protected void run() throws Exception {
+ adminCachePermission.checkCurrentUserAllowed(e -> stderr.println(e.getLocalizedMessage()));
+
Set<Path> h2Files = getH2CacheFiles();
stdout.println("Extracting information from H2 caches...");
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 d668884..a9f8bb7 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
@@ -43,6 +43,7 @@
private final DynamicMap<Cache<?, ?>> cacheMap;
private final ChronicleMapCacheConfig.Factory configFactory;
private final Path cacheDir;
+ private final AdministerCachePermission adminCachePermission;
@Option(
name = "--dry-run",
@@ -55,14 +56,18 @@
@GerritServerConfig Config cfg,
SitePaths site,
DynamicMap<Cache<?, ?>> cacheMap,
- ChronicleMapCacheConfig.Factory configFactory) {
+ ChronicleMapCacheConfig.Factory configFactory,
+ AdministerCachePermission adminCachePermission) {
this.cacheMap = cacheMap;
this.configFactory = configFactory;
this.cacheDir = getCacheDir(site, cfg.getString("cache", null, "directory"));
+ this.adminCachePermission = adminCachePermission;
}
@Override
protected void run() throws Exception {
+ adminCachePermission.checkCurrentUserAllowed(e -> stderr.println(e.getLocalizedMessage()));
+
Config outputChronicleMapConfig = new Config();
Map<String, ChronicleMapCacheImpl<Object, Object>> chronicleMapCaches = getChronicleMapCaches();
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CapabilityModule.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CapabilityModule.java
new file mode 100644
index 0000000..9764571
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CapabilityModule.java
@@ -0,0 +1,28 @@
+// 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.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.config.CapabilityDefinition;
+import com.google.inject.AbstractModule;
+
+public class CapabilityModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(CapabilityDefinition.class)
+ .annotatedWith(Exports.named(AdministerCachesCapability.ID))
+ .to(AdministerCachesCapability.class);
+ }
+}
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 97ecb1c..ad18aa6 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
@@ -50,7 +50,7 @@
private final InMemoryLRU<K> hotEntries;
private final PersistentCacheDef<K, V> cacheDefinition;
- @SuppressWarnings("unchecked")
+ @SuppressWarnings({"unchecked", "cast", "rawtypes"})
ChronicleMapCacheImpl(
PersistentCacheDef<K, V> def,
ChronicleMapCacheConfig config,
@@ -181,9 +181,8 @@
hitCount.increment();
hotEntries.add((K) objKey);
return vTimedValue.getValue();
- } else {
- invalidate(objKey);
}
+ invalidate(objKey);
}
missCount.increment();
return null;
@@ -249,7 +248,7 @@
/**
* Associates the specified value with the specified key. This method should be used when the
* creation time of the value needs to be preserved, rather than computed at insertion time
- * ({@link #put(K,V)}. This is typically the case when migrating from an existing cache where the
+ * ({@link #put}. This is typically the case when migrating from an existing cache where the
* creation timestamp needs to be preserved. See ({@link H2MigrationServlet} for an example.
*
* @param key
@@ -266,9 +265,9 @@
/**
* 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(K, V)}. This is typically the case
- * when the key/value are extracted from another chronicle-map cache see ({@link AutoAdjustCaches}
- * for an example.
+ * 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.
*
* @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 3dc7ef8..63cf659 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
@@ -15,7 +15,6 @@
package com.googlesource.gerrit.modules.cache.chroniclemap;
import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.H2_SUFFIX;
-import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.appendToConfig;
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;
@@ -26,7 +25,6 @@
import com.google.gerrit.entities.CachedProjectConfig;
import com.google.gerrit.extensions.auth.oauth.OAuthToken;
import com.google.gerrit.extensions.client.ChangeKind;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.httpd.WebSessionManager;
import com.google.gerrit.metrics.DisabledMetricMaker;
@@ -46,14 +44,12 @@
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.permissions.GlobalPermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ConflictKey;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
+import java.io.File;
import java.io.IOException;
import java.io.PrintWriter;
import java.nio.file.Files;
@@ -81,7 +77,7 @@
private final ChronicleMapCacheConfig.Factory configFactory;
private final SitePaths site;
private final Config gerritConfig;
- private final PermissionBackend permissionBackend;
+ private final AdministerCachePermission adminCachePermission;
public static int DEFAULT_SIZE_MULTIPLIER = 3;
public static int DEFAULT_MAX_BLOAT_FACTOR = 3;
@@ -96,7 +92,7 @@
@GerritServerConfig Config cfg,
SitePaths site,
ChronicleMapCacheConfig.Factory configFactory,
- PermissionBackend permissionBackend,
+ AdministerCachePermission permissionBackend,
@Named("web_sessions") PersistentCacheDef<String, WebSessionManager.Val> webSessionsCacheDef,
@Named("accounts")
PersistentCacheDef<CachedAccountDetails.Key, CachedAccountDetails> accountsCacheDef,
@@ -121,7 +117,7 @@
this.configFactory = configFactory;
this.site = site;
this.gerritConfig = cfg;
- this.permissionBackend = permissionBackend;
+ this.adminCachePermission = permissionBackend;
this.persistentCacheDefs =
Stream.of(
webSessionsCacheDef,
@@ -150,15 +146,11 @@
return;
}
- try {
- permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
- } catch (AuthException | PermissionBackendException e) {
- setResponse(
- rsp,
- HttpServletResponse.SC_FORBIDDEN,
- "administrateServer for plugin cache-chroniclemap not permitted");
+ if (!adminCachePermission.isCurrentUserAllowed()) {
+ setResponse(rsp, HttpServletResponse.SC_FORBIDDEN, "not permitted to administer caches");
return;
}
+
Optional<Path> cacheDir = getCacheDir();
int maxBloatFactor =
@@ -188,21 +180,55 @@
try {
for (PersistentCacheDef<?, ?> in : persistentCacheDefs) {
Optional<Path> h2CacheFile = getH2CacheFile(cacheDir.get(), in.name());
+ Optional<ChronicleMapCacheConfig> chronicleMapConfig;
if (h2CacheFile.isPresent()) {
- H2AggregateData stats = getStats(h2CacheFile.get());
+ if (hasFullPersistentCacheConfiguration(in)) {
+ if (sizeMultiplier != DEFAULT_SIZE_MULTIPLIER) {
+ logger.atWarning().log(
+ "Size multiplier = %d ignored because of existing configuration found",
+ sizeMultiplier);
+ }
+ if (maxBloatFactor != DEFAULT_MAX_BLOAT_FACTOR) {
+ logger.atWarning().log(
+ "Max Bloat Factor = %d ignored because of existing configuration found",
+ maxBloatFactor);
+ }
- if (!stats.isEmpty()) {
+ File cacheFile =
+ ChronicleMapCacheFactory.fileName(cacheDir.get(), in.name(), in.version());
+ chronicleMapConfig =
+ Optional.of(
+ configFactory.create(
+ in.name(), cacheFile, in.expireAfterWrite(), in.refreshAfterWrite()));
+
+ } else {
+ if (hasPartialPersistentCacheConfiguration(in)) {
+ logger.atWarning().log(
+ "Existing configuration for cache %s found gerrit.config and will be ignored because incomplete",
+ in.name());
+ }
+ chronicleMapConfig =
+ optionalOf(getStats(h2CacheFile.get()))
+ .map(
+ (stats) ->
+ makeChronicleMapConfig(
+ configFactory,
+ cacheDir.get(),
+ in,
+ stats,
+ sizeMultiplier,
+ maxBloatFactor));
+ }
+
+ if (chronicleMapConfig.isPresent()) {
+ ChronicleMapCacheConfig cacheConfig = chronicleMapConfig.get();
ChronicleMapCacheImpl<?, ?> chronicleMapCache =
- new ChronicleMapCacheImpl<>(
- in,
- makeChronicleMapConfig(
- configFactory, cacheDir.get(), in, stats, sizeMultiplier, maxBloatFactor),
- null,
- new DisabledMetricMaker());
+ new ChronicleMapCacheImpl<>(in, cacheConfig, null, new DisabledMetricMaker());
+
doMigrate(h2CacheFile.get(), in, chronicleMapCache);
chronicleMapCache.close();
- appendBloatedConfig(outputChronicleMapConfig, stats, maxBloatFactor, sizeMultiplier);
+ copyExistingCacheSettingsToConfig(outputChronicleMapConfig, cacheConfig);
}
}
}
@@ -215,6 +241,27 @@
setResponse(rsp, HttpServletResponse.SC_OK, outputChronicleMapConfig.toText());
}
+ private Optional<H2AggregateData> optionalOf(H2AggregateData stats) {
+ if (stats.isEmpty()) {
+ return Optional.empty();
+ }
+ return Optional.of(stats);
+ }
+
+ private boolean hasFullPersistentCacheConfiguration(PersistentCacheDef<?, ?> in) {
+ return gerritConfig.getLong("cache", in.name(), "avgKeySize", 0L) > 0
+ && gerritConfig.getLong("cache", in.name(), "avgValueSize", 0L) > 0
+ && gerritConfig.getLong("cache", in.name(), "maxEntries", 0L) > 0
+ && gerritConfig.getInt("cache", in.name(), "maxBloatFactor", 0) > 0;
+ }
+
+ private boolean hasPartialPersistentCacheConfiguration(PersistentCacheDef<?, ?> in) {
+ return gerritConfig.getLong("cache", in.name(), "avgKeySize", 0L) > 0
+ || gerritConfig.getLong("cache", in.name(), "avgValueSize", 0L) > 0
+ || gerritConfig.getLong("cache", in.name(), "maxEntries", 0L) > 0
+ || gerritConfig.getInt("cache", in.name(), "maxBloatFactor", 0) > 0;
+ }
+
protected Optional<Path> getCacheDir() throws IOException {
String name = gerritConfig.getString("cache", null, "directory");
if (name == null) {
@@ -240,18 +287,6 @@
return Optional.empty();
}
- private void appendBloatedConfig(
- Config config, H2AggregateData stats, int maxBloatFactor, int sizeMultiplier) {
- appendToConfig(
- config,
- H2AggregateData.create(
- stats.cacheName(),
- stats.size() * sizeMultiplier,
- stats.avgKeySize(),
- stats.avgValueSize()));
- config.setLong("cache", stats.cacheName(), "maxBloatFactor", maxBloatFactor);
- }
-
protected static ChronicleMapCacheConfig makeChronicleMapConfig(
ChronicleMapCacheConfig.Factory configFactory,
Path cacheDir,
@@ -318,4 +353,13 @@
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();
+ outputConfig.setLong("cache", cacheName, "avgKeySize", cacheConfig.getAverageKeySize());
+ outputConfig.setLong("cache", cacheName, "avgValueSize", cacheConfig.getAverageValueSize());
+ outputConfig.setLong("cache", cacheName, "maxEntries", cacheConfig.getMaxEntries());
+ outputConfig.setLong("cache", cacheName, "maxBloatFactor", cacheConfig.getMaxBloatFactor());
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryLRU.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryLRU.java
index 7f450d5..dcf7a68 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryLRU.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/InMemoryLRU.java
@@ -31,6 +31,8 @@
LRUMap =
Collections.synchronizedMap(
new LinkedHashMap<K, Boolean>(capacity, 0.75f, true) {
+ private static final long serialVersionUID = 1L;
+
@Override
protected boolean removeEldestEntry(Map.Entry<K, Boolean> eldest) {
return size() > capacity;
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/KeyWrapperMarshaller.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/KeyWrapperMarshaller.java
index 7c7801c..53de07c 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/KeyWrapperMarshaller.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/KeyWrapperMarshaller.java
@@ -34,7 +34,7 @@
return new KeyWrapperMarshaller<>(name);
}
- @SuppressWarnings("unchecked")
+ @SuppressWarnings({"unchecked", "rawtypes"})
@Override
public KeyWrapper<V> read(Bytes in, KeyWrapper<V> using) {
int serializedLength = (int) in.readUnsignedInt();
@@ -46,6 +46,7 @@
return using;
}
+ @SuppressWarnings("rawtypes")
@Override
public void write(Bytes out, KeyWrapper<V> toWrite) {
final byte[] serialized = CacheSerializers.getKeySerializer(name).serialize(toWrite.getValue());
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshaller.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshaller.java
index ec30043..f85f57e 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshaller.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshaller.java
@@ -35,7 +35,7 @@
return new TimedValueMarshaller<>(name);
}
- @SuppressWarnings("unchecked")
+ @SuppressWarnings({"rawtypes", "unchecked"})
@Override
public TimedValue<V> read(Bytes in, TimedValue<V> using) {
long initialPosition = in.readPosition();
@@ -64,6 +64,7 @@
return using;
}
+ @SuppressWarnings("rawtypes")
@Override
public void write(Bytes out, TimedValue<V> toWrite) {
byte[] serialized = CacheSerializers.getValueSerializer(name).serialize(toWrite.getValue());
diff --git a/src/main/resources/Documentation/migration.md b/src/main/resources/Documentation/migration.md
index 0370232..261f08f 100644
--- a/src/main/resources/Documentation/migration.md
+++ b/src/main/resources/Documentation/migration.md
@@ -1,8 +1,8 @@
## Migration from H2 Caches
This module provides a REST API to help converting existing cache from H2 to
-chronicle-map, which requires the `Administrate Server` capability to be
-executed.
+chronicle-map, which requires the `Administrate Caches` or `Administrate Server`
+capabilities to be executed.
The migration must be executed _before_ switching to use chronicle-map, while
Gerrit cache is still backed by H2.
@@ -14,10 +14,15 @@
The migration would do the following:
1. scan all existing cache key-value pairs
-2. calculate the parameters for the new cache
+2. calculate the parameters for the new cache, if not already defined in gerrit.config.
3. create the new cache
4. read all existing key-value pairs and insert them into the new cache-chroniclemap files
+> **NOTE**: The existing cache parameters are kept in `gerrit.config` only when they are all
+> defined (avgKeySize, avgValueSize, maxEntries and maxBloatFactor), otherwise the
+> migration process will recalculate them and create the new cache based on the new
+> values.
+
The following caches will be migrated (if they exist and contain any data):
* accounts
diff --git a/src/main/resources/Documentation/tuning.md b/src/main/resources/Documentation/tuning.md
index 1571723..7ed7868 100644
--- a/src/main/resources/Documentation/tuning.md
+++ b/src/main/resources/Documentation/tuning.md
@@ -23,6 +23,9 @@
The idea is to read from the _actual_ H2 persisted files and output the
information that will be required to configure chronicle-map as an alternative.
+The Gerrit/SSH command to analyze H2 caches requires the user to have
+ `Administrate Caches` or `Administrate Server` capabilities.
+
You can do this _before_ installing cache-chroniclemap as a lib module so that
your Gerrit server will not need downtime. As follows:
@@ -130,6 +133,9 @@
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
+ `Administrate Caches` or `Administrate Server` capabilities.
+
* Symlink the `cache-chroniclemap.jar` file in the `plugins/` directory (from
the `lib/` directory).
* Wait for the pluginLoader to acknowledge and load the new plugin. You will see
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 5fce93a..482b9da 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
@@ -56,6 +56,12 @@
}
@Test
+ public void shouldDenyAccessToAnalyzeH2Cache() throws Exception {
+ userSshSession.exec(cmd);
+ userSshSession.assertFailure("not permitted");
+ }
+
+ @Test
public void shouldProduceWarningWhenCacheFileIsEmpty() throws Exception {
List<String> expected =
ImmutableList.of(
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 543f4c3..a55c02a 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
@@ -99,6 +99,12 @@
assertThat(tunedCaches.size()).isEqualTo(EXPECTED_CACHES.size());
}
+ @Test
+ public void shouldDenyAccessToCreateNewCacheFiles() throws Exception {
+ userSshSession.exec(cmd);
+ userSshSession.assertFailure("not permitted");
+ }
+
private Config configResult(String result) throws ConfigInvalidException {
Config configResult = new Config();
configResult.fromText((result.split(CONFIG_HEADER))[1]);
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheIT.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheIT.java
index 36ce53c..6f53dea 100644
--- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheIT.java
+++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheIT.java
@@ -19,7 +19,6 @@
import com.google.common.cache.Cache;
import com.google.common.truth.Truth8;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.accounts.AccountInput;
@@ -65,7 +64,7 @@
@Test
public void shouldCacheNewProject() throws Exception {
String newProjectName = name("newProject");
- RestResponse r = adminRestSession.put("/projects/" + newProjectName);
+ adminRestSession.put("/projects/" + newProjectName).assertCreated();
Truth8.assertThat(projectCache.get(Project.nameKey(newProjectName))).isPresent();
}
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java
index 5d380ff..739c688 100644
--- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java
+++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java
@@ -406,7 +406,7 @@
gerritConfig.setInt("cache", TEST_CACHE_NAME, "percentageHotKeys", percentageHotKeys);
gerritConfig.save();
- ChronicleMapCacheImpl<String, String> cache = newCacheWithMetrics(TEST_CACHE_NAME, cachedValue);
+ newCacheWithMetrics(TEST_CACHE_NAME, cachedValue);
assertThat(getMetric(hotKeysCapacityMetricName).getValue()).isEqualTo(expectedCapacity);
}
@@ -503,16 +503,6 @@
return newCache(true, cacheName, cachedValue, null, null, null, null, 1, metricMaker);
}
- private ChronicleMapCacheImpl<String, String> newCacheWithMetrics(
- String cacheName,
- String cachedValue,
- CacheSerializer<String> keySerializer,
- CacheSerializer<String> valueSerializer)
- throws IOException {
- return newCache(
- true, cacheName, cachedValue, null, null, null, null, 1, new DisabledMetricMaker());
- }
-
private ChronicleMapCacheImpl<String, String> newCache(
Boolean withLoader,
String cacheName,
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 5480001..dbca0df 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
@@ -23,8 +23,6 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.RestSession;
import com.google.gerrit.acceptance.TestPlugin;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.inject.Inject;
import java.io.IOException;
import org.apache.http.message.BasicHeader;
import org.junit.Test;
@@ -35,8 +33,6 @@
public class MigrateH2CachesInMemoryIT extends LightweightPluginDaemonTest {
private static final String MIGRATION_ENDPOINT = "/plugins/cache-chroniclemap/migrate";
- @Inject protected GitRepositoryManager repoManager;
-
@Test
public void shouldReturnTexPlain() throws Exception {
RestResponse result = runMigration(adminRestSession);
@@ -52,8 +48,7 @@
public void shouldFailWhenUserHasNoAdminServerCapability() throws Exception {
RestResponse result = runMigration(userRestSession);
result.assertForbidden();
- assertThat(result.getEntityContent())
- .contains("administrateServer for plugin cache-chroniclemap not permitted");
+ assertThat(result.getEntityContent()).contains("not permitted");
}
@Test
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 d2523f0..93b0297 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
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.modules.cache.chroniclemap;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.googlesource.gerrit.modules.cache.chroniclemap.H2CacheCommand.H2_SUFFIX;
import static com.googlesource.gerrit.modules.cache.chroniclemap.H2MigrationServlet.DEFAULT_MAX_BLOAT_FACTOR;
import static com.googlesource.gerrit.modules.cache.chroniclemap.H2MigrationServlet.DEFAULT_SIZE_MULTIPLIER;
@@ -31,6 +32,8 @@
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.acceptance.WaitUtil;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.CachedProjectConfig;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
@@ -40,12 +43,12 @@
import com.google.gerrit.server.cache.h2.H2CacheImpl;
import com.google.gerrit.server.cache.proto.Cache;
import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
-import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
-import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.inject.Binding;
import com.google.inject.Inject;
import com.google.inject.Key;
+import com.google.inject.Module;
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.nio.file.Path;
@@ -67,9 +70,8 @@
private String PERSISTED_PROJECTS_CACHE_NAME = "persisted_projects";
private String MIGRATION_ENDPOINT = "/plugins/cache-chroniclemap/migrate";
- @Inject protected GitRepositoryManager repoManager;
@Inject private SitePaths sitePaths;
- @Inject @GerritServerConfig Config cfg;
+ @Inject private ProjectOperations projectOperations;
private ChronicleMapCacheConfig.Factory chronicleMapCacheConfigFactory;
@@ -79,6 +81,12 @@
plugin.getHttpInjector().getInstance(ChronicleMapCacheConfig.Factory.class);
}
+ /** Override to bind an additional Guice module */
+ @Override
+ public Module createModule() {
+ return new CapabilityModule();
+ }
+
@Test
public void shouldRunAndCompleteSuccessfullyWhenCacheDirectoryIsDefined() throws Exception {
runMigration(adminRestSession).assertOK();
@@ -120,6 +128,25 @@
}
@Test
+ public void shouldDenyH2MigrationForNonAdminsAndUsersWithoutAdministerCachePermission()
+ throws Exception {
+ waitForCacheToLoad(ACCOUNTS_CACHE_NAME);
+ waitForCacheToLoad(PERSISTED_PROJECTS_CACHE_NAME);
+
+ runMigration(userRestSession).assertForbidden();
+
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(
+ allowCapability("cache-chroniclemap-" + AdministerCachesCapability.ID)
+ .group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+
+ runMigration(userRestSession).assertOK();
+ }
+
+ @Test
public void shouldOutputChronicleMapBloatedProvidedConfiguration() throws Exception {
waitForCacheToLoad(ACCOUNTS_CACHE_NAME);
waitForCacheToLoad(PERSISTED_PROJECTS_CACHE_NAME);
@@ -146,6 +173,53 @@
}
@Test
+ @GerritConfig(name = "cache.accounts.maxBloatFactor", value = "1")
+ @GerritConfig(name = "cache.accounts.maxEntries", value = "10")
+ @GerritConfig(name = "cache.accounts.avgKeySize", value = "100")
+ @GerritConfig(name = "cache.accounts.avgValueSize", value = "1000")
+ public void shouldKeepExistingChronicleMapConfiguration() throws Exception {
+ waitForCacheToLoad(ACCOUNTS_CACHE_NAME);
+
+ int sizeMultiplier = 2;
+ int maxBloatFactor = 3;
+ RestResponse result = runMigration(sizeMultiplier, maxBloatFactor);
+ result.assertOK();
+
+ Config configResult = new Config();
+ String entityContent = result.getEntityContent();
+ configResult.fromText(entityContent);
+
+ assertThat(configResult.getInt("cache", ACCOUNTS_CACHE_NAME, "maxBloatFactor", 0)).isEqualTo(1);
+ assertThat(configResult.getInt("cache", ACCOUNTS_CACHE_NAME, "maxEntries", 0)).isEqualTo(10);
+ assertThat(configResult.getInt("cache", ACCOUNTS_CACHE_NAME, "avgKeySize", 0)).isEqualTo(100);
+ assertThat(configResult.getInt("cache", ACCOUNTS_CACHE_NAME, "avgValueSize", 0))
+ .isEqualTo(1000);
+ }
+
+ @Test
+ @GerritConfig(name = "cache.accounts.maxBloatFactor", value = "1")
+ @GerritConfig(name = "cache.accounts.maxEntries", value = "10")
+ @GerritConfig(name = "cache.accounts.avgValueSize", value = "1000")
+ public void shouldIgnoreIncompleteChronicleMapConfiguration() throws Exception {
+ waitForCacheToLoad(ACCOUNTS_CACHE_NAME);
+
+ int sizeMultiplier = 2;
+ int maxBloatFactor = 3;
+ RestResponse result = runMigration(sizeMultiplier, maxBloatFactor);
+ result.assertOK();
+
+ Config configResult = new Config();
+ String entityContent = result.getEntityContent();
+ configResult.fromText(entityContent);
+
+ assertThat(configResult.getInt("cache", ACCOUNTS_CACHE_NAME, "maxBloatFactor", 0))
+ .isEqualTo(maxBloatFactor);
+ assertThat(configResult.getInt("cache", ACCOUNTS_CACHE_NAME, "maxEntries", 0)).isNotEqualTo(10);
+ assertThat(configResult.getInt("cache", ACCOUNTS_CACHE_NAME, "avgValueSize", 0))
+ .isNotEqualTo(1000);
+ }
+
+ @Test
public void shouldMigrateAccountsCache() throws Exception {
waitForCacheToLoad(ACCOUNTS_CACHE_NAME);
