jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From meteata...@apache.org
Subject svn commit: r1416202 - in /jackrabbit/oak/trunk/oak-mongomk/src: main/java/org/apache/jackrabbit/mongomk/impl/action/ main/java/org/apache/jackrabbit/mongomk/impl/command/ main/java/org/apache/jackrabbit/mongomk/impl/instruction/ test/java/org/apache/j...
Date Sun, 02 Dec 2012 17:03:17 GMT
Author: meteatamel
Date: Sun Dec  2 17:03:15 2012
New Revision: 1416202

URL: http://svn.apache.org/viewvc?rev=1416202&view=rev
Log:
OAK-440 - Concurrency issue when 3 microkernels are writing in the same db

Removed fetchDescendants from FetchNodesAction, it's no longer used.

Modified:
    jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesAction.java
    jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/GetNodesCommand.java
    jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java
    jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesActionTest.java

Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesAction.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesAction.java?rev=1416202&r1=1416201&r2=1416202&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesAction.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesAction.java
Sun Dec  2 17:03:15 2012
@@ -50,7 +50,6 @@ public class FetchNodesAction extends Ba
     private String branchId;
     private List<MongoCommit> validCommits;
     private int depth = LIMITLESS_DEPTH;
-    private boolean fetchDescendants;
 
     /**
      * Constructs a new {@code FetchNodesAction} to fetch a node and optionally
@@ -58,16 +57,12 @@ public class FetchNodesAction extends Ba
      *
      * @param nodeStore Node store.
      * @param path The path.
-     * @param fetchDescendants Determines whether the descendants of the path
-     * will be fetched as well.
      * @param revisionId The revision id.
      */
-    public FetchNodesAction(MongoNodeStore nodeStore, String path,
-            boolean fetchDescendants, long revisionId) {
+    public FetchNodesAction(MongoNodeStore nodeStore, String path, long revisionId) {
         super(nodeStore);
         paths = new HashSet<String>();
         paths.add(path);
-        this.fetchDescendants = fetchDescendants;
         this.revisionId = revisionId;
     }
 
@@ -79,8 +74,7 @@ public class FetchNodesAction extends Ba
      * @param paths The exact paths to fetch nodes for.
      * @param revisionId The revision id.
      */
-    public FetchNodesAction(MongoNodeStore nodeStore,  Set<String> paths,
-            long revisionId) {
+    public FetchNodesAction(MongoNodeStore nodeStore,  Set<String> paths, long revisionId)
{
         super(nodeStore);
         this.paths = paths;
         this.revisionId = revisionId;
@@ -129,12 +123,8 @@ public class FetchNodesAction extends Ba
             queryBuilder = queryBuilder.in(paths);
         } else {
             String path = paths.toArray(new String[0])[0];
-            if (fetchDescendants) {
-                Pattern pattern = createPrefixRegExp(path);
-                queryBuilder = queryBuilder.regex(pattern);
-            } else {
-                queryBuilder = queryBuilder.is(path);
-            }
+            Pattern pattern = createPrefixRegExp(path);
+            queryBuilder = queryBuilder.regex(pattern);
         }
 
         if (revisionId > 0) {
@@ -156,10 +146,11 @@ public class FetchNodesAction extends Ba
             queryBuilder = queryBuilder.and(branchQuery);
         }
 
+        DBObject orderBy = new BasicDBObject(MongoCommit.KEY_REVISION_ID, -1);
         DBObject query = queryBuilder.get();
         LOG.debug("Executing query: {}", query);
 
-        return nodeStore.getNodeCollection().find(query);
+        return nodeStore.getNodeCollection().find(query).sort(orderBy);
     }
 
     private Pattern createPrefixRegExp(String path) {
@@ -209,6 +200,19 @@ public class FetchNodesAction extends Ba
                 continue;
             }
 
+            // This assumes that revision ids are ordered and nodes were fetched
+            // in sorted order.
+            if (nodeMongos.containsKey(path)) {
+                LOG.debug("Converted nodes @{} with path {} was not put into map"
+                        + " because a newer version is available", revisionId, path);
+                continue;
+            }
+            nodeMongos.put(path, nodeMongo);
+            LOG.debug("Converted node @{} with path {} was put into map", revisionId, path);
+
+
+            // This is for unordered revision ids.
+            /*
             MongoNode existingNodeMongo = nodeMongos.get(path);
             if (existingNodeMongo != null) {
                 long existingRevId = existingNodeMongo.getRevisionId();
@@ -222,10 +226,11 @@ public class FetchNodesAction extends Ba
                 }
             } else {
                 nodeMongos.put(path, nodeMongo);
-                LOG.debug("Converted node was put into map");
+                LOG.debug("Converted node @{} with path {} was put into map", revisionId,
path);
             }
+            */
         }
