You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by md...@apache.org on 2016/04/20 13:45:45 UTC
svn commit: r1740121 -
/jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/segment/RecordCache.java
Author: mduerig
Date: Wed Apr 20 11:45:45 2016
New Revision: 1740121
URL: http://svn.apache.org/viewvc?rev=1740121&view=rev
Log:
OAK-3348: Cross gc sessions might introduce references to pre-compacted segments
Remove race in record cache by using memoised supplier for producing cache generations
Modified:
jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/segment/RecordCache.java
Modified: jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/segment/RecordCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/segment/RecordCache.java?rev=1740121&r1=1740120&r2=1740121&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/segment/RecordCache.java (original)
+++ jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/segment/RecordCache.java Wed Apr 20 11:45:45 2016
@@ -20,6 +20,7 @@
package org.apache.jackrabbit.oak.segment;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Suppliers.memoize;
import static com.google.common.collect.Lists.newArrayList;
import static com.google.common.collect.Maps.newConcurrentMap;
import static com.google.common.collect.Sets.newHashSet;
@@ -32,6 +33,8 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentMap;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -45,7 +48,7 @@ public class RecordCache<T> {
// FIXME OAK-3348 make this a feature flag
private static final int RETENTION_THRESHOLD = 1;
- private final ConcurrentMap<Integer, Cache<T>> generations = newConcurrentMap();
+ private final ConcurrentMap<Integer, Supplier<Cache<T>>> generations = newConcurrentMap();
public abstract static class Cache<T> {
public static <T> Cache<T> disabled() {
@@ -70,27 +73,25 @@ public class RecordCache<T> {
};
}
- /**
- * FIXME OAK-3348 The getCache might get called multiple times per generation
- * as per the comment below. Either come up with a fix for this race
- * or clearly state that API consumers need to be prepared for this.
- */
protected Cache<T> getCache(int generation) {
return Cache.disabled();
}
- public Cache<T> generation(int generation) {
- // Avoid calling getCache on every invocation.
+ public Cache<T> generation(final int generation) {
+ // Preemptive check to limit the number of wasted Supplier instances
if (!generations.containsKey(generation)) {
- // The small race here might still result in getCache being called
- // more than once. Implementors much take this into account.
- generations.putIfAbsent(generation, getCache(generation));
+ generations.putIfAbsent(generation, memoize(new Supplier<Cache<T>>() {
+ @Override
+ public Cache<T> get() {
+ return getCache(generation);
+ }
+ }));
}
- return generations.get(generation);
+ return generations.get(generation).get();
}
public void put(Cache<T> cache, int generation) {
- generations.put(generation, cache);
+ generations.put(generation, Suppliers.ofInstance(cache));
}
public void clearUpTo(int maxGen) {