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 mr...@apache.org on 2020/09/25 13:09:30 UTC

svn commit: r1882013 - in /jackrabbit/oak/branches/1.8: ./ oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/ oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/

Author: mreutegg
Date: Fri Sep 25 13:09:30 2020
New Revision: 1882013

URL: http://svn.apache.org/viewvc?rev=1882013&view=rev
Log:
OAK-9230: CachingCommitValueResolver with negative cache

Merge revision 1881984 from trunk

Modified:
    jackrabbit/oak/branches/1.8/   (props changed)
    jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/CachingCommitValueResolver.java
    jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
    jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
    jackrabbit/oak/branches/1.8/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitValueResolverTest.java

Propchange: jackrabbit/oak/branches/1.8/
------------------------------------------------------------------------------
  Merged /jackrabbit/oak/trunk:r1881984

Modified: jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/CachingCommitValueResolver.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/CachingCommitValueResolver.java?rev=1882013&r1=1882012&r2=1882013&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/CachingCommitValueResolver.java (original)
+++ jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/CachingCommitValueResolver.java Fri Sep 25 13:09:30 2020
@@ -22,6 +22,7 @@ import com.google.common.base.Supplier;
 import com.google.common.cache.Cache;
 
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
+import org.apache.jackrabbit.oak.stats.Clock;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -38,13 +39,54 @@ final class CachingCommitValueResolver i
 
     private final Cache<Revision, String> commitValueCache;
 
+    private final int cacheSize;
+
     private final Supplier<RevisionVector> sweepRevisions;
 
     CachingCommitValueResolver(int cacheSize, Supplier<RevisionVector> sweepRevisions) {
         this.commitValueCache = newBuilder().maximumSize(cacheSize).build();
+        this.cacheSize = cacheSize;
         this.sweepRevisions = sweepRevisions;
     }
 
+    CommitValueResolver withEmptyCommitValueCache(boolean enable,
+                                                  Clock clock,
+                                                  long maxAgeMillis) {
+        if (enable) {
+            return new CommitValueResolver() {
+
+                private final Cache<Revision, String> emptyCommitValueCache
+                        = newBuilder().maximumSize(cacheSize).build();
+
+                @Override
+                public String resolve(@NotNull Revision changeRevision,
+                                      @NotNull NodeDocument doc) {
+                    // check cache first
+                    String value = commitValueCache.getIfPresent(changeRevision);
+                    if (value != null) {
+                        return value;
+                    }
+                    // check negative cache
+                    if (emptyCommitValueCache.getIfPresent(changeRevision) != null) {
+                        return null;
+                    }
+                    value = CachingCommitValueResolver.this.resolve(changeRevision, doc);
+                    if (value == null && isOld(changeRevision, clock, maxAgeMillis)) {
+                        // remember
+                        emptyCommitValueCache.put(changeRevision, "");
+                    }
+                    return value;
+                }
+
+                private boolean isOld(Revision r, Clock c, long maxAgeMillis) {
+                    return r.getTimestamp() + maxAgeMillis < c.getTime();
+                }
+            };
+        } else {
+            return this;
+        }
+    }
+
     @Override
     public String resolve(@NotNull Revision changeRevision,
                           @NotNull NodeDocument doc) {

Modified: jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1882013&r1=1882012&r2=1882013&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Fri Sep 25 13:09:30 2020
@@ -527,7 +527,10 @@ public final class DocumentNodeStore
         this.nodeCachePredicate = builder.getNodeCachePredicate();
         this.updateLimit = builder.getUpdateLimit();
         this.commitValueResolver = new CachingCommitValueResolver(
-                builder.getCommitValueCacheSize(), this::getSweepRevisions);
+                builder.getCommitValueCacheSize(), this::getSweepRevisions)
+                .withEmptyCommitValueCache(
+                        builder.getCacheEmptyCommitValue() && builder.getReadOnlyMode(),
+                        builder.getClock(), builder.getJournalGCMaxAge());
         this.blobStore = builder.getBlobStore();
         this.nodeStoreStatsCollector = builder.getNodeStoreStatsCollector();
         if (builder.isUseSimpleRevision()) {

Modified: jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java?rev=1882013&r1=1882012&r2=1882013&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java (original)
+++ jackrabbit/oak/branches/1.8/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java Fri Sep 25 13:09:30 2020
@@ -158,6 +158,7 @@ public class DocumentNodeStoreBuilder<T
             new JournalPropertyHandlerFactory();
     private int updateLimit = UPDATE_LIMIT;
     private int commitValueCacheSize = 10000;
+    private boolean cacheEmptyCommitValue = false;
     private long maxRevisionAgeMillis = DEFAULT_JOURNAL_GC_MAX_AGE_MILLIS;
     private GCMonitor gcMonitor = new LoggingGCMonitor(
             LoggerFactory.getLogger(VersionGarbageCollector.class));
@@ -529,6 +530,28 @@ public class DocumentNodeStoreBuilder<T
         return commitValueCacheSize;
     }
 
