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());
