Author: mreutegg
Date: Mon Nov 28 12:44:01 2016
New Revision: 1771729
URL: http://svn.apache.org/viewvc?rev=1771729&view=rev
Log:
OAK-5156: Limit JournalDiffLoader to subtree
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoader.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoaderTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java
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=1771729&r1=1771728&r2=1771729&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
Mon Nov 28 12:44:01 2016
@@ -1994,7 +1994,8 @@ public final class DocumentNodeStore
// then there were external changes and reading them
// was successful -> apply them to the diff cache
try {
- JournalEntry.applyTo(externalSort, diffCache,
oldHead, newHead);
+ JournalEntry.applyTo(externalSort, diffCache,
+ PathUtils.ROOT_PATH, oldHead, newHead);
} catch (Exception e1) {
LOG.error("backgroundRead: Exception while
processing external changes from journal: {}", e1, e1);
}
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoader.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoader.java?rev=1771729&r1=1771728&r2=1771729&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoader.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoader.java
Mon Nov 28 12:44:01 2016
@@ -33,6 +33,8 @@ import org.apache.jackrabbit.oak.plugins
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
import static org.apache.commons.io.FileUtils.byteCountToDisplaySize;
import static org.apache.jackrabbit.oak.plugins.document.JournalEntry.asId;
import static
org.apache.jackrabbit.oak.plugins.document.JournalEntry.fillExternalChanges;
@@ -55,16 +57,20 @@ class JournalDiffLoader implements DiffC
JournalDiffLoader(@Nonnull AbstractDocumentNodeState base,
@Nonnull AbstractDocumentNodeState node,
@Nonnull DocumentNodeStore ns) {
- this.base = base;
- this.node = node;
- this.ns = ns;
+ this.base = checkNotNull(base);
+ this.node = checkNotNull(node);
+ this.ns = checkNotNull(ns);
+ checkArgument(base.getPath().equals(node.getPath()),
+ "nodes must have matching paths: {} != {}",
+ base.getPath(), node.getPath());
}
@Override
public String call() {
- stats = new Stats();
+ String path = node.getPath();
RevisionVector afterRev = node.getRootRevision();
RevisionVector beforeRev = base.getRootRevision();
+ stats = new Stats(path, beforeRev, afterRev);
JournalEntry localPending = ns.getCurrentJournalEntry();
DocumentStore store = ns.getDocumentStore();
@@ -79,15 +85,15 @@ class JournalDiffLoader implements DiffC
StringSort changes = JournalEntry.newSorter();
try {
- readTrunkChanges(beforeRev, afterRev, localPending, localLastRev,
changes);
+ readTrunkChanges(path, beforeRev, afterRev, localPending,
localLastRev, changes);
- readBranchChanges(beforeRev, changes);
- readBranchChanges(afterRev, changes);
+ readBranchChanges(path, beforeRev, changes);
+ readBranchChanges(path, afterRev, changes);
changes.sort();
DiffCache df = ns.getDiffCache();
WrappedDiffCache wrappedCache = new
WrappedDiffCache(node.getPath(), df, stats);
- JournalEntry.applyTo(changes, wrappedCache, beforeRev, afterRev);
+ JournalEntry.applyTo(changes, wrappedCache, path, beforeRev,
afterRev);
return wrappedCache.changes;
} catch (IOException e) {
@@ -98,7 +104,8 @@ class JournalDiffLoader implements DiffC
}
}
- private void readBranchChanges(RevisionVector rv,
+ private void readBranchChanges(String path,
+ RevisionVector rv,
StringSort changes) throws IOException {
if (!rv.isBranch() || ns.isDisableBranches()) {
return;
@@ -116,7 +123,7 @@ class JournalDiffLoader implements DiffC
if (!bc.isRebase()) {
JournalEntry entry = store.find(Collection.JOURNAL, asId(br));
if (entry != null) {
- entry.addTo(changes);
+ entry.addTo(changes, path);
stats.numJournalEntries++;
} else {
LOG.warn("Missing journal entry for {}", asId(br));
@@ -125,7 +132,8 @@ class JournalDiffLoader implements DiffC
}
}
- private void readTrunkChanges(RevisionVector beforeRev,
+ private void readTrunkChanges(String path,
+ RevisionVector beforeRev,
RevisionVector afterRev,
JournalEntry localPending,
Revision localLastRev,
@@ -151,13 +159,13 @@ class JournalDiffLoader implements DiffC
// use revision with a timestamp of zero
from = new Revision(0, 0, to.getClusterId());
}
- stats.numJournalEntries += fillExternalChanges(changes, from, to,
ns.getDocumentStore());
+ stats.numJournalEntries += fillExternalChanges(changes, path,
from, to, ns.getDocumentStore());
}
// do we need to include changes from pending local changes?
if (!max.isRevisionNewer(localLastRev)
&& !localLastRev.equals(max.getRevision(clusterId))) {
// journal does not contain all local changes
- localPending.addTo(changes);
+ localPending.addTo(changes, path);
stats.numJournalEntries++;
}
}
@@ -197,16 +205,25 @@ class JournalDiffLoader implements DiffC
private static class Stats {
private final Stopwatch sw = Stopwatch.createStarted();
+ private final String path;
+ private final RevisionVector from, to;
private long numJournalEntries;
private long numDiffEntries;
private long keyMemory;
private long valueMemory;
+ Stats(String path, RevisionVector from, RevisionVector to) {
+ this.path = path;
+ this.from = from;
+ this.to = to;
+ }
+
@Override
public String toString() {
- String msg = "%d diffs loaded from %d journal entries in %s. " +
+ String msg = "%d diffs for %s (%s/%s) loaded from %d journal
entries in %s. " +
"Keys: %s, values: %s, total: %s";
- return String.format(msg, numDiffEntries, numJournalEntries, sw,
+ return String.format(msg, numDiffEntries, path, from, to,
+ numJournalEntries, sw,
byteCountToDisplaySize(keyMemory),
byteCountToDisplaySize(valueMemory),
byteCountToDisplaySize(keyMemory + valueMemory));
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java?rev=1771729&r1=1771728&r2=1771729&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java
Mon Nov 28 12:44:01 2016
@@ -101,9 +101,10 @@ public final class JournalEntry extends
static void applyTo(@Nonnull StringSort externalSort,
@Nonnull DiffCache diffCache,
+ @Nonnull String path,
@Nonnull RevisionVector from,
@Nonnull RevisionVector to) throws IOException {
- LOG.debug("applyTo: starting for {} to {}", from, to);
+ LOG.debug("applyTo: starting for {} from {} to {}", path, from, to);
// note that it is not de-duplicated yet
LOG.debug("applyTo: sorting done.");
@@ -113,8 +114,8 @@ public final class JournalEntry extends
if (!it.hasNext()) {
// nothing at all? that's quite unusual..
- // we apply this diff as one '/' to the entry then
- entry.append("/", "");
+ // we apply this diff as no change at the given path
+ entry.append(path, "");
entry.done();
return;
}
@@ -139,8 +140,10 @@ public final class JournalEntry extends
// part of that hierarchy anymore and we 'move elsewhere'.
// eg if 'currentPath' is /a/b/e, then we must flush /a/b/c/d and
/a/b/c
while (node != null && !node.isAncestorOf(currentNode)) {
- // add parent to the diff entry
- entry.append(node.getPath(), getChanges(node));
+ // add parent to the diff entry if within scope
+ if (inScope(node, path)) {
+ entry.append(node.getPath(), getChanges(node));
+ }
deDuplicatedCnt++;
// clean up the hierarchy when we are done with this
// part of the tree to avoid excessive memory usage
@@ -165,7 +168,7 @@ public final class JournalEntry extends
// once we're done we still have the last hierarchy line contained in
'node',
// eg /x, /x/y, /x/y/z
// and that one we must now append to the diff cache entry:
- while (node != null) {
+ while (node != null && inScope(node, path)) {
entry.append(node.getPath(), getChanges(node));
deDuplicatedCnt++;
node = node.parent;
@@ -176,6 +179,14 @@ public final class JournalEntry extends
LOG.debug("applyTo: done. totalCnt: {}, deDuplicatedCnt: {}",
totalCnt, deDuplicatedCnt);
}
+ private static boolean inScope(TreeNode node, String path) {
+ if (PathUtils.denotesRoot(path)) {
+ return true;
+ }
+ String p = node.getPath();
+ return p.startsWith(path) && (p.length() == path.length() ||
p.charAt(path.length()) == '/');
+ }
+
/**
* Reads all external changes between the two given revisions (with the
same
* clusterId) from the journal and appends the paths therein to the
provided
@@ -192,10 +203,38 @@ public final class JournalEntry extends
* @throws IOException
*/
static int fillExternalChanges(@Nonnull StringSort sorter,
- @Nonnull Revision from,
- @Nonnull Revision to,
- @Nonnull DocumentStore store)
+ @Nonnull Revision from,
+ @Nonnull Revision to,
+ @Nonnull DocumentStore store)
throws IOException {
+ return fillExternalChanges(sorter, PathUtils.ROOT_PATH, from, to,
store);
+ }
+
+ /**
+ * Reads external changes between the two given revisions (with the same
+ * clusterId) from the journal and appends the paths therein to the
provided
+ * sorter. If there is no exact match of a journal entry for the given
+ * {@code to} revision, this method will fill external changes from the
+ * next higher journal entry that contains the revision. The {@code path}
+ * defines the scope of the external changes that should be read and filled
+ * into the {@code sorter}.
+ *
+ * @param sorter the StringSort to which all externally changed paths
+ * between the provided revisions will be added
+ * @param path a path that defines the scope of the changes to read.
+ * @param from the lower bound of the revision range (exclusive).
+ * @param to the upper bound of the revision range (inclusive).
+ * @param store the document store to query.
+ * @return the number of journal entries read from the store.
+ * @throws IOException
+ */
+ static int fillExternalChanges(@Nonnull StringSort sorter,
+ @Nonnull String path,
+ @Nonnull Revision from,
+ @Nonnull Revision to,
+ @Nonnull DocumentStore store)
+ throws IOException {
+ checkNotNull(path);
checkArgument(checkNotNull(from).getClusterId() ==
checkNotNull(to).getClusterId());
if (from.compareRevisionTime(to) >= 0) {
@@ -231,7 +270,7 @@ public final class JournalEntry extends
}
for (JournalEntry d : partialResult) {
- d.addTo(sorter);
+ d.addTo(sorter, path);
}
if (partialResult.size() < READ_CHUNK_SIZE) {
break;
@@ -248,7 +287,7 @@ public final class JournalEntry extends
|| (lastEntry != null &&
!lastEntry.getId().equals(inclusiveToId))) {
String maxId = asId(new Revision(Long.MAX_VALUE, 0,
to.getClusterId()));
for (JournalEntry d : store.query(JOURNAL, inclusiveToId, maxId,
1)) {
- d.addTo(sorter);
+ d.addTo(sorter, path);
numEntries++;
}
}
@@ -287,14 +326,6 @@ public final class JournalEntry extends
put(BRANCH_COMMITS, branchCommits);
}
- String getChanges(String path) {
- TreeNode node = getNode(path);
- if (node == null) {
- return "";
- }
- return getChanges(node);
- }
-
UpdateOp asUpdateOp(@Nonnull Revision revision) {
String id = asId(revision);
UpdateOp op = new UpdateOp(id, true);
@@ -309,18 +340,31 @@ public final class JournalEntry extends
return op;
}
- void addTo(final StringSort sort) throws IOException {
- TreeNode n = getChanges();
+ /**
+ * Add changed paths in this journal entry that are in the scope of
+ * {@code path} to {@code sort}.
+ *
+ * @param sort where changed paths are added to.
+ * @param path the scope for added paths.
+ * @throws IOException if an exception occurs while adding a path to
+ * {@code sort}. In this case only some paths may have been added.
+ */
+ void addTo(final StringSort sort, String path) throws IOException {
TraversingVisitor v = new TraversingVisitor() {
-
@Override
- public void node(TreeNode node, String path) throws IOException {
- sort.add(path);
+ public void node(TreeNode node, String p) throws IOException {
+ sort.add(p);
}
};
- n.accept(v, "/");
+ TreeNode n = getNode(path);
+ if (n != null) {
+ n.accept(v, path);
+ }
for (JournalEntry e : getBranchCommits()) {
- e.getChanges().accept(v, "/");
+ n = e.getNode(path);
+ if (n != null) {
+ n.accept(v, path);
+ }
}
}
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoaderTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoaderTest.java?rev=1771729&r1=1771728&r2=1771729&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoaderTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalDiffLoaderTest.java
Mon Nov 28 12:44:01 2016
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.plugin
import java.util.Set;
import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.cache.CacheStats;
import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
@@ -29,6 +30,8 @@ import org.junit.Test;
import static com.google.common.collect.Sets.newHashSet;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
public class JournalDiffLoaderTest {
@@ -162,6 +165,48 @@ public class JournalDiffLoaderTest {
assertEquals(newHashSet("foo", "bar", "baz"), changeChildNodes(ns1,
s1, s2));
}
+ @Test
+ public void withPath() throws Exception {
+ DocumentNodeStore ns = builderProvider.newBuilder()
+ .setAsyncDelay(0).getNodeStore();
+ NodeBuilder builder = ns.getRoot().builder();
+ builder.child("foo");
+ merge(ns, builder);
+ ns.runBackgroundOperations();
+ DocumentNodeState before = (DocumentNodeState)
ns.getRoot().getChildNode("foo");
+ builder = ns.getRoot().builder();
+ builder.child("bar");
+ merge(ns, builder);
+ ns.runBackgroundOperations();
+ builder = ns.getRoot().builder();
+ builder.child("foo").child("a").child("b").child("c");
+ merge(ns, builder);
+ ns.runBackgroundOperations();
+ builder = ns.getRoot().builder();
+ builder.child("bar").child("a").child("b").child("c");
+ merge(ns, builder);
+ ns.runBackgroundOperations();
+ DocumentNodeState after = (DocumentNodeState)
ns.getRoot().getChildNode("foo");
+
+ CacheStats cs = getMemoryDiffStats(ns);
+ assertNotNull(cs);
+ cs.resetStats();
+ Set<String> changes = changeChildNodes(ns, before, after);
+ assertEquals(1, changes.size());
+ assertTrue(changes.contains("a"));
+ // must only push /foo, /foo/a, /foo/a/b and /foo/a/b/c into cache
+ assertEquals(4, cs.getElementCount());
+ }
+
+ private static CacheStats getMemoryDiffStats(DocumentNodeStore ns) {
+ for (CacheStats cs : ns.getDiffCache().getStats()) {
+ if (cs.getName().equals("Document-MemoryDiff")) {
+ return cs;
+ }
+ }
+ return null;
+ }
+
private static Set<String> changeChildNodes(DocumentNodeStore store,
AbstractDocumentNodeState
before,
AbstractDocumentNodeState
after) {
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java?rev=1771729&r1=1771728&r2=1771729&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java
Mon Nov 28 12:44:01 2016
@@ -56,7 +56,7 @@ public class JournalEntryTest {
RevisionVector from = new RevisionVector(new Revision(1, 0, 1));
RevisionVector to = new RevisionVector(new Revision(2, 0, 1));
sort.sort();
- JournalEntry.applyTo(sort, cache, from, to);
+ JournalEntry.applyTo(sort, cache, "/", from, to);
for (String p : paths) {
String changes = cache.getChanges(from, to, p, null);
@@ -68,6 +68,29 @@ public class JournalEntryTest {
sort.close();
}
+ @Test
+ public void applyToWithPath() throws Exception {
+ DiffCache cache = new MemoryDiffCache(new DocumentMK.Builder());
+ StringSort sort = JournalEntry.newSorter();
+ sort.add("/");
+ sort.add("/foo");
+ sort.add("/foo/a");
+ sort.add("/foo/b");
+ sort.add("/bar");
+ sort.add("/bar/a");
+ sort.add("/bar/b");
+ RevisionVector from = new RevisionVector(Revision.newRevision(1));
+ RevisionVector to = new RevisionVector(Revision.newRevision(1));
+ sort.sort();
+ JournalEntry.applyTo(sort, cache, "/foo", from, to);
+ assertNotNull(cache.getChanges(from, to, "/foo", null));
+ assertNotNull(cache.getChanges(from, to, "/foo/a", null));
+ assertNotNull(cache.getChanges(from, to, "/foo/b", null));
+ assertNull(cache.getChanges(from, to, "/bar", null));
+ assertNull(cache.getChanges(from, to, "/bar/a", null));
+ assertNull(cache.getChanges(from, to, "/bar/b", null));
+ }
+
//OAK-3494
@Test
public void useParentDiff() throws Exception {
@@ -98,7 +121,7 @@ public class JournalEntryTest {
StringSort sort = JournalEntry.newSorter();
add(sort, paths);
sort.sort();
- JournalEntry.applyTo(sort, cache, from, to);
+ JournalEntry.applyTo(sort, cache, "/", from, to);
validateCacheUsage(cache, from, to, "/topUnchanged", true);
validateCacheUsage(cache, from, to, "/content/changed/unchangedLeaf",
true);
@@ -206,6 +229,31 @@ public class JournalEntryTest {
}
@Test
+ public void fillExternalChangesWithPath() throws Exception {
+ Revision r1 = new Revision(1, 0, 1);
+ Revision r2 = new Revision(2, 0, 1);
+ DocumentStore store = new MemoryDocumentStore();
+ JournalEntry entry = JOURNAL.newDocument(store);
+ entry.modified("/");
+ entry.modified("/foo");
+ entry.modified("/foo/a");
+ entry.modified("/foo/b");
+ entry.modified("/foo/c");
+ entry.modified("/bar");
+ entry.modified("/bar/a");
+ entry.modified("/bar/b");
+ entry.modified("/bar/c");
+
+ UpdateOp op = entry.asUpdateOp(r2);
+ assertTrue(store.create(JOURNAL, Collections.singletonList(op)));
+
+ StringSort sort = JournalEntry.newSorter();
+ JournalEntry.fillExternalChanges(sort, "/foo", r1, r2, store);
+ assertEquals(4, sort.getSize());
+ sort.close();
+ }
+
+ @Test
public void getRevisionTimestamp() throws Exception {
DocumentStore store = new MemoryDocumentStore();
JournalEntry entry = JOURNAL.newDocument(store);
@@ -234,7 +282,7 @@ public class JournalEntryTest {
t.start();
StringSort sort = JournalEntry.newSorter();
try {
- entry.addTo(sort);
+ entry.addTo(sort, PathUtils.ROOT_PATH);
} finally {
sort.close();
}
@@ -244,6 +292,25 @@ public class JournalEntryTest {
}
}
+ @Test
+ public void addToWithPath() throws Exception {
+ DocumentStore store = new MemoryDocumentStore();
+ JournalEntry entry = JOURNAL.newDocument(store);
+ entry.modified("/");
+ entry.modified("/foo");
+ entry.modified("/foo/a");
+ entry.modified("/foo/b");
+ entry.modified("/foo/c");
+ entry.modified("/bar");
+ entry.modified("/bar/a");
+ entry.modified("/bar/b");
+ entry.modified("/bar/c");
+ StringSort sort = JournalEntry.newSorter();
+ entry.addTo(sort, "/foo");
+ assertEquals(4, sort.getSize());
+ sort.close();
+ }
+
private static void addRandomPaths(java.util.Collection<String> paths)
throws IOException {
paths.add("/");
Random random = new Random(42);
