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 ca...@apache.org on 2018/02/27 15:58:01 UTC
svn commit: r1825466 - in /jackrabbit/oak/trunk/oak-run/src:
main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/
test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/
Author: catholicon
Date: Tue Feb 27 15:58:01 2018
New Revision: 1825466
URL: http://svn.apache.org/viewvc?rev=1825466&view=rev
Log:
OAK-7285: Reindexing using --doc-traversal-mode can OOM while aggregation in some cases
Instead of using number_of_preferred_children, we now pass along preferred_childred set.
While gettingChildNode by name which is present in preffered set, we break iteration on encountering any non-preferred child.
Fixed tests to adhere to this new parameter and in particular fixed ChildNodeStateProviderTest#hasChildNode_InLimit.
The test earlier assumed that some children won't come in getChildNode if they aren't preferred.
Modified:
jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProvider.java
jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilder.java
jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStore.java
jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java
jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProviderTest.java
jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIteratorTest.java
Modified: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProvider.java?rev=1825466&r1=1825465&r2=1825466&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProvider.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProvider.java Tue Feb 27 15:58:01 2018
@@ -25,15 +25,16 @@ import javax.annotation.Nonnull;
import com.google.common.base.Optional;
import com.google.common.collect.AbstractIterator;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import com.google.common.collect.PeekingIterator;
+import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.index.indexer.document.NodeStateEntry;
import org.apache.jackrabbit.oak.plugins.memory.MemoryChildNodeEntry;
import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import static com.google.common.base.Preconditions.checkState;
-import static com.google.common.collect.Iterators.limit;
import static com.google.common.collect.Iterators.size;
import static com.google.common.collect.Iterators.transform;
import static java.util.Collections.emptyIterator;
@@ -45,12 +46,12 @@ import static org.apache.jackrabbit.oak.
class ChildNodeStateProvider {
private final Iterable<NodeStateEntry> entries;
private final String path;
- private final int checkChildLimit;
+ private final Iterable<String> preferredPathElements;
- public ChildNodeStateProvider(Iterable<NodeStateEntry> entries, String path, int checkChildLimit) {
+ public ChildNodeStateProvider(Iterable<NodeStateEntry> entries, String path, Iterable<String> preferredPathElements) {
this.entries = entries;
this.path = path;
- this.checkChildLimit = checkChildLimit;
+ this.preferredPathElements = preferredPathElements;
}
public boolean hasChildNode(@Nonnull String name) {
@@ -59,7 +60,8 @@ class ChildNodeStateProvider {
@Nonnull
public NodeState getChildNode(@Nonnull String name) throws IllegalArgumentException {
- Optional<NodeStateEntry> o = Iterators.tryFind(limit(children(), checkChildLimit), p -> name.equals(name(p)));
+ boolean isPreferred = Iterables.contains(preferredPathElements, name);
+ Optional<NodeStateEntry> o = Iterators.tryFind(children(isPreferred), p -> name.equals(name(p)));
return o.isPresent() ? o.get().getNodeState() : MISSING_NODE;
}
@@ -80,6 +82,10 @@ class ChildNodeStateProvider {
}
Iterator<NodeStateEntry> children() {
+ return children(false);
+ }
+
+ Iterator<NodeStateEntry> children(boolean preferred) {
PeekingIterator<NodeStateEntry> pitr = Iterators.peekingIterator(entries.iterator());
if (!pitr.hasNext()) {
return emptyIterator();
@@ -100,7 +106,12 @@ class ChildNodeStateProvider {
@Override
protected NodeStateEntry computeNext() {
if (pitr.hasNext() && isAncestor(path, pitr.peek().getPath())) {
- return pitr.next();
+ NodeStateEntry nextEntry = pitr.next();
+ String nextEntryName = PathUtils.getName(nextEntry.getPath());
+ if (preferred && !Iterables.contains(preferredPathElements, nextEntryName)) {
+ return endOfData();
+ }
+ return nextEntry;
}
return endOfData();
}
Modified: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilder.java?rev=1825466&r1=1825465&r2=1825466&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileNodeStoreBuilder.java Tue Feb 27 15:58:01 2018
@@ -30,7 +30,7 @@ import org.apache.jackrabbit.oak.spi.blo
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static com.google.common.collect.Iterables.size;
+import static com.google.common.collect.Iterables.unmodifiableIterable;
public class FlatFileNodeStoreBuilder {
public static final String OAK_INDEXER_USE_ZIP = "oak.indexer.useZip";
@@ -70,7 +70,7 @@ public class FlatFileNodeStoreBuilder {
comparator = new PathElementComparator(preferredPathElements);
entryWriter = new NodeStateEntryWriter(blobStore);
FlatFileStore store = new FlatFileStore(createdSortedStoreFile(), new NodeStateEntryReader(blobStore),
- size(preferredPathElements), useZip);
+ unmodifiableIterable(preferredPathElements), useZip);
if (entryCount > 0) {
store.setEntryCount(entryCount);
}
Modified: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStore.java?rev=1825466&r1=1825465&r2=1825466&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStore.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStore.java Tue Feb 27 15:58:01 2018
@@ -35,14 +35,14 @@ public class FlatFileStore implements It
private final Closer closer = Closer.create();
private final File storeFile;
private final NodeStateEntryReader entryReader;
- private final int checkChildLimit;
+ private final Iterable<String> preferredPathElements;
private final boolean compressionEnabled;
private long entryCount = -1;
- public FlatFileStore(File storeFile, NodeStateEntryReader entryReader, int checkChildLimit, boolean compressionEnabled) {
+ public FlatFileStore(File storeFile, NodeStateEntryReader entryReader, Iterable<String> preferredPathElements, boolean compressionEnabled) {
this.storeFile = storeFile;
this.entryReader = entryReader;
- this.checkChildLimit = checkChildLimit;
+ this.preferredPathElements = preferredPathElements;
this.compressionEnabled = compressionEnabled;
}
@@ -56,7 +56,7 @@ public class FlatFileStore implements It
@Override
public Iterator<NodeStateEntry> iterator() {
- return new FlatFileStoreIterator(createBaseIterator(), checkChildLimit);
+ return new FlatFileStoreIterator(createBaseIterator(), preferredPathElements);
}
private Iterator<NodeStateEntry> createBaseIterator() {
Modified: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java?rev=1825466&r1=1825465&r2=1825466&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java Tue Feb 27 15:58:01 2018
@@ -37,12 +37,12 @@ class FlatFileStoreIterator extends Abst
private final Iterator<NodeStateEntry> baseItr;
private final CursorableLinkedList buffer = new CursorableLinkedList();
private NodeStateEntry current;
- private final int checkChildLimit;
+ private final Iterable<String> preferredPathElements;
private int maxBufferSize;
- public FlatFileStoreIterator(Iterator<NodeStateEntry> baseItr, int checkChildLimit) {
+ public FlatFileStoreIterator(Iterator<NodeStateEntry> baseItr, Iterable<String> preferredPathElements) {
this.baseItr = baseItr;
- this.checkChildLimit = checkChildLimit;
+ this.preferredPathElements = preferredPathElements;
}
int getBufferSize(){
@@ -77,7 +77,7 @@ class FlatFileStoreIterator extends Abst
private NodeStateEntry wrap(NodeStateEntry baseEntry) {
NodeState state = new LazyChildrenNodeState(baseEntry.getNodeState(),
- new ChildNodeStateProvider(getEntries(), baseEntry.getPath(), checkChildLimit));
+ new ChildNodeStateProvider(getEntries(), baseEntry.getPath(), preferredPathElements));
return new NodeStateEntry(state, baseEntry.getPath());
}
Modified: jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProviderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProviderTest.java?rev=1825466&r1=1825465&r2=1825466&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProviderTest.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/ChildNodeStateProviderTest.java Tue Feb 27 15:58:01 2018
@@ -47,7 +47,8 @@ public class ChildNodeStateProviderTest
@Test
public void emptyCase() {
- ChildNodeStateProvider p = new ChildNodeStateProvider(emptyList(), "/a", 5);
+ Set<String> preferred = ImmutableSet.of("u", "v", "x", "y", "z");
+ ChildNodeStateProvider p = new ChildNodeStateProvider(emptyList(), "/a", preferred);
assertEquals(0, p.getChildNodeCount(1));
assertEquals(0, Iterables.size(p.getChildNodeNames()));
assertEquals(0, Iterables.size(p.getChildNodeEntries()));
@@ -59,18 +60,18 @@ public class ChildNodeStateProviderTest
public void children() {
Set<String> preferred = ImmutableSet.of("jcr:content", "x");
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/jcr:content", "/a/c", "/a/d", "/e", "/e/f", "/g", "/h"));
- ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", 100);
+ ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", preferred);
assertEquals(asList("jcr:content", "c", "d"), copyOf(childNames(p.children())));
assertEquals(5, citr.getCount());
citr.reset();
- p = new ChildNodeStateProvider(citr, "/e", 100);
+ p = new ChildNodeStateProvider(citr, "/e", preferred);
assertEquals(singletonList("f"), copyOf(childNames(p.children())));
assertEquals(7, citr.getCount());
- p = new ChildNodeStateProvider(citr, "/g", 100);
+ p = new ChildNodeStateProvider(citr, "/g", preferred);
assertEquals(emptyList(), copyOf(childNames(p.children())));
}
@@ -78,23 +79,23 @@ public class ChildNodeStateProviderTest
public void children2() {
Set<String> preferred = ImmutableSet.of("b");
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/b", "/a/b/c", "/a/b/c/d", "/e", "/e/f", "/g", "/h"));
- ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", 100);
+ ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", preferred);
assertEquals(singletonList("b"), copyOf(childNames(p.children())));
assertEquals(5, citr.getCount());
citr.reset();
- p = new ChildNodeStateProvider(citr, "/a/b", 100);
+ p = new ChildNodeStateProvider(citr, "/a/b", preferred);
assertEquals(singletonList("c"), copyOf(childNames(p.children())));
assertEquals(5, citr.getCount());
- p = new ChildNodeStateProvider(citr, "/a/b/c", 100);
+ p = new ChildNodeStateProvider(citr, "/a/b/c", preferred);
assertEquals(singletonList("d"), copyOf(childNames(p.children())));
- p = new ChildNodeStateProvider(citr, "/a/b/c/d", 100);
+ p = new ChildNodeStateProvider(citr, "/a/b/c/d", preferred);
assertEquals(emptyList(), copyOf(childNames(p.children())));
- p = new ChildNodeStateProvider(citr, "/h", 100);
+ p = new ChildNodeStateProvider(citr, "/h", preferred);
assertEquals(emptyList(), copyOf(childNames(p.children())));
}
@@ -102,18 +103,36 @@ public class ChildNodeStateProviderTest
public void hasChildNode_InLimit() {
Set<String> preferred = ImmutableSet.of("jcr:content", "x");
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/jcr:content", "/a/c", "/a/d", "/e", "/e/f"));
- ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", preferred.size());
+ ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", preferred);
+ citr.reset();
assertTrue(p.hasChildNode("jcr:content"));
+ assertEquals("Unexpected number of reads to get jcr:content", 2, citr.getCount());
+
+ citr.reset();
+ assertFalse(p.hasChildNode("x"));
+ assertEquals("Unexpected number reads to conclude that preferred child 'x' is missing",
+ 3, citr.getCount());
+
+ citr.reset();
assertTrue(p.hasChildNode("c"));
- assertFalse(p.hasChildNode("d"));
+ assertEquals("Unexpected number reads to get 'c'", 3, citr.getCount());
+
+ citr.reset();
+ assertTrue(p.hasChildNode("d"));
+ assertEquals("Unexpected number reads to get 'd'", 4, citr.getCount());
+
+ citr.reset();
+ assertFalse(p.hasChildNode("y"));
+ assertEquals("Unexpected number reads to conclude that non-preferred child 'x' is missing",
+ 5, citr.getCount());
}
@Test
public void childCount() {
Set<String> preferred = ImmutableSet.of("jcr:content", "x");
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/jcr:content", "/a/c", "/a/d", "/e", "/e/f"));
- ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", preferred.size());
+ ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", preferred);
assertEquals(1, p.getChildNodeCount(1));
assertEquals(3, p.getChildNodeCount(2));
}
@@ -122,7 +141,7 @@ public class ChildNodeStateProviderTest
public void childNames() {
Set<String> preferred = ImmutableSet.of("jcr:content");
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/jcr:content", "/a/c", "/a/d", "/e", "/e/f"));
- ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", 100);
+ ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", preferred);
assertEquals(asList("jcr:content", "c", "d"), copyOf(childNames(p.children())));
assertEquals(5, citr.getCount());
@@ -133,7 +152,7 @@ public class ChildNodeStateProviderTest
Set<String> preferred = ImmutableSet.of("jcr:content");
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/jcr:content", "/a/jcr:content/metadata",
"/a/c", "/a/c/status","/a/d", "/e", "/e/f"));
- ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", 100);
+ ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", preferred);
assertEquals(asList("jcr:content", "c", "d"), copyOf(childNames(p.children())));
assertEquals(7, citr.getCount());
@@ -143,7 +162,7 @@ public class ChildNodeStateProviderTest
public void childEntries() {
Set<String> preferred = ImmutableSet.of("jcr:content");
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/jcr:content", "/a/c", "/a/d", "/e", "/e/f"));
- ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", 100);
+ ChildNodeStateProvider p = new ChildNodeStateProvider(citr, "/a", preferred);
Map<String, NodeState> children = new HashMap<>();
p.getChildNodeEntries().forEach(e -> children.put(e.getName(), e.getNodeState()));
Modified: jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIteratorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIteratorTest.java?rev=1825466&r1=1825465&r2=1825466&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIteratorTest.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIteratorTest.java Tue Feb 27 15:58:01 2018
@@ -41,7 +41,7 @@ public class FlatFileStoreIteratorTest {
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/jcr:content", "/a/jcr:content/metadata",
"/a/d", "/e"));
- FlatFileStoreIterator fitr = new FlatFileStoreIterator(citr.iterator(), preferred.size());
+ FlatFileStoreIterator fitr = new FlatFileStoreIterator(citr.iterator(), preferred);
NodeStateEntry a = fitr.next();
assertEquals("/a", a.getPath());
@@ -76,7 +76,7 @@ public class FlatFileStoreIteratorTest {
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/jcr:content", "/a/jcr:content/metadata",
"/a/d", "/e"));
- FlatFileStoreIterator fitr = new FlatFileStoreIterator(citr.iterator(), preferred.size());
+ FlatFileStoreIterator fitr = new FlatFileStoreIterator(citr.iterator(), preferred);
NodeStateEntry a = fitr.next();
assertEquals("/a", a.getPath());
@@ -106,7 +106,7 @@ public class FlatFileStoreIteratorTest {
CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/j:c", "/a/j:c/j:c", "/a/b"));
- FlatFileStoreIterator fitr = new FlatFileStoreIterator(citr.iterator(), preferred.size());
+ FlatFileStoreIterator fitr = new FlatFileStoreIterator(citr.iterator(), preferred);
NodeStateEntry a = fitr.next();
assertEquals("/a", a.getPath());
@@ -122,4 +122,24 @@ public class FlatFileStoreIteratorTest {
}
}
}
+
+ // OAK-7285
+ @Test
+ public void getChildNodeLimitedByNonPreferred() {
+ // have more than 1 preferred names
+ Set<String> preferred = ImmutableSet.of("j:c", "md");
+
+ CountingIterable<NodeStateEntry> citr = createList(preferred, asList("/a", "/a/b", "/a/c"));
+
+ FlatFileStoreIterator fitr = new FlatFileStoreIterator(citr.iterator(), preferred);
+
+ NodeStateEntry a = fitr.next();
+ assertEquals("/a", a.getPath());
+
+ NodeState aNS = a.getNodeState();
+ aNS.getChildNode("j:c");
+
+ // Don't read whole tree to conclude that "j:c" doesn't exist (reading /a/b should imply that it doesn't exist)
+ assertEquals(1, fitr.getBufferSize());
+ }
}
\ No newline at end of file