Merge branch 'stable-3.2' into stable-3.3
* stable-3.2:
Introduce cache.threads option to enable a custom cache executor
GitwebServlet: Fix project root computation
Release-Notes: skip
Change-Id: I342a210680ff43796f9fa4403f1b70f78e0ddb3e
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index d4382a7..1a9ffdb 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -706,6 +706,19 @@
[[cache]]
=== Section cache
+[[cache.threads]]cache.threads::
++
+Number of threads to use when running asynchronous cache tasks.
+The threads executor is delegated to when sending removal notifications to listeners,
+when asynchronous computations like refresh, refreshAfterWrite are performed, or when
+performing periodic maintenance.
++
+**NOTE**: Setting it to 0 disables the dedicated thread pool and indexing will be done in the
+same thread as the operation. This may result in evictions taking longer because the
+listeners are executed in the caller's thread.
++
+By default, the JVM common ForkJoinPool is used.
+
[[cache.directory]]cache.directory::
+
Path to a local directory where Gerrit can write cached entities for
diff --git a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
index 0875317..4c18afd 100644
--- a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
+++ b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
@@ -32,6 +32,7 @@
import static java.nio.charset.StandardCharsets.ISO_8859_1;
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.CharMatcher;
import com.google.common.base.Splitter;
import com.google.common.flogger.FluentLogger;
@@ -640,20 +641,39 @@
return env.getEnvArray();
}
- private String getProjectRoot(Project.NameKey nameKey)
- throws RepositoryNotFoundException, IOException {
+ /**
+ * Return the project root under which the specified project is stored.
+ *
+ * @param nameKey the name of the project
+ * @return base directory
+ */
+ @VisibleForTesting
+ String getProjectRoot(Project.NameKey nameKey) throws RepositoryNotFoundException, IOException {
try (Repository repo = repoManager.openRepository(nameKey)) {
- return getProjectRoot(repo);
+ return getRepositoryRoot(repo, nameKey).toString();
}
}
- private String getProjectRoot(Repository repo) {
+ /**
+ * Return the repository root under which the specified repository is stored.
+ *
+ * @param repo the name of the repository
+ * @param nameKey project name
+ * @return base path
+ * @throws ProvisionException if the repo is not DelegateRepository or FileRepository.
+ */
+ private static Path getRepositoryRoot(Repository repo, Project.NameKey nameKey) {
if (repo instanceof DelegateRepository) {
- return getProjectRoot(((DelegateRepository) repo).delegate());
+ return getRepositoryRoot(((DelegateRepository) repo).delegate(), nameKey);
}
if (repo instanceof FileRepository) {
- return repo.getDirectory().getAbsolutePath();
+ String name = nameKey.get();
+ Path current = repo.getDirectory().toPath();
+ for (int i = 0; i <= CharMatcher.is('/').countIn(name); i++) {
+ current = current.getParent();
+ }
+ return current;
}
throw new ProvisionException("Gitweb can only be used with FileRepository");
diff --git a/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java b/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java
index ee672cd..357cbbb 100644
--- a/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java
+++ b/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.cache;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.cache.RemovalListener;
import com.google.common.cache.RemovalNotification;
@@ -40,7 +41,8 @@
private String pluginName = PluginName.GERRIT;
@Inject
- ForwardingRemovalListener(
+ @VisibleForTesting
+ protected ForwardingRemovalListener(
PluginSetContext<CacheRemovalListener> listeners, @Assisted String cacheName) {
this.listeners = listeners;
this.cacheName = cacheName;
diff --git a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
index 23caca7..d792a39 100644
--- a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
+++ b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
@@ -27,6 +27,7 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.cache.RemovalNotification;
+import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.server.cache.CacheBackend;
import com.google.gerrit.server.cache.CacheDef;
@@ -34,20 +35,37 @@
import com.google.gerrit.server.cache.MemoryCacheFactory;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.WorkQueue;
import com.google.inject.Inject;
import java.time.Duration;
+import java.util.concurrent.Executor;
import org.eclipse.jgit.lib.Config;
class DefaultMemoryCacheFactory implements MemoryCacheFactory {
+ static final String CACHE_EXECUTOR_PREFIX = "InMemoryCacheExecutor";
+ private static final int DEFAULT_CACHE_EXECUTOR_THREADS = -1;
+
private final Config cfg;
private final ForwardingRemovalListener.Factory forwardingRemovalListenerFactory;
+ private int executorThreads;
+ private final Executor executor;
@Inject
DefaultMemoryCacheFactory(
@GerritServerConfig Config config,
- ForwardingRemovalListener.Factory forwardingRemovalListenerFactory) {
+ ForwardingRemovalListener.Factory forwardingRemovalListenerFactory,
+ WorkQueue workQueue) {
this.cfg = config;
this.forwardingRemovalListenerFactory = forwardingRemovalListenerFactory;
+ this.executorThreads = config.getInt("cache", "threads", DEFAULT_CACHE_EXECUTOR_THREADS);
+
+ if (executorThreads == 0) {
+ executor = MoreExecutors.newDirectExecutorService();
+ } else if (executorThreads > DEFAULT_CACHE_EXECUTOR_THREADS) {
+ executor = workQueue.createQueue(executorThreads, CACHE_EXECUTOR_PREFIX);
+ } else {
+ executor = null;
+ }
}
@Override
@@ -129,6 +147,10 @@
builder.maximumWeight(
cfg.getLong("cache", def.configKey(), "memoryLimit", def.maximumWeight()));
builder = builder.removalListener(newRemovalListener(def.name()));
+
+ if (executor != null) {
+ builder.executor(executor);
+ }
builder.weigher(newWeigher(def.weigher()));
Duration expireAfterWrite = def.expireAfterWrite();
diff --git a/javatests/com/google/gerrit/httpd/gitweb/GitwebServletTest.java b/javatests/com/google/gerrit/httpd/gitweb/GitwebServletTest.java
new file mode 100644
index 0000000..d1598b9
--- /dev/null
+++ b/javatests/com/google/gerrit/httpd/gitweb/GitwebServletTest.java
@@ -0,0 +1,107 @@
+// Copyright (C) 2022 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.google.gerrit.httpd.gitweb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.mock;
+
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.AllProjectsNameProvider;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.GitwebCgiConfig;
+import com.google.gerrit.server.config.GitwebConfig;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.git.LocalDiskRepositoryManager;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class GitwebServletTest {
+ @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ private Config cfg;
+ private SitePaths site;
+ private LocalDiskRepositoryManager repoManager;
+ private ProjectCache projectCache;
+ private PermissionBackend permissionBackendMock;
+ private GitwebCgiConfig gitWebCgiConfig;
+ private GitwebConfig gitWebConfig;
+ private GitwebServlet servlet;
+ private AllProjectsName allProjectsName;
+
+ @Before
+ public void setUp() throws Exception {
+ site = new SitePaths(temporaryFolder.newFolder().toPath());
+ site.resolve("git").toFile().mkdir();
+ cfg = new Config();
+ cfg.setString("gerrit", null, "basePath", "git");
+ repoManager =
+ Guice.createInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(SitePaths.class).toInstance(site);
+ bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(cfg);
+ }
+ })
+ .getInstance(LocalDiskRepositoryManager.class);
+ projectCache = mock(ProjectCache.class);
+ permissionBackendMock = mock(PermissionBackend.class);
+ gitWebCgiConfig = mock(GitwebCgiConfig.class);
+ gitWebConfig = mock(GitwebConfig.class);
+ allProjectsName = new AllProjectsName(AllProjectsNameProvider.DEFAULT);
+ // All-Projects must exist prior to calling GitwebServlet ctor
+ repoManager.createRepository(allProjectsName);
+ servlet =
+ new GitwebServlet(
+ repoManager,
+ projectCache,
+ permissionBackendMock,
+ null,
+ site,
+ cfg,
+ null,
+ null,
+ gitWebConfig,
+ gitWebCgiConfig,
+ allProjectsName);
+ }
+
+ @Test
+ public void projectRootSetToBasePathForSimpleRepository() throws Exception {
+ Project.NameKey foo = Project.nameKey("foo");
+ try (Repository repo = repoManager.createRepository(foo)) {
+ assertThat(servlet.getProjectRoot(foo))
+ .isEqualTo(repoManager.getBasePath(foo).toAbsolutePath().toString());
+ }
+ }
+
+ @Test
+ public void projectRootSetToBasePathForNestedRepository() throws Exception {
+ Project.NameKey baz = Project.nameKey("foo/bar/baz");
+ try (Repository repo = repoManager.createRepository(baz)) {
+ assertThat(servlet.getProjectRoot(baz))
+ .isEqualTo(repoManager.getBasePath(baz).toAbsolutePath().toString());
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactoryTest.java b/javatests/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactoryTest.java
new file mode 100644
index 0000000..9e345c0
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactoryTest.java
@@ -0,0 +1,263 @@
+// Copyright (C) 2022 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.google.gerrit.server.cache.mem;
+
+import static com.google.common.base.Functions.identity;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.cache.RemovalNotification;
+import com.google.common.cache.Weigher;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.metrics.DisabledMetricMaker;
+import com.google.gerrit.server.cache.CacheBackend;
+import com.google.gerrit.server.cache.CacheDef;
+import com.google.gerrit.server.cache.ForwardingRemovalListener;
+import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.util.IdGenerator;
+import com.google.inject.Guice;
+import com.google.inject.TypeLiteral;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.BrokenBarrierException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
+import org.apache.mina.util.ConcurrentHashSet;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Before;
+import org.junit.Test;
+
+public class DefaultMemoryCacheFactoryTest {
+
+ private static final String TEST_CACHE = "test-cache";
+ private static final long TEST_TIMEOUT_SEC = 1;
+ private static final int TEST_CACHE_KEY = 1;
+ private static final int TEST_CACHE_VALUE = 2;
+
+ private DefaultMemoryCacheFactory memoryCacheFactory;
+ private DefaultMemoryCacheFactory memoryCacheFactoryDirectExecutor;
+ private DefaultMemoryCacheFactory memoryCacheFactoryWithThreadPool;
+ private Config memoryCacheConfig;
+ private Config memoryCacheConfigDirectExecutor;
+ private Config memoryCacheConfigWithThreadPool;
+ private CyclicBarrier cacheGetStarted;
+ private CyclicBarrier cacheGetCompleted;
+ private CyclicBarrier evictionReceived;
+ private ForwardingRemovalTrackerListener forwardingRemovalListener;
+ private WorkQueue workQueue;
+
+ @Before
+ public void setUp() {
+
+ IdGenerator idGenerator = Guice.createInjector().getInstance(IdGenerator.class);
+ workQueue = new WorkQueue(idGenerator, 10, new DisabledMetricMaker());
+
+ memoryCacheConfig = new Config();
+ memoryCacheConfigDirectExecutor = new Config();
+ memoryCacheConfigDirectExecutor.setInt("cache", null, "threads", 0);
+ memoryCacheConfigWithThreadPool = new Config();
+ memoryCacheConfigWithThreadPool.setInt("cache", null, "threads", 1);
+
+ forwardingRemovalListener = new ForwardingRemovalTrackerListener();
+ memoryCacheFactory =
+ new DefaultMemoryCacheFactory(
+ memoryCacheConfig, (cache) -> forwardingRemovalListener, workQueue);
+ memoryCacheFactoryDirectExecutor =
+ new DefaultMemoryCacheFactory(
+ memoryCacheConfigDirectExecutor, (cache) -> forwardingRemovalListener, workQueue);
+ memoryCacheFactoryWithThreadPool =
+ new DefaultMemoryCacheFactory(
+ memoryCacheConfigWithThreadPool, (cache) -> forwardingRemovalListener, workQueue);
+ cacheGetStarted = new CyclicBarrier(2);
+ cacheGetCompleted = new CyclicBarrier(2);
+ evictionReceived = new CyclicBarrier(2);
+ }
+
+ @Test
+ public void shouldRunEvictionListenerInBackgroundByDefault() throws Exception {
+ shouldRunEvictionListenerInThreadPool(memoryCacheFactory, "ForkJoinPool");
+ }
+
+ @Test
+ public void shouldRunEvictionListenerInThreadPool() throws Exception {
+ shouldRunEvictionListenerInThreadPool(
+ memoryCacheFactoryWithThreadPool, DefaultMemoryCacheFactory.CACHE_EXECUTOR_PREFIX);
+ }
+
+ private void shouldRunEvictionListenerInThreadPool(
+ DefaultMemoryCacheFactory cacheFactory, String threadPoolPrefix) throws Exception {
+ LoadingCache<Integer, Integer> cache =
+ cacheFactory.build(newCacheDef(1), newCacheLoader(identity()), CacheBackend.CAFFEINE);
+
+ cache.put(TEST_CACHE_KEY, TEST_CACHE_VALUE);
+ cache.invalidate(TEST_CACHE_KEY);
+
+ assertThat(forwardingRemovalListener.contains(TEST_CACHE_KEY, TEST_CACHE_VALUE)).isFalse();
+
+ evictionReceived.await(TEST_TIMEOUT_SEC, TimeUnit.SECONDS);
+
+ assertThat(forwardingRemovalListener.contains(TEST_CACHE_KEY, TEST_CACHE_VALUE)).isTrue();
+ assertThat(forwardingRemovalListener.removalThreadName(TEST_CACHE_KEY))
+ .startsWith(threadPoolPrefix);
+ }
+
+ @Test
+ public void shouldRunEvictionListenerWithDirectExecutor() throws Exception {
+ LoadingCache<Integer, Integer> cache =
+ memoryCacheFactoryDirectExecutor.build(
+ newCacheDef(1), newCacheLoader(identity()), CacheBackend.CAFFEINE);
+
+ cache.put(TEST_CACHE_KEY, TEST_CACHE_VALUE);
+ cache.invalidate(TEST_CACHE_KEY);
+
+ assertThat(forwardingRemovalListener.contains(TEST_CACHE_KEY, TEST_CACHE_VALUE)).isTrue();
+ }
+
+ @Test
+ public void shouldLoadAllKeysWithDisabledCache() throws Exception {
+ LoadingCache<Integer, Integer> disabledCache =
+ memoryCacheFactory.build(newCacheDef(0), newCacheLoader(identity()), CacheBackend.CAFFEINE);
+
+ List<Integer> keys = Arrays.asList(1, 2);
+ ImmutableMap<Integer, Integer> entries = disabledCache.getAll(keys);
+
+ assertThat(entries).containsExactly(1, 1, 2, 2);
+ }
+
+ private CacheLoader<Integer, Integer> newCacheLoader(Function<Integer, Integer> loadFunc) {
+ return new CacheLoader<Integer, Integer>() {
+
+ @Override
+ public Integer load(Integer n) throws Exception {
+ Integer v = 0;
+ try {
+ cacheGetStarted.await(TEST_TIMEOUT_SEC, TimeUnit.SECONDS);
+ v = loadFunc.apply(n);
+ cacheGetCompleted.await(TEST_TIMEOUT_SEC, TimeUnit.SECONDS);
+ } catch (TimeoutException | BrokenBarrierException e) {
+ }
+ return v;
+ }
+
+ @Override
+ public Map<Integer, Integer> loadAll(Iterable<? extends Integer> keys) throws Exception {
+ return StreamSupport.stream(keys.spliterator(), false)
+ .collect(Collectors.toMap(identity(), identity()));
+ }
+ };
+ }
+
+ private class ForwardingRemovalTrackerListener extends ForwardingRemovalListener<Object, Object> {
+ private final ConcurrentHashMap<Object, Set<Object>> removalEvents;
+ private final ConcurrentHashMap<Object, String> removalThreads;
+
+ public ForwardingRemovalTrackerListener() {
+ super(null, null);
+
+ removalEvents = new ConcurrentHashMap<>();
+ removalThreads = new ConcurrentHashMap<>();
+ }
+
+ @Override
+ public void onRemoval(RemovalNotification<Object, Object> notification) {
+ Set<Object> setOfValues =
+ removalEvents.computeIfAbsent(
+ notification.getKey(),
+ (key) -> {
+ Set<Object> elements = new ConcurrentHashSet<>();
+ return elements;
+ });
+ setOfValues.add(notification.getValue());
+
+ removalThreads.put(notification.getKey(), Thread.currentThread().getName());
+
+ try {
+ evictionReceived.await(TEST_TIMEOUT_SEC, TimeUnit.SECONDS);
+ } catch (InterruptedException | BrokenBarrierException | TimeoutException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ private boolean contains(Object key, Object value) {
+ return Optional.ofNullable(removalEvents.get(key))
+ .map(sv -> sv.contains(value))
+ .orElse(false);
+ }
+
+ private String removalThreadName(Object key) {
+ return removalThreads.get(key);
+ }
+ }
+
+ private CacheDef<Integer, Integer> newCacheDef(long maximumWeight) {
+ return new CacheDef<Integer, Integer>() {
+
+ @Override
+ public String name() {
+ return TEST_CACHE;
+ }
+
+ @Override
+ public String configKey() {
+ return TEST_CACHE;
+ }
+
+ @Override
+ public TypeLiteral<Integer> keyType() {
+ return null;
+ }
+
+ @Override
+ public TypeLiteral<Integer> valueType() {
+ return null;
+ }
+
+ @Override
+ public long maximumWeight() {
+ return maximumWeight;
+ }
+
+ @Override
+ public Duration expireAfterWrite() {
+ return null;
+ }
+
+ @Override
+ public Duration expireFromMemoryAfterAccess() {
+ return null;
+ }
+
+ @Override
+ public Weigher<Integer, Integer> weigher() {
+ return null;
+ }
+
+ @Override
+ public CacheLoader<Integer, Integer> loader() {
+ return null;
+ }
+ };
+ }
+}
