You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "thomasmueller (via GitHub)" <gi...@apache.org> on 2023/11/15 15:56:54 UTC
[PR] OAK-10549 Improve performance of facet count at scale (Lucene) [jackrabbit-oak]
thomasmueller opened a new pull request, #1215:
URL: https://github.com/apache/jackrabbit-oak/pull/1215
I didn't run all the tests yet; will do this now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10549 Improve performance of facet count at scale (Lucene) [jackrabbit-oak]
Posted by "thomasmueller (via GitHub)" <gi...@apache.org>.
thomasmueller commented on code in PR #1215:
URL: https://github.com/apache/jackrabbit-oak/pull/1215#discussion_r1395684061
##########
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java:
##########
@@ -1677,6 +1708,28 @@ private List<Facet> getFacetsUncached(int numberOfFacets, String columnName) thr
indexNode.release();
}
}
+
+ private List<Facet> getFacetsUncached(Facets facets, int numberOfFacets, String columnName) throws IOException {
Review Comment:
I did this on purpose actually: to keep the old code as is as much as possible, in case we need to switch back to it. I agree it would be better, but hopefully we don't need the old code in the future - in which case we should remove it. I'm fine to create a jira issue about removing the old code in 6 month, so we don't forget. OK?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10549 Improve performance of facet count at scale (Lucene) [jackrabbit-oak]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1215:
URL: https://github.com/apache/jackrabbit-oak/pull/1215#issuecomment-1814509482
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&metric=new_coverage&view=list) [87.0% Coverage](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1215&metric=new_duplicated_lines_density&view=list)
 The version of Java (11.0.21) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10549 Improve performance of facet count at scale (Lucene) [jackrabbit-oak]
