diff --git a/oak-core/pom.xml b/oak-core/pom.xml index 0136378..eaa2516 100644 --- a/oak-core/pom.xml +++ b/oak-core/pom.xml @@ -276,6 +276,12 @@ test + org.assertj + assertj-core + 3.6.2 + test + + com.googlecode.json-simple json-simple 1.1.1 diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java index 3564480..ae4d3ab 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java @@ -983,6 +983,7 @@ public class QueryImpl implements Query { // track similar costs QueryIndex almostBestIndex = null; double almostBestCost = Double.POSITIVE_INFINITY; + IndexPlan almostBestPlan = null; // Sort the indexes according to their minimum cost to be able to skip the remaining indexes if the cost of the // current index is below the minimum cost of the next index. @@ -1033,10 +1034,18 @@ public class QueryImpl implements Query { String msg = String.format("cost for [%s] of type (%s) with plan [%s] is %1.2f", p.getPlanName(), indexName, plan, c); logDebug(msg); } + if (c < bestCost) { + almostBestCost = bestCost; + almostBestIndex = bestIndex; + almostBestPlan = bestPlan; - if (c < cost) { - cost = c; - indexPlan = p; + bestCost = c; + bestIndex = index; + bestPlan = p; + } else if (c - bestCost <= 0.1) { + almostBestCost = c; + almostBestIndex = index; + almostBestPlan = p; } } @@ -1045,14 +1054,6 @@ public class QueryImpl implements Query { } } else { cost = index.getCost(filter, rootState); - } - if (LOG.isDebugEnabled()) { - logDebug("cost for " + indexName + " is " + cost); - } - if (cost < 0) { - LOG.error("cost below 0 for " + indexName + " is " + cost); - } - if (cost < bestCost) { almostBestCost = bestCost; almostBestIndex = bestIndex; @@ -1065,10 +1066,22 @@ public class QueryImpl implements Query { almostBestIndex = index; } } + if (LOG.isDebugEnabled()) { + logDebug("cost for " + indexName + " is " + cost); + } + if (cost < 0) { + LOG.error("cost below 0 for " + indexName + " is " + cost); + } + + } if (LOG.isDebugEnabled() && Math.abs(bestCost - almostBestCost) <= 0.1) { - LOG.debug("selected index {} and {} have similar costs {} and {} for query {} - check query explanation / index definitions", + String msg = (bestPlan != null && almostBestPlan != null) ? String.format("selected index %s with plan %s and %s with plan %s have similar costs %s and %s for query %s - " + + "check query explanation / index definitions", + bestIndex, bestPlan.getPlanName(), almostBestIndex, almostBestPlan.getPlanName(), bestCost, almostBestCost, filter.toString()) + :String.format("selected index %s and %s have similar costs %s and %s for query %s - check query explanation / index definitions", bestIndex, almostBestIndex, bestCost, almostBestCost, filter.toString()); + LOG.debug(msg); } potentiallySlowTraversalQuery = bestIndex == null; diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/stats/QuerySimilarCostTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/stats/QuerySimilarCostTest.java new file mode 100644 index 0000000..db17082 --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/stats/QuerySimilarCostTest.java @@ -0,0 +1,159 @@ +/* + * 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.query.stats; +import com.google.common.collect.ImmutableList; +import org.apache.jackrabbit.JcrConstants; +import org.apache.jackrabbit.oak.InitialContent; +import org.apache.jackrabbit.oak.Oak; +import org.apache.jackrabbit.oak.api.ContentRepository; +import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexEditorProvider; +import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexPlan; +import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexProvider; +import org.apache.jackrabbit.oak.query.*; +import org.apache.jackrabbit.oak.query.index.FilterImpl; +import org.apache.jackrabbit.oak.spi.query.Cursor; +import org.apache.jackrabbit.oak.spi.query.Filter; +import org.apache.jackrabbit.oak.spi.query.QueryIndex; +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.util.NodeUtil; +import org.assertj.core.groups.Tuple; +import org.jetbrains.annotations.NotNull; +import org.junit.Test; +import org.slf4j.LoggerFactory; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import org.assertj.core.api.Assertions; + + +public class QuerySimilarCostTest extends AbstractQueryTest { + + private TestIndexProvider testIndexProvider = new TestIndexProvider(); + private final SQL2Parser p = SQL2ParserTest.createTestSQL2Parser(); + @Override + protected ContentRepository createRepository() { + return new Oak() + .with(new OpenSecurityProvider()) + .with(new InitialContent()) + .with(new PropertyIndexEditorProvider()) + .with(new PropertyIndexProvider()) + .with(testIndexProvider) + .createContentRepository(); + } + + + /* + Given 2 index plan with similar cost + we expect a log at debug level to intimate user to either modify either of the indices or the query + */ + @Test + public void testSimilarCostIndices() throws Exception{ + Logger queryImplLogger = (Logger) LoggerFactory.getLogger(QueryImpl.class); + queryImplLogger.setLevel(ch.qos.logback.classic.Level.DEBUG); + + ListAppender listAppender = new ListAppender<>(); + listAppender.start(); + queryImplLogger.addAppender(listAppender); + NodeUtil node = new NodeUtil(root.getTree("/")); + String uuid = UUID.randomUUID().toString(); + node.setString(JcrConstants.JCR_UUID, uuid); + root.commit(); + + executeQuery("SELECT * FROM [nt:base] WHERE [jcr:uuid] is not null", SQL2, true, false); + + + String expectedLogMessage = String.format("selected index %s " + + "with plan testIndexPlan1 and %s with plan testIndexPlan2 have similar costs 11.0 and 11.0 " + + "for query Filter(query=SELECT * FROM [nt:base] WHERE [jcr:uuid] is not null, path=*, property=[jcr:uuid=[is not null]]) - check query explanation / index definitions",testIndexProvider.index,testIndexProvider.index); + + Assertions.assertThat(listAppender.list) + .extracting(ILoggingEvent::getMessage, ILoggingEvent::getLevel) + .contains(Tuple.tuple(expectedLogMessage, Level.DEBUG)); + + queryImplLogger.addAppender(listAppender); + } + + private static class TestIndexProvider implements QueryIndexProvider { + TestIndex index = new TestIndex(); + @Override + public @NotNull List getQueryIndexes(NodeState nodeState) { + return ImmutableList.of(index); + } + } + + private static class TestIndex implements QueryIndex,QueryIndex.AdvancedQueryIndex { + int invocationCount = 0; + @Override + public double getMinimumCost() { + return PropertyIndexPlan.COST_OVERHEAD + 0.1; + } + + @Override + public double getCost(Filter filter, NodeState rootState) { + invocationCount++; + return Double.POSITIVE_INFINITY; + } + + @Override + public Cursor query(Filter filter, NodeState rootState) { + return null; + } + + @Override + public String getPlan(Filter filter, NodeState rootState) { + return null; + } + + @Override + public String getIndexName() { + return "test-index"; + } + + + @Override + public List getPlans(Filter filter, List sortOrder, NodeState rootState) { + IndexPlan.Builder b = new IndexPlan.Builder(); + Filter f = new FilterImpl(null, "SELECT * FROM [nt:file]", new QueryEngineSettings()); + IndexPlan plan1 = b.setEstimatedEntryCount(10).setPlanName("testIndexPlan1").setFilter(f).build(); + IndexPlan plan2 = b.setEstimatedEntryCount(10).setPlanName("testIndexPlan2").setFilter(f).build(); + + List indexList = new ArrayList(); + + indexList.add(plan1); + indexList.add(plan2); + return indexList; + } + + @Override + public String getPlanDescription(QueryIndex.IndexPlan plan, NodeState root) { + return null; + } + + @Override + public Cursor query(QueryIndex.IndexPlan plan, NodeState rootState) { + return null; + } + } + +}