summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLuca Milanesio <luca.milanesio@gmail.com>2022-04-22 08:36:43 +0100
committerLuca Milanesio <luca.milanesio@gmail.com>2022-04-23 00:25:24 +0100
commitac92778ab8d08136b6a0268419e0bd244fa98dca (patch)
tree34a1e5bfcbe98dfc2f142da13cd9979da3f62f26
parent4b327bbe8b74e7ee7d66fbf88ed81bb60dd79016 (diff)
Revert "Cache change /meta ref SHA1 for each REST API request"
This reverts commit 23db8a546b800b2c91629d80b6f5117ff635238a. Reason: refs caching on stable-3.2 can be achieve using [1]. [1] https://gerrit.googlesource.com/modules/cached-refdb/ Release-Notes: skip Change-Id: I44ad6fbe165bed57c3d639a6a0878f704139b7b3
-rw-r--r--java/com/google/gerrit/httpd/restapi/RestApiServlet.java2
-rw-r--r--java/com/google/gerrit/server/cache/PerThreadCache.java29
-rw-r--r--java/com/google/gerrit/server/git/RepoRefCache.java12
-rw-r--r--java/com/google/gerrit/server/notedb/ChangeNotes.java7
-rw-r--r--javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java47
5 files changed, 11 insertions, 86 deletions
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index fb1f2353ff..d427caa0c6 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -326,7 +326,7 @@ public class RestApiServlet extends HttpServlet {
try (TraceContext traceContext = enableTracing(req, res)) {
List<IdString> path = splitPath(req);
- try (PerThreadCache ignored = PerThreadCache.create(req)) {
+ try (PerThreadCache ignored = PerThreadCache.create()) {
RequestInfo requestInfo = createRequestInfo(traceContext, requestUri(req), path);
globals.requestListeners.runEach(l -> l.onRequest(requestInfo));
diff --git a/java/com/google/gerrit/server/cache/PerThreadCache.java b/java/com/google/gerrit/server/cache/PerThreadCache.java
index 609fce7b25..b4f79d10ab 100644
--- a/java/com/google/gerrit/server/cache/PerThreadCache.java
+++ b/java/com/google/gerrit/server/cache/PerThreadCache.java
@@ -21,9 +21,7 @@ import com.google.common.collect.ImmutableList;
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.
@@ -60,12 +58,6 @@ public class PerThreadCache implements AutoCloseable {
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.
@@ -110,9 +102,9 @@ public class PerThreadCache implements AutoCloseable {
}
}
- public static PerThreadCache create(@Nullable HttpServletRequest httpRequest) {
+ public static PerThreadCache create() {
checkState(CACHE.get() == null, "called create() twice on the same request");
- PerThreadCache cache = new PerThreadCache(httpRequest);
+ PerThreadCache cache = new PerThreadCache();
CACHE.set(cache);
return cache;
}
@@ -129,9 +121,7 @@ public class PerThreadCache implements AutoCloseable {
private final Map<Key<?>, Object> cache = Maps.newHashMapWithExpectedSize(PER_THREAD_CACHE_SIZE);
- private PerThreadCache(@Nullable HttpServletRequest req) {
- httpRequest = Optional.ofNullable(req);
- }
+ private PerThreadCache() {}
/**
* Returns an instance of {@code T} that was either loaded from the cache or obtained from the
@@ -149,19 +139,6 @@ public class PerThreadCache implements AutoCloseable {
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 9086da7fb5..6b2493a9f1 100644
--- a/java/com/google/gerrit/server/git/RepoRefCache.java
+++ b/java/com/google/gerrit/server/git/RepoRefCache.java
@@ -14,7 +14,6 @@
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;
@@ -30,17 +29,6 @@ public class RepoRefCache implements RefCache {
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 cc31115fe5..51acf16e52 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -56,7 +56,6 @@ import com.google.gerrit.server.ReviewerByEmailSet;
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;
@@ -577,10 +576,6 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
@Override
protected ObjectId readRef(Repository repo) throws IOException {
- 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);
+ return refs != null ? refs.get(getRefName()).orElse(null) : super.readRef(repo);
}
}
diff --git a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
index 9d0e0e0665..0453fa1532 100644
--- a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
+++ b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
@@ -18,10 +18,7 @@ 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 {
@@ -43,7 +40,7 @@ public class PerThreadCacheTest {
@Test
public void endToEndCache() {
- try (PerThreadCache ignored = PerThreadCache.create(null)) {
+ try (PerThreadCache ignored = PerThreadCache.create()) {
PerThreadCache cache = PerThreadCache.get();
PerThreadCache.Key<String> key1 = PerThreadCache.Key.create(String.class);
@@ -61,7 +58,7 @@ public class PerThreadCacheTest {
@Test
public void cleanUp() {
PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class);
- try (PerThreadCache ignored = PerThreadCache.create(null)) {
+ try (PerThreadCache ignored = PerThreadCache.create()) {
PerThreadCache cache = PerThreadCache.get();
String value1 = cache.get(key, () -> "value1");
assertThat(value1).isEqualTo("value1");
@@ -69,7 +66,7 @@ public class PerThreadCacheTest {
// 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(null)) {
+ try (PerThreadCache ignored = PerThreadCache.create()) {
PerThreadCache cache = PerThreadCache.get();
String value1 = cache.get(key, () -> "value2");
assertThat(value1).isEqualTo("value2");
@@ -78,48 +75,16 @@ public class PerThreadCacheTest {
@Test
public void doubleInstantiationFails() {
- try (PerThreadCache ignored = PerThreadCache.create(null)) {
+ try (PerThreadCache ignored = PerThreadCache.create()) {
IllegalStateException thrown =
- assertThrows(IllegalStateException.class, () -> PerThreadCache.create(null));
+ assertThrows(IllegalStateException.class, () -> PerThreadCache.create());
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(null)) {
+ try (PerThreadCache cache = PerThreadCache.create()) {
// Fill the cache
for (int i = 0; i < 50; i++) {
PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class, i);