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 2015/08/27 11:04:53 UTC

svn commit: r1698096 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java

Author: mreutegg
Date: Thu Aug 27 09:04:53 2015
New Revision: 1698096

URL: http://svn.apache.org/r1698096
Log:
OAK-3079: LastRevRecoveryAgent can update _lastRev of children but not the root

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

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java?rev=1698096&r1=1698095&r2=1698096&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java Thu Aug 27 09:04:53 2015
@@ -19,12 +19,10 @@
 
 package org.apache.jackrabbit.oak.plugins.document;
 
-import static com.google.common.collect.ImmutableList.of;
-import static com.google.common.collect.Iterables.filter;
-import static com.google.common.collect.Iterables.mergeSorted;
+import static com.google.common.collect.Maps.filterKeys;
 import static java.util.Collections.singletonList;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.JOURNAL;
-import static org.apache.jackrabbit.oak.plugins.document.UnsavedModifications.Snapshot.IGNORE;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.PROPERTY_OR_DELETED;
 
 import java.util.Iterator;
 import java.util.List;
@@ -35,6 +33,7 @@ import javax.annotation.CheckForNull;
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.plugins.document.mongo.MongoDocumentStore;
@@ -152,13 +151,17 @@ public class LastRevRecoveryAgent {
             }
 
             Revision currentLastRev = doc.getLastRev().get(clusterId);
-            if (currentLastRev != null) {
-                knownLastRevs.put(doc.getPath(), currentLastRev);
-            }
+
             // 1. determine last committed modification on document
             Revision lastModifiedRev = determineLastModification(doc, clusterId);
 
             Revision lastRevForParents = Utils.max(lastModifiedRev, currentLastRev);
+            // remember the higher of the two revisions. this is the
+            // most recent revision currently obtained from either a
+            // _lastRev entry or an explicit modification on the document
+            if (lastRevForParents != null) {
+                knownLastRevs.put(doc.getPath(), lastRevForParents);
+            }
 
             //If both currentLastRev and lostLastRev are null it means
             //that no change is done by suspect cluster on this document
