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/07/08 10:33:28 UTC

svn commit: r1689810 - 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/DocumentSplitTest.java

Author: mreutegg
Date: Wed Jul  8 08:33:28 2015
New Revision: 1689810

URL: http://svn.apache.org/r1689810
Log:
OAK-3081: SplitOperations may undo committed changes

Added test to reproduce the problem (currently ignored)

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/DocumentSplitTest.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=1689810&r1=1689809&r2=1689810&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 Wed Jul  8 08:33:28 2015
@@ -1984,6 +1984,11 @@ public final class DocumentNodeStore
         }
     }
 
+    @Nonnull
+    Set<String> getSplitCandidates() {
+        return Collections.unmodifiableSet(splitCandidates.keySet());
+    }
+
     BackgroundWriteStats backgroundWrite() {
         return unsavedLastRevisions.persist(this, new UnsavedModifications.Snapshot() {
             @Override

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java?rev=1689810&r1=1689809&r2=1689810&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java Wed Jul  8 08:33:28 2015
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.plugin
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -25,12 +26,16 @@ import java.util.Random;
 import java.util.Set;
 import java.util.TreeSet;
 
+import javax.annotation.Nonnull;
+
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterators;
 
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.plugins.document.UpdateOp.Key;
+import org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation;
 import org.apache.jackrabbit.oak.spi.blob.MemoryBlobStore;
 import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
 import org.apache.jackrabbit.oak.plugins.document.util.Utils;
@@ -39,6 +44,7 @@ import org.apache.jackrabbit.oak.spi.com
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import com.google.common.collect.Iterables;
@@ -540,7 +546,7 @@ public class DocumentSplitTest extends B
         // second update op is for the main document
         op = splitOps.get(1);
         assertEquals(id, op.getId());
-        for (Map.Entry<UpdateOp.Key, UpdateOp.Operation> entry : op.getChanges().entrySet()) {
+        for (Map.Entry<Key, Operation> entry : op.getChanges().entrySet()) {
             Revision r = entry.getKey().getRevision();
             assertNotNull(r);
             assertEquals(clusterId, r.getClusterId());
@@ -732,6 +738,99 @@ public class DocumentSplitTest extends B
         assertTrue(doc.getLocalCommitRoot().size() < NUM_REVS_THRESHOLD);
     }
 
+    // OAK-3081
+    @Ignore
+    @Test
+    public void removeGarbage() throws Exception {
+        final DocumentStore store = mk.getDocumentStore();
+        final DocumentNodeStore ns = mk.getNodeStore();
+        final List<Exception> exceptions = Lists.newArrayList();
+        final List<Revision> revisions = Lists.newArrayList();
+
+        Thread t = new Thread(new Runnable() {
+            @Override
+            public void run() {
+                try {
+                    for (int i = 0; i < 1000; i++) {
+                        NodeBuilder builder = ns.getRoot().builder();
+                        builder.child("foo").child("node").child("node").child("node").child("node");
+                        builder.child("bar").child("node").child("node").child("node").child("node");
+                        merge(ns, builder);
+                        revisions.add(ns.getHeadRevision());
+
+                        builder = ns.getRoot().builder();
+                        builder.child("foo").child("node").remove();
+                        builder.child("bar").child("node").remove();
+                        merge(ns, builder);
+                        revisions.add(ns.getHeadRevision());
+                    }
+                } catch (CommitFailedException e) {
+                    exceptions.add(e);
+                }
+            }
+        });
+        t.start();
+
+        // Use a revision context, which wraps the DocumentNodeStore and
+        // randomly delays calls to get the head revision
+        RevisionContext rc = new TestRevisionContext(ns);
+        while (t.isAlive()) {
+            for (String id : ns.getSplitCandidates()) {
+                NodeDocument doc = store.find(NODES, id);
+                List<UpdateOp> ops = SplitOperations.forDocument(doc, rc);
+                for (UpdateOp op : ops) {
+                    for (Map.Entry<Key, Operation> e : op.getChanges().entrySet()) {
+                        assertFalse("SplitOperations must not remove committed changes",
+                                "_deleted".equals(e.getKey().getName()));
+                    }
+                }
+            }
+            // perform the actual cleanup
+            ns.runBackgroundOperations();
+        }
+
+    }
+
+    private static class TestRevisionContext implements RevisionContext {
+
+        private final RevisionContext rc;
+
+        TestRevisionContext(RevisionContext rc) {
+            this.rc = rc;
+        }
+
+        @Override
+        public UnmergedBranches getBranches() {
+            return rc.getBranches();
+        }
+
+        @Override
+        public UnsavedModifications getPendingModifications() {
+            return rc.getPendingModifications();
+        }
+
+        @Override
+        public Comparator<Revision> getRevisionComparator() {
+            return rc.getRevisionComparator();
+        }
+
+        @Override
+        public int getClusterId() {
+            return rc.getClusterId();
+        }
+
+        @Nonnull
+        @Override
+        public Revision getHeadRevision() {
+            try {
+                Thread.sleep((long) (Math.random() * 100));
+            } catch (InterruptedException e) {
+                // ignore
+            }
+            return rc.getHeadRevision();
+        }
+    }
+
     private static NodeState merge(NodeStore store, NodeBuilder root)
             throws CommitFailedException {
         return store.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY);