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 2019/02/20 16:17:53 UTC
svn commit: r1853969 - in /jackrabbit/oak/trunk:
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/
oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/
oak-search/src/main/java/org/apache/jackrabbit/oak/plugin...
Author: catholicon
Date: Wed Feb 20 16:17:53 2019
New Revision: 1853969
URL: http://svn.apache.org/viewvc?rev=1853969&view=rev
Log:
OAK-8046: Result items are not always correctly counted against the configured read limit if a query uses a lucene index
Added:
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/ReopenedLuceneIndexTest.java (with props)
Modified:
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java
jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java
Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java?rev=1853969&r1=1853968&r2=1853969&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java Wed Feb 20 16:17:53 2019
@@ -299,7 +299,7 @@ public class LuceneIndex implements Adva
final boolean nonFullTextConstraints = parent.isEmpty();
final int parentDepth = getDepth(parent);
QueryLimits settings = filter.getQueryLimits();
- Iterator<LuceneResultRow> itr = new AbstractIterator<LuceneResultRow>() {
+ LuceneResultRowIterator itr = new LuceneResultRowIterator() {
private final Deque<LuceneResultRow> queue = Queues.newArrayDeque();
private final Set<String> seenPaths = Sets.newHashSet();
private ScoreDoc lastDoc;
@@ -316,6 +316,11 @@ public class LuceneIndex implements Adva
return endOfData();
}
+ @Override
+ public int rewoundCount() {
+ return reloadCount;
+ }
+
private LuceneResultRow convertToRow(ScoreDoc doc, IndexSearcher searcher, String excerpt) throws IOException {
IndexReader reader = searcher.getIndexReader();
PathStoredFieldVisitor visitor = new PathStoredFieldVisitor();
@@ -483,7 +488,7 @@ public class LuceneIndex implements Adva
throw new IllegalStateException("Too many version changes");
}
lastDoc = null;
- LOG.debug("Change in index version detected {} => {}. Query would be performed without " +
+ LOG.info("Change in index version detected {} => {}. Query would be performed without " +
"offset; reload {}", currentVersion, lastSearchIndexerVersion, reloadCount);
}
this.lastSearchIndexerVersion = currentVersion;
@@ -1139,19 +1144,20 @@ public class LuceneIndex implements Adva
*/
static class LucenePathCursor implements Cursor {
- private static final int TRAVERSING_WARNING = Integer.getInteger("oak.traversing.warning", 10000);
+ private final int TRAVERSING_WARNING = Integer.getInteger("oak.traversing.warning", 10000);
private final Cursor pathCursor;
LuceneResultRow currentRow;
private final SizeEstimator sizeEstimator;
private long estimatedSize;
- LucenePathCursor(final Iterator<LuceneResultRow> it, QueryLimits settings, SizeEstimator sizeEstimator, Filter filter) {
+ LucenePathCursor(final LuceneResultRowIterator it, QueryLimits settings, SizeEstimator sizeEstimator, Filter filter) {
this.sizeEstimator = sizeEstimator;
Iterator<String> pathIterator = new Iterator<String>() {
private int readCount;
+ private int rewoundCount;
@Override
public boolean hasNext() {
@@ -1160,6 +1166,10 @@ public class LuceneIndex implements Adva
@Override
public String next() {
+ if (it.rewoundCount() > rewoundCount) {
+ readCount = 0;
+ rewoundCount = it.rewoundCount();
+ }
currentRow = it.next();
readCount++;
if (readCount % TRAVERSING_WARNING == 0) {
@@ -1230,4 +1240,8 @@ public class LuceneIndex implements Adva
return estimatedSize = sizeEstimator.getSize();
}
}
+
+ static abstract class LuceneResultRowIterator extends AbstractIterator<LuceneResultRow> {
+ abstract int rewoundCount();
+ }
}
Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java?rev=1853969&r1=1853968&r2=1853969&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java Wed Feb 20 16:17:53 2019
@@ -251,7 +251,7 @@ public class LucenePropertyIndex extends
final Sort sort = getSort(plan);
final PlanResult pr = getPlanResult(plan);
QueryLimits settings = filter.getQueryLimits();
- Iterator<FulltextResultRow> itr = new AbstractIterator<FulltextResultRow>() {
+ LuceneResultRowIterator rItr = new LuceneResultRowIterator() {
private final Deque<FulltextResultRow> queue = Queues.newArrayDeque();
private final Set<String> seenPaths = Sets.newHashSet();
private ScoreDoc lastDoc;
@@ -260,6 +260,7 @@ public class LucenePropertyIndex extends
private IndexSearcher indexSearcher;
private int indexNodeId = -1;
private LuceneFacetProvider facetProvider = null;
+ private int rewoundCount = 0;
@Override
protected FulltextResultRow computeNext() {
@@ -270,6 +271,11 @@ public class LucenePropertyIndex extends
return endOfData();
}
+ @Override
+ public int rewoundCount() {
+ return rewoundCount;
+ }
+
private FulltextResultRow convertToRow(ScoreDoc doc, IndexSearcher searcher, Map<String, String> excerpts,
LuceneFacetProvider facetProvider,
String explanation) throws IOException {
@@ -535,7 +541,8 @@ public class LucenePropertyIndex extends
if (indexNodeId != indexNode.getIndexNodeId()){
//if already initialized then log about change
if (indexNodeId > 0){
- LOG.debug("Change in index version detected. Query would be performed without offset");
+ LOG.info("Change in index version detected. Query would be performed without offset");
+ rewoundCount++;
}
indexSearcher = indexNode.getSearcher();
@@ -550,13 +557,14 @@ public class LucenePropertyIndex extends
indexSearcher = null;
}
};
+ Iterator<FulltextResultRow> itr = rItr;
SizeEstimator sizeEstimator = getSizeEstimator(plan);
if (pr.hasPropertyIndexResult() || pr.evaluateSyncNodeTypeRestriction()) {
itr = mergePropertyIndexResult(plan, rootState, itr);
}
- return new FulltextPathCursor(itr, plan, settings, sizeEstimator);
+ return new FulltextPathCursor(itr, rItr, plan, settings, sizeEstimator);
}
private static Query addDescendantClauseIfRequired(Query query, IndexPlan plan) {
@@ -1687,4 +1695,6 @@ public class LucenePropertyIndex extends
}
+ static abstract class LuceneResultRowIterator extends AbstractIterator<FulltextResultRow> implements IteratorRewoundStateProvider {
+ }
}
Added: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/ReopenedLuceneIndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/ReopenedLuceneIndexTest.java?rev=1853969&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/ReopenedLuceneIndexTest.java (added)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/ReopenedLuceneIndexTest.java Wed Feb 20 16:17:53 2019
@@ -0,0 +1,224 @@
+/*
+ * 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;
+
+import com.google.common.io.Closer;
+import org.apache.jackrabbit.oak.Oak;
+import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.api.QueryEngine;
+import org.apache.jackrabbit.oak.api.Result;
+import org.apache.jackrabbit.oak.api.ResultRow;
+import org.apache.jackrabbit.oak.api.Root;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
+import org.apache.jackrabbit.oak.commons.junit.TemporarySystemProperty;
+import org.apache.jackrabbit.oak.plugins.index.lucene.util.IndexDefinitionBuilder;
+import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants;
+import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
+import org.apache.jackrabbit.oak.plugins.memory.MultiStringPropertyState;
+import org.apache.jackrabbit.oak.query.QueryEngineSettings;
+import org.apache.jackrabbit.oak.spi.commit.Observer;
+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.hamcrest.core.StringContains;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.query.Query;
+import javax.security.auth.login.LoginException;
+import java.io.File;
+import java.io.IOException;
+import java.text.ParseException;
+import java.util.Iterator;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+import static com.google.common.collect.ImmutableList.of;
+import static org.apache.jackrabbit.oak.InitialContentHelper.INITIAL_CONTENT;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class ReopenedLuceneIndexTest {
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder(new File("target"));
+
+ @Rule
+ public TemporarySystemProperty systemProperty = new TemporarySystemProperty();
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ private Closer closer;
+
+ private Root root;
+ private QueryEngine qe;
+
+ private static final int NUM_NODES = 1000;
+ // Use sufficiently large number to cross batched results from index
+ private static final int READ_BEFORE_REOPEN = 400;
+ private static final int READ_LIMIT = NUM_NODES + READ_BEFORE_REOPEN / 2;
+
+ private void createRepository() throws RepositoryException, IOException, LoginException {
+ System.setProperty("oak.traversing.warning", "100");
+
+ ExecutorService executorService = Executors.newFixedThreadPool(2);
+ closer.register(new ExecutorCloser(executorService));
+ IndexCopier copier = new IndexCopier(executorService, temporaryFolder.getRoot());
+ LuceneIndexEditorProvider editorProvider = new LuceneIndexEditorProvider(copier);
+ LuceneIndexProvider queryIndexProvider = new LuceneIndexProvider(copier);
+
+ QueryEngineSettings qeSettings = new QueryEngineSettings();
+ qeSettings.setLimitReads(READ_LIMIT);
+ qeSettings.setFailTraversal(true);
+
+ NodeStore nodeStore = new MemoryNodeStore(INITIAL_CONTENT);
+ Oak oak = new Oak(nodeStore)
+ .with(new OpenSecurityProvider())
+ .with((QueryIndexProvider) queryIndexProvider)
+ .with((Observer) queryIndexProvider)
+ .with(editorProvider)
+ .with(qeSettings)
+ ;
+
+ root = oak.createContentRepository().login(null, null).getLatestRoot();
+ qe = root.getQueryEngine();
+ }
+
+ @Before
+ public void setup() throws Exception {
+ closer = Closer.create();
+ createRepository();
+ createIndex();
+ createData();
+ }
+
+ @After
+ public void after() throws IOException {
+ closer.close();
+ IndexDefinition.setDisableStoredIndexDefinition(false);
+ }
+
+ @Test
+ public void resultSizeWithinLimitCompatV1() throws Exception {
+ int resultSize = iterateResultWhileReopening("v1");
+
+ assertEquals("Unexpected number of result rows", NUM_NODES, resultSize);
+ }
+
+ @Test
+ public void resultSizeWithinLimitCompatV2() throws Exception {
+ int resultSize = iterateResultWhileReopening("v2");
+
+ assertEquals("Unexpected number of result rows", NUM_NODES, resultSize);
+ }
+
+ @Test
+ public void resultSizeAboveLimitCompatV1() throws Exception {
+ thrown.expect(UnsupportedOperationException.class);
+ thrown.expectMessage(StringContains.containsString("The query read or traversed more than " + READ_LIMIT + " nodes. To avoid affecting other tasks, processing was stopped."));
+
+ // Add more data such that the query genuinely supasses query limit
+ createData(NUM_NODES, READ_LIMIT + 100);
+
+ iterateResultWhileReopening("v1");
+ }
+
+ @Test
+ public void resultSizeAboveLimitCompatV2() throws Exception {
+ thrown.expect(UnsupportedOperationException.class);
+ thrown.expectMessage(StringContains.containsString("The query read or traversed more than " + READ_LIMIT + " nodes. To avoid affecting other tasks, processing was stopped."));
+
+ // Add more data such that the query genuinely supasses query limit
+ createData(NUM_NODES, READ_LIMIT + 100);
+
+ iterateResultWhileReopening("v2");
+ }
+
+ private void createIndex() throws CommitFailedException {
+ IndexDefinitionBuilder idxBuilder = new IndexDefinitionBuilder();
+ idxBuilder.noAsync().evaluatePathRestrictions()
+ .indexRule("nt:base")
+ .property("cons").nodeScopeIndex()
+ // to make a change in index but we won't query for this
+ .enclosingRule().property("foo").propertyIndex();
+
+ Tree indexNode;
+
+ idxBuilder.getBuilderTree().setProperty(new MultiStringPropertyState("tags", of("v2")));
+ indexNode = root.getTree("/").addChild("oak:index").addChild("index-v2");
+ idxBuilder.build(indexNode);
+
+ idxBuilder.getBuilderTree().setProperty(new MultiStringPropertyState("tags", of("v1")));
+ idxBuilder.getBuilderTree().setProperty(FulltextIndexConstants.COMPAT_MODE, 1); // to force aggregate index
+ indexNode = root.getTree("/oak:index").addChild("index-v2");
+ idxBuilder.build(indexNode);
+
+ root.commit();
+ }
+
+ private void createData() throws CommitFailedException {
+ createData(0, NUM_NODES);
+ }
+
+ private void createData(int initialIndex, int lastIndex /* exclusive */) throws CommitFailedException {
+ Tree par = root.getTree("/").addChild("parent");
+
+ for (int i = initialIndex; i < lastIndex; i++) {
+ par.addChild("c" + i).setProperty("cons", "val");
+ }
+
+ root.commit();
+ }
+
+ private int iterateResultWhileReopening(String indexTag) throws ParseException, CommitFailedException {
+ String query = "SELECT * FROM [nt:base] WHERE CONTAINS(*, 'val') option(index tag " + indexTag + ")";
+
+ int resultSize = 0;
+
+ Result result = qe.executeQuery(query, Query.JCR_SQL2, QueryEngine.NO_BINDINGS, QueryEngine.NO_MAPPINGS);
+ Iterator<? extends ResultRow> rows = result.getRows().iterator();
+
+ // get few rows to open the cursor
+ for (int i = 0; i < READ_BEFORE_REOPEN; i++) {
+ assertTrue("Insufficient result rows. Current iteration count: " + i, rows.hasNext());
+ rows.next();
+ resultSize++;
+ }
+
+ // make a change while we still haven't expected the query cursor
+ root.getTree("/").addChild("parent").addChild("c-new").setProperty("foo", "val");
+ root.commit();
+
+ // iterate over rest of the results
+ while (rows.hasNext()) {
+ rows.next();
+ resultSize++;
+ }
+
+ return resultSize;
+ }
+}
Propchange: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/ReopenedLuceneIndexTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java?rev=1853969&r1=1853968&r2=1853969&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java (original)
+++ jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndex.java Wed Feb 20 16:17:53 2019
@@ -365,7 +365,7 @@ public abstract class FulltextIndex impl
protected static class FulltextPathCursor implements Cursor {
private final Logger log = LoggerFactory.getLogger(getClass());
- private static final int TRAVERSING_WARNING = Integer.getInteger("oak.traversing.warning", 10000);
+ private final int TRAVERSING_WARNING = Integer.getInteger("oak.traversing.warning", 10000);
private final Cursor pathCursor;
private final String pathPrefix;
@@ -374,12 +374,13 @@ public abstract class FulltextIndex impl
private long estimatedSize;
private final int numberOfFacets;
- public FulltextPathCursor(final Iterator<FulltextResultRow> it, final IndexPlan plan, QueryLimits settings, SizeEstimator sizeEstimator) {
+ public FulltextPathCursor(final Iterator<FulltextResultRow> it, final IteratorRewoundStateProvider iterStateProvider, final IndexPlan plan, QueryLimits settings, SizeEstimator sizeEstimator) {
pathPrefix = plan.getPathPrefix();
this.sizeEstimator = sizeEstimator;
Iterator<String> pathIterator = new Iterator<String>() {
private int readCount;
+ private int rewoundCount;
@Override
public boolean hasNext() {
@@ -388,6 +389,10 @@ public abstract class FulltextIndex impl
@Override
public String next() {
+ if (iterStateProvider.rewoundCount() > rewoundCount) {
+ readCount = 0;
+ rewoundCount = iterStateProvider.rewoundCount();
+ }
currentRow = it.next();
readCount++;
if (readCount % TRAVERSING_WARNING == 0) {
@@ -493,6 +498,10 @@ public abstract class FulltextIndex impl
}
}
+ public interface IteratorRewoundStateProvider {
+ int rewoundCount();
+ }
+
/**
* A query result facet, composed by its label and count.
*/