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/10 11:03:37 UTC

svn commit: r1694997 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/ main/java/org/apache/jackrabbit/oak/plugins/document/util/ test/java/org/apache/jackrabbit/oak/plugins/document/

Author: mreutegg
Date: Mon Aug 10 09:03:37 2015
New Revision: 1694997

URL: http://svn.apache.org/r1694997
Log:
OAK-1974: Fail fast on branch conflict

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.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/DocumentNodeStoreBranch.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java?rev=1694997&r1=1694996&r2=1694997&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java Mon Aug 10 09:03:37 2015
@@ -20,11 +20,14 @@ import static com.google.common.base.Pre
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.apache.jackrabbit.oak.api.CommitFailedException.MERGE;
 import static org.apache.jackrabbit.oak.api.CommitFailedException.OAK;
+import static org.apache.jackrabbit.oak.api.CommitFailedException.STATE;
 import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
 import static org.apache.jackrabbit.oak.commons.PathUtils.getName;
 import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.COLLISIONS;
 
 import java.util.Random;
+import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.locks.Lock;
@@ -34,8 +37,11 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 
 import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.plugins.document.util.Utils;
 import org.apache.jackrabbit.oak.spi.commit.CommitHook;
 import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
 import org.apache.jackrabbit.oak.spi.state.ConflictAnnotatingRebaseDiff;
@@ -652,6 +658,7 @@ class DocumentNodeStoreBranch implements
                 DocumentNodeState newRoot = withCurrentBranch(new Callable<DocumentNodeState>() {
                     @Override
                     public DocumentNodeState call() throws Exception {
+                        checkForConflicts();
                         NodeState toCommit = checkNotNull(hook).processCommit(base, head, info);
                         head = DocumentNodeStoreBranch.this.persist(toCommit, head, info);
                         return store.getRoot(store.merge(head.getRevision(), info));
@@ -691,6 +698,27 @@ class DocumentNodeStoreBranch implements
                 branchState = new ResetFailed(base, ex);
             }
         }
+
+        /**
+         * Checks if any of the commits on this branch have a collision marker
+         * set.
+         *
+         * @throws CommitFailedException if a collision marker is set for one
+         *          of the commits on this branch.
+         */
+        private void checkForConflicts() throws CommitFailedException {
+            Branch b = store.getBranches().getBranch(head.getRevision());
+            if (b == null) {
+                return;
+            }
+            NodeDocument doc = Utils.getRootDocument(store.getDocumentStore());
+            Set<Revision> collisions = doc.getLocalMap(COLLISIONS).keySet();
+            Set<Revision> conflicts = Sets.intersection(collisions, b.getCommits());
+            if (!conflicts.isEmpty()) {
+                throw new CommitFailedException(STATE, 2,
+                        "Conflicting concurrent change on branch commits " + conflicts);
+            }
+        }
     }
 
     /**

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java?rev=1694997&r1=1694996&r2=1694997&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java Mon Aug 10 09:03:37 2015
@@ -523,6 +523,27 @@ public class Utils {
     }
 
     /**
+     * Returns the root node document of the given document store. The returned
+     * document is retrieved from the document store via
+     * {@link DocumentStore#find(Collection, String)}, which means the
+     * implementation is allowed to return a cached version of the document.
+     * The document is therefore not guaranteed to be up-to-date.
+     *
+     * @param store a document store.
+     * @return the root document.
+     * @throws IllegalStateException if there is no root document.
+     */
+    @Nonnull
+    public static NodeDocument getRootDocument(@Nonnull DocumentStore store) {
+        String rootId = Utils.getIdFromPath("/");
+        NodeDocument root = store.find(Collection.NODES, rootId);
+        if (root == null) {
+            throw new IllegalStateException("missing root document");
+        }
+        return root;
+    }
+
+    /**
      * Returns an {@link Iterable} over all {@link NodeDocument}s in the given
      * store matching a condition on an <em>indexed property</em>. The returned
      * {@link Iterable} does not guarantee a consistent view on the store.

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=1694997&r1=1694996&r2=1694997&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 Mon Aug 10 09:03:37 2015
@@ -1915,6 +1915,54 @@ public class DocumentNodeStoreTest {
         }
     }
 
+    @Test
+    public void failFastOnBranchConflict() throws Exception {
+        final AtomicInteger mergeAttempts = new AtomicInteger();
+        MemoryDocumentStore store = new MemoryDocumentStore() {
+            @Override
+            public <T extends Document> T findAndUpdate(Collection<T> collection,
+                                                        UpdateOp update) {
+                for (Key k : update.getConditions().keySet()) {
+                    if (k.getName().equals(NodeDocument.COLLISIONS)) {
+                        mergeAttempts.incrementAndGet();
+                        break;
+                    }
+                }
+                return super.findAndUpdate(collection, update);
+            }
+        };
+        DocumentNodeStore ds = new DocumentMK.Builder()
+                .setDocumentStore(store)
+                .setAsyncDelay(0).getNodeStore();
+
+        DocumentNodeState root = ds.getRoot();
+        DocumentNodeStoreBranch b = ds.createBranch(root);
+        // branch state is now Unmodified
+        NodeBuilder builder = root.builder();
+        builder.child("foo");
+        b.setRoot(builder.getNodeState());
+        // branch state is now InMemory
+        builder.child("bar").setProperty("p", "foo");
+        b.setRoot(builder.getNodeState());
+        // branch state is now Persisted
+
+        // create conflict with persisted branch
+        NodeBuilder nb = ds.getRoot().builder();
+        nb.child("bar").setProperty("p", "bar");
+        merge(ds, nb);
+
+        mergeAttempts.set(0);
+        try {
+            b.merge(EmptyHook.INSTANCE, CommitInfo.EMPTY);
+            fail("must fail with CommitFailedException");
+        } catch (CommitFailedException e) {
+            // expected
+        }
+
+        assertTrue("too many merge attempts: " + mergeAttempts.get(),
+                mergeAttempts.get() <= 1);
+    }
+
     private static DocumentNodeState asDocumentNodeState(NodeState state) {
         if (!(state instanceof DocumentNodeState)) {
             throw new IllegalArgumentException("Not a DocumentNodeState");