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 2014/12/11 15:29:01 UTC

svn commit: r1644650 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java

Author: mreutegg
Date: Thu Dec 11 14:29:01 2014
New Revision: 1644650

URL: http://svn.apache.org/r1644650
Log:
OAK-2345: Diff reads too many nodes

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

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1644650&r1=1644649&r2=1644650&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Thu Dec 11 14:29:01 2014
@@ -1532,6 +1532,13 @@ public final class DocumentNodeStore
             Revision last = lastKnownRevision.get(machineId);
             if (last == null || r.compareRevisionTime(last) > 0) {
                 lastKnownRevision.put(machineId, r);
+                // OAK-2345
+                // only consider as external change if
+                // - the revision changed for the machineId
+                // or
+                // - the revision is within the time frame we remember revisions
+                if (last != null
+                        || r.getTimestamp() > revisionPurgeMillis())
                 externalChanges.put(r, otherSeen);
             }
         }
@@ -1562,7 +1569,17 @@ public final class DocumentNodeStore
                 backgroundOperationLock.writeLock().unlock();
             }
         }
-        revisionComparator.purge(Revision.getCurrentTimestamp() - REMEMBER_REVISION_ORDER_MILLIS);
+        revisionComparator.purge(revisionPurgeMillis());
+    }
+
+    /**
+     * Returns the time in milliseconds when revisions can be purged from the
+     * revision comparator.
+     *
+     * @return time in milliseconds.
+     */
+    private static long revisionPurgeMillis() {
+        return Revision.getCurrentTimestamp() - REMEMBER_REVISION_ORDER_MILLIS;
     }
 
     private void backgroundSplit() {

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1644650&r1=1644649&r2=1644650&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java Thu Dec 11 14:29:01 2014
@@ -60,12 +60,14 @@ import com.google.common.collect.Sets;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.jackrabbit.oak.api.CommitFailedException.CONSTRAINT;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
+import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore.REMEMBER_REVISION_ORDER_MILLIS;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.MODIFIED_IN_SECS_RESOLUTION;
 import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.NUM_REVS_THRESHOLD;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -373,7 +375,7 @@ public class DocumentNodeStoreTest {
         assertEquals(1, created.getClusterId());
 
         clock.waitUntil(System.currentTimeMillis() +
-                DocumentNodeStore.REMEMBER_REVISION_ORDER_MILLIS / 2);
+                REMEMBER_REVISION_ORDER_MILLIS / 2);
 
         NodeBuilder builder = nodeStore2.getRoot().builder();
         builder.setProperty("prop", "value");
@@ -381,7 +383,7 @@ public class DocumentNodeStoreTest {
         nodeStore2.runBackgroundOperations();
 
         clock.waitUntil(System.currentTimeMillis() +
-                DocumentNodeStore.REMEMBER_REVISION_ORDER_MILLIS + 1000);
+                REMEMBER_REVISION_ORDER_MILLIS + 1000);
         nodeStore3.runBackgroundOperations();
 
         doc = docStore.find(NODES, Utils.getIdFromPath("/"));
@@ -751,6 +753,41 @@ public class DocumentNodeStoreTest {
         ns.dispose();
     }
 
+    // OAK-2345
+    @Test
+    public void inactiveClusterId() throws Exception {
+        Clock clock = new Clock.Virtual();
+        clock.waitUntil(System.currentTimeMillis());
+        Revision.setClock(clock);
+        MemoryDocumentStore docStore = new MemoryDocumentStore();
+        DocumentNodeStore ns1 = new DocumentMK.Builder()
+                .setDocumentStore(docStore).setClusterId(1)
+                .setAsyncDelay(0).clock(clock).getNodeStore();
+        NodeBuilder builder = ns1.getRoot().builder();
+        builder.child("test");
+        merge(ns1, builder);
+        Revision r = ns1.getHeadRevision();
+        ns1.dispose();
+
+        // start other cluster node
+        DocumentNodeStore ns2 = new DocumentMK.Builder()
+                .setDocumentStore(docStore).setClusterId(2)
+                .setAsyncDelay(0).clock(clock).getNodeStore();
+        assertNotNull(ns2.getRevisionComparator().getRevisionSeen(r));
+        ns2.dispose();
+
+        // wait until revision is old
+        clock.waitUntil(System.currentTimeMillis()
+                + REMEMBER_REVISION_ORDER_MILLIS + 1000);
+
+        // start cluster 2 again
+        ns2 = new DocumentMK.Builder()
+                .setDocumentStore(docStore).setClusterId(2)
+                .setAsyncDelay(0).clock(clock).getNodeStore();
+        // now r is considered old and revisionSeen is null
+        assertNull(ns2.getRevisionComparator().getRevisionSeen(r));
+        ns2.dispose();
+    }
 
     private static void merge(NodeStore store, NodeBuilder root)
             throws CommitFailedException {