Author: thomasm
Date: Fri Oct 6 14:15:37 2017
New Revision: 1811334
URL: http://svn.apache.org/viewvc?rev=1811334&view=rev
Log:
OAK-6776 Correctly use IndexPlan.supportsPathRestrictions
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/TraversalAvoidanceTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java?rev=1811334&r1=1811333&r2=1811334&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java
Fri Oct 6 14:15:37 2017
@@ -36,6 +36,7 @@ import org.apache.jackrabbit.oak.api.Res
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.namepath.JcrPathParser;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.plugins.index.counter.jmx.NodeCounter;
import org.apache.jackrabbit.oak.plugins.memory.PropertyValues;
import org.apache.jackrabbit.oak.query.QueryOptions.Traversal;
import org.apache.jackrabbit.oak.query.ast.AndImpl;
@@ -82,6 +83,7 @@ import org.apache.jackrabbit.oak.query.p
import org.apache.jackrabbit.oak.query.plan.SelectorExecutionPlan;
import
org.apache.jackrabbit.oak.query.stats.QueryStatsData.QueryExecutionStats;
import org.apache.jackrabbit.oak.spi.query.Filter;
+import org.apache.jackrabbit.oak.spi.query.Filter.PathRestriction;
import org.apache.jackrabbit.oak.spi.query.QueryConstants;
import org.apache.jackrabbit.oak.spi.query.QueryIndex;
import org.apache.jackrabbit.oak.spi.query.QueryIndex.AdvancedQueryIndex;
@@ -997,8 +999,13 @@ public class QueryImpl implements Query
filter, sortOrder, rootState);
cost = Double.POSITIVE_INFINITY;
for (IndexPlan p : ipList) {
- // TODO limit is after all conditions
- long entryCount = Math.min(maxEntryCount,
p.getEstimatedEntryCount());
+
+ long entryCount = p.getEstimatedEntryCount();
+ if (p.getSupportsPathRestriction()) {
+ entryCount = scaleEntryCount(rootState, filter,
entryCount);
+ }
+
+ entryCount = Math.min(maxEntryCount, entryCount);
double c = p.getCostPerExecution() + entryCount *
p.getCostPerEntry();
if (LOG.isDebugEnabled()) {
@@ -1032,34 +1039,58 @@ public class QueryImpl implements Query
}
}
potentiallySlowTraversalQuery = bestIndex == null;
- if (bestPlan != null &&
- (filter.getPathRestriction() ==
Filter.PathRestriction.NO_RESTRICTION ||
- bestPlan.getSupportsPathRestriction())) {
+ if (traversalEnabled) {
+ TraversingIndex traversal = new TraversingIndex();
+ double cost = traversal.getCost(filter, rootState);
if (LOG.isDebugEnabled()) {
- logDebug("Ignoring traversal. Params:: best index:" +
bestIndex + ";" +
- " property restriction: " +
filter.getPathRestriction() + ";" +
- " best plans supports path restriction: " +
bestPlan.getSupportsPathRestriction());
+ logDebug("cost for " + traversal.getIndexName() + " is " +
cost);
}
- } else {
- if (traversalEnabled) {
- TraversingIndex traversal = new TraversingIndex();
- double cost = traversal.getCost(filter, rootState);
- if (LOG.isDebugEnabled()) {
- logDebug("cost for " + traversal.getIndexName() + " is " +
cost);
- }
- if (cost < bestCost || bestCost == Double.POSITIVE_INFINITY) {
- bestCost = cost;
- bestPlan = null;
- bestIndex = traversal;
- if (potentiallySlowTraversalQuery) {
- potentiallySlowTraversalQuery =
traversal.isPotentiallySlow(filter, rootState);
- }
+ if (cost < bestCost || bestCost == Double.POSITIVE_INFINITY) {
+ bestCost = cost;
+ bestPlan = null;
+ bestIndex = traversal;
+ if (potentiallySlowTraversalQuery) {
+ potentiallySlowTraversalQuery =
traversal.isPotentiallySlow(filter, rootState);
}
}
}
return new SelectorExecutionPlan(filter.getSelector(), bestIndex,
bestPlan, bestCost);
}
+
+ private long scaleEntryCount(NodeState rootState, FilterImpl filter, long
count) {
+ PathRestriction r = filter.getPathRestriction();
+ if (r != PathRestriction.ALL_CHILDREN) {
+ return count;
+ }
+ String path = filter.getPath();
+ if (path.startsWith(JoinConditionImpl.SPECIAL_PATH_PREFIX)) {
+ // don't know the path currently, could be root
+ return count;
+ }
+ long filterPathCount = NodeCounter.getEstimatedNodeCount(rootState,
path, true);
+ if (filterPathCount < 0) {
+ // don't know
+ return count;
+ }
+ long totalNodesCount = NodeCounter.getEstimatedNodeCount(rootState,
"/", true);
+ if (totalNodesCount <= 0) {
+ totalNodesCount = 1;
+ }
+ // same logic as for the property index (see
ContentMirrorStoreStrategy):
+
+ // assume nodes in the index are evenly distributed in the repository
(old idea)
+ long countScaledDown = (long) ((double) count / totalNodesCount *
filterPathCount);
+ // assume 80% of the indexed nodes are in this subtree
+ long mostNodesFromThisSubtree = (long) (filterPathCount * 0.8);
+ // count can at most be the assumed subtree size
+ count = Math.min(count, mostNodesFromThisSubtree);
+ // this in theory should not have any effect,
+ // except if the above estimates are incorrect,
+ // so this is just for safety feature
+ count = Math.max(count, countScaledDown);
+ return count;
+ }
@Override
public boolean isPotentiallySlow() {
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/TraversalAvoidanceTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/TraversalAvoidanceTest.java?rev=1811334&r1=1811333&r2=1811334&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/TraversalAvoidanceTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/TraversalAvoidanceTest.java
Fri Oct 6 14:15:37 2017
@@ -13,12 +13,22 @@
*/
package org.apache.jackrabbit.oak.query;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
+import static org.junit.Assert.assertNotNull;
+
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
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.Result;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.plugins.index.AsyncIndexUpdate;
+import
org.apache.jackrabbit.oak.plugins.index.counter.NodeCounterEditorProvider;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
import org.apache.jackrabbit.oak.spi.query.Cursor;
import org.apache.jackrabbit.oak.spi.query.Filter;
import org.apache.jackrabbit.oak.spi.query.IndexRow;
@@ -26,27 +36,81 @@ import org.apache.jackrabbit.oak.spi.que
import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider;
import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
import org.apache.jackrabbit.oak.spi.state.NodeState;
+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.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
-import javax.annotation.Nonnull;
-import java.util.List;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
public class TraversalAvoidanceTest extends AbstractQueryTest {
+
+ Whiteboard wb;
+ NodeStore nodeStore;
+
private static final String QUERY = "SELECT * FROM [nt:base]";
- private static final String PATH_RESTRICTED_QUERY = "SELECT * FROM
[nt:base] WHERE ISDESCENDANTNODE('/content')";
- private static final String PATH_RESTRICTED_SLOW_TRAVERSAL_QUERY = "SELECT
* FROM [nt:base] WHERE ISDESCENDANTNODE('/jcr:system')";
+
+ private static final String PATH_RESTRICTED_QUERY =
+ "SELECT * FROM [nt:base] WHERE ISDESCENDANTNODE('/content/test0')";
+
+ private static final String PATH_RESTRICTED_SLOW_TRAVERSAL_QUERY =
+ "SELECT * FROM [nt:base] WHERE ISDESCENDANTNODE('/content')";
private TestQueryIndexProvider testIndexProvider = new
TestQueryIndexProvider();
@Override
protected ContentRepository createRepository() {
- return new Oak()
+ nodeStore = new MemoryNodeStore();
+ Oak oak = new Oak(nodeStore)
.with(new OpenSecurityProvider())
.with(new InitialContent())
+ .with(new NodeCounterEditorProvider())
.with(testIndexProvider)
- .createContentRepository();
+ //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();
+ return oak.createContentRepository();
+ }
+
+ @Before
+ public void before() throws Exception {
+ session = createRepository().login(null, null);
+ root = session.getLatestRoot();
+ qe = root.getQueryEngine();
+
+ root.getTree("/oak:index/counter").setProperty("resolution", 100);
+ root.getTree("/oak:index/counter").setProperty("seed", 1);
+
+ Tree content = root.getTree("/").addChild("content");
+ // add 200'000 nodes under /content
+ for (int i = 0; i < 2000; i++) {
+ Tree t = content.addChild("test" + i);
+ for (int j = 0; j < 100; j++) {
+ t.addChild("n" + j);
+ }
+ }
+ root.commit();
+
+ runAsyncIndex();
}
+ private void runAsyncIndex() {
+ Runnable async = WhiteboardUtils.getService(wb, Runnable.class, new
Predicate<Runnable>() {
+ @Override
+ public boolean apply(@Nullable Runnable input) {
+ return input instanceof AsyncIndexUpdate;
+ }
+ });
+ assertNotNull(async);
+ async.run();
+ root.refresh();
+ }
+
@Test
public void noPlansLetTraversalWin() {
assertPlanSelection(QUERY, "traverse", "Traversal must be used if
nothing else participates");
@@ -58,20 +122,20 @@ public class TraversalAvoidanceTest exte
@Test
public void singlePlanWithoutPathRestrictionWins() {
- testIndexProvider.addPlan("plan1", 1000000, false);
+ testIndexProvider.addPlan("plan1", 10000, false);
assertPlanSelection(QUERY, "plan1", "Valid plan without path
restriction must win");
}
@Test
public void singlePlanWithPathRestriction() {
- testIndexProvider.addPlan("plan1", 1000000, true);
+ testIndexProvider.addPlan("plan1", 10000, true);
assertPlanSelection(PATH_RESTRICTED_QUERY, "plan1", "Valid plan which
evaluate path" +
" restrictions wins with query having path restriction");
testIndexProvider.restPlans();
- testIndexProvider.addPlan("plan1", 1000000, false);
+ testIndexProvider.addPlan("plan1", 10000, false);
assertPlanSelection(PATH_RESTRICTED_QUERY, "traverse", "Valid plan
which evaluate path" +
" restrictions wins with query having path restriction");
@@ -83,7 +147,7 @@ public class TraversalAvoidanceTest exte
@Test
public void competingPlans() {
- testIndexProvider.addPlan("plan1", 1000000, true);
+ testIndexProvider.addPlan("plan1", 100000, true);
testIndexProvider.addPlan("plan2", 100, true);
assertPlanSelection(QUERY, "plan2", "Low cost must win");
@@ -91,7 +155,7 @@ public class TraversalAvoidanceTest exte
assertPlanSelection(PATH_RESTRICTED_SLOW_TRAVERSAL_QUERY, "plan2",
"Low cost must win");
testIndexProvider.restPlans();
- testIndexProvider.addPlan("plan1", 1000000, false);
+ testIndexProvider.addPlan("plan1", 100000, false);
testIndexProvider.addPlan("plan2", 100, true);
assertPlanSelection(QUERY, "plan2", "Low cost must win");
@@ -99,16 +163,16 @@ public class TraversalAvoidanceTest exte
assertPlanSelection(PATH_RESTRICTED_SLOW_TRAVERSAL_QUERY, "plan2",
"Low cost must win");
testIndexProvider.restPlans();
- testIndexProvider.addPlan("plan1", 1000000, true);
- testIndexProvider.addPlan("plan2", 100, false);
+ testIndexProvider.addPlan("plan1", 200000, true);
+ testIndexProvider.addPlan("plan2", 10000, false);
assertPlanSelection(QUERY, "plan2", "Low cost must win");
assertPlanSelection(PATH_RESTRICTED_QUERY, "traverse", "Low cost must
win");
assertPlanSelection(PATH_RESTRICTED_SLOW_TRAVERSAL_QUERY, "plan2",
"Low cost must win");
testIndexProvider.restPlans();
- testIndexProvider.addPlan("plan1", 1000000, false);
- testIndexProvider.addPlan("plan2", 100, false);
+ testIndexProvider.addPlan("plan1", 200000, false);
+ testIndexProvider.addPlan("plan2", 1000, false);
assertPlanSelection(QUERY, "plan2", "Low cost must win");
assertPlanSelection(PATH_RESTRICTED_QUERY, "traverse", "Low cost must
win");
@@ -190,7 +254,7 @@ public class TraversalAvoidanceTest exte
@Override
public String getPlanDescription(IndexPlan plan, NodeState root) {
- return "unimportant plan description (" + plan.getPlanName() + ")";
+ return "plan=" + plan.getPlanName();
}
void addPlan(String name, long cost, boolean supportsPathRestriction) {
@@ -233,6 +297,6 @@ public class TraversalAvoidanceTest exte
private void assertPlanSelection(String query, String expectedPlan, String
message) {
String explain = explain(query);
- Assert.assertTrue(message, explain.contains(expectedPlan));
+ Assert.assertTrue(message + ", but got: " + explain,
explain.contains(expectedPlan));
}
}
