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 2016/12/05 10:49:56 UTC

svn commit: r1772629 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java

Author: mreutegg
Date: Mon Dec  5 10:49:56 2016
New Revision: 1772629

URL: http://svn.apache.org/viewvc?rev=1772629&view=rev
Log:
OAK-5207: Reduce reads in NodeDocument.getVisibleChanges()

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1772629&r1=1772628&r2=1772629&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Mon Dec  5 10:49:56 2016
@@ -1549,38 +1549,45 @@ public final class NodeDocument extends
             changes.add(filter(localChanges.entrySet(), p));
         }
 
-        boolean overlapping = false;
-        List<Range> ranges = Lists.newArrayList();
-        long lowStamp = Long.MAX_VALUE;
-        for (Map.Entry<Revision, Range> e : getPreviousRanges().entrySet()) {
-            Range range = e.getValue();
-            if (!readRevision.isRevisionNewer(range.low)) {
-                ranges.add(range);
-                // check if overlapping
-                if (!overlapping) {
-                    overlapping = range.high.getTimestamp() >= lowStamp;
+        for (Revision r : readRevision) {
+            // collect changes per clusterId
+            boolean overlapping = false;
+            List<Range> ranges = Lists.newArrayList();
+            Revision lowRev = new Revision(Long.MAX_VALUE, 0, r.getClusterId());
+            for (Map.Entry<Revision, Range> e : getPreviousRanges().entrySet()) {
+                Range range = e.getValue();
+                if (range.low.getClusterId() != r.getClusterId()) {
+                    continue;
+                }
+                if (r.compareRevisionTime(range.low) >= 0) {
+                    ranges.add(range);
+                    // check if overlapping
+                    if (!overlapping) {
+                        overlapping = range.high.compareRevisionTime(lowRev) >= 0;
+                    }
+                    lowRev = Utils.min(lowRev, range.low);
                 }
-                lowStamp = Math.min(lowStamp, range.low.getTimestamp());
             }
-        }
 
-        if (!ranges.isEmpty()) {
-            Iterable<Iterable<Map.Entry<Revision, String>>> revs = transform(filter(transform(ranges,
-                    new Function<Range, NodeDocument>() {
-                @Override
-                public NodeDocument apply(Range input) {
-                    return getPreviousDoc(input.high, input);
-                }
-            }), Predicates.notNull()), new Function<NodeDocument, Iterable<Map.Entry<Revision, String>>>() {
-                @Override
-                public Iterable<Map.Entry<Revision, String>> apply(NodeDocument prev) {
-                    return prev.getVisibleChanges(property, readRevision);
+            if (!ranges.isEmpty()) {
+                final RevisionVector readRev = new RevisionVector(r);
+                Iterable<Iterable<Map.Entry<Revision, String>>> revs = transform(filter(transform(ranges,
+                        new Function<Range, NodeDocument>() {
+                            @Override
+                            public NodeDocument apply(Range input) {
+                                return getPreviousDoc(input.high, input);
+                            }
+                        }), Predicates.notNull()), new Function<NodeDocument, Iterable<Map.Entry<Revision, String>>>() {
+                    @Override
+                    public Iterable<Map.Entry<Revision, String>> apply(NodeDocument prev) {
+                        return prev.getVisibleChanges(property, readRev);
+                    }
+                });
+                if (overlapping) {
+                    changes.add(mergeSorted(revs, ValueComparator.REVERSE));
+                } else {
+                    changes.add(concat(revs));
                 }
-            });
-            if (overlapping) {
-                changes.add(mergeSorted(revs, ValueComparator.REVERSE));
-            } else {
-                changes.add(concat(revs));
             }
         }
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java?rev=1772629&r1=1772628&r2=1772629&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java Mon Dec  5 10:49:56 2016
@@ -651,6 +651,42 @@ public class NodeDocumentTest {
         ns.dispose();
     }
 
+    // OAK-5207
+    @Test
+    public void tooManyReadsOnGetVisibleChanges() throws Exception {
+        final int numChanges = 500;
+        final Set<String> prevDocCalls = newHashSet();
+        MemoryDocumentStore store = new MemoryDocumentStore() {
+            @Override
+            public <T extends Document> T find(Collection<T> collection,
+                                               String key) {
+                if (Utils.getPathFromId(key).startsWith("p")) {
+                    prevDocCalls.add(key);
+                }
+                return super.find(collection, key);
+            }
+        };
+        Random random = new Random(42);
+        DocumentNodeStore ns1 = createTestStore(store, 1, 0);
+        DocumentNodeStore ns2 = createTestStore(store, 2, 0);
+        List<DocumentNodeStore> nodeStores = Lists.newArrayList(ns1, ns2);
+        List<RevisionVector> headRevisions = Lists.reverse(
+                createTestData(nodeStores, random, numChanges));
+
+        NodeDocument doc = getRootDocument(store);
+
+        for (int i = 0; i < 20; i++) {
+            prevDocCalls.clear();
+            String value = doc.getVisibleChanges("p", headRevisions.get(i)).iterator().next().getValue();
+            assertEquals(String.valueOf(numChanges - (i + 1)), value);
+            assertTrue("too many calls for previous documents: " + prevDocCalls,
+                    prevDocCalls.size() <= 3);
+        }
+
+        ns1.dispose();
+        ns2.dispose();
+    }
+
     @Test
     public void getVisibleChanges() throws Exception {
         final int numChanges = 200;