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 2013/04/16 10:20:12 UTC
svn commit: r1468326 - in /jackrabbit/oak/trunk/oak-mongomk/src:
main/java/org/apache/jackrabbit/mongomk/
test/java/org/apache/jackrabbit/mongomk/
Author: mreutegg
Date: Tue Apr 16 08:20:11 2013
New Revision: 1468326
URL: http://svn.apache.org/r1468326
Log:
OAK-619 Lock-free MongoMK implementation
- Improved conflict handling with tests (one still failing and ignored)
Added:
jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java (with props)
Modified:
jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java
jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java
jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java
Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java?rev=1468326&r1=1468325&r2=1468326&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java (original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/Commit.java Tue Apr 16 08:20:11 2013
@@ -272,7 +272,8 @@ public class Commit {
op.path + " was already added in revision " +
newestRev + "; before " + revision + "; document " + map);
}
- if (mk.isRevisionNewer(newestRev, baseRevision)) {
+ if (mk.isRevisionNewer(newestRev, baseRevision)
+ && isConflicting(map, op)) {
throw new MicroKernelException("The node " +
op.path + " was changed in revision " +
newestRev +
@@ -302,7 +303,49 @@ public class Commit {
}
}
}
-
+
+ /**
+ * Checks whether the given <code>UpdateOp</code> conflicts with the
+ * existing content in <code>nodeMap</code>. The check is done based on the
+ * {@link #baseRevision} of this commit. An <code>UpdateOp</code> conflicts
+ * when there were changes after {@link #baseRevision} on properties also
+ * contained in <code>UpdateOp</code>.
+ *
+ * @param nodeMap the contents of the nodes before the update.
+ * @param op the update to perform.
+ * @return <code>true</code> if the update conflicts; <code>false</code>
+ * otherwise.
+ */
+ private boolean isConflicting(Map<String, Object> nodeMap,
+ UpdateOp op) {
+ if (baseRevision == null) {
+ // no conflict is possible when there is no baseRevision
+ return false;
+ }
+ for (Map.Entry<String, UpdateOp.Operation> entry : op.changes.entrySet()) {
+ if (entry.getValue().type != UpdateOp.Operation.Type.SET_MAP_ENTRY) {
+ continue;
+ }
+ int idx = entry.getKey().indexOf('.');
+ String name = entry.getKey().substring(0, idx);
+ if (!Utils.isPropertyName(name)) {
+ continue;
+ }
+ // was this property touched after baseRevision?
+ @SuppressWarnings("unchecked")
+ Map<String, Object> changes = (Map<String, Object>) nodeMap.get(name);
+ if (changes == null) {
+ continue;
+ }
+ for (String rev : changes.keySet()) {
+ if (mk.isRevisionNewer(Revision.fromString(rev), baseRevision)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
private UpdateOp[] splitDocument(Map<String, Object> map) {
String id = (String) map.get(UpdateOp.ID);
String path = Utils.getPathFromId(id);
Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java?rev=1468326&r1=1468325&r2=1468326&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java (original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/MongoMK.java Tue Apr 16 08:20:11 2013
@@ -51,6 +51,7 @@ import org.apache.jackrabbit.oak.commons
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Maps;
import com.mongodb.DB;
/**
@@ -1056,15 +1057,19 @@ public class MongoMK implements MicroKer
if (nodeMap == null) {
return null;
}
+ Map<String, String> revisions = Maps.newHashMap();
+ if (nodeMap.containsKey(UpdateOp.REVISIONS)) {
+ //noinspection unchecked
+ revisions.putAll((Map<String, String>) nodeMap.get(UpdateOp.REVISIONS));
+ }
@SuppressWarnings("unchecked")
- Map<String, String> valueMap = (Map<String, String>) nodeMap
+ Map<String, String> deletedMap = (Map<String, String>) nodeMap
.get(UpdateOp.DELETED);
- if (valueMap == null) {
- return null;
+ if (deletedMap != null) {
+ revisions.putAll(deletedMap);
}
Revision newestRev = null;
- String newestValue = null;
- for (String r : valueMap.keySet()) {
+ for (String r : revisions.keySet()) {
Revision propRev = Revision.fromString(r);
if (newestRev == null || isRevisionNewer(propRev, newestRev)) {
if (isRevisionNewer(before, propRev)) {
@@ -1072,15 +1077,20 @@ public class MongoMK implements MicroKer
|| isValidRevision(propRev, before,
nodeMap, new HashSet<Revision>())) {
newestRev = propRev;
- newestValue = valueMap.get(r);
}
}
}
}
- if ("true".equals(newestValue)) {
- // deleted in the newest revision
+ if (newestRev == null) {
return null;
}
+ if (deletedMap != null) {
+ String value = deletedMap.get(newestRev.toString());
+ if ("true".equals(value)) {
+ // deleted in the newest revision
+ return null;
+ }
+ }
return newestRev;
}
Added: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java?rev=1468326&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java (added)
+++ jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java Tue Apr 16 08:20:11 2013
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.mongomk;
+
+import org.apache.jackrabbit.mk.api.MicroKernelException;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import static org.junit.Assert.fail;
+
+/**
+ * <code>ConflictTest</code> checks
+ * <a href="http://wiki.apache.org/jackrabbit/Conflict%20handling%20through%20rebasing%20branches">conflict handling</a>.
+ */
+public class ConflictTest extends BaseMongoMKTest {
+
+ @Test
+ public void addExistingProperty() {
+ String rev = mk.commit("/", "+\"foo\":{}", null, null);
+ mk.commit("/foo", "^\"prop\":\"value\"", rev, null);
+
+ try {
+ mk.commit("/foo", "^\"prop\":\"value\"", rev, null);
+ fail("Must fail with conflict for addExistingProperty");
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void removeRemovedProperty() {
+ String rev = mk.commit("/", "+\"foo\":{\"prop\":\"value\"}", null, null);
+ mk.commit("/foo", "^\"prop\":null", rev, null);
+
+ try {
+ mk.commit("/foo", "^\"prop\":null", rev, null);
+ fail("Must fail with conflict for removeRemovedProperty");
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void removeChangedProperty() {
+ String rev = mk.commit("/", "+\"foo\":{\"prop\":\"value\"}", null, null);
+ mk.commit("/foo", "^\"prop\":\"bar\"", rev, null);
+
+ try {
+ mk.commit("/foo", "^\"prop\":null", rev, null);
+ fail("Must fail with conflict for removeChangedProperty");
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void changeRemovedProperty() {
+ String rev = mk.commit("/", "+\"foo\":{\"prop\":\"value\"}", null, null);
+ mk.commit("/foo", "^\"prop\":null", rev, null);
+
+ try {
+ mk.commit("/foo", "^\"prop\":\"bar\"", rev, null);
+ fail("Must fail with conflict for changeRemovedProperty");
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void changeChangedProperty() {
+ String rev = mk.commit("/", "+\"foo\":{\"prop\":\"value\"}", null, null);
+ mk.commit("/foo", "^\"prop\":\"bar\"", rev, null);
+
+ try {
+ mk.commit("/foo", "^\"prop\":\"baz\"", rev, null);
+ fail("Must fail with conflict for changeChangedProperty");
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void addExistingNode() {
+ String rev = mk.commit("/", "+\"foo\":{}", null, null);
+ mk.commit("/foo", "+\"bar\":{}", rev, null);
+
+ try {
+ mk.commit("/foo", "+\"bar\":{}", rev, null);
+ fail("Must fail with conflict for addExistingNode");
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void removeRemovedNode() {
+ String rev = mk.commit("/", "+\"foo\":{}", null, null);
+ mk.commit("/", "-\"foo\"", rev, null);
+
+ try {
+ mk.commit("/", "-\"foo\"", rev, null);
+ fail("Must fail with conflict for removeRemovedNode");
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ }
+
+ @Test
+ @Ignore
+ public void removeChangedNode() {
+ String rev = mk.commit("/", "+\"foo\":{}", null, null);
+ mk.commit("/foo", "^\"prop\":\"value\"", rev, null);
+
+ try {
+ mk.commit("/", "-\"foo\"", rev, null);
+ fail("Must fail with conflict for removeChangedNode");
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ }
+
+ @Test
+ public void changeRemovedNode() {
+ String rev = mk.commit("/", "+\"foo\":{}", null, null);
+ mk.commit("/", "-\"foo\"", rev, null);
+
+ try {
+ mk.commit("/foo", "^\"prop\":\"value\"", rev, null);
+ fail("Must fail with conflict for changeRemovedNode");
+ } catch (MicroKernelException e) {
+ // expected
+ }
+ }
+}
Propchange: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/ConflictTest.java
------------------------------------------------------------------------------
svn:keywords = Author Date Id Revision Rev URL
Modified: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java?rev=1468326&r1=1468325&r2=1468326&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java (original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/MongoMKCommitAddTest.java Tue Apr 16 08:20:11 2013
@@ -20,6 +20,7 @@ import static org.junit.Assert.assertFal
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import org.apache.jackrabbit.mk.api.MicroKernelException;
import org.json.simple.JSONObject;
import org.junit.Ignore;
import org.junit.Test;
@@ -143,9 +144,14 @@ public class MongoMKCommitAddTest extend
public void setPropertyIllegalKey() throws Exception {
mk.commit("/", "+\"a\" : {}", null, null);
- mk.commit("/", "^\"a/ke.y1\" : \"value\"", null, null);
+ mk.commit("/", "^\"a/_id\" : \"value\"", null, null);
String nodes = mk.getNodes("/a", null, 0 /*depth*/, 0 /*offset*/, -1 /*maxChildNodes*/, null /*filter*/);
JSONObject obj = parseJSONObject(nodes);
+ assertPropertyValue(obj, "_id", "value");
+
+ mk.commit("/", "^\"a/ke.y1\" : \"value\"", null, null);
+ nodes = mk.getNodes("/a", null, 0 /*depth*/, 0 /*offset*/, -1 /*maxChildNodes*/, null /*filter*/);
+ obj = parseJSONObject(nodes);
assertPropertyValue(obj, "ke.y1", "value");
mk.commit("/", "^\"a/ke.y.1\" : \"value\"", null, null);
@@ -174,10 +180,18 @@ public class MongoMKCommitAddTest extend
String rev1 = mk.commit("/", "+\"a\" : {} ^\"a/key1\" : \"value1\"", null, null);
// Commit with rev1
- mk.commit("/", "^\"a/key1\" : \"value2\"", rev1, null);
+ String rev2 = mk.commit("/", "^\"a/key1\" : \"value2\"", rev1, null);
+
+ // try to commit with rev1 again (to overwrite rev2)
+ try {
+ mk.commit("/", "^\"a/key1\" : \"value3\"", rev1, null);
+ fail("commit must fail with conflicting change");
+ } catch (MicroKernelException e) {
+ // expected
+ }
- // Commit with rev1 again (to overwrite rev2)
- mk.commit("/", "^\"a/key1\" : \"value3\"", rev1, null);
+ // now overwrite with correct base revision
+ mk.commit("/", "^\"a/key1\" : \"value3\"", rev2, null);
String nodes = mk.getNodes("/a", null, 0 /*depth*/, 0 /*offset*/, -1 /*maxChildNodes*/, null /*filter*/);
JSONObject obj = parseJSONObject(nodes);