Merge branch 'stable-3.0' into stable-3.1
* stable-3.0:
Cache change /meta ref SHA1 for each REST API request
Release-Notes: skip
Change-Id: Ifaca2a4a00f9b7a3c9bb72f9a2b0d95c8c039009
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 8160754..1296c49 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -316,7 +316,7 @@
RequestInfo requestInfo = createRequestInfo(traceContext, requestUri(req), path);
globals.requestListeners.runEach(l -> l.onRequest(requestInfo));
- try (PerThreadCache ignored = PerThreadCache.create()) {
+ try (PerThreadCache ignored = PerThreadCache.create(req)) {
// It's important that the PerformanceLogContext is closed before the response is sent to
// the client. Only this way it is ensured that the invocation of the PerformanceLogger
// plugins happens before the client sees the response. This is needed for being able to
diff --git a/java/com/google/gerrit/server/cache/PerThreadCache.java b/java/com/google/gerrit/server/cache/PerThreadCache.java
index b4f79d1..609fce7 100644
--- a/java/com/google/gerrit/server/cache/PerThreadCache.java
+++ b/java/com/google/gerrit/server/cache/PerThreadCache.java
@@ -21,7 +21,9 @@
import com.google.common.collect.Maps;
import com.google.gerrit.common.Nullable;
import java.util.Map;
+import java.util.Optional;
import java.util.function.Supplier;
+import javax.servlet.http.HttpServletRequest;
/**
* Caches object instances for a request as {@link ThreadLocal} in the serving thread.
@@ -58,6 +60,12 @@
private static final int PER_THREAD_CACHE_SIZE = 25;
/**
+ * Optional HTTP request associated with the per-thread cache, should the thread be associated
+ * with the incoming HTTP thread pool.
+ */
+ private final Optional<HttpServletRequest> httpRequest;
+
+ /**
* Unique key for key-value mappings stored in PerThreadCache. The key is based on the value's
* class and a list of identifiers that in combination uniquely set the object apart form others
* of the same class.
@@ -102,9 +110,9 @@
}
}
- public static PerThreadCache create() {
+ public static PerThreadCache create(@Nullable HttpServletRequest httpRequest) {
checkState(CACHE.get() == null, "called create() twice on the same request");
- PerThreadCache cache = new PerThreadCache();
+ PerThreadCache cache = new PerThreadCache(httpRequest);
CACHE.set(cache);
return cache;
}
@@ -121,7 +129,9 @@
private final Map<Key<?>, Object> cache = Maps.newHashMapWithExpectedSize(PER_THREAD_CACHE_SIZE);
- private PerThreadCache() {}
+ private PerThreadCache(@Nullable HttpServletRequest req) {
+ httpRequest = Optional.ofNullable(req);
+ }
/**
* Returns an instance of {@code T} that was either loaded from the cache or obtained from the
@@ -139,6 +149,19 @@
return value;
}
+ /** Returns the optional HTTP request associated with the local thread cache. */
+ public Optional<HttpServletRequest> getHttpRequest() {
+ return httpRequest;
+ }
+
+ /** Returns true if there is an HTTP request associated and is a GET or HEAD */
+ public boolean hasReadonlyRequest() {
+ return httpRequest
+ .map(HttpServletRequest::getMethod)
+ .filter(m -> m.equalsIgnoreCase("GET") || m.equalsIgnoreCase("HEAD"))
+ .isPresent();
+ }
+
@Override
public void close() {
CACHE.remove();
diff --git a/java/com/google/gerrit/server/git/RepoRefCache.java b/java/com/google/gerrit/server/git/RepoRefCache.java
index 6b2493a..9086da7 100644
--- a/java/com/google/gerrit/server/git/RepoRefCache.java
+++ b/java/com/google/gerrit/server/git/RepoRefCache.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.git;
+import com.google.gerrit.server.cache.PerThreadCache;
import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
@@ -29,6 +30,17 @@
private final RefDatabase refdb;
private final Map<String, Optional<ObjectId>> ids;
+ public static Optional<RefCache> getOptional(Repository repo) {
+ PerThreadCache cache = PerThreadCache.get();
+ if (cache != null && cache.hasReadonlyRequest()) {
+ return Optional.of(
+ cache.get(
+ PerThreadCache.Key.create(RepoRefCache.class, repo), () -> new RepoRefCache(repo)));
+ }
+
+ return Optional.empty();
+ }
+
public RepoRefCache(Repository repo) {
this.refdb = repo.getRefDatabase();
this.ids = new HashMap<>();
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index bd673d6..df4bc24 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -55,6 +55,7 @@
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.git.RefCache;
+import com.google.gerrit.server.git.RepoRefCache;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.change.ChangeData;
@@ -566,6 +567,10 @@
@Override
protected ObjectId readRef(Repository repo) throws IOException {
- return refs != null ? refs.get(getRefName()).orElse(null) : super.readRef(repo);
+ Optional<RefCache> refsCache =
+ Optional.ofNullable(refs).map(Optional::of).orElse(RepoRefCache.getOptional(repo));
+ return refsCache.isPresent()
+ ? refsCache.get().get(getRefName()).orElse(null)
+ : super.readRef(repo);
}
}
diff --git a/javatests/com/google/gerrit/server/cache/BUILD b/javatests/com/google/gerrit/server/cache/BUILD
index c255e61..bb6ea30 100644
--- a/javatests/com/google/gerrit/server/cache/BUILD
+++ b/javatests/com/google/gerrit/server/cache/BUILD
@@ -6,7 +6,10 @@
deps = [
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/testing:gerrit-test-util",
+ "//javatests/com/google/gerrit/util/http/testutil",
"//lib:junit",
"//lib/truth",
+ "//lib/truth:truth-java8-extension",
+ "@servlet-api//jar",
],
)
diff --git a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
index d19073d..9d0e0e0 100644
--- a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
+++ b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
@@ -15,9 +15,13 @@
package com.google.gerrit.server.cache;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
import java.util.function.Supplier;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
import org.junit.Test;
public class PerThreadCacheTest {
@@ -39,7 +43,7 @@
@Test
public void endToEndCache() {
- try (PerThreadCache ignored = PerThreadCache.create()) {
+ try (PerThreadCache ignored = PerThreadCache.create(null)) {
PerThreadCache cache = PerThreadCache.get();
PerThreadCache.Key<String> key1 = PerThreadCache.Key.create(String.class);
@@ -57,7 +61,7 @@
@Test
public void cleanUp() {
PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class);
- try (PerThreadCache ignored = PerThreadCache.create()) {
+ try (PerThreadCache ignored = PerThreadCache.create(null)) {
PerThreadCache cache = PerThreadCache.get();
String value1 = cache.get(key, () -> "value1");
assertThat(value1).isEqualTo("value1");
@@ -65,7 +69,7 @@
// Create a second cache and assert that it is not connected to the first one.
// This ensures that the cleanup is actually working.
- try (PerThreadCache ignored = PerThreadCache.create()) {
+ try (PerThreadCache ignored = PerThreadCache.create(null)) {
PerThreadCache cache = PerThreadCache.get();
String value1 = cache.get(key, () -> "value2");
assertThat(value1).isEqualTo("value2");
@@ -74,16 +78,48 @@
@Test
public void doubleInstantiationFails() {
- try (PerThreadCache ignored = PerThreadCache.create()) {
+ try (PerThreadCache ignored = PerThreadCache.create(null)) {
IllegalStateException thrown =
- assertThrows(IllegalStateException.class, () -> PerThreadCache.create());
+ assertThrows(IllegalStateException.class, () -> PerThreadCache.create(null));
assertThat(thrown).hasMessageThat().contains("called create() twice on the same request");
}
}
@Test
+ public void isAssociatedWithHttpReadonlyRequest() {
+ HttpServletRequest getRequest = new FakeHttpServletRequest();
+ try (PerThreadCache cache = PerThreadCache.create(getRequest)) {
+ assertThat(cache.getHttpRequest()).hasValue(getRequest);
+ assertThat(cache.hasReadonlyRequest()).isTrue();
+ }
+ }
+
+ @Test
+ public void isAssociatedWithHttpWriteRequest() {
+ HttpServletRequest putRequest =
+ new HttpServletRequestWrapper(new FakeHttpServletRequest()) {
+ @Override
+ public String getMethod() {
+ return "PUT";
+ }
+ };
+ try (PerThreadCache cache = PerThreadCache.create(putRequest)) {
+ assertThat(cache.getHttpRequest()).hasValue(putRequest);
+ assertThat(cache.hasReadonlyRequest()).isFalse();
+ }
+ }
+
+ @Test
+ public void isNotAssociatedWithHttpRequest() {
+ try (PerThreadCache cache = PerThreadCache.create(null)) {
+ assertThat(cache.getHttpRequest()).isEmpty();
+ assertThat(cache.hasReadonlyRequest()).isFalse();
+ }
+ }
+
+ @Test
public void enforceMaxSize() {
- try (PerThreadCache cache = PerThreadCache.create()) {
+ try (PerThreadCache cache = PerThreadCache.create(null)) {
// Fill the cache
for (int i = 0; i < 50; i++) {
PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class, i);