-
+        dbCursor.close();
         return nodeMongos;
     }
 

Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/GetNodesCommand.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/GetNodesCommand.java?rev=1416202&r1=1416201&r2=1416202&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/GetNodesCommand.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/GetNodesCommand.java
Sun Dec  2 17:03:15 2012
@@ -168,8 +168,7 @@ public class GetNodesCommand extends Bas
     }
 
     private void readNodesByPath() {
-        FetchNodesAction query = new FetchNodesAction(nodeStore,
-                path, true, revisionId);
+        FetchNodesAction query = new FetchNodesAction(nodeStore, path, revisionId);
         query.setBranchId(branchId);
         query.setValidCommits(lastCommits);
         query.setDepth(depth);
@@ -177,15 +176,11 @@ public class GetNodesCommand extends Bas
     }
 
     private boolean verifyNodeHierarchy() {
-        boolean verified = false;
-
-        verified = verifyNodeHierarchyRec(path, 0);
-
+        boolean verified = verifyNodeHierarchyRec(path, 0);
         if (!verified) {
-            LOG.error(String.format("Node hierarchy could not be verified because"
-                    + " some nodes were inconsistent: %s", path));
+            LOG.error("Node hierarchy could not be verified because some nodes"
+                    + " were inconsistent: {}", path);
         }
-
         return verified;
     }
 