@@ -306,21 +309,17 @@ public class LastRevRecoveryAgent {
     private Revision determineLastModification(NodeDocument doc, int clusterId) {
         ClusterPredicate cp = new ClusterPredicate(clusterId);
 
-        // Merge sort the revs for which changes have been made
-        // to this doc
-
-        // localMap always keeps the most recent valid commit entry
-        // per cluster node so looking into that should be sufficient
-        Iterable<Revision> revs = mergeSorted(of(
-                filter(doc.getLocalCommitRoot().keySet(), cp),
-                filter(doc.getLocalRevisions().keySet(), cp)),
-                StableRevisionComparator.REVERSE
-                );
-
         Revision lastModified = null;
-        // Look for latest valid revision
-        for (Revision rev : revs) {
-            lastModified = Utils.max(lastModified, doc.getCommitRevision(rev));
+        for (String property : Sets.filter(doc.keySet(), PROPERTY_OR_DELETED)) {
+            Map<Revision, String> valueMap = doc.getLocalMap(property);
+            // collect committed changes of this cluster node
+            for (Map.Entry<Revision, String> entry : filterKeys(valueMap, cp).entrySet()) {
+                Revision rev = entry.getKey();
+                if (doc.isCommitted(rev)) {
+                    lastModified = Utils.max(lastModified, doc.getCommitRevision(rev));
+                    break;
+                }
+            }
         }
         return lastModified;
     }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java?rev=1698096&r1=1698095&r2=1698096&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java Thu Aug 27 09:04:53 2015
@@ -19,19 +19,31 @@
 
 package org.apache.jackrabbit.oak.plugins.document;
 
+import java.util.List;
+import java.util.Map;
+
 import com.google.common.collect.Iterators;
+
+import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
 import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
 import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.apache.jackrabbit.oak.stats.Clock;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import static com.google.common.collect.Lists.newArrayList;
+import static org.apache.jackrabbit.oak.plugins.document.Collection.CLUSTER_NODES;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 public class LastRevRecoveryTest {
+    private Clock clock;
     private DocumentNodeStore ds1;
     private DocumentNodeStore ds2;
     private int c1Id;
@@ -39,21 +51,37 @@ public class LastRevRecoveryTest {
     private MemoryDocumentStore sharedStore;
 
     @Before
-    public void setUp(){
+    public void setUp() throws Exception {
+        clock = new Clock.Virtual();
+        clock.waitUntil(System.currentTimeMillis());
+        Revision.setClock(clock);
+        // disable lease check because we fiddle with the virtual clock
+        final boolean leaseCheck = false;
         sharedStore = new MemoryDocumentStore();
         ds1 = new DocumentMK.Builder()
+                .clock(clock)
+                .setLeaseCheck(leaseCheck)
                 .setAsyncDelay(0)
                 .setDocumentStore(sharedStore)
                 .getNodeStore();
         c1Id = ds1.getClusterId();
 
         ds2 = new DocumentMK.Builder()
+                .clock(clock)
+                .setLeaseCheck(leaseCheck)
                 .setAsyncDelay(0)
                 .setDocumentStore(sharedStore)
                 .getNodeStore();
         c2Id = ds2.getClusterId();
     }
 
+    @After
+    public void tearDown() {
+        ds1.dispose();
+        ds2.dispose();
+        Revision.resetClockToDefault();
+    }
+
 
     @Test
     public void testRecover() throws Exception {
@@ -104,7 +132,56 @@ public class LastRevRecoveryTest {
         assertEquals(head2, getDocument(ds1, "/").getLastRev().get(c2Id));
     }
 
+    // OAK-3079
+    @Test
+    public void recoveryWithoutRootUpdate() throws Exception {
+        String clusterId = String.valueOf(c1Id);
+        ClusterNodeInfoDocument doc = sharedStore.find(CLUSTER_NODES, clusterId);
+
+        NodeBuilder builder = ds1.getRoot().builder();
+        builder.child("x").child("y").child("z");
+        merge(ds1, builder);
+        ds1.dispose();
+
+        // reset clusterNodes entry to simulate a crash
+        sharedStore.remove(CLUSTER_NODES, clusterId);
+        sharedStore.create(CLUSTER_NODES, newArrayList(updateOpFromDocument(doc)));
+
+        // 'wait' until lease expires
+        clock.waitUntil(doc.getLeaseEndTime() + 1);
+
+        // run recovery on ds2
+        LastRevRecoveryAgent agent = new LastRevRecoveryAgent(ds2);
+        List<Integer> clusterIds = agent.getRecoveryCandidateNodes();
+        assertTrue(clusterIds.contains(c1Id));
+        assertEquals("must not recover any documents",
+                0, agent.recover(c1Id));
+    }
+
+
     private NodeDocument getDocument(DocumentNodeStore nodeStore, String path) {
         return nodeStore.getDocumentStore().find(Collection.NODES, Utils.getIdFromPath(path));
     }
+
+    private static void merge(NodeStore store, NodeBuilder builder)
+            throws CommitFailedException {
+        store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+    }
+
+    private static UpdateOp updateOpFromDocument(Document doc) {
+        UpdateOp op = new UpdateOp(doc.getId(), true);
+        for (String key : doc.keySet()) {
+            Object obj = doc.get(key);
+            if (obj instanceof Map) {
+                @SuppressWarnings("unchecked")
+                Map<Revision, String> map = (Map<Revision, String>) obj;
+                for (Map.Entry<Revision, String> entry : map.entrySet()) {
+                    op.setMapEntry(key, entry.getKey(), entry.getValue());
+                }
+            } else {
+                op.set(key, obj);
+            }
+        }
+        return op;
+    }
 }