diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java index ca8b67d03a7..9c73ee0e677 100644 --- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java +++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeState.java @@ -268,16 +268,27 @@ public long getChildNodeCount(long max) { return bundledChildCount; } - if (max > DocumentNodeStore.NUM_CHILDREN_CACHE_LIMIT) { - // count all - return Iterables.size(getChildNodeEntries()); + String name = null; + long count = 0; + int fetchSize = INITIAL_FETCH_SIZE; + long remaining = Math.max(max, 1); // fetch at least once + Children c = NO_CHILDREN; + while (remaining > 0) { + c = store.getChildren(this, name, fetchSize); + count += c.children.size(); + remaining -= c.children.size(); + if (!c.hasMore) { + break; + } + name = c.children.get(c.children.size() - 1); + fetchSize = Math.min(fetchSize << 1, MAX_FETCH_SIZE); } - Children c = store.getChildren(this, null, (int) max); - if (c.hasMore) { - return Long.MAX_VALUE; - } else { + if (!c.hasMore) { // we know the exact value - return c.children.size() + bundledChildCount; + return count + bundledChildCount; + } else { + // there are more than max + return Long.MAX_VALUE; } } diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java index 56d78cd2184..8c326dec2a7 100644 --- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java +++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java @@ -147,11 +147,6 @@ public final class DocumentNodeStore public static final FormatVersion VERSION = FormatVersion.V1_8; - /** - * Do not cache more than this number of children for a document. - */ - static final int NUM_CHILDREN_CACHE_LIMIT = Integer.getInteger("oak.documentMK.childrenCacheLimit", 16 * 1024); - /** * List of meta properties which are created by DocumentNodeStore and which needs to be * retained in any cloned copy of DocumentNodeState. diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java index 08d5dec127c..af15449bd0a 100644 --- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java +++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java @@ -16,6 +16,7 @@ */ package org.apache.jackrabbit.oak.plugins.document; +import static com.google.common.collect.ImmutableList.of; import static java.util.Collections.emptyList; import static java.util.Collections.synchronizedList; import static java.util.concurrent.TimeUnit.SECONDS; @@ -52,6 +53,7 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Random; @@ -3549,7 +3551,6 @@ public void diffWithBrokenJournal() throws Exception { } // OAK-2621 - @Ignore("OAK-2621") @Test public void getChildNodeCount() throws Exception { CountingDocumentStore store = new CountingDocumentStore(new MemoryDocumentStore()); @@ -3573,6 +3574,56 @@ public void getChildNodeCount() throws Exception { assertEquals(1, store.getNumQueryCalls(NODES)); } + @Test + public void getChildNodeCountTest() throws Exception { + final long UL = Long.MAX_VALUE; // unknown + // childNodeCount = none + getChildNodeCountTest(0, + of(0L, 1L), + of(0L, 0L) + ); + // childNodeCount = less than initial fetch size 42 + getChildNodeCountTest(42, + of( 0L, 1L, 41L, 42L, 43L, 100L), + of(42L, 42L, 42L, 42L, 42L, 42L) + ); + // childNodeCount = initial fetch size (100) + getChildNodeCountTest(100, + of( 0L, 1L, 99L, 100L, 101L, 200L), + of(100L, 100L, 100L, 100L, 100L, 100L) + ); + // childNodeCount = initial fetch size + 1 (100 + 1) + getChildNodeCountTest(101, + of(0L, 1L, 99L, 100L, 101L, 200L), + of(UL, UL, UL, UL, 101L, 101L) + ); + // childNodeCount = first two fetches (100 + 200) + getChildNodeCountTest(300, + of(0L, 1L, 99L, 100L, 101L, 200L, 299L, 300L, 301L, 400L), + of(UL, UL, UL, UL, 300L, 300L, 300L, 300L, 300L, 300L) + ); + } + + private void getChildNodeCountTest(int numChildren, + Iterable maxValues, + Iterable expectedValues) + throws Exception { + DocumentNodeStore ns = builderProvider.newBuilder() + .setAsyncDelay(0).getNodeStore(); + NodeBuilder builder = ns.getRoot().builder(); + for (int i = 0; i < numChildren; i++) { + builder.child("test").child("node-" + i); + } + merge(ns, builder); + ns.getNodeChildrenCache().invalidateAll(); + + NodeState test = ns.getRoot().getChildNode("test"); + Iterator expected = expectedValues.iterator(); + for (long max : maxValues) { + assertEquals(expected.next().longValue(), test.getChildNodeCount(max)); + } + } + private static class WriteCountingStore extends MemoryDocumentStore { private final ThreadLocal createMulti = new ThreadLocal<>(); int count; diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/GetChildNodeCountTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/GetChildNodeCountTest.java new file mode 100644 index 00000000000..f45f4ab36c1 --- /dev/null +++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/GetChildNodeCountTest.java @@ -0,0 +1,93 @@ +/* + * 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.document; + +import javax.jcr.SimpleCredentials; + +import org.apache.jackrabbit.oak.Oak; +import org.apache.jackrabbit.oak.api.ContentRepository; +import org.apache.jackrabbit.oak.api.ContentSession; +import org.apache.jackrabbit.oak.api.Root; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore; +import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexEditorProvider; +import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import static com.google.common.collect.Lists.newArrayList; +import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES; +import static org.apache.jackrabbit.oak.plugins.document.util.Utils.closeIfCloseable; +import static org.hamcrest.Matchers.lessThan; +import static org.junit.Assert.assertThat; + +public class GetChildNodeCountTest { + + @Rule + public DocumentMKBuilderProvider builderProvider = new DocumentMKBuilderProvider(); + + private CountingDocumentStore store = new CountingDocumentStore(new MemoryDocumentStore()); + + private DocumentNodeStore ns; + + private ContentRepository repository; + + private ContentSession session; + + @Before + public void before() throws Exception { + ns = builderProvider.newBuilder().setAsyncDelay(0) + .setDocumentStore(store).getNodeStore(); + repository = new Oak(ns) + .with(new PropertyIndexEditorProvider()) + .with(new OpenSecurityProvider()) + .createContentRepository(); + session = repository.login(new SimpleCredentials("admin", "admin".toCharArray()), null); + Root root = session.getLatestRoot(); + Tree idx = root.getTree("/").addChild("oak:index").addChild("p"); + idx.setProperty("type", "property"); + idx.setProperty("propertyNames", newArrayList("p"), Type.NAMES); + root.commit(); + } + + @After + public void after() throws Exception { + session.close(); + closeIfCloseable(repository); + ns.dispose(); + } + + @Test + public void removeIndexedNodes() throws Exception { + Root root = session.getLatestRoot(); + Tree t = root.getTree("/").addChild("test"); + for (int i = 0; i < 200; i ++) { + t.addChild("node-" + i).setProperty("p", "v"); + } + root.commit(); + ns.getNodeChildrenCache().invalidateAll(); + store.resetCounters(); + // remove nodes + root = session.getLatestRoot(); + root.getTree("/test").remove(); + root.commit(); + assertThat(store.getNumQueryCalls(NODES), lessThan(13)); + } +}