@@ -217,26 +212,19 @@ public class GetNodesCommand extends Bas
     }
 
     private boolean verifyProblematicNodes() {
-        boolean verified = true;
-
         for (Map.Entry<String, Long> entry : problematicNodes.entrySet()) {
             String path = entry.getKey();
             Long revisionId = entry.getValue();
-
             MongoNode nodeMongo = pathAndNodeMap.get(path);
             if (nodeMongo != null) {
                 if (!revisionId.equals(nodeMongo.getRevisionId())) {
-                    verified = false;
-
-                    LOG.error(String
-                            .format("Node could not be verified because the expected revisionId
did not match: %d (expected) vs %d (actual)",
-                                    revisionId, nodeMongo.getRevisionId()));
-
-                    break;
+                    LOG.error("Node could not be verified because revisionIds"
+                            + " did not match: {} (expected) vs {} (actual)",
+                            revisionId, nodeMongo.getRevisionId());
+                    return false;
                 }
             }
         }
-
-        return verified;
+        return true;
     }
 }

Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java?rev=1416202&r1=1416201&r2=1416202&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java
Sun Dec  2 17:03:15 2012
@@ -156,7 +156,7 @@ public class CommitCommandInstructionVis
 
         // First, copy the existing nodes.
         Map<String, MongoNode> nodesToCopy = new FetchNodesAction(nodeStore,
-                srcPath, true, headRevisionId).execute();
+                srcPath, headRevisionId).execute();
         for (MongoNode nodeMongo : nodesToCopy.values()) {
             String oldPath = nodeMongo.getPath();
             String oldPathRel = PathUtils.relativize(srcPath, oldPath);
@@ -204,7 +204,7 @@ public class CommitCommandInstructionVis
 
         // First, copy the existing nodes.
         Map<String, MongoNode> nodesToCopy = new FetchNodesAction(nodeStore,
-                srcPath, true, headRevisionId).execute();
+                srcPath, headRevisionId).execute();
         for (MongoNode nodeMongo : nodesToCopy.values()) {
             String oldPath = nodeMongo.getPath();
             String oldPathRel = PathUtils.relativize(srcPath, oldPath);

Modified: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesActionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesActionTest.java?rev=1416202&r1=1416201&r2=1416202&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesActionTest.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/action/FetchNodesActionTest.java
Sun Dec  2 17:03:15 2012
@@ -51,8 +51,7 @@ public class FetchNodesActionTest extend
         invalidateCommit(revisionId1);
         updateBaseRevisionId(revisionId2, 0L);
 
-        FetchNodesAction query = new FetchNodesAction(getNodeStore(),
-                "/", true, revisionId3);
+        FetchNodesAction query = new FetchNodesAction(getNodeStore(), "/", revisionId3);
         List<Node> actuals = toNode(query.execute());
 
         String json = String.format("{\"/#%2$s\" : { \"b#%1$s\" : {}, \"c#%2$s\" : {} }}",
@@ -68,10 +67,7 @@ public class FetchNodesActionTest extend
         Long revisionId3 = addNode("c");
 
         invalidateCommit(revisionId3);
-
-        FetchNodesAction query = new FetchNodesAction(getNodeStore(),
-                "/", true, revisionId3);
-        List<Node> actuals = toNode(query.execute());
+        List<Node> actuals = createAndExecuteQuery(revisionId3);
 
         String json = String.format("{\"/#%2$s\" : { \"a#%1$s\" : {}, \"b#%2$s\" : {} }}",
                 revisionId1, revisionId2);
@@ -87,10 +83,7 @@ public class FetchNodesActionTest extend
 
         invalidateCommit(revisionId2);
         updateBaseRevisionId(revisionId3, revisionId1);
-
-        FetchNodesAction query = new FetchNodesAction(getNodeStore(),
-                "/", true, revisionId3);
-        List<Node> actuals = toNode(query.execute());
+        List<Node> actuals = createAndExecuteQuery(revisionId3);
 
         String json = String.format("{\"/#%2$s\" : { \"a#%1$s\" : {}, \"c#%2$s\" : {} }}",
                 revisionId1, revisionId3);
@@ -105,68 +98,53 @@ public class FetchNodesActionTest extend
         Long firstRevisionId = scenario.create();
         Long secondRevisionId = scenario.update_A_and_add_D_and_E();
 
-        FetchNodesAction query = new FetchNodesAction(getNodeStore(),
-                "/", true, firstRevisionId);
-        query.setDepth(0);
-        List<Node> actuals = toNode(query.execute());
+        List<Node> actuals = createAndExecuteQuery(firstRevisionId, 0);
         String json = String.format("{ \"/#%1$s\" : {} }", firstRevisionId);
         Node expected = NodeBuilder.build(json);
         Iterator<Node> expecteds = expected.getChildNodeEntries(0, -1);
         NodeAssert.assertEquals(expecteds, actuals);
 
-        query = new FetchNodesAction(getNodeStore(), "/", true, secondRevisionId);
-        query.setDepth(0);
-        actuals = toNode(query.execute());
+        actuals = createAndExecuteQuery(secondRevisionId, 0);
         json = String.format("{ \"/#%1$s\" : {} }", firstRevisionId);
         expected = NodeBuilder.build(json);
         expecteds = expected.getChildNodeEntries(0, -1);
         NodeAssert.assertEquals(expecteds, actuals);
 
-        query = new FetchNodesAction(getNodeStore(), "/", true, firstRevisionId);
-        query.setDepth(1);
-        actuals = toNode(query.execute());
+        actuals = createAndExecuteQuery(firstRevisionId, 1);
         json = String.format("{ \"/#%1$s\" : { \"a#%1$s\" : { \"int\" : 1 } } }", firstRevisionId);
         expected = NodeBuilder.build(json);
         expecteds = expected.getChildNodeEntries(0, -1);
         NodeAssert.assertEquals(expecteds, actuals);
 
-        query = new FetchNodesAction(getNodeStore(), "/", true, secondRevisionId);
-        query.setDepth(1);
-        actuals = toNode(query.execute());
+        actuals = createAndExecuteQuery(secondRevisionId, 1);
         json = String.format("{ \"/#%1$s\" : { \"a#%2$s\" : { \"int\" : 1 , \"double\" :
0.123 } } }",
                 firstRevisionId, secondRevisionId);
         expected = NodeBuilder.build(json);
         expecteds = expected.getChildNodeEntries(0, -1);
         NodeAssert.assertEquals(expecteds, actuals);
 
