diff options
author | Luca Milanesio <luca.milanesio@gmail.com> | 2022-04-22 08:36:43 +0100 |
---|---|---|
committer | Luca Milanesio <luca.milanesio@gmail.com> | 2022-04-23 00:25:24 +0100 |
commit | ac92778ab8d08136b6a0268419e0bd244fa98dca (patch) | |
tree | 34a1e5bfcbe98dfc2f142da13cd9979da3f62f26 | |
parent | 4b327bbe8b74e7ee7d66fbf88ed81bb60dd79016 (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
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); |