+    /**
+     * Controls whether caching of empty commit values (negative cache) is
+     * enabled. This cache is disabled by default. The cache can only be enabled
+     * on a {@link #setReadOnlyMode() read-only} store. In read-write mode, the
+     * cache is always be disabled.
+     *
+     * @param enable {@code true} to enable the empty commit value cache.
+     * @return this builder.
+     */
+    public T setCacheEmptyCommitValue(boolean enable) {
+        this.cacheEmptyCommitValue = enable;
+        return thisBuilder();
+    }
+
+    /**
+     * @return {@code true} when caching of empty commit values is enabled,
+     *      {@code false} otherwise.
+     */
+    public boolean getCacheEmptyCommitValue() {
+        return cacheEmptyCommitValue;
+    }
+
     public T setJournalGCMaxAge(long maxRevisionAgeMillis) {
         this.maxRevisionAgeMillis = maxRevisionAgeMillis;
         return thisBuilder();

Modified: jackrabbit/oak/branches/1.8/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitValueResolverTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.8/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitValueResolverTest.java?rev=1882013&r1=1882012&r2=1882013&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.8/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitValueResolverTest.java (original)
+++ jackrabbit/oak/branches/1.8/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitValueResolverTest.java Fri Sep 25 13:09:30 2020
@@ -16,10 +16,11 @@
  */
 package org.apache.jackrabbit.oak.plugins.document;
 
-import com.google.common.base.Supplier;
+import java.util.concurrent.Callable;
 
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.junit.Before;
@@ -28,9 +29,12 @@ import org.junit.Test;
 
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
 import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getIdFromPath;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 public class CommitValueResolverTest {
@@ -38,19 +42,17 @@ public class CommitValueResolverTest {
     @Rule
     public DocumentMKBuilderProvider builderProvider = new DocumentMKBuilderProvider();
 
+    private CountingDocumentStore store = new CountingDocumentStore(new MemoryDocumentStore());
+
     private DocumentNodeStore ns;
 
     private CommitValueResolver resolver;
 
     @Before
     public void setup() {
-        ns = builderProvider.newBuilder().setUpdateLimit(20).setAsyncDelay(0).getNodeStore();
-        resolver = new CachingCommitValueResolver(0, new Supplier<RevisionVector>() {
-            @Override
-            public RevisionVector get() {
-                return ns.getSweepRevisions();
-            }
-        });
+        ns = builderProvider.newBuilder().setDocumentStore(store)
+                .setUpdateLimit(20).setAsyncDelay(0).getNodeStore();
+        resolver = newCachingCommitValueResolver(0);
     }
 
     @Test
@@ -203,6 +205,46 @@ public class CommitValueResolverTest {
         assertEquals(value, resolver.resolve(r, root));
     }
 
+    @Test
+    public void cacheEmptyCommitValue() throws Exception {
+        addNode("/foo");
+        // add changes and remove commit value
+        NodeBuilder builder = ns.getRoot().builder();
+        builder.child("foo").setProperty("p", "v");
+        builder.child("bar");
+        TestUtils.merge(ns, builder);
+        Revision commitRev = ns.getHeadRevision().getRevision(ns.getClusterId());
+        assertNotNull(commitRev);
+        UpdateOp op = new UpdateOp(Utils.getIdFromPath("/"), false);
+        NodeDocument.removeRevision(op, commitRev);
+        assertNotNull(store.findAndUpdate(NODES, op));
+
+        waitABit();
+
+        CommitValueResolver cvr = newCachingCommitValueResolver(100)
+                .withEmptyCommitValueCache(true, ns.getClock(), 0);
+        NodeDocument foo = getDocument("/foo");
+
+        // resolver without negative cache will look up previous docs every time
+        assertNull(resolver.resolve(commitRev, foo));
+        assertThat(countDocumentLookUps(() -> resolver.resolve(commitRev, foo)), greaterThan(0));
+
+        // resolver with negative cache will look up only the first time
+        assertNull(cvr.resolve(commitRev, foo));
+        assertThat(countDocumentLookUps(() -> cvr.resolve(commitRev, foo)), equalTo(0));
+    }
+
+    private int countDocumentLookUps(Callable<?> c) throws Exception {
+        int numCalls = store.getNumFindCalls(NODES);
+        c.call();
+        return store.getNumFindCalls(NODES) - numCalls;
+    }
+
+    private void waitABit() throws InterruptedException {
+        ns.getClock().getTimeIncreasing();
+        ns.getClock().getTimeIncreasing();
+    }
+
     private Revision addNode(String path) throws Exception {
         NodeBuilder builder = ns.getRoot().builder();
         NodeBuilder nb = builder;
@@ -260,4 +302,8 @@ public class CommitValueResolverTest {
         NodeDocument doc = getDocument(path);
         return doc != null ? doc.getLocalRevisions().size() : 0;
     }
+
+    private CachingCommitValueResolver newCachingCommitValueResolver(int cacheSize) {
+        return new CachingCommitValueResolver(cacheSize, () -> ns.getSweepRevisions());
+    }
 }