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 2017/03/21 13:23:12 UTC
svn commit: r1787966 - in /jackrabbit/oak/trunk/oak-segment-tar/src:
main/java/org/apache/jackrabbit/oak/segment/
main/java/org/apache/jackrabbit/oak/segment/file/
test/java/org/apache/jackrabbit/oak/segment/
Author: mduerig
Date: Tue Mar 21 13:23:11 2017
New Revision: 1787966
URL: http://svn.apache.org/viewvc?rev=1787966&view=rev
Log:
OAK-5954: Unify and simplify the deduplication caches
Replace the ad-hoc collection of deduplication cache statistics in the SegmentWriter with statistics exposed to Metrics.
Modified:
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentWriter.java
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/WriterCacheManager.java
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStoreBuilder.java
jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/NodeRecordTest.java
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentWriter.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentWriter.java?rev=1787966&r1=1787965&r2=1787966&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentWriter.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentWriter.java Tue Mar 21 13:23:11 2017
@@ -291,7 +291,7 @@ public class SegmentWriter {
@Nonnull
@Override
public RecordId execute(@Nonnull SegmentBufferWriter writer) throws IOException {
- return with(writer).writeNodeWithStats(state);
+ return with(writer).writeNode(state);
}
});
return new SegmentNodeState(reader, this, nodeId);
@@ -319,7 +319,7 @@ public class SegmentWriter {
@Nonnull
@Override
public RecordId execute(@Nonnull SegmentBufferWriter writer) throws IOException {
- return with(writer).writeNodeWithStats(state);
+ return with(writer).writeNode(state);
}
});
return new SegmentNodeState(reader, this, nodeId);
@@ -335,53 +335,6 @@ public class SegmentWriter {
* <em>not thread safe</em>.
*/
private abstract class SegmentWriteOperation implements WriteOperation {
- private class NodeWriteStats {
- /*
- * Total number of nodes in the subtree rooted at the node passed
- * to {@link #writeNode(SegmentWriteOperation, SegmentBufferWriter, NodeState)}
- */
- public int nodeCount;
-
- /*
- * Number of cache hits for a deferred compacted node
- */
- public int cacheHits;
-
- /*
- * Number of cache misses for a deferred compacted node
- */
- public int cacheMiss;
-
- /*
- * Number of nodes that where de-duplicated as the store already contained
- * them.
- */
- public int deDupNodes;
-
- /*
- * Number of nodes that actually had to be written as there was no de-duplication
- * and a cache miss (in case of a deferred compaction).
- */
- public int writesOps;
-
- /*
- * {@code true} for if the node written was a compaction operation, false otherwise
- */
- boolean isCompactOp;
-
- @Override
- public String toString() {
- return "NodeStats{" +
- "op=" + (isCompactOp ? "compact" : "write") +
- ", nodeCount=" + nodeCount +
- ", writeOps=" + writesOps +
- ", deDupNodes=" + deDupNodes +
- ", cacheHits=" + cacheHits +
- ", cacheMiss=" + cacheMiss +
- ", hitRate=" + (100*(double) cacheHits / ((double) cacheHits + (double) cacheMiss)) +
- '}';
- }
- }
/**
* This exception is used internally to signal cancellation of a (recursive)
@@ -396,12 +349,9 @@ public class SegmentWriter {
@Nonnull
private final Supplier<Boolean> cancel;
- @CheckForNull
- private NodeWriteStats nodeWriteStats;
-
private SegmentBufferWriter writer;
- private RecordCache<String> stringCache;
- private RecordCache<Template> templateCache;
+ private Cache<String, RecordId> stringCache;
+ private Cache<Template, RecordId> templateCache;
private Cache<String, RecordId> nodeCache;
protected SegmentWriteOperation(@Nonnull Supplier<Boolean> cancel) {
@@ -936,30 +886,18 @@ public class SegmentWriter {
return tid;
}
- private RecordId writeNodeWithStats(@Nonnull NodeState state) throws IOException {
- this.nodeWriteStats = new NodeWriteStats();
- try {
- return writeNode(state);
- } finally {
- LOG.debug("{}", nodeWriteStats);
- }
- }
-
private RecordId writeNode(@Nonnull NodeState state) throws IOException {
if (cancel.get()) {
// Poor man's Either Monad
throw new CancelledWriteException();
}
- checkState(nodeWriteStats != null);
- nodeWriteStats.nodeCount++;
- RecordId compactedId = deduplicateNode(state, nodeWriteStats);
+ RecordId compactedId = deduplicateNode(state);
if (compactedId != null) {
return compactedId;
}
- nodeWriteStats.writesOps++;
RecordId recordId = writeNodeUncached(state);
if (state instanceof SegmentNodeState) {
// This node state has been rewritten because it is from an older
@@ -967,7 +905,6 @@ public class SegmentWriter {
// deduplication of hard links to it (e.g. checkpoints).
SegmentNodeState sns = (SegmentNodeState) state;
nodeCache.put(sns.getStableId(), recordId, cost(sns));
- nodeWriteStats.isCompactOp = true;
compactionMonitor.compacted();
}
return recordId;
@@ -990,7 +927,7 @@ public class SegmentWriter {
if (after != null) {
// Pass null to indicate we don't want to update the node write statistics
// when deduplicating the base state
- beforeId = deduplicateNode(after.getBaseState(), null);
+ beforeId = deduplicateNode(after.getBaseState());
}
SegmentNodeState before = null;
@@ -1096,12 +1033,9 @@ public class SegmentWriter {
* and is still in the de-duplication cache for nodes.
*
* @param node The node states to de-duplicate.
- * @param nodeWriteStats write statistics to update if not {@code null}.
* @return the id of the de-duplicated node or {@code null} if none.
*/
- private RecordId deduplicateNode(
- @Nonnull NodeState node,
- @CheckForNull NodeWriteStats nodeWriteStats) {
+ private RecordId deduplicateNode(@Nonnull NodeState node) {
if (!(node instanceof SegmentNodeState)) {
// De-duplication only for persisted node states
return null;
@@ -1117,26 +1051,13 @@ public class SegmentWriter {
if (!isOldGeneration(sns.getRecordId())) {
// This segment node state is already in this store, no need to
// write it again
- if (nodeWriteStats != null) {
- nodeWriteStats.deDupNodes++;
- }
return sns.getRecordId();
}
// This is a segment node state from an old generation. Check
// whether an equivalent one of the current generation is in the
// cache
- RecordId compacted = nodeCache.get(sns.getStableId());
-
- if (nodeWriteStats != null) {
- if (compacted == null) {
- nodeWriteStats.cacheMiss++;
- } else {
- nodeWriteStats.cacheHits++;
- }
- }
-
- return compacted;
+ return nodeCache.get(sns.getStableId());
}
/**
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/WriterCacheManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/WriterCacheManager.java?rev=1787966&r1=1787965&r2=1787966&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/WriterCacheManager.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/WriterCacheManager.java Tue Mar 21 13:23:11 2017
@@ -39,6 +39,9 @@ import com.google.common.base.Supplier;
import com.google.common.cache.CacheStats;
import org.apache.jackrabbit.oak.api.jmx.CacheStatsMBean;
import org.apache.jackrabbit.oak.segment.file.PriorityCache;
+import org.apache.jackrabbit.oak.stats.CounterStats;
+import org.apache.jackrabbit.oak.stats.StatisticsProvider;
+import org.apache.jackrabbit.oak.stats.StatsOptions;
/**
* Instances of this class manage the deduplication caches used
@@ -91,14 +94,14 @@ public abstract class WriterCacheManager
* @return cache for string records of the given {@code generation}.
*/
@Nonnull
- public abstract RecordCache<String> getStringCache(int generation);
+ public abstract Cache<String, RecordId> getStringCache(int generation);
/**
* @param generation
* @return cache for template records of the given {@code generation}.
*/
@Nonnull
- public abstract RecordCache<Template> getTemplateCache(int generation);
+ public abstract Cache<Template, RecordId> getTemplateCache(int generation);
/**
* @param generation
@@ -221,6 +224,13 @@ public abstract class WriterCacheManager
private final Supplier<PriorityCache<String, RecordId>> nodeCache;
/**
+ * The {@code StatisticsProvider} instance used to expose statistics of
+ * the caches managed by this instance.
+ */
+ @Nonnull
+ private final StatisticsProvider statisticsProvider;
+
+ /**
* New instance using the passed factories for creating cache instances.
* The factories will be invoked exactly once when a generation of a
* cache is requested that has not been requested before.
@@ -228,14 +238,18 @@ public abstract class WriterCacheManager
* @param stringCacheFactory factory for the string cache
* @param templateCacheFactory factory for the template cache
* @param nodeCacheFactory factory for the node cache
+ * @param statisticsProvider The {@code StatisticsProvider} instance to expose
+ * statistics of the caches managed by this instance.
*/
public Default(
@Nonnull Supplier<RecordCache<String>> stringCacheFactory,
@Nonnull Supplier<RecordCache<Template>> templateCacheFactory,
- @Nonnull Supplier<PriorityCache<String, RecordId>> nodeCacheFactory) {
+ @Nonnull Supplier<PriorityCache<String, RecordId>> nodeCacheFactory,
+ @Nonnull StatisticsProvider statisticsProvider) {
this.stringCaches = new Generations<>(stringCacheFactory);
this.templateCaches = new Generations<>(templateCacheFactory);
this.nodeCache = memoize(nodeCacheFactory);
+ this.statisticsProvider = checkNotNull(statisticsProvider);
}
/**
@@ -247,7 +261,8 @@ public abstract class WriterCacheManager
public Default() {
this(RecordCache.<String>factory(DEFAULT_STRING_CACHE_SIZE),
RecordCache.<Template>factory(DEFAULT_TEMPLATE_CACHE_SIZE),
- PriorityCache.<String, RecordId>factory(DEFAULT_NODE_CACHE_SIZE));
+ PriorityCache.<String, RecordId>factory(DEFAULT_NODE_CACHE_SIZE),
+ StatisticsProvider.NOOP);
}
private static class Generations<T> implements Iterable<T> {
@@ -289,14 +304,16 @@ public abstract class WriterCacheManager
@Nonnull
@Override
- public RecordCache<String> getStringCache(int generation) {
- return stringCaches.getGeneration(generation);
+ public Cache<String, RecordId> getStringCache(int generation) {
+ return new AccessTrackingCache<>("oak.segment.string-deduplication-cache",
+ stringCaches.getGeneration(generation));
}
@Nonnull
@Override
- public RecordCache<Template> getTemplateCache(int generation) {
- return templateCaches.getGeneration(generation);
+ public Cache<Template, RecordId> getTemplateCache(int generation) {
+ return new AccessTrackingCache<>("oak.segment.template-deduplication-cache",
+ templateCaches.getGeneration(generation));
}
private PriorityCache<String, RecordId> nodeCache() {
@@ -306,7 +323,8 @@ public abstract class WriterCacheManager
@Override
@Nonnull
public Cache<String, RecordId> getNodeCache(final int generation) {
- return new Cache<String, RecordId>() {
+ return new AccessTrackingCache<>("oak.segment.node-deduplication-cache",
+ new Cache<String, RecordId>() {
@Override
public void put(@Nonnull String stableId, @Nonnull RecordId recordId, byte cost) {
nodeCache().put(stableId, recordId, generation, cost);
@@ -322,7 +340,7 @@ public abstract class WriterCacheManager
public RecordId get(@Nonnull String stableId) {
return nodeCache().get(stableId, generation);
}
- };
+ });
}
@CheckForNull
@@ -421,5 +439,45 @@ public abstract class WriterCacheManager
templateCaches.evictGenerations(generations);
nodeCache().purgeGenerations(generations);
}
+
+ /**
+ * {@code Cache} wrapper exposing the number of read accesses and the
+ * number of misses ot the underlying cache via the {@link #statisticsProvider}.
+ */
+ private class AccessTrackingCache<K, V> implements Cache<K,V> {
+ private final Cache<K, V> delegate;
+ private final CounterStats accessCount;
+ private final CounterStats missCount;
+
+ private AccessTrackingCache(@Nonnull String name, @Nonnull Cache<K, V> delegate) {
+ this.delegate = delegate;
+ this.accessCount = statisticsProvider.getCounterStats(
+ name + ".access-count", StatsOptions.DEFAULT);
+ this.missCount = statisticsProvider.getCounterStats(
+ name + ".miss-count", StatsOptions.DEFAULT);
+ }
+
+ @Override
+ public void put(@Nonnull K key, @Nonnull V value) {
+ delegate.put(key, value);
+ }
+
+ @Override
+ public void put(@Nonnull K key, @Nonnull V value, byte cost) {
+ delegate.put(key, value, cost);
+ }
+
+ @CheckForNull
+ @Override
+ public V get(@Nonnull K key) {
+ V v = delegate.get(key);
+ accessCount.inc();
+ if (v == null) {
+ missCount.inc();
+ }
+ return v;
+ }
+ }
+
}
}
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStoreBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStoreBuilder.java?rev=1787966&r1=1787965&r2=1787966&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStoreBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStoreBuilder.java Tue Mar 21 13:23:11 2017
@@ -430,8 +430,8 @@ public class FileStoreBuilder {
@Nonnull
public WriterCacheManager getCacheManager() {
if (cacheManager == null) {
- cacheManager = new EvictingWriteCacheManager(
- stringDeduplicationCacheSize, templateDeduplicationCacheSize, nodeDeduplicationCacheSize);
+ cacheManager = new EvictingWriteCacheManager(stringDeduplicationCacheSize,
+ templateDeduplicationCacheSize, nodeDeduplicationCacheSize, statsProvider);
}
return cacheManager;
}
@@ -458,10 +458,15 @@ public class FileStoreBuilder {
}
private static class EvictingWriteCacheManager extends WriterCacheManager.Default {
- public EvictingWriteCacheManager(int stringCacheSize, int templateCacheSize, int nodeCacheSize) {
+ public EvictingWriteCacheManager(
+ int stringCacheSize,
+ int templateCacheSize,
+ int nodeCacheSize,
+ @Nonnull StatisticsProvider statisticsProvider) {
super(RecordCache.factory(stringCacheSize, new StringCacheWeigher()),
RecordCache.factory(templateCacheSize, new TemplateCacheWeigher()),
- PriorityCache.factory(nodeCacheSize, new NodeCacheWeigher()));
+ PriorityCache.factory(nodeCacheSize, new NodeCacheWeigher()),
+ statisticsProvider);
}
void evictOldGeneration(final int newGeneration) {
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/NodeRecordTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/NodeRecordTest.java?rev=1787966&r1=1787965&r2=1787966&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/NodeRecordTest.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/NodeRecordTest.java Tue Mar 21 13:23:11 2017
@@ -169,13 +169,13 @@ public class NodeRecordTest {
@Nonnull
@Override
- public RecordCache<String> getStringCache(int generation) {
+ public Cache<String, RecordId> getStringCache(int generation) {
return Empty.INSTANCE.getStringCache(generation);
}
@Nonnull
@Override
- public RecordCache<Template> getTemplateCache(int generation) {
+ public Cache<Template, RecordId> getTemplateCache(int generation) {
return Empty.INSTANCE.getTemplateCache(generation);
}