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 2016/07/07 08:03:15 UTC
svn commit: r1751753 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/
Author: mreutegg
Date: Thu Jul 7 08:03:14 2016
New Revision: 1751753
URL: http://svn.apache.org/viewvc?rev=1751753&view=rev
Log:
OAK-4539: Calculate children cache entry on commit
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java?rev=1751753&r1=1751752&r2=1751753&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java Thu Jul 7 08:03:14 2016
@@ -644,7 +644,9 @@ public class Commit {
}
list.add(p);
}
- RevisionVector after = before.update(revision);
+ // the commit revision with branch flag if this is a branch commit
+ Revision rev = isBranchCommit ? revision.asBranchRevision() : revision;
+ RevisionVector after = before.update(rev);
DiffCache.Entry cacheEntry = nodeStore.getDiffCache().newEntry(before, after, true);
LastRevTracker tracker = nodeStore.createTracker(revision, isBranchCommit);
List<String> added = new ArrayList<String>();
@@ -672,7 +674,7 @@ public class Commit {
// track intermediate node and root
tracker.track(path);
}
- nodeStore.applyChanges(after, path, isNew,
+ nodeStore.applyChanges(before, after, rev, path, isNew,
added, removed, changed, cacheEntry);
}
cacheEntry.done();
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1751753&r1=1751752&r2=1751753&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Thu Jul 7 08:03:14 2016
@@ -505,9 +505,7 @@ public final class DocumentNodeStore
DocumentNodeState n = new DocumentNodeState(this, "/", head);
commit.addNode(n);
commit.applyToDocumentStore();
- // use dummy Revision as before
- RevisionVector before = new RevisionVector(new Revision(0, 0, clusterId));
- commit.applyToCache(before, false);
+ unsavedLastRevisions.put("/", commitRev);
setRoot(head);
// make sure _lastRev is written back to store
backgroundWrite();
@@ -1164,27 +1162,101 @@ public final class DocumentNodeStore
/**
* Apply the changes of a node to the cache.
*
+ * @param before the before revision (old head)
+ * @param after the after revision (new head)
* @param rev the commit revision
* @param path the path
* @param isNew whether this is a new node
* @param added the list of added child nodes
* @param removed the list of removed child nodes
- * @param changed the list of changed child nodes.
+ * @param changed the list of changed child nodes
*
*/
- void applyChanges(RevisionVector rev, String path,
+ void applyChanges(RevisionVector before, RevisionVector after,
+ Revision rev, String path,
boolean isNew, List<String> added,
List<String> removed, List<String> changed,
DiffCache.Entry cacheEntry) {
- if (isNew && !added.isEmpty()) {
- DocumentNodeState.Children c = new DocumentNodeState.Children();
- Set<String> set = Sets.newTreeSet();
- for (String p : added) {
- set.add(Utils.unshareString(PathUtils.getName(p)));
- }
- c.children.addAll(set);
- PathRev key = childNodeCacheKey(path, rev, null);
- nodeChildrenCache.put(key, c);
+ if (isNew) {
+ if (added.isEmpty()) {
+ // this is a leaf node.
+ // check if it has the children flag set
+ NodeDocument doc = store.find(NODES, getIdFromPath(path));
+ if (doc != null && doc.hasChildren()) {
+ PathRev key = childNodeCacheKey(path, after, null);
+ LOG.debug("nodeChildrenCache.put({},{})", key, "NO_CHILDREN");
+ nodeChildrenCache.put(key, DocumentNodeState.NO_CHILDREN);
+ }
+ } else {
+ DocumentNodeState.Children c = new DocumentNodeState.Children();
+ Set<String> set = Sets.newTreeSet();
+ for (String p : added) {
+ set.add(Utils.unshareString(PathUtils.getName(p)));
+ }
+ c.children.addAll(set);
+ PathRev key = childNodeCacheKey(path, after, null);
+ LOG.debug("nodeChildrenCache.put({},{})", key, c);
+ nodeChildrenCache.put(key, c);
+ }
+ } else {
+ // existed before
+ DocumentNodeState beforeState = getRoot(before);
+ // do we have a cached before state that can be used
+ // to calculate the new children?
+ int depth = PathUtils.getDepth(path);
+ for (int i = 1; i <= depth && beforeState != null; i++) {
+ String p = PathUtils.getAncestorPath(path, depth - i);
+ PathRev key = new PathRev(p, beforeState.getLastRevision());
+ beforeState = nodeCache.getIfPresent(key);
+ }
+ DocumentNodeState.Children children = null;
+ if (beforeState != null) {
+ if (beforeState.hasNoChildren()) {
+ children = DocumentNodeState.NO_CHILDREN;
+ } else {
+ PathRev key = childNodeCacheKey(path, beforeState.getLastRevision(), null);
+ children = nodeChildrenCache.getIfPresent(key);
+ }
+ }
+ if (children != null) {
+ PathRev afterKey = new PathRev(path, before.update(rev));
+ // are there any added or removed children?
+ if (added.isEmpty() && removed.isEmpty()) {
+ // simply use the same list
+ LOG.debug("nodeChildrenCache.put({},{})", afterKey, children);
+ nodeChildrenCache.put(afterKey, children);
+ } else if (!children.hasMore){
+ // list is complete. use before children as basis
+ Set<String> afterChildren = Sets.newTreeSet(children.children);
+ for (String p : added) {
+ afterChildren.add(Utils.unshareString(PathUtils.getName(p)));
+ }
+ for (String p : removed) {
+ afterChildren.remove(PathUtils.getName(p));
+ }
+ DocumentNodeState.Children c = new DocumentNodeState.Children();
+ c.children.addAll(afterChildren);
+ if (c.children.size() <= DocumentNodeState.MAX_FETCH_SIZE) {
+ LOG.debug("nodeChildrenCache.put({},{})", afterKey, c);
+ nodeChildrenCache.put(afterKey, c);
+ } else {
+ LOG.info("not caching more than {} child names for {}",
+ DocumentNodeState.MAX_FETCH_SIZE, path);
+ }
+ } else if (added.isEmpty()) {
+ // incomplete list, but we only removed nodes
+ // use linked hash set to retain order
+ Set<String> afterChildren = Sets.newLinkedHashSet(children.children);
+ for (String p : removed) {
+ afterChildren.remove(PathUtils.getName(p));
+ }
+ DocumentNodeState.Children c = new DocumentNodeState.Children();
+ c.children.addAll(afterChildren);
+ c.hasMore = true;
+ LOG.debug("nodeChildrenCache.put({},{})", afterKey, c);
+ nodeChildrenCache.put(afterKey, c);
+ }
+ }
}
// update diff cache
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1751753&r1=1751752&r2=1751753&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java Thu Jul 7 08:03:14 2016
@@ -218,14 +218,8 @@ public class DocumentNodeStoreTest {
}
store.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY);
counter.set(0);
- // the following should just make one call to DocumentStore.query()
- for (ChildNodeEntry e : store.getRoot().getChildNodeEntries()) {
- e.getNodeState();
- }
- assertEquals(1, counter.get());
-
- counter.set(0);
- // now the child node entries are cached and no call should happen
+ // the following must read from the nodeChildrenCache populated by
+ // the commit and not use a query on the document store (OAK-1322)
for (ChildNodeEntry e : store.getRoot().getChildNodeEntries()) {
e.getNodeState();
}
@@ -2212,6 +2206,8 @@ public class DocumentNodeStoreTest {
NodeState afterTest = after.getChildNode("test");
startValues.clear();
+ // make sure diff is not served from node children cache entries
+ ns.invalidateNodeChildrenCache();
afterTest.compareAgainstBaseState(beforeTest, new DefaultNodeStateDiff());
assertEquals(1, startValues.size());
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java?rev=1751753&r1=1751752&r2=1751753&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java Thu Jul 7 08:03:14 2016
@@ -20,10 +20,12 @@
package org.apache.jackrabbit.oak.plugins.document;
import java.io.IOException;
+import java.util.List;
import java.util.Set;
import javax.annotation.Nonnull;
+import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.commons.PathUtils;
@@ -135,11 +137,18 @@ public class NodeStoreDiffTest {
//but less then lastRev of the of readRevision. Where readRevision is the rev of root node when
//rebase was performed
+ // remember paths accessed so far
+ List<String> paths = Lists.newArrayList(tds.paths);
+
//This is not to be done in actual cases as CommitHooks are invoked in critical sections
//and creating nodes from within CommitHooks would cause deadlock. This is done here to ensure
//that changes are done when rebase has been performed and merge is about to happen
createNodes("/oak:index/prop-b/b1");
+ // reset accessed paths
+ tds.reset();
+ tds.paths.addAll(paths);
+
//For now we the cache is disabled (size 0) so this is not required
//ns.nodeCache.invalidateAll();