Revert "Reduce chance of deadlock in account cache"
This reverts commit 00fc15ac0073b86270e7c0f40d386f95dfe31e86.
With Caffeine, accessing the cache from the cache loader is causing an
infinite loop [1]. On stable-2.14, the only circular dependency was removed
but it looks like there is another one on stable-2.15. Revert the change
until circular dependency is removed or a solution is found to access
the cache from the cache loader.
The test going into an infinite loop is
RevisionDiffIT.rebaseHunksOneLineApartFromRegularHunkAreIdentified.
[1]https://github.com/ben-manes/caffeine/issues/89
Bug: Issue 8464
Change-Id: If65560b4a9bfcf0a03decaedd83ad000c6b28f4f
diff --git a/WORKSPACE b/WORKSPACE
index f975c8d..5f994da 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -213,18 +213,6 @@
)
maven_jar(
- name = "caffeine",
- artifact = "com.github.ben-manes.caffeine:caffeine:2.6.1",
- sha1 = "fc7a29feda0a3c5aaf1ff55e0df5417025e6d5f4",
-)
-
-maven_jar(
- name = "caffeine_guava",
- artifact = "com.github.ben-manes.caffeine:guava:2.6.1",
- sha1 = "e1fbe0d8c06639d6fee74404f687f00da25671eb",
-)
-
-maven_jar(
name = "velocity",
artifact = "org.apache.velocity:velocity:1.7",
sha1 = "2ceb567b8f3f21118ecdec129fe1271dbc09aa7a",
diff --git a/gerrit-cache-h2/BUILD b/gerrit-cache-h2/BUILD
index 55d47c2..45cf416 100644
--- a/gerrit-cache-h2/BUILD
+++ b/gerrit-cache-h2/BUILD
@@ -8,8 +8,6 @@
"//gerrit-common:server",
"//gerrit-extension-api:api",
"//gerrit-server:server",
- "//lib:caffeine",
- "//lib:caffeine_guava",
"//lib:guava",
"//lib:h2",
"//lib/guice",
@@ -24,8 +22,6 @@
deps = [
":cache-h2",
"//gerrit-server:server",
- "//lib:caffeine",
- "//lib:caffeine_guava",
"//lib:guava",
"//lib:h2",
"//lib:junit",
diff --git a/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/DefaultCacheFactory.java b/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/DefaultCacheFactory.java
index 7b85834..3566955 100644
--- a/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/DefaultCacheFactory.java
+++ b/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/DefaultCacheFactory.java
@@ -14,16 +14,12 @@
package com.google.gerrit.server.cache.h2;
-import com.github.benmanes.caffeine.cache.Caffeine;
-import com.github.benmanes.caffeine.cache.RemovalCause;
-import com.github.benmanes.caffeine.cache.RemovalListener;
-import com.github.benmanes.caffeine.cache.Weigher;
-import com.github.benmanes.caffeine.guava.CaffeinatedGuava;
import com.google.common.base.Strings;
import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
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.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.cache.CacheBinding;
import com.google.gerrit.server.cache.ForwardingRemovalListener;
@@ -61,48 +57,35 @@
@Override
public <K, V> Cache<K, V> build(CacheBinding<K, V> def) {
- return CaffeinatedGuava.build(create(def, false));
+ return create(def, false).build();
}
@Override
public <K, V> LoadingCache<K, V> build(CacheBinding<K, V> def, CacheLoader<K, V> loader) {
- return CaffeinatedGuava.build(create(def, false), loader);
+ return create(def, false).build(loader);
}
@SuppressWarnings("unchecked")
- <K, V> Caffeine<K, V> create(CacheBinding<K, V> def, boolean unwrapValueHolder) {
- Caffeine<K, V> builder = newCacheBuilder();
+ <K, V> CacheBuilder<K, V> create(CacheBinding<K, V> def, boolean unwrapValueHolder) {
+ CacheBuilder<K, V> builder = newCacheBuilder();
builder.recordStats();
builder.maximumWeight(cfg.getLong("cache", def.name(), "memoryLimit", def.maximumWeight()));
- builder =
- builder.removalListener(
- new CaffeineToGuavaRemovalListener<K, V>(
- forwardingRemovalListenerFactory.create(def.name())));
+ builder = builder.removalListener(forwardingRemovalListenerFactory.create(def.name()));
- Weigher<K, V> weigher = null;
- com.google.common.cache.Weigher<K, V> guavaWeigher = def.weigher();
- if (guavaWeigher != null) {
- if (unwrapValueHolder) {
- weigher =
- (Weigher<K, V>)
- new Weigher<K, ValueHolder<V>>() {
- @Override
- public int weigh(K key, ValueHolder<V> value) {
- return guavaWeigher.weigh(key, value.value);
- }
- };
- } else {
- weigher =
- new Weigher<K, V>() {
- @Override
- public int weigh(K key, V value) {
- return guavaWeigher.weigh(key, value);
- }
- };
- }
- } else {
- weigher = Weigher.singletonWeigher();
+ Weigher<K, V> weigher = def.weigher();
+ if (weigher != null && unwrapValueHolder) {
+ final Weigher<K, V> impl = weigher;
+ weigher =
+ (Weigher<K, V>)
+ new Weigher<K, ValueHolder<V>>() {
+ @Override
+ public int weigh(K key, ValueHolder<V> value) {
+ return impl.weigh(key, value.value);
+ }
+ };
+ } else if (weigher == null) {
+ weigher = unitWeight();
}
builder.weigher(weigher);
@@ -124,24 +107,16 @@
}
@SuppressWarnings("unchecked")
- private static <K, V> Caffeine<K, V> newCacheBuilder() {
- return (Caffeine<K, V>) Caffeine.newBuilder();
+ private static <K, V> CacheBuilder<K, V> newCacheBuilder() {
+ return (CacheBuilder<K, V>) CacheBuilder.newBuilder();
}
- private static class CaffeineToGuavaRemovalListener<K, V> implements RemovalListener<K, V> {
-
- private com.google.common.cache.RemovalListener<K, V> guavalistener;
-
- private CaffeineToGuavaRemovalListener(
- com.google.common.cache.RemovalListener<K, V> guavalistener) {
- this.guavalistener = guavalistener;
- }
-
- @Override
- public void onRemoval(K key, V value, RemovalCause cause) {
- guavalistener.onRemoval(
- RemovalNotification.create(
- key, value, com.google.common.cache.RemovalCause.valueOf(cause.name())));
- }
+ private static <K, V> Weigher<K, V> unitWeight() {
+ return new Weigher<K, V>() {
+ @Override
+ public int weigh(K key, V value) {
+ return 1;
+ }
+ };
}
}
diff --git a/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java b/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
index 7e49bd0..1283452 100644
--- a/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
+++ b/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.cache.h2;
-import com.github.benmanes.caffeine.guava.CaffeinatedGuava;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -169,7 +168,7 @@
executor,
store,
def.keyType(),
- (Cache<K, ValueHolder<V>>) CaffeinatedGuava.build(defaultFactory.create(def, true)));
+ (Cache<K, ValueHolder<V>>) defaultFactory.create(def, true).build());
synchronized (caches) {
caches.add(cache);
}
@@ -189,9 +188,9 @@
newSqlStore(def.name(), def.keyType(), limit, def.expireAfterWrite(TimeUnit.SECONDS));
Cache<K, ValueHolder<V>> mem =
(Cache<K, ValueHolder<V>>)
- CaffeinatedGuava.build(
- defaultFactory.create(def, true),
- (CacheLoader<K, V>) new H2CacheImpl.Loader<>(executor, store, loader));
+ defaultFactory
+ .create(def, true)
+ .build((CacheLoader<K, V>) new H2CacheImpl.Loader<>(executor, store, loader));
H2CacheImpl<K, V> cache = new H2CacheImpl<>(executor, store, def.keyType(), mem);
caches.add(cache);
return cache;
diff --git a/gerrit-plugin-api/BUILD b/gerrit-plugin-api/BUILD
index c788888..fe9ce19 100644
--- a/gerrit-plugin-api/BUILD
+++ b/gerrit-plugin-api/BUILD
@@ -16,8 +16,6 @@
"//gerrit-server:metrics",
"//gerrit-reviewdb:server",
"//gerrit-server:prolog-common",
- "//lib:caffeine",
- "//lib:caffeine_guava",
"//lib/commons:dbcp",
"//lib/commons:lang",
"//lib/commons:lang3",
diff --git a/lib/BUILD b/lib/BUILD
index 8e752c0..91334cb 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -83,20 +83,6 @@
)
java_library(
- name = "caffeine",
- data = ["//lib:LICENSE-Apache2.0"],
- visibility = ["//visibility:public"],
- exports = ["@caffeine//jar"],
-)
-
-java_library(
- name = "caffeine_guava",
- data = ["//lib:LICENSE-Apache2.0"],
- visibility = ["//visibility:public"],
- exports = ["@caffeine_guava//jar"],
-)
-
-java_library(
name = "velocity",
data = ["//lib:LICENSE-Apache2.0"],
visibility = ["//visibility:public"],
