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 to...@apache.org on 2019/02/15 09:20:58 UTC

svn commit: r1853626 - in /jackrabbit/oak/trunk/oak-core: pom.xml src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java src/test/java/org/apache/jackrabbit/oak/query/stats/QuerySimilarCostTest.java

Author: tommaso
Date: Fri Feb 15 09:20:58 2019
New Revision: 1853626

URL: http://svn.apache.org/viewvc?rev=1853626&view=rev
Log:
OAK-8035 - a message is logged when similar costs come from the same index type

Added:
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/stats/QuerySimilarCostTest.java
Modified:
    jackrabbit/oak/trunk/oak-core/pom.xml
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java

Modified: jackrabbit/oak/trunk/oak-core/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/pom.xml?rev=1853626&r1=1853625&r2=1853626&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-core/pom.xml Fri Feb 15 09:20:58 2019
@@ -281,5 +281,11 @@
       <version>1.1.1</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.assertj</groupId>
+      <artifactId>assertj-core</artifactId>
+      <version>3.6.2</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 </project>

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=1853626&r1=1853625&r2=1853626&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 Feb 15 09:20:58 2019
@@ -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 < cost) {
-                        cost = c;
-                        indexPlan = p;
+                    if (c < bestCost) {
+                        almostBestCost = bestCost;
+                        almostBestIndex = bestIndex;
+                        almostBestPlan = bestPlan;
+
+                        bestCost = c;
+                        bestIndex = index;
+                        bestPlan = p;
+                    } else if (c - bestCost <= 0.1) {
+                        almostBestCost = c;
+                        almostBestIndex = index;
+                        almostBestPlan = p;
                     }
                 }
 
@@ -1067,8 +1076,12 @@ public class QueryImpl implements Query
         }
 
         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;

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/stats/QuerySimilarCostTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/stats/QuerySimilarCostTest.java?rev=1853626&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/stats/QuerySimilarCostTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/query/stats/QuerySimilarCostTest.java Fri Feb 15 09:20:58 2019
@@ -0,0 +1,161 @@
+/*
+ * 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;
+
+/**
+ * Tests for cases where two or more indices return a similar cost estimation for the same query
+ */
+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<ILoggingEvent> 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<? extends QueryIndex> getQueryIndexes(NodeState nodeState) {
+            return ImmutableList.<QueryIndex>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<QueryIndex.IndexPlan> getPlans(Filter filter, List<QueryIndex.OrderEntry> 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<QueryIndex.IndexPlan> indexList = new ArrayList<QueryIndex.IndexPlan>();
+
+            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;
+        }
+    }
+
+}