Author: mreutegg
Date: Thu Apr 24 10:16:48 2014
New Revision: 1589658

URL: http://svn.apache.org/r1589658
Log:
OAK-1751: DocumentNodeStore may detect conflict too late

Added:
    
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentPropertyUpdateTest.java
      - copied unchanged from r1589442, 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentPropertyUpdateTest.java
Modified:
    jackrabbit/oak/branches/1.0/   (props changed)
    
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java
    
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
    
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
    
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java

Propchange: jackrabbit/oak/branches/1.0/
------------------------------------------------------------------------------
  Merged /jackrabbit/oak/trunk:r1589442

Modified: 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java?rev=1589658&r1=1589657&r2=1589658&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java
 (original)
+++ 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java
 Thu Apr 24 10:16:48 2014
@@ -81,7 +81,7 @@ class Collision {
         document.deepCopy(newDoc);
         UpdateUtils.applyChanges(newDoc, ourOp, 
context.getRevisionComparator());
         if (!markCommitRoot(newDoc, ourRev, store)) {
-            throw new MicroKernelException("Unable to annotate our revision "
+            throw new IllegalStateException("Unable to annotate our revision "
                     + "with collision marker. Our revision: " + ourRev
                     + ", document:\n" + newDoc.format());
         }

Modified: 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java?rev=1589658&r1=1589657&r2=1589658&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
 (original)
+++ 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
 Thu Apr 24 10:16:48 2014
@@ -275,6 +275,8 @@ public class Commit {
             }
         }
         int commitRootDepth = PathUtils.getDepth(commitRootPath);
+        // check if there are real changes on the commit root
+        boolean commitRootHasChanges = operations.containsKey(commitRootPath);
         // create a "root of the commit" if there is none
         UpdateOp commitRoot = getUpdateOperationForNode(commitRootPath);
         for (String p : operations.keySet()) {
@@ -283,7 +285,10 @@ public class Commit {
                 NodeDocument.setDeleted(op, revision, false);
             }
             if (op == commitRoot) {
-                // apply at the end
+                if (!op.isNew() && commitRootHasChanges) {
+                    // commit root already exists and this is an update
+                    changedNodes.add(op);
+                }
             } else {
                 NodeDocument.setCommitRoot(op, revision, commitRootDepth);
                 if (op.isNew()) {
@@ -327,11 +332,10 @@ public class Commit {
                 }
             }
             for (UpdateOp op : changedNodes) {
-                // set commit root on changed nodes unless it's the
-                // commit root itself
-                if (op != commitRoot) {
-                    NodeDocument.setCommitRoot(op, revision, commitRootDepth);
-                }
+                // set commit root on changed nodes. this may even apply
+                // to the commit root. the _commitRoot entry is removed
+                // again when the _revisions entry is set at the end
+                NodeDocument.setCommitRoot(op, revision, commitRootDepth);
                 opLog.add(op);
                 createOrUpdateNode(store, op);
             }
@@ -340,7 +344,12 @@ public class Commit {
             // first to check if there was a conflict, and only then to commit
             // the revision, with the revision property set)
             if (changedNodes.size() > 0 || !commitRoot.isNew()) {
+                // set revision to committed
                 NodeDocument.setRevision(commitRoot, revision, commitValue);
+                if (commitRootHasChanges) {
+                    // remove previously added commit root
+                    NodeDocument.removeCommitRoot(commitRoot, revision);
+                }
                 opLog.add(commitRoot);
                 if (baseBranchRevision == null) {
                     // create a clone of the commitRoot in order
@@ -554,7 +563,7 @@ public class Commit {
             // or document did not exist before
             return false;
         }
-        return doc.isConflicting(op, baseRevision, nodeStore);
+        return doc.isConflicting(op, baseRevision, revision, nodeStore);
     }
 
     /**

Modified: 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1589658&r1=1589657&r2=1589658&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
 (original)
+++ 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
 Thu Apr 24 10:16:48 2014
@@ -814,17 +814,22 @@ public final class NodeDocument extends 
      *
      * @param op the update operation.
      * @param baseRevision the base revision for the update operation.
+     * @param commitRevision the commit revision of the update operation.
      * @param context the revision context.
      * @return <code>true</code> if conflicting, <code>false</code> otherwise.
      */
-    public boolean isConflicting(@Nonnull UpdateOp op,
+    boolean isConflicting(@Nonnull UpdateOp op,
                                  @Nonnull Revision baseRevision,
+                                 @Nonnull Revision commitRevision,
                                  @Nonnull RevisionContext context) {
         // did existence of node change after baseRevision?
         // only check local deleted map, which contains the most
         // recent values
         Map<Revision, String> deleted = getLocalDeleted();
         for (Map.Entry<Revision, String> entry : deleted.entrySet()) {
+            if (entry.getKey().equals(commitRevision)) {
+                continue;
+            }
             if (isRevisionNewer(context, entry.getKey(), baseRevision)) {
                 return true;
             }
@@ -845,6 +850,9 @@ public final class NodeDocument extends 
             }
             // was this property touched after baseRevision?
             for (Revision rev : getValueMap(name).keySet()) {
+                if (rev.equals(commitRevision)) {
+                    continue;
+                }
                 if (isRevisionNewer(context, rev, baseRevision)) {
                     return true;
                 }

Modified: 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java?rev=1589658&r1=1589657&r2=1589658&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java
 (original)
+++ 
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java
 Thu Apr 24 10:16:48 2014
@@ -32,6 +32,7 @@ import static org.apache.jackrabbit.oak.
 
 import javax.annotation.Nonnull;
 
+import org.apache.jackrabbit.mk.api.MicroKernelException;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.spi.commit.ChangeDispatcher;
@@ -500,9 +501,12 @@ public abstract class AbstractNodeStoreB
                     dispatcher.contentChanged(newHead, info);
                     branchState = new Merged(base);
                     return newHead;
-                } catch (Exception e) {
+                } catch (MicroKernelException e) {
                     throw new CommitFailedException(MERGE, 1,
                             "Failed to merge changes to the underlying store", 
e);
+                } catch (Exception e) {
+                    throw new CommitFailedException(OAK, 1,
+                            "Failed to merge changes to the underlying store", 
e);
                 }
             } finally {
                 dispatcher.contentChanged(getRoot(), null);


Reply via email to