-        query = new FetchNodesAction(getNodeStore(), "/", true, firstRevisionId);
-        query.setDepth(2);
-        actuals = toNode(query.execute());
+        actuals = createAndExecuteQuery(firstRevisionId, 2);
         json = String.format("{ \"/#%1$s\" : { \"a#%1$s\" : { \"int\" : 1, \"b#%1$s\" : {
\"string\" : \"foo\" } , \"c#%1$s\" : { \"bool\" : true } } } }",
                 firstRevisionId);
         expected = NodeBuilder.build(json);
         expecteds = expected.getChildNodeEntries(0, -1);
         NodeAssert.assertEquals(expecteds, actuals);
 
-        query = new FetchNodesAction(getNodeStore(), "/", true, secondRevisionId);
-        query.setDepth(2);
-        actuals = toNode(query.execute());
+        actuals = createAndExecuteQuery(secondRevisionId, 2);
         json = String.format("{ \"/#%1$s\" : { \"a#%2$s\" : { \"int\" : 1 , \"double\" :
0.123 , \"b#%2$s\" : { \"string\" : \"foo\" } , \"c#%1$s\" : { \"bool\" : true }, \"d#%2$s\"
: { \"null\" : null } } } }",
                 firstRevisionId, secondRevisionId);
         expected = NodeBuilder.build(json);
         expecteds = expected.getChildNodeEntries(0, -1);
         NodeAssert.assertEquals(expecteds, actuals);
 
-        query = new FetchNodesAction(getNodeStore(), "/", true, firstRevisionId);
-        actuals = toNode(query.execute());
+        actuals = createAndExecuteQuery(firstRevisionId);
         json = String.format("{ \"/#%1$s\" : { \"a#%1$s\" : { \"int\" : 1 , \"b#%1$s\" :
{ \"string\" : \"foo\" } , \"c#%1$s\" : { \"bool\" : true } } } }",
                 firstRevisionId);
         expected = NodeBuilder.build(json);
         expecteds = expected.getChildNodeEntries(0, -1);
         NodeAssert.assertEquals(expecteds, actuals);
 
-        query = new FetchNodesAction(getNodeStore(), "/", true, secondRevisionId);
-        actuals = toNode(query.execute());
+        actuals = createAndExecuteQuery(secondRevisionId);
         json = String.format("{ \"/#%1$s\" : { \"a#%2$s\" : { \"int\" : 1 , \"double\" :
0.123 , \"b#%2$s\" : { \"string\" : \"foo\", \"e#%2$s\" : { \"array\" : [ 123, null, 123.456,
\"for:bar\", true ] } } , \"c#%1$s\" : { \"bool\" : true }, \"d#%2$s\" : { \"null\" : null
} } } }",
                 firstRevisionId, secondRevisionId);
         expected = NodeBuilder.build(json);
@@ -231,6 +209,18 @@ public class FetchNodesActionTest extend
         NodeAssert.assertEquals(expecteds, actuals);
     }
 
+    private List<Node> createAndExecuteQuery(Long revisionId) {
+        return createAndExecuteQuery(revisionId, -1);
+    }
+
+    private List<Node> createAndExecuteQuery(Long revisionId, int depth) {
+        FetchNodesAction query = new FetchNodesAction(getNodeStore(), "/", revisionId);
+        if (depth > -1) {
+            query.setDepth(depth);
+        }
+        return toNode(query.execute());
+    }
+
     private Set<String> getPathSet(String... paths) {
         return new HashSet<String>(Arrays.asList(paths));
     }



Mime
View raw message