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 2014/01/27 14:46:43 UTC
svn commit: r1561676 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/mongomk/
test/java/org/apache/jackrabbit/oak/plugins/mongomk/
Author: mreutegg
Date: Mon Jan 27 13:46:42 2014
New Revision: 1561676
URL: http://svn.apache.org/r1561676
Log:
OAK-1275: Efficient MongoNodeState.getChildNodeEntries() for many child nodes
Added:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/ManyChildNodesTest.java (with props)
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/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/MongoNodeState.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStore.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStoreBranch.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/UnsavedModifications.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/SimpleTest.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=1561676&r1=1561675&r2=1561676&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 Mon Jan 27 13:46:42 2014
@@ -98,7 +98,10 @@ class Branch {
Revision last = commits.lastKey();
checkArgument(commits.comparator().compare(head, last) > 0);
BranchCommit bc = new BranchCommit(base);
- bc.getModifications().put("/", head);
+ // set all previously touched paths as modified
+ for (BranchCommit c : commits.values()) {
+ c.getModifications().applyTo(bc.getModifications(), head);
+ }
commits.put(head, bc);
}
@@ -198,7 +201,7 @@ class Branch {
*/
@CheckForNull
public Revision getUnsavedLastRevision(String path,
- Revision readRevision) {
+ Revision readRevision) {
readRevision = readRevision.asBranchRevision();
for (Revision r : commits.descendingKeySet()) {
if (readRevision.compareRevisionTime(r) < 0) {
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=1561676&r1=1561675&r2=1561676&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 Mon Jan 27 13:46:42 2014
@@ -452,6 +452,10 @@ public class Commit {
addOrRemove.addAll(addedNodes);
addOrRemove.addAll(removedNodes);
for (String p : addOrRemove) {
+ if (PathUtils.denotesRoot(p)) {
+ // special case: root node was added
+ continue;
+ }
String parent = PathUtils.getParentPath(p);
ArrayList<String> list = nodesWithChangedChildren.get(parent);
if (list == 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=1561676&r1=1561675&r2=1561676&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 Mon Jan 27 13:46:42 2014
@@ -229,8 +229,8 @@ public class MongoMK implements MicroKer
// use a MongoDB index instead
int max = MANY_CHILDREN_THRESHOLD;
Children fromChildren, toChildren;
- fromChildren = nodeStore.getChildren(path, fromRev, max);
- toChildren = nodeStore.getChildren(path, toRev, max);
+ fromChildren = nodeStore.getChildren(from, null, max);
+ toChildren = nodeStore.getChildren(to, null, max);
if (!fromChildren.hasMore && !toChildren.hasMore) {
diffFewChildren(w, fromChildren, fromRev, toChildren, toRev);
} else {
@@ -238,8 +238,8 @@ public class MongoMK implements MicroKer
diffManyChildren(w, path, fromRev, toRev);
} else {
max = Integer.MAX_VALUE;
- fromChildren = nodeStore.getChildren(path, fromRev, max);
- toChildren = nodeStore.getChildren(path, toRev, max);
+ fromChildren = nodeStore.getChildren(from, null, max);
+ toChildren = nodeStore.getChildren(to, null, max);
diffFewChildren(w, fromChildren, fromRev, toChildren, toRev);
}
}
@@ -383,7 +383,7 @@ public class MongoMK implements MicroKer
long m = ((long) maxChildNodes) + offset;
max = (int) Math.min(m, Integer.MAX_VALUE);
}
- Children c = nodeStore.getChildren(path, rev, max);
+ Children c = nodeStore.getChildren(n, null, max);
for (long i = offset; i < c.children.size(); i++) {
if (maxChildNodes-- <= 0) {
break;
@@ -529,8 +529,8 @@ public class MongoMK implements MicroKer
//------------------------------< internal >--------------------------------
private void parseJsonDiff(Commit commit, String json, String rootPath) {
- String baseRevId = commit.getBaseRevision() != null ?
- commit.getBaseRevision().toString() : null;
+ Revision baseRev = commit.getBaseRevision();
+ String baseRevId = baseRev != null ? baseRev.toString() : null;
JsopReader t = new JsopTokenizer(json);
while (true) {
int r = t.read();
@@ -545,8 +545,12 @@ public class MongoMK implements MicroKer
parseAddNode(commit, t, path);
break;
case '-':
+ Node toRemove = nodeStore.getNode(path, commit.getBaseRevision());
+ if (toRemove == null) {
+ throw new MicroKernelException("Node not found: " + path + " in revision " + baseRevId);
+ }
commit.removeNode(path);
- nodeStore.markAsDeleted(path, commit, true);
+ nodeStore.markAsDeleted(toRemove, commit, true);
commit.removeNodeDiff(path);
break;
case '^':
@@ -565,35 +569,35 @@ public class MongoMK implements MicroKer
case '>': {
// TODO support moving nodes that were modified within this commit
t.read(':');
- String sourcePath = path;
String targetPath = t.readString();
if (!PathUtils.isAbsolute(targetPath)) {
targetPath = PathUtils.concat(rootPath, targetPath);
}
- if (!nodeExists(sourcePath, baseRevId)) {
- throw new MicroKernelException("Node not found: " + sourcePath + " in revision " + baseRevId);
+ Node source = nodeStore.getNode(path, baseRev);
+ if (source == null) {
+ throw new MicroKernelException("Node not found: " + path + " in revision " + baseRevId);
} else if (nodeExists(targetPath, baseRevId)) {
throw new MicroKernelException("Node already exists: " + targetPath + " in revision " + baseRevId);
}
- commit.moveNode(sourcePath, targetPath);
- nodeStore.moveNode(sourcePath, targetPath, commit);
+ commit.moveNode(path, targetPath);
+ nodeStore.moveNode(source, targetPath, commit);
break;
}
case '*': {
// TODO support copying nodes that were modified within this commit
t.read(':');
- String sourcePath = path;
String targetPath = t.readString();
if (!PathUtils.isAbsolute(targetPath)) {
targetPath = PathUtils.concat(rootPath, targetPath);
}
- if (!nodeExists(sourcePath, baseRevId)) {
- throw new MicroKernelException("Node not found: " + sourcePath + " in revision " + baseRevId);
+ Node source = nodeStore.getNode(path, baseRev);
+ if (source == null) {
+ throw new MicroKernelException("Node not found: " + path + " in revision " + baseRevId);
} else if (nodeExists(targetPath, baseRevId)) {
throw new MicroKernelException("Node already exists: " + targetPath + " in revision " + baseRevId);
}
- commit.copyNode(sourcePath, targetPath);
- nodeStore.copyNode(sourcePath, targetPath, commit);
+ commit.copyNode(path, targetPath);
+ nodeStore.copyNode(source, targetPath, commit);
break;
}
default:
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeState.java?rev=1561676&r1=1561675&r2=1561676&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeState.java Mon Jan 27 13:46:42 2014
@@ -17,9 +17,12 @@
package org.apache.jackrabbit.oak.plugins.mongomk;
import java.util.Collections;
+import java.util.Iterator;
import java.util.List;
+import java.util.NoSuchElementException;
import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import javax.jcr.PropertyType;
import org.apache.jackrabbit.mk.json.JsopReader;
@@ -61,6 +64,16 @@ import static org.apache.jackrabbit.oak.
*/
final class MongoNodeState extends AbstractNodeState {
+ /**
+ * The number of child nodes to fetch initially.
+ */
+ static final int INITIAL_FETCH_SIZE = 100;
+
+ /**
+ * The maximum number of child nodes to fetch in one call. (1600).
+ */
+ static final int MAX_FETCH_SIZE = INITIAL_FETCH_SIZE << 4;
+
private final MongoNodeStore store;
private final Node node;
@@ -173,32 +186,12 @@ final class MongoNodeState extends Abstr
if (node.hasNoChildren()) {
return Collections.emptyList();
}
- // TODO: handle many child nodes better
- Node.Children children = store.getChildren(getPath(),
- node.getLastRevision(), 100);
- if (children.hasMore) {
- children = store.getChildren(getPath(),
- node.getLastRevision(), Integer.MAX_VALUE);
- }
- return Iterables.transform(children.children, new Function<String, ChildNodeEntry>() {
+ return new Iterable<ChildNodeEntry>() {
@Override
- public ChildNodeEntry apply(String path) {
- final String name = PathUtils.getName(path);
- return new AbstractChildNodeEntry() {
- @Nonnull
- @Override
- public String getName() {
- return name;
- }
-
- @Nonnull
- @Override
- public NodeState getNodeState() {
- return getChildNode(name);
- }
- };
+ public Iterator<ChildNodeEntry> iterator() {
+ return new ChildNodeEntryIterator();
}
- });
+ };
}
@Nonnull
@@ -259,7 +252,7 @@ final class MongoNodeState extends Abstr
* @param reader the reader
* @return new property state
*/
- public static PropertyState readProperty(String name, MongoNodeStore store, JsopReader reader) {
+ static PropertyState readProperty(String name, MongoNodeStore store, JsopReader reader) {
if (reader.matches(JsopReader.NUMBER)) {
String number = reader.getToken();
try {
@@ -317,7 +310,7 @@ final class MongoNodeState extends Abstr
* @param reader the reader
* @return new property state
*/
- public static PropertyState readArrayProperty(String name, MongoNodeStore store, JsopReader reader) {
+ static PropertyState readArrayProperty(String name, MongoNodeStore store, JsopReader reader) {
int type = PropertyType.STRING;
List<Object> values = Lists.newArrayList();
while (!reader.matches(']')) {
@@ -362,4 +355,89 @@ final class MongoNodeState extends Abstr
}
return createProperty(name, values, Type.fromTag(type, true));
}
+
+ //-----------------------< internal >---------------------------------------
+
+ /**
+ * Returns up to {@code limit} child node entries, starting after the given
+ * {@code name}.
+ *
+ * @param name the name of the lower bound child node entry (exclusive) or
+ * {@code null}, if the method should start with the first known
+ * child node.
+ * @param limit the maximum number of child node entries to return.
+ * @return the child node entries.
+ */
+ @Nonnull
+ private Iterable<ChildNodeEntry> getChildNodeEntries(@Nullable String name,
+ int limit) {
+ Iterable<Node> children = store.getChildNodes(node, name, limit);
+ return Iterables.transform(children, new Function<Node, ChildNodeEntry>() {
+ @Override
+ public ChildNodeEntry apply(final Node input) {
+ return new AbstractChildNodeEntry() {
+ @Nonnull
+ @Override
+ public String getName() {
+ return PathUtils.getName(input.path);
+ }
+
+ @Nonnull
+ @Override
+ public NodeState getNodeState() {
+ return new MongoNodeState(store, input);
+ }
+ };
+ }
+ });
+ }
+
+ private class ChildNodeEntryIterator implements Iterator<ChildNodeEntry> {
+
+ private String previousName;
+ private Iterator<ChildNodeEntry> current;
+ private int fetchSize = INITIAL_FETCH_SIZE;
+
+ ChildNodeEntryIterator() {
+ fetchMore();
+ }
+
+ @Override
+ public boolean hasNext() {
+ while (true) {
+ if (current == null) {
+ return false;
+ } else if (current.hasNext()) {
+ return true;
+ }
+ fetchMore();
+ }
+ }
+
+ @Override
+ public ChildNodeEntry next() {
+ if (!hasNext()) {
+ throw new NoSuchElementException();
+ }
+ ChildNodeEntry entry = current.next();
+ previousName = entry.getName();
+ return entry;
+ }
+
+ @Override
+ public void remove() {
+ throw new UnsupportedOperationException();
+ }
+
+ private void fetchMore() {
+ Iterator<ChildNodeEntry> entries = getChildNodeEntries(
+ previousName, fetchSize).iterator();
+ fetchSize = Math.min(fetchSize * 2, MAX_FETCH_SIZE);
+ if (entries.hasNext()) {
+ current = entries;
+ } else {
+ current = null;
+ }
+ }
+ }
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStore.java?rev=1561676&r1=1561675&r2=1561676&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStore.java Mon Jan 27 13:46:42 2014
@@ -18,7 +18,6 @@ package org.apache.jackrabbit.oak.plugin
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Preconditions.checkState;
import static org.apache.jackrabbit.oak.api.CommitFailedException.MERGE;
import java.io.Closeable;
@@ -211,7 +210,7 @@ public final class MongoNodeStore
/**
* Child node cache.
*
- * Key: path@rev, value: children
+ * Key: start-name/path@rev, value: children
*/
private final Cache<String, Node.Children> nodeChildrenCache;
private final CacheStats nodeChildrenCacheStats;
@@ -496,29 +495,22 @@ public final class MongoNodeStore
splitCandidates.put(id, id);
}
- void copyNode(String sourcePath, String targetPath, Commit commit) {
- moveOrCopyNode(false, sourcePath, targetPath, commit);
+ void copyNode(Node source, String targetPath, Commit commit) {
+ moveOrCopyNode(false, source, targetPath, commit);
}
- void moveNode(String sourcePath, String targetPath, Commit commit) {
- moveOrCopyNode(true, sourcePath, targetPath, commit);
+ void moveNode(Node source, String targetPath, Commit commit) {
+ moveOrCopyNode(true, source, targetPath, commit);
}
- void markAsDeleted(String path, Commit commit, boolean subTreeAlso) {
- Revision rev = commit.getBaseRevision();
- checkState(rev != null, "Base revision of commit must not be null");
- commit.removeNode(path);
+ void markAsDeleted(Node node, Commit commit, boolean subTreeAlso) {
+ commit.removeNode(node.getPath());
if (subTreeAlso) {
// recurse down the tree
// TODO causes issue with large number of children
- Node n = getNode(path, rev);
-
- if (n != null) {
- Node.Children c = getChildren(path, rev, Integer.MAX_VALUE);
- for (String childPath : c.children) {
- markAsDeleted(childPath, commit, true);
- }
+ for (Node child : getChildNodes(node, null, Integer.MAX_VALUE)) {
+ markAsDeleted(child, commit, true);
}
}
}
@@ -553,54 +545,59 @@ public final class MongoNodeStore
}
}
- Node.Children getChildren(final String path, final Revision rev, final int limit)
+ Node.Children getChildren(@Nonnull final Node parent,
+ @Nullable final String name,
+ final int limit)
throws MicroKernelException {
- checkRevisionAge(rev, path);
-
- //Preemptive check. If we know there are no child then
- //return straight away
- final Node node = getNode(path, rev);
- if (node.hasNoChildren()) {
- return new Node.Children();
+ if (checkNotNull(parent).hasNoChildren()) {
+ return Node.NO_CHILDREN;
}
-
- String key = path + "@" + rev;
+ final String path = checkNotNull(parent).getPath();
+ final Revision readRevision = parent.getLastRevision();
+ String key = childNodeCacheKey(path, readRevision, name);
Node.Children children;
- try {
- children = nodeChildrenCache.get(key, new Callable<Node.Children>() {
- @Override
- public Node.Children call() throws Exception {
- return readChildren(path, rev, limit);
- }
- });
- } catch (ExecutionException e) {
- throw new MicroKernelException("Error occurred while fetching children nodes for path "+path, e);
- }
-
- //In case the limit > cached children size and there are more child nodes
- //available then refresh the cache
- if (children.hasMore) {
- if (limit > children.children.size()) {
- children = readChildren(path, rev, limit);
- if (children != null) {
- nodeChildrenCache.put(key, children);
- }
+ for (;;) {
+ try {
+ children = nodeChildrenCache.get(key, new Callable<Node.Children>() {
+ @Override
+ public Node.Children call() throws Exception {
+ return readChildren(parent, name, limit);
+ }
+ });
+ } catch (ExecutionException e) {
+ throw new MicroKernelException(
+ "Error occurred while fetching children for path "
+ + path, e.getCause());
+ }
+ if (children.hasMore && limit > children.children.size()) {
+ // there are potentially more children and
+ // current cache entry contains less than requested limit
+ // -> need to refresh entry with current limit
+ nodeChildrenCache.invalidate(key);
+ } else {
+ // use this cache entry
+ break;
}
}
return children;
}
- Node.Children readChildren(String path, Revision rev, int limit) {
+ Node.Children readChildren(Node parent, String name, int limit) {
// TODO use offset, to avoid O(n^2) and running out of memory
// to do that, use the *name* of the last entry of the previous batch of children
// as the starting point
+ String path = parent.getPath();
+ Revision rev = parent.getLastRevision();
Iterable<NodeDocument> docs;
Node.Children c = new Node.Children();
+ // add one to the requested limit for the raw limit
+ // this gives us a chance to detect whether there are more
+ // child nodes than requested.
int rawLimit = (int) Math.min(Integer.MAX_VALUE, ((long) limit) + 1);
Set<Revision> validRevisions = new HashSet<Revision>();
for (;;) {
c.children.clear();
- docs = readChildren(path, rawLimit);
+ docs = readChildDocs(path, name, rawLimit);
int numReturned = 0;
for (NodeDocument doc : docs) {
numReturned++;
@@ -630,12 +627,32 @@ public final class MongoNodeStore
}
}
+ /**
+ * Returns the child documents at the given {@code path} and returns up to
+ * {@code limit} documents. The returned child documents are sorted in
+ * ascending child node name order. If a {@code name} is passed, the first
+ * child document returned is after the given name. That is, the name is the
+ * lower exclusive bound.
+ *
+ * @param path the path of the parent document.
+ * @param name the lower exclusive bound or {@code null}.
+ * @param limit the maximum number of child documents to return.
+ * @return the child documents.
+ */
@Nonnull
- Iterable<NodeDocument> readChildren(final String path, int limit) {
- String from = Utils.getKeyLowerLimit(path);
- String to = Utils.getKeyUpperLimit(path);
- if (limit > NUM_CHILDREN_CACHE_LIMIT) {
- // do not use cache
+ Iterable<NodeDocument> readChildDocs(@Nonnull final String path,
+ @Nullable String name,
+ int limit) {
+ String to = Utils.getKeyUpperLimit(checkNotNull(path));
+ String from;
+ if (name != null) {
+ from = Utils.getIdFromPath(PathUtils.concat(path, name));
+ } else {
+ from = Utils.getKeyLowerLimit(path);
+ }
+ if (name != null || limit > NUM_CHILDREN_CACHE_LIMIT) {
+ // do not use cache when there is a lower bound name
+ // or more than 16k child docs are requested
return store.query(Collection.NODES, from, to, limit);
}
// check cache
@@ -649,6 +666,7 @@ public final class MongoNodeStore
}
c.isComplete = docs.size() < limit;
docChildrenCache.put(path, c);
+ return docs;
} else if (c.childNames.size() < limit && !c.isComplete) {
// fetch more and update cache
String lastName = c.childNames.get(c.childNames.size() - 1);
@@ -679,6 +697,37 @@ public final class MongoNodeStore
return it;
}
+ /**
+ * Returns up to {@code limit} child nodes, starting at the given
+ * {@code name} (exclusive).
+ *
+ * @param parent the parent node.
+ * @param name the name of the lower bound child node (exclusive) or
+ * {@code null}, if the method should start with the first known
+ * child node.
+ * @param limit the maximum number of child nodes to return.
+ * @return the child nodes.
+ */
+ @Nonnull
+ Iterable<Node> getChildNodes(final @Nonnull Node parent,
+ final @Nullable String name,
+ final int limit) {
+ // Preemptive check. If we know there are no children then
+ // return straight away
+ if (checkNotNull(parent).hasNoChildren()) {
+ return Collections.emptyList();
+ }
+
+ final Revision readRevision = parent.getLastRevision();
+ return Iterables.transform(getChildren(parent, name, limit).children,
+ new Function<String, Node>() {
+ @Override
+ public Node apply(String input) {
+ return getNode(input, readRevision);
+ }
+ });
+ }
+
@CheckForNull
Node readNode(String path, Revision readRevision) {
String id = Utils.getIdFromPath(path);
@@ -727,7 +776,7 @@ public final class MongoNodeStore
}
}
}
- String key = path + "@" + rev;
+ String key = childNodeCacheKey(path, rev, null);
Node.Children c = nodeChildrenCache.getIfPresent(key);
if (isNew || (!isDelete && c != null)) {
Node.Children c2 = new Node.Children();
@@ -1232,6 +1281,12 @@ public final class MongoNodeStore
//-----------------------------< internal >---------------------------------
+ private static String childNodeCacheKey(@Nonnull String path,
+ @Nonnull Revision readRevision,
+ @Nullable String name) {
+ return (name == null ? "" : name) + path + "@" + readRevision;
+ }
+
private static MongoRootBuilder asMongoRootBuilder(NodeBuilder builder)
throws IllegalArgumentException {
if (!(builder instanceof MongoRootBuilder)) {
@@ -1242,7 +1297,7 @@ public final class MongoNodeStore
}
private void moveOrCopyNode(boolean move,
- String sourcePath,
+ Node source,
String targetPath,
Commit commit) {
// TODO Optimize - Move logic would not work well with very move of very large subtrees
@@ -1253,25 +1308,17 @@ public final class MongoNodeStore
// of this commit i.e. transient nodes. If its required it would need to be looked
// into
- Node n = getNode(sourcePath, commit.getBaseRevision());
-
- // Node might be deleted already
- if (n == null) {
- return;
- }
-
Node newNode = new Node(targetPath, commit.getRevision());
- n.copyTo(newNode);
+ source.copyTo(newNode);
commit.addNode(newNode);
if (move) {
- markAsDeleted(sourcePath, commit, false);
+ markAsDeleted(source, commit, false);
}
- Node.Children c = getChildren(sourcePath, commit.getBaseRevision(), Integer.MAX_VALUE);
- for (String srcChildPath : c.children) {
- String childName = PathUtils.getName(srcChildPath);
+ for (Node child : getChildNodes(source, null, Integer.MAX_VALUE)) {
+ String childName = PathUtils.getName(child.getPath());
String destChildPath = PathUtils.concat(targetPath, childName);
- moveOrCopyNode(move, srcChildPath, destChildPath, commit);
+ moveOrCopyNode(move, child, destChildPath, commit);
}
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStoreBranch.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStoreBranch.java?rev=1561676&r1=1561675&r2=1561676&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStoreBranch.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/MongoNodeStoreBranch.java Mon Jan 27 13:46:42 2014
@@ -24,6 +24,8 @@ import org.apache.jackrabbit.oak.spi.com
import org.apache.jackrabbit.oak.spi.state.AbstractNodeStoreBranch;
import org.apache.jackrabbit.oak.spi.state.NodeState;
+import static com.google.common.base.Preconditions.checkState;
+
/**
* Implementation of a MongoMK based node store branch.
*/
@@ -86,10 +88,13 @@ public class MongoNodeStoreBranch
protected MongoNodeState copy(final String source,
final String target,
MongoNodeState base) {
+ final Node src = store.getNode(source, base.getRevision());
+ checkState(src != null, "Source node %s@%s does not exist",
+ source, base.getRevision());
return persist(new Changes() {
@Override
public void with(Commit c) {
- store.copyNode(source, target, c);
+ store.copyNode(src, target, c);
}
}, base, null);
}
@@ -98,10 +103,13 @@ public class MongoNodeStoreBranch
protected MongoNodeState move(final String source,
final String target,
MongoNodeState base) {
+ final Node src = store.getNode(source, base.getRevision());
+ checkState(src != null, "Source node %s@%s does not exist",
+ source, base.getRevision());
return persist(new Changes() {
@Override
public void with(Commit c) {
- store.moveNode(source, target, c);
+ store.moveNode(src, target, c);
}
}, base, null);
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java?rev=1561676&r1=1561675&r2=1561676&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/Node.java Mon Jan 27 13:46:42 2014
@@ -143,6 +143,8 @@ public class Node implements CacheValue
return size;
}
+ static final Children NO_CHILDREN = new Children();
+
/**
* A list of children for a node.
*/
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=1561676&r1=1561675&r2=1561676&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 Mon Jan 27 13:46:42 2014
@@ -90,14 +90,14 @@ 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 than the
- * merge commit revision.
+ * {@code commit} revision.
*
* @param other the other <code>UnsavedModifications</code>.
- * @param mergeCommit the merge commit revision.
+ * @param commit the commit revision.
*/
- public void applyTo(UnsavedModifications other, Revision mergeCommit) {
+ public void applyTo(UnsavedModifications other, Revision commit) {
for (Map.Entry<String, Revision> entry : map.entrySet()) {
- other.put(entry.getKey(), mergeCommit);
+ other.put(entry.getKey(), commit);
}
}
Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/ManyChildNodesTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/ManyChildNodesTest.java?rev=1561676&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/ManyChildNodesTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/ManyChildNodesTest.java Mon Jan 27 13:46:42 2014
@@ -0,0 +1,81 @@
+/*
+ * 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.oak.plugins.mongomk;
+
+import java.util.List;
+import java.util.Map;
+
+import javax.annotation.Nonnull;
+
+import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
+import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.junit.Test;
+
+import com.google.common.collect.Maps;
+
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Checks that traversing over many child nodes requests them in batches with
+ * an upper limit.
+ */
+public class ManyChildNodesTest {
+
+ @Test
+ public void manyChildNodes() throws Exception {
+ TestStore store = new TestStore();
+ MongoMK mk = new MongoMK.Builder().setDocumentStore(store).open();
+ NodeStore ns = mk.getNodeStore();
+
+ NodeBuilder builder = ns.getRoot().builder();
+ for (int i = 0; i < MongoNodeState.MAX_FETCH_SIZE * 2; i++) {
+ builder.child("c-" + i);
+ }
+ ns.merge(builder, EmptyHook.INSTANCE, null);
+ store.queries.clear();
+ // must fetch in batches
+ for (ChildNodeEntry entry : ns.getRoot().getChildNodeEntries()) {
+ entry.getName();
+ }
+ // maximum fetch size is MAX_FETCH_SIZE plus one because
+ // MongoNodeStore will use this value to find out if there
+ // are more child nodes than requested
+ int maxFetchSize = MongoNodeState.MAX_FETCH_SIZE + 1;
+ for (Map.Entry<String, Integer> e : store.queries.entrySet()) {
+ assertTrue(e.getValue() + " > " + maxFetchSize,
+ e.getValue() <= maxFetchSize);
+ }
+ mk.dispose();
+ }
+
+ private final class TestStore extends MemoryDocumentStore {
+
+ Map<String, Integer> queries = Maps.newHashMap();
+
+ @Nonnull
+ @Override
+ public <T extends Document> List<T> query(Collection<T> collection,
+ String fromKey,
+ String toKey,
+ int limit) {
+ queries.put(fromKey, limit);
+ return super.query(collection, fromKey, toKey, limit);
+ }
+ }
+}
Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/ManyChildNodesTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/ManyChildNodesTest.java
------------------------------------------------------------------------------
svn:keywords = Author Date Id Revision Rev URL
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/SimpleTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/SimpleTest.java?rev=1561676&r1=1561675&r2=1561676&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/SimpleTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/SimpleTest.java Mon Jan 27 13:46:42 2014
@@ -244,10 +244,13 @@ public class SimpleTest {
String r0 = mk.commit("/test", "+\"a\":{\"name\": \"World\"}", null, null);
String r1 = mk.commit("/test", "+\"b\":{\"name\": \"!\"}", null, null);
test = mk.getNodes("/test", r0, 0, 0, Integer.MAX_VALUE, null);
- Children c;
- c = ns.getChildren("/", Revision.fromString(r0), Integer.MAX_VALUE);
+ Node n = ns.getNode("/", Revision.fromString(r0));
+ assertNotNull(n);
+ Children c = ns.getChildren(n, null, Integer.MAX_VALUE);
assertEquals("[/test]", c.toString());
- c = ns.getChildren("/test", Revision.fromString(r1), Integer.MAX_VALUE);
+ n = ns.getNode("/test", Revision.fromString(r1));
+ assertNotNull(n);
+ c = ns.getChildren(n, null, Integer.MAX_VALUE);
assertEquals("[/test/a, /test/b]", c.toString());
rev = mk.commit("", "^\"/test\":1", null, null);
@@ -268,17 +271,19 @@ public class SimpleTest {
mk.commit("/testDel", "+\"b\":{\"name\": \"!\"}", null, null);
String r1 = mk.commit("/testDel", "+\"c\":{\"name\": \"!\"}", null, null);
- Children c = ns.getChildren("/testDel", Revision.fromString(r1),
- Integer.MAX_VALUE);
+ Node n = ns.getNode("/testDel", Revision.fromString(r1));
+ assertNotNull(n);
+ Children c = ns.getChildren(n, null, Integer.MAX_VALUE);
assertEquals(3, c.children.size());
String r2 = mk.commit("/testDel", "-\"c\"", null, null);
- c = ns.getChildren("/testDel", Revision.fromString(r2),
- Integer.MAX_VALUE);
+ n = ns.getNode("/testDel", Revision.fromString(r2));
+ assertNotNull(n);
+ c = ns.getChildren(n, null, Integer.MAX_VALUE);
assertEquals(2, c.children.size());
String r3 = mk.commit("/", "-\"testDel\"", null, null);
- Node n = ns.getNode("/testDel", Revision.fromString(r3));
+ n = ns.getNode("/testDel", Revision.fromString(r3));
assertNull(n);
}