Posted by "fabriziofortino (via GitHub)" <gi...@apache.org>.
fabriziofortino commented on code in PR #1215:
URL: https://github.com/apache/jackrabbit-oak/pull/1215#discussion_r1395604781
##########
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java:
##########
@@ -1644,11 +1647,39 @@ public List<Facet> getFacets(int numberOfFacets, String columnName) throws IOExc
return cachedResults.get(cacheKey);
}
LOG.trace("columnName = {} facet Data not present in cache...", columnName);
+ if (EAGER_FACET_CACHE_FILL) {
+ fillFacetCache(numberOfFacets);
+ if (cachedResults.containsKey(cacheKey)) {
+ LOG.trace("columnName = {} now found");
Review Comment:
`cacheKey` is missing
```suggestion
LOG.trace("columnName = {} now found", cacheKey);
```
##########
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java:
##########
@@ -1677,6 +1708,28 @@ private List<Facet> getFacetsUncached(int numberOfFacets, String columnName) thr
indexNode.release();
}
}
+
+ private List<Facet> getFacetsUncached(Facets facets, int numberOfFacets, String columnName) throws IOException {
+ String facetFieldName = FulltextIndex.parseFacetField(columnName);
+ try {
+ ImmutableList.Builder<Facet> res = new ImmutableList.Builder<>();
+ FacetResult topChildren = facets.getTopChildren(numberOfFacets, facetFieldName);
+ if (topChildren == null) {
+ return null;
+ }
+ for (LabelAndValue lav : topChildren.labelValues) {
+ res.add(new Facet(
+ lav.label, lav.value.intValue()
+ ));
+ }
+ return res.build();
+ } catch (IllegalArgumentException iae) {
+ LOG.debug(iae.getMessage(), iae);
+ LOG.warn("facets for {} not yet indexed: " + iae, facetFieldName);
Review Comment:
this message mixes placeholders + interpolation. I would prefer to use just placeholders:
```suggestion
LOG.warn("facets for {} not yet indexed: {}", facetFieldName, iae);
```
##########
oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ManyFacetsTest.java:
##########
@@ -0,0 +1,280 @@
+/*
+ * 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.index.lucene.hybrid;
+
+import static org.apache.jackrabbit.guava.common.util.concurrent.MoreExecutors.newDirectExecutorService;
+import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.PROP_FACETS;
+import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.STATISTICAL_FACET_SAMPLE_SIZE_DEFAULT;
+import static org.apache.jackrabbit.oak.spi.mount.Mounts.defaultMountInfoProvider;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Properties;
+import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+
+import javax.jcr.GuestCredentials;
+import javax.jcr.Repository;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.query.Query;
+import javax.jcr.query.QueryManager;
+import javax.jcr.query.QueryResult;
+import javax.jcr.query.Row;
+import javax.jcr.query.RowIterator;
+
+import org.apache.jackrabbit.oak.InitialContent;
+import org.apache.jackrabbit.oak.Oak;
+import org.apache.jackrabbit.oak.api.ContentRepository;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
+import org.apache.jackrabbit.oak.commons.json.JsonObject;
+import org.apache.jackrabbit.oak.jcr.Jcr;
+import org.apache.jackrabbit.oak.plugins.index.AsyncIndexUpdate;
+import org.apache.jackrabbit.oak.plugins.index.counter.NodeCounterEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.lucene.IndexCopier;
+import org.apache.jackrabbit.oak.plugins.index.lucene.IndexTracker;
+import org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexProvider;
+import org.apache.jackrabbit.oak.plugins.index.lucene.LucenePropertyIndex;
+import org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil;
+import org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil.OptionalEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.lucene.reader.DefaultIndexReaderFactory;
+import org.apache.jackrabbit.oak.plugins.index.lucene.reader.LuceneIndexReaderFactory;
+import org.apache.jackrabbit.oak.plugins.index.lucene.util.LuceneIndexDefinitionBuilder;
+import org.apache.jackrabbit.oak.plugins.index.nodetype.NodeTypeIndexProvider;
+import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants;
+import org.apache.jackrabbit.oak.plugins.index.search.util.IndexDefinitionBuilder.PropertyRule;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
+import org.apache.jackrabbit.oak.query.AbstractQueryTest;
+import org.apache.jackrabbit.oak.spi.commit.Observer;
+import org.apache.jackrabbit.oak.spi.mount.MountInfoProvider;
+import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider;
+import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
+import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
+import org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils;
+import org.apache.jackrabbit.oak.stats.Clock;
+import org.apache.jackrabbit.oak.stats.StatisticsProvider;
+import org.jetbrains.annotations.Nullable;
+import org.junit.After;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class ManyFacetsTest extends AbstractQueryTest {
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder(new File("target"));
+
+ private static final int NUM_LABELS = 4;
+ private static final int NUM_LEAF_NODES = STATISTICAL_FACET_SAMPLE_SIZE_DEFAULT;
+ private static final String FACET_PROP = "facets";
+ private static final long REFRESH_DELTA = TimeUnit.SECONDS.toMillis(1);
+
+ private static final int FACET_COUNT = 200;
+
+ private ExecutorService executorService = Executors.newFixedThreadPool(2);
+ private OptionalEditorProvider optionalEditorProvider = new OptionalEditorProvider();
+ private NRTIndexFactory nrtIndexFactory;
+ private LuceneIndexProvider luceneIndexProvider;
+ private NodeStore nodeStore;
+ private DocumentQueue queue;
+ private Clock clock = new Clock.Virtual();
+ private Whiteboard wb;
+ private QueryManager qm;
+ private Repository jcrRepo;
+ private Jcr jcr;
+ private Oak oak;
+ // backup original system properties i.e. before test started
+ private final Properties backupProperties = (Properties) System.getProperties().clone();
+
+ @After
+ public void tearDown() throws IOException {
+ luceneIndexProvider.close();
+ new ExecutorCloser(executorService).close();
+ nrtIndexFactory.close();
+ // restore original system properties i.e. before test started
+ System.setProperties(backupProperties);
+ }
+
+ @Override
+ protected ContentRepository createRepository() {
+ IndexCopier copier;
+ try {
+ copier = new IndexCopier(executorService, temporaryFolder.getRoot());
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ MountInfoProvider mip = defaultMountInfoProvider();
+ nrtIndexFactory = new NRTIndexFactory(copier, clock, TimeUnit.MILLISECONDS.toSeconds(REFRESH_DELTA), StatisticsProvider.NOOP);
+ nrtIndexFactory.setAssertAllResourcesClosed(true);
+ LuceneIndexReaderFactory indexReaderFactory = new DefaultIndexReaderFactory(mip, copier);
+ IndexTracker tracker = new IndexTracker(indexReaderFactory, nrtIndexFactory);
+ luceneIndexProvider = new LuceneIndexProvider(tracker);
+ queue = new DocumentQueue(100, tracker, newDirectExecutorService());
+ LuceneIndexEditorProvider editorProvider = new LuceneIndexEditorProvider(copier,
+ tracker,
+ null,
+ null,
+ mip);
+ editorProvider.setIndexingQueue(queue);
+ LocalIndexObserver localIndexObserver = new LocalIndexObserver(queue, StatisticsProvider.NOOP);
+ nodeStore = new MemoryNodeStore();
+ oak = new Oak(nodeStore)
+ .with(new InitialContent())
+ .with(new OpenSecurityProvider())
+ .with((QueryIndexProvider) luceneIndexProvider)
+ .with((Observer) luceneIndexProvider)
+ .with(localIndexObserver)
+ .with(editorProvider)
+ .with(new PropertyIndexEditorProvider())
+ .with(new NodeTypeIndexProvider())
+ .with(optionalEditorProvider)
+ .with(new NodeCounterEditorProvider())
+ //Effectively disable async indexing auto run
+ //such that we can control run timing as per test requirement
+ .withAsyncIndexing("async", TimeUnit.DAYS.toSeconds(1));
+
+ wb = oak.getWhiteboard();
+ ContentRepository repo = oak.createContentRepository();
+ return repo;
+ }
+
+ private void createSmallDataset(int k) throws RepositoryException {
+ Random random = new Random(42);
+ Tree par = createPath("/parent" + k);
+ par.setProperty("foo", "bar");
+ for (int i = 0; i < NUM_LABELS * 2; i++) {
+ Tree subPar = par.addChild("par" + i);
+ for (int j = 0; j < NUM_LEAF_NODES / (2 * NUM_LABELS); j++) {
+ Tree child = subPar.addChild("c" + j);
+ child.setProperty("cons", "val");
+ for (int f = 0; f < FACET_COUNT; f++) {
+ int labelNum = random.nextInt(NUM_LABELS);
+ child.setProperty("foo" + f, "foo" + f + "x" + labelNum);
+ }
+ }
+ }
+ }
+
+ private Tree createPath(String path) {
+ Tree base = root.getTree("/");
+ for (String e : PathUtils.elements(path)) {
+ base = base.addChild(e);
+ }
+ return base;
+ }
+
+ private void runAsyncIndex() {
+ AsyncIndexUpdate async = (AsyncIndexUpdate) WhiteboardUtils.getService(wb, Runnable.class, new Predicate<Runnable>() {
+ @Override
+ public boolean test(@Nullable Runnable input) {
+ return input instanceof AsyncIndexUpdate;
+ }
+ });
+ assertNotNull(async);
+ async.run();
+ if (async.isFailing()) {
+ fail("AsyncIndexUpdate failed");
+ }
+ root.refresh();
+ }
+
+ @Test
+ public void facet() throws Exception {
+ // Explicitly setting following configs to run DelayedLuceneFacetProvider and a thread sleep of 50 ms in refresh readers. Refer: OAK-8898
+ System.setProperty(LucenePropertyIndex.OLD_FACET_PROVIDER_CONFIG_NAME, "false");
+ // The variable is static final so once set it remains same for all tests and which will lead to slow execution
+ // of other tests as this add a sleep of specified milliseconds in refresh reader method in LuceneIndexNodeManager.
+ // System.setProperty(LuceneIndexNodeManager.OLD_FACET_PROVIDER_TEST_FAILURE_SLEEP_INSTRUMENT_NAME, "40");
Review Comment:
nitpick: fix indentation
```suggestion
// System.setProperty(LuceneIndexNodeManager.OLD_FACET_PROVIDER_TEST_FAILURE_SLEEP_INSTRUMENT_NAME, "40");
```
##########
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java:
##########
@@ -1677,6 +1708,28 @@ private List<Facet> getFacetsUncached(int numberOfFacets, String columnName) thr
indexNode.release();
}
}
+
+ private List<Facet> getFacetsUncached(Facets facets, int numberOfFacets, String columnName) throws IOException {
Review Comment:
`getFacetsUncached(int numberOfFacets, String columnName)` has the exact same logic at lines 1690-1705. What about reducing the code in this class (that is already huge) and call the new `getFacetsUncached` in there?
##########
oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/ManyFacetsTest.java:
##########
@@ -0,0 +1,280 @@
+/*
+ * 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.index.lucene.hybrid;
+
+import static org.apache.jackrabbit.guava.common.util.concurrent.MoreExecutors.newDirectExecutorService;
+import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.PROP_FACETS;
+import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.STATISTICAL_FACET_SAMPLE_SIZE_DEFAULT;
+import static org.apache.jackrabbit.oak.spi.mount.Mounts.defaultMountInfoProvider;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Properties;
+import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+
+import javax.jcr.GuestCredentials;
+import javax.jcr.Repository;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.query.Query;
+import javax.jcr.query.QueryManager;
+import javax.jcr.query.QueryResult;
+import javax.jcr.query.Row;
+import javax.jcr.query.RowIterator;
+
+import org.apache.jackrabbit.oak.InitialContent;
+import org.apache.jackrabbit.oak.Oak;
+import org.apache.jackrabbit.oak.api.ContentRepository;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
+import org.apache.jackrabbit.oak.commons.json.JsonObject;
+import org.apache.jackrabbit.oak.jcr.Jcr;
+import org.apache.jackrabbit.oak.plugins.index.AsyncIndexUpdate;
+import org.apache.jackrabbit.oak.plugins.index.counter.NodeCounterEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.lucene.IndexCopier;
+import org.apache.jackrabbit.oak.plugins.index.lucene.IndexTracker;
+import org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexProvider;
+import org.apache.jackrabbit.oak.plugins.index.lucene.LucenePropertyIndex;
+import org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil;
+import org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil.OptionalEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.lucene.reader.DefaultIndexReaderFactory;
+import org.apache.jackrabbit.oak.plugins.index.lucene.reader.LuceneIndexReaderFactory;
+import org.apache.jackrabbit.oak.plugins.index.lucene.util.LuceneIndexDefinitionBuilder;
+import org.apache.jackrabbit.oak.plugins.index.nodetype.NodeTypeIndexProvider;
+import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants;
+import org.apache.jackrabbit.oak.plugins.index.search.util.IndexDefinitionBuilder.PropertyRule;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
+import org.apache.jackrabbit.oak.query.AbstractQueryTest;
+import org.apache.jackrabbit.oak.spi.commit.Observer;
+import org.apache.jackrabbit.oak.spi.mount.MountInfoProvider;
+import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider;
+import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
+import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
+import org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils;
+import org.apache.jackrabbit.oak.stats.Clock;
+import org.apache.jackrabbit.oak.stats.StatisticsProvider;
+import org.jetbrains.annotations.Nullable;
+import org.junit.After;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class ManyFacetsTest extends AbstractQueryTest {
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder(new File("target"));
+
+ private static final int NUM_LABELS = 4;
+ private static final int NUM_LEAF_NODES = STATISTICAL_FACET_SAMPLE_SIZE_DEFAULT;
+ private static final String FACET_PROP = "facets";
+ private static final long REFRESH_DELTA = TimeUnit.SECONDS.toMillis(1);
+
+ private static final int FACET_COUNT = 200;
+
+ private ExecutorService executorService = Executors.newFixedThreadPool(2);
+ private OptionalEditorProvider optionalEditorProvider = new OptionalEditorProvider();
+ private NRTIndexFactory nrtIndexFactory;
+ private LuceneIndexProvider luceneIndexProvider;
+ private NodeStore nodeStore;
+ private DocumentQueue queue;
+ private Clock clock = new Clock.Virtual();
+ private Whiteboard wb;
+ private QueryManager qm;
+ private Repository jcrRepo;
+ private Jcr jcr;
+ private Oak oak;
+ // backup original system properties i.e. before test started
+ private final Properties backupProperties = (Properties) System.getProperties().clone();
+
+ @After
+ public void tearDown() throws IOException {
+ luceneIndexProvider.close();
+ new ExecutorCloser(executorService).close();
+ nrtIndexFactory.close();
+ // restore original system properties i.e. before test started
+ System.setProperties(backupProperties);
+ }
+
+ @Override
+ protected ContentRepository createRepository() {
+ IndexCopier copier;
+ try {
+ copier = new IndexCopier(executorService, temporaryFolder.getRoot());
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ MountInfoProvider mip = defaultMountInfoProvider();
+ nrtIndexFactory = new NRTIndexFactory(copier, clock, TimeUnit.MILLISECONDS.toSeconds(REFRESH_DELTA), StatisticsProvider.NOOP);
+ nrtIndexFactory.setAssertAllResourcesClosed(true);
+ LuceneIndexReaderFactory indexReaderFactory = new DefaultIndexReaderFactory(mip, copier);
+ IndexTracker tracker = new IndexTracker(indexReaderFactory, nrtIndexFactory);
+ luceneIndexProvider = new LuceneIndexProvider(tracker);
+ queue = new DocumentQueue(100, tracker, newDirectExecutorService());
+ LuceneIndexEditorProvider editorProvider = new LuceneIndexEditorProvider(copier,
+ tracker,
+ null,
+ null,
+ mip);
+ editorProvider.setIndexingQueue(queue);
+ LocalIndexObserver localIndexObserver = new LocalIndexObserver(queue, StatisticsProvider.NOOP);
+ nodeStore = new MemoryNodeStore();
+ oak = new Oak(nodeStore)
+ .with(new InitialContent())
+ .with(new OpenSecurityProvider())
+ .with((QueryIndexProvider) luceneIndexProvider)
+ .with((Observer) luceneIndexProvider)
+ .with(localIndexObserver)
+ .with(editorProvider)
+ .with(new PropertyIndexEditorProvider())
+ .with(new NodeTypeIndexProvider())
+ .with(optionalEditorProvider)
+ .with(new NodeCounterEditorProvider())
+ //Effectively disable async indexing auto run
+ //such that we can control run timing as per test requirement
+ .withAsyncIndexing("async", TimeUnit.DAYS.toSeconds(1));
+
+ wb = oak.getWhiteboard();
+ ContentRepository repo = oak.createContentRepository();
+ return repo;
+ }
+
+ private void createSmallDataset(int k) throws RepositoryException {
+ Random random = new Random(42);
+ Tree par = createPath("/parent" + k);
+ par.setProperty("foo", "bar");
+ for (int i = 0; i < NUM_LABELS * 2; i++) {
+ Tree subPar = par.addChild("par" + i);
+ for (int j = 0; j < NUM_LEAF_NODES / (2 * NUM_LABELS); j++) {
+ Tree child = subPar.addChild("c" + j);
+ child.setProperty("cons", "val");
+ for (int f = 0; f < FACET_COUNT; f++) {
+ int labelNum = random.nextInt(NUM_LABELS);
+ child.setProperty("foo" + f, "foo" + f + "x" + labelNum);
+ }
+ }
+ }
+ }
+
+ private Tree createPath(String path) {
+ Tree base = root.getTree("/");
+ for (String e : PathUtils.elements(path)) {
+ base = base.addChild(e);
+ }
+ return base;
+ }
+
+ private void runAsyncIndex() {
+ AsyncIndexUpdate async = (AsyncIndexUpdate) WhiteboardUtils.getService(wb, Runnable.class, new Predicate<Runnable>() {
+ @Override
+ public boolean test(@Nullable Runnable input) {
+ return input instanceof AsyncIndexUpdate;
+ }
+ });
+ assertNotNull(async);
+ async.run();
+ if (async.isFailing()) {
+ fail("AsyncIndexUpdate failed");
+ }
+ root.refresh();
+ }
+
+ @Test
+ public void facet() throws Exception {
+ // Explicitly setting following configs to run DelayedLuceneFacetProvider and a thread sleep of 50 ms in refresh readers. Refer: OAK-8898
+ System.setProperty(LucenePropertyIndex.OLD_FACET_PROVIDER_CONFIG_NAME, "false");
+ // The variable is static final so once set it remains same for all tests and which will lead to slow execution
+ // of other tests as this add a sleep of specified milliseconds in refresh reader method in LuceneIndexNodeManager.
+ // System.setProperty(LuceneIndexNodeManager.OLD_FACET_PROVIDER_TEST_FAILURE_SLEEP_INSTRUMENT_NAME, "40");
+ Thread.currentThread().setName("main");
+ String idxName = "hybridtest";
+ Tree idx = createIndex(root.getTree("/"), idxName);
+ TestUtil.enableIndexingMode(idx, FulltextIndexConstants.IndexingMode.NRT);
+ setTraversalEnabled(false);
+ root.commit();
+ jcr = new Jcr(oak);
+ jcrRepo = jcr.createRepository();
+ createSmallDataset(0);
+ clock.waitUntil(clock.getTime() + REFRESH_DELTA + 1);
+ root.commit();
+ runAsyncIndex();
+ createSmallDataset(2);
+ clock.waitUntil(clock.getTime() + REFRESH_DELTA + 1);
+ root.commit();
+ Session anonSession = jcrRepo.login(new GuestCredentials());
+ qm = anonSession.getWorkspace().getQueryManager();
+ String facetList = "";
+ for (int i = 0; i < FACET_COUNT; i++) {
+ if (i > 0) {
+ facetList += ", ";
+ }
+ facetList += "[rep:facet(foo" + i + ")]";
+ }
+ String queryString = "SELECT " + facetList +
+ " FROM [nt:base] WHERE [cons] = 'val'";
+ Query q = qm.createQuery(queryString, SQL2);
+ QueryResult qr = q.execute();
+ try {
+ RowIterator it = qr.getRows();
+ assertTrue(it.hasNext());
+ while (it.hasNext()) {
+ Row r = it.nextRow();
+ for (int i = 0; i < qr.getColumnNames().length; i++) {
+ String columnName = qr.getColumnNames()[i];
+ String v = r.getValue(columnName).getString();
+ JsonObject json = JsonObject.fromJson(v, true);
+ for (int j = 0; j < NUM_LABELS; j++) {
+ String n = json.getProperties().get("foo" + i + "x" + j);
+ assertTrue(n != null);
Review Comment:
`assertNotNull` is a better option here
```suggestion
assertNotNull(n);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10549 Improve performance of facet count at scale (Lucene) [jackrabbit-oak]
Posted by "thomasmueller (via GitHub)" <gi...@apache.org>.
thomasmueller merged PR #1215:
URL: https://github.com/apache/jackrabbit-oak/pull/1215
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10549 Improve performance of facet count at scale (Lucene) [jackrabbit-oak]
Posted by "tihom88 (via GitHub)" <gi...@apache.org>.
tihom88 commented on code in PR #1215:
URL: https://github.com/apache/jackrabbit-oak/pull/1215#discussion_r1396835757
##########
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java:
##########
@@ -1644,11 +1647,39 @@ public List<Facet> getFacets(int numberOfFacets, String columnName) throws IOExc
return cachedResults.get(cacheKey);
}
LOG.trace("columnName = {} facet Data not present in cache...", columnName);
+ if (EAGER_FACET_CACHE_FILL) {
Review Comment:
As we are filling cache with all facets. Would it make sense to initialise it in constructor itself using future. Like @fabriziofortino did in https://github.com/apache/jackrabbit-oak/pull/1217
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10549 Improve performance of facet count at scale (Lucene) [jackrabbit-oak]
Posted by "thomasmueller (via GitHub)" <gi...@apache.org>.
thomasmueller commented on code in PR #1215:
URL: https://github.com/apache/jackrabbit-oak/pull/1215#discussion_r1396951519
##########
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java:
##########
@@ -1644,11 +1647,39 @@ public List<Facet> getFacets(int numberOfFacets, String columnName) throws IOExc
return cachedResults.get(cacheKey);
}
LOG.trace("columnName = {} facet Data not present in cache...", columnName);
+ if (EAGER_FACET_CACHE_FILL) {
Review Comment:
I prefer to avoid concurrency. it is filled when the first one is accessed, so I think it "should be fine". (I know, it sounds like "famous last words")
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org