jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mreut...@apache.org
Subject svn commit: r1502207 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/ ...
Date Thu, 11 Jul 2013 12:37:27 GMT
Author: mreutegg
Date: Thu Jul 11 12:37:26 2013
New Revision: 1502207

URL: http://svn.apache.org/r1502207
Log:
OAK-893: MongoMK may fail with MicroKernelException under concurrent commits

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Branch.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnmergedBranches.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnsavedModifications.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/Utils.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/IsolationTest.java
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ConcurrentFileOperationsTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Branch.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Branch.java?rev=1502207&r1=1502206&r2=1502207&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Branch.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Branch.java
Thu Jul 11 12:37:26 2013
@@ -130,16 +130,16 @@ class Branch {
 
     /**
      * Applies all unsaved modification of this branch to the given collection
-     * of unsaved trunk modifications. A modification is only applied if there
-     * is no modification in <code>trunk</code> for a given path or if the
-     * <code>trunk</code> modification is earlier.
+     * of unsaved trunk modifications with the given merge commit revision.
      *
      * @param trunk the unsaved trunk modifications.
+     * @param mergeCommit the revision of the merge commit.
      */
-    public synchronized void applyTo(@Nonnull UnsavedModifications trunk) {
+    public synchronized void applyTo(@Nonnull UnsavedModifications trunk,
+                                     @Nonnull Revision mergeCommit) {
         checkNotNull(trunk);
         for (Commit c : commits.values()) {
-            c.getModifications().applyTo(trunk);
+            c.getModifications().applyTo(trunk, mergeCommit);
         }
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java?rev=1502207&r1=1502206&r2=1502207&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Collision.java
Thu Jul 11 12:37:26 2013
@@ -39,7 +39,8 @@ import static com.google.common.base.Pre
  * or some other branch.</li>
  * </ul>
  * Other collisions like concurrent commits to trunk are handled earlier and
- * do not require collision marking. See {@link Commit#createOrUpdateNode()}.
+ * do not require collision marking.
+ * See {@link Commit#createOrUpdateNode(DocumentStore, UpdateOp)}.
  */
 class Collision {
 
@@ -105,7 +106,7 @@ class Collision {
         Map<String, String> revisions = (Map<String, String>) document.get(UpdateOp.REVISIONS);
         if (revisions != null && revisions.containsKey(revision)) {
             String value = revisions.get(revision);
-            if ("true".equals(value)) {
+            if (Utils.isCommitted(value)) {
                 // already committed
                 return false;
             }
@@ -165,10 +166,6 @@ class Collision {
     private static boolean isCommitted(String revision, Map<String, Object> document)
{
         @SuppressWarnings("unchecked")
         Map<String, String> revisions = (Map<String, String>) document.get(UpdateOp.REVISIONS);
-        if (revisions != null && revisions.containsKey(revision)) {
-            String value = revisions.get(revision);
-            return "true".equals(value);
-        }
-        return false;
+        return revisions != null && Utils.isCommitted(revisions.get(revision));
     }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java?rev=1502207&r1=1502206&r2=1502207&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Commit.java
Thu Jul 11 12:37:26 2013
@@ -161,10 +161,10 @@ public class Commit {
      */
     void applyToDocumentStore(Revision baseBranchRevision) {
         // the value in _revisions.<revision> property of the commit root node
-        // regular commits use "true", which makes the commit visible to
+        // regular commits use "c", which makes the commit visible to
         // other readers. branch commits use the base revision to indicate
         // the visibility of the commit
-        String commitValue = baseBranchRevision != null ? baseBranchRevision.toString() :
"true";
+        String commitValue = baseBranchRevision != null ? baseBranchRevision.toString() :
"c";
         DocumentStore store = mk.getDocumentStore();
         String commitRootPath = null;
         if (baseBranchRevision != null) {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java?rev=1502207&r1=1502206&r2=1502207&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoMK.java
Thu Jul 11 12:37:26 2013
@@ -564,9 +564,9 @@ public class MongoMK implements MicroKer
     }
 
     /**
-     * Returns <code>true</code> if the given revision is set to committed in
-     * the revisions map. That is, the revision exists in the map and the string
-     * value is <code>"true"</code> or equals the <code>readRevision</code>.
+     * Returns <code>true</code> if the given revision
+     * {@link Utils#isCommitted(String)} in the revisions map and is visible
+     * from the <code>readRevision</code>.
      *
      * @param revision  the revision to check.
      * @param readRevision the read revision.
@@ -587,9 +587,13 @@ public class MongoMK implements MicroKer
         if (value == null) {
             return false;
         }
-        if (value.equals("true")) {
+        if (Utils.isCommitted(value)) {
+            // resolve commit revision
+            revision = Utils.resolveCommitRevision(revision, value);
             if (branches.getBranch(readRevision) == null) {
-                return true;
+                // readRevision is not from a branch
+                // compare resolved revision as is
+                return !isRevisionNewer(revision, readRevision);
             }
         } else {
             // branch commit
@@ -1423,16 +1427,17 @@ public class MongoMK implements MicroKer
         // make branch commits visible
         UpdateOp op = new UpdateOp("/", Utils.getIdFromPath("/"), false);
         Revision revision = Revision.fromString(revisionId);
-        Commit.setModified(op, revision);
         Branch b = branches.getBranch(revision);
+        Revision mergeCommit = newRevision();
+        Commit.setModified(op, mergeCommit);
         if (b != null) {
             for (Revision rev : b.getCommits()) {
-                op.setMapEntry(UpdateOp.REVISIONS, rev.toString(), "true");
+                op.setMapEntry(UpdateOp.REVISIONS, rev.toString(), "c-" + mergeCommit.toString());
                 op.containsMapEntry(UpdateOp.COLLISIONS, rev.toString(), false);
             }
             if (store.findAndUpdate(DocumentStore.Collection.NODES, op) != null) {
                 // remove from branchCommits map after successful update
-                b.applyTo(unsavedLastRevisions);
+                b.applyTo(unsavedLastRevisions, mergeCommit);
                 branches.remove(b);
             } else {
                 throw new MicroKernelException("Conflicting concurrent change. Update operation
failed: " + op);
@@ -1440,7 +1445,7 @@ public class MongoMK implements MicroKer
         } else {
             // no commits in this branch -> do nothing
         }
-        headRevision = newRevision();
+        headRevision = mergeCommit;
         return headRevision.toString();
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnmergedBranches.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnmergedBranches.java?rev=1502207&r1=1502206&r2=1502207&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnmergedBranches.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnmergedBranches.java
Thu Jul 11 12:37:26 2013
@@ -76,7 +76,7 @@ class UnmergedBranches {
             SortedMap<Revision, Revision> tmp =
                     new TreeMap<Revision, Revision>(comparator);
             for (Map.Entry<String, String> commit : valueMap.entrySet()) {
-                if (!commit.getValue().equals("true")) {
+                if (!Utils.isCommitted(commit.getValue())) {
                     Revision r = Revision.fromString(commit.getKey());
                     if (r.getClusterId() == clusterId) {
                         Revision b = Revision.fromString(commit.getValue());

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnsavedModifications.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnsavedModifications.java?rev=1502207&r1=1502206&r2=1502207&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnsavedModifications.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnsavedModifications.java
Thu Jul 11 12:37:26 2013
@@ -56,16 +56,18 @@ class UnsavedModifications {
     /**
      * Applies all modifications from this instance to the <code>other</code>.
      * A modification is only applied if there is no modification in other
-     * for a given path or if the other modification is earlier.
+     * for a given path or if the other modification is earlier than the
+     * merge commit revision.
      *
      * @param other the other <code>UnsavedModifications</code>.
+     * @param mergeCommit the merge commit revision.
      */
-    public void applyTo(UnsavedModifications other) {
+    public void applyTo(UnsavedModifications other, Revision mergeCommit) {
         for (Map.Entry<String, Revision> entry : map.entrySet()) {
             Revision r = other.map.putIfAbsent(entry.getKey(), entry.getValue());
             if (r != null) {
-                if (r.compareRevisionTime(entry.getValue()) < 0) {
-                    other.map.put(entry.getKey(), entry.getValue());
+                if (r.compareRevisionTime(mergeCommit) < 0) {
+                    other.map.put(entry.getKey(), mergeCommit);
                 }
             }
         }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/Utils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/Utils.java?rev=1502207&r1=1502206&r2=1502207&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/Utils.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/util/Utils.java
Thu Jul 11 12:37:26 2013
@@ -22,11 +22,17 @@ import java.util.Map.Entry;
 import java.util.Set;
 import java.util.TreeMap;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
 import com.mongodb.BasicDBObject;
 
 import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.plugins.mongomk.Revision;
 import org.bson.types.ObjectId;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Utility methods.
  */
@@ -232,4 +238,31 @@ public class Utils {
         to = to.substring(0, to.length() - 2) + "0";
         return to;
     }
+
+    /**
+     * Returns <code>true</code> if a revision tagged with the given revision
+     * should be considered committed, <code>false</code> otherwise. Committed
+     * revisions have a tag, which equals 'c' or starts with 'c-'.
+     *
+     * @param tag the tag (may be <code>null</code>).
+     * @return <code>true</code> if committed; <code>false</code>
otherwise.
+     */
+    public static boolean isCommitted(@Nullable String tag) {
+        return tag != null && (tag.equals("c") || tag.startsWith("c-"));
+    }
+
+    /**
+     * Resolve the commit revision for the given revision <code>rev</code> and
+     * the associated commit tag.
+     *
+     * @param rev a revision.
+     * @param tag the associated commit tag.
+     * @return the actual commit revision for <code>rev</code>.
+     */
+    @Nonnull
+    public static Revision resolveCommitRevision(@Nonnull Revision rev,
+                                                 @Nonnull String tag) {
+        return checkNotNull(tag).startsWith("c-") ?
+                Revision.fromString(tag.substring(2)) : rev;
+    }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/IsolationTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/IsolationTest.java?rev=1502207&r1=1502206&r2=1502207&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/IsolationTest.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/IsolationTest.java
Thu Jul 11 12:37:26 2013
@@ -23,7 +23,6 @@ import org.junit.Test;
  * Tests if commits to branches and trunk are properly isolated and repository
  * state on a given revision is stable.
  */
-@Ignore("OAK-893")
 public class IsolationTest extends BaseMongoMKTest {
 
     @Test

Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ConcurrentFileOperationsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ConcurrentFileOperationsTest.java?rev=1502207&r1=1502206&r2=1502207&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ConcurrentFileOperationsTest.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ConcurrentFileOperationsTest.java
Thu Jul 11 12:37:26 2013
@@ -72,8 +72,6 @@ public class ConcurrentFileOperationsTes
      */
     @Test
     public void concurrent() throws Exception {
-        // OAK-893
-        assumeTrue(fixture != NodeStoreFixture.MONGO_MK);
         List<Session> sessions = new ArrayList<Session>();
         for (int i = 0; i < NUM_WRITERS; i++) {
             sessions.add(createAdminSession());



Mime
View raw message