madrob commented on a change in pull request #592:
URL: https://github.com/apache/solr/pull/592#discussion_r806048246
##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -854,20 +880,92 @@ private DocSet getAndCacheDocSet(Query query) throws
IOException {
return filterCache.computeIfAbsent(query, q -> getDocSetNC(q, null));
}
- private static Query matchAllDocsQuery = new MatchAllDocsQuery();
- private volatile BitDocSet liveDocs;
+ private static final MatchAllDocsQuery MATCH_ALL_DOCS_QUERY = new
MatchAllDocsQuery();
+
+ /**
+ * A naively cached canonical `liveDocs` DocSet. This does not need to be
volatile. It may be set multiple times,
+ * but should always be set to the same value, as all set values should pass
through `liveDocsCache.computeIfAbsent`
+ */
+ private BitDocSet liveDocs;
+ private final IOFunction<MatchAllDocsQuery, BitDocSet> computeLiveDocs =
this::computeLiveDocs;
+
+ private static final BitDocSet EMPTY = new BitDocSet(new FixedBitSet(0), 0);
+
+ private BitDocSet computeLiveDocs(Query q) {
+ assert q == MATCH_ALL_DOCS_QUERY;
+ switch (leafContexts.size()) {
+ case 0:
+ assert numDocs() == 0;
+ return EMPTY;
+ case 1:
+ final Bits onlySegLiveDocs =
leafContexts.get(0).reader().getLiveDocs();
+ final FixedBitSet fbs;
+ if (onlySegLiveDocs == null) {
+ // `LeafReader.getLiveDocs()` returns null if no deleted docs --
accordingly, set all bits
+ final int onlySegMaxDoc = maxDoc();
+ fbs = new FixedBitSet(onlySegMaxDoc);
+ fbs.set(0, onlySegMaxDoc);
+ } else {
+ fbs = FixedBitSet.copyOf(onlySegLiveDocs);
+ }
+ assert fbs.cardinality() == numDocs();
+ return new BitDocSet(fbs, numDocs());
+ default:
+ final FixedBitSet bs = new FixedBitSet(maxDoc());
+ for (LeafReaderContext ctx : leafContexts) {
+ final LeafReader r = ctx.reader();
+ final Bits segLiveDocs = r.getLiveDocs();
+ final int segDocBase = ctx.docBase;
+ if (segLiveDocs == null) {
+ // `LeafReader.getLiveDocs()` returns null if no deleted docs --
accordingly, set all bits in seg range
+ bs.set(segDocBase, segDocBase + r.maxDoc());
+ } else {
+ copyTo(segLiveDocs, r.maxDoc(), bs, segDocBase);
+ }
+ }
+ assert bs.cardinality() == numDocs();
+ return new BitDocSet(bs, numDocs());
+ }
+ }
+
+ private static void copyTo(Bits segLiveDocs, int sourceMaxDoc, FixedBitSet
bs, int segDocBase) {
Review comment:
This is the kind of method that seems like it really wants unit tests. I
spent quite a bit of time, and I think your logic is all correct, but I know I
would be terrified to touch it in the future.
##########
File path: solr/core/src/java/org/apache/solr/search/QueryUtils.java
##########
@@ -45,6 +46,39 @@ public static boolean isNegative(Query q) {
return true;
}
+ /**
+ * Recursively unwraps the specified query to determine whether it is
capable of producing a score
+ * that varies across different documents. Returns true if this query is not
capable of producing a
+ * varying score (i.e., it is a constant score query).
+ */
+ public static boolean isConstantScoreQuery(Query q) {
Review comment:
This whole method makes me want to add an isScoring method to lucene
Query
##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -883,16 +981,30 @@ public Bits getLiveDocsBits() throws IOException {
return getIndexReader().hasDeletions() ? getLiveDocSet().getBits() : null;
}
- /** @lucene.internal */
- public boolean isLiveDocsInstantiated() {
- return liveDocs != null;
- }
-
- /** @lucene.internal */
- public void setLiveDocs(DocSet docs) {
- // a few places currently expect BitDocSet
- assert docs.size() == numDocs();
- this.liveDocs = makeBitDocSet(docs);
+ /**
+ * If some process external to {@link SolrIndexSearcher} has produced a
DocSet whose cardinality matches
+ * that of `liveDocs`, this method provides such caller the ability to offer
its own DocSet to be cached
+ * in the searcher. The caller should then use the returned value (which may
or may not be derived from
+ * the DocSet instance supplied), allowing more efficient memory use.
+ * @lucene.internal
+ */
+ public BitDocSet offerLiveDocs(Supplier<DocSet> docSetSupplier, int
suppliedSize) {
Review comment:
very clear javadoc, this makes sense, thank you!
We don't currently have the convention to do so, but what do you think about
annotating this with `com.google.errorprone.annotations.@CheckReturnValue` so
that it would cause compilation error if the return value was ignored?
##########
File path: solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
##########
@@ -0,0 +1,313 @@
+/*
+ * 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.solr.search;
+
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.solr.common.util.Utils.fromJSONString;
+
+/**
+ * Verify caching interactions between main query and filterCache
+ */
+public class TestMainQueryCaching extends SolrTestCaseJ4 {
+
+ private static final int MOST_DOCS = 100;
+ private static final int ALL_DOCS = MOST_DOCS + 1;
+ private static final String TEST_UFFSQ_PROPNAME =
"solr.test.useFilterForSortedQuery";
+ static String RESTORE_UFFSQ_PROP;
+ static boolean USE_FILTER_FOR_SORTED_QUERY;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // TODO: figure out why the line below (accepting this property as
overridden on test invocation) isn't working
+ // as expected.
+ final String uffsq = System.getProperty(TEST_UFFSQ_PROPNAME,
Boolean.toString(random().nextBoolean()));
+ USE_FILTER_FOR_SORTED_QUERY = Boolean.parseBoolean(uffsq);
+ RESTORE_UFFSQ_PROP = System.setProperty(TEST_UFFSQ_PROPNAME, uffsq);
+ initCore("solrconfig-deeppaging.xml", "schema-sorts.xml");
+ createIndex();
+ }
+
+ @AfterClass
+ public static void afterClass() throws Exception {
+ if (RESTORE_UFFSQ_PROP == null) {
+ System.clearProperty(TEST_UFFSQ_PROPNAME);
+ } else {
+ System.setProperty(TEST_UFFSQ_PROPNAME, RESTORE_UFFSQ_PROP);
+ }
+ }
+
+ public static void createIndex() {
+ for (int i = 0; i < MOST_DOCS; i++) {
+ assertU(adoc("id", Integer.toString(i), "str", "d" + i));
+ if (random().nextInt(MOST_DOCS) == 0) {
+ assertU(commit()); // sometimes make multiple segments
+ }
+ }
+ // add an extra doc to distinguish scoring query from `*:*`
+ assertU(adoc("id", Integer.toString(MOST_DOCS), "str", "e" + MOST_DOCS));
+ assertU(commit());
+ }
+
+ @Before
+ public void beforeTest() throws Exception {
+ // testing caching, it's far simpler to just reload the core every time to
prevent
+ // subsequent requests from affecting each other
+ h.reload();
+ }
+
+ private static long coreToInserts(SolrCore core) {
+ return (long)((MetricsMap)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("CACHE.searcher.filterCache")).getGauge())
+ .getValue().get("inserts");
+ }
+
+ private static long coreToSortCount(SolrCore core, String skipOrFull) {
+ return (long)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("SEARCHER.searcher." +
skipOrFull + "SortCount")).getGauge()
+ .getValue();
+ }
+
+ private static long coreToLiveDocsNaiveCacheHitCount(SolrCore core) {
+ return (long)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("SEARCHER.searcher.liveDocsNaiveCacheHitCount")).getGauge()
+ .getValue();
+ }
+
+ private static long coreToMatchAllDocsInsertCount(SolrCore core) {
+ return (long) coreToLiveDocsCacheMetrics(core).get("inserts");
+ }
+
+ private static Map<String, Object> coreToLiveDocsCacheMetrics(SolrCore core)
{
+ return
((MetricsMap)((SolrMetricManager.GaugeWrapper<?>)core.getCoreMetricManager().getRegistry()
+
.getMetrics().get("CACHE.searcher.liveDocsCache")).getGauge()).getValue();
+ }
+ private static final String SCORING_QUERY = "str:d*";
+ private static final String CONSTANT_SCORE_QUERY = "(" + SCORING_QUERY +
")^=1.0"; // wrapped as a ConstantScoreQuery
+ private static final String MATCH_ALL_DOCS_QUERY = "*:*";
+
+ private static final String[] ALL_QUERIES = new String[] { SCORING_QUERY,
CONSTANT_SCORE_QUERY, MATCH_ALL_DOCS_QUERY };
+
+ @Test
+ public void testScoringQuery() throws Exception {
+ // plain request should have no caching or sorting optimization
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true"));
+ assertMetricCounts(response, false, 0, 1, 0);
+ }
+
+ @Test
+ public void testConstantScoreFlScore() throws Exception {
+ // explicitly requesting scores should unconditionally disable caching and
sorting optimizations
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true",
"rows", "0", "fl", "id,score", "sort", (random().nextBoolean() ? "id asc" :
"score desc")));
+ assertMetricCounts(response, false, 0, 1, 0);
+ }
+
+ @Test
+ public void testScoringQueryNonScoreSort() throws Exception {
+ // plain request with no score in sort should consult filterCache, but
need full sorting
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true", "sort", "id
asc"));
+ assertMetricCounts(response, false, USE_FILTER_FOR_SORTED_QUERY ? 1 : 0,
1, 0);
+ }
+
+ @Test
+ public void testScoringQueryZeroRows() throws Exception {
+ // always hit cache, optimize sort because rows=0
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true", "rows",
"0", "sort", (random().nextBoolean() ? "id asc" : "score desc")));
+ final int insertAndSkipCount = USE_FILTER_FOR_SORTED_QUERY ? 1 : 0;
+ assertMetricCounts(response, false, insertAndSkipCount,
USE_FILTER_FOR_SORTED_QUERY ? 0 : 1, insertAndSkipCount);
+ }
+
+ @Test
+ public void testConstantScoreSortByScore() throws Exception {
+ // hit cache and skip sort because constant score query
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true"));
+ final int insertAndSkipCount = USE_FILTER_FOR_SORTED_QUERY ? 1 : 0;
+ assertMetricCounts(response, false, insertAndSkipCount,
USE_FILTER_FOR_SORTED_QUERY ? 0 : 1, insertAndSkipCount);
+ }
+
+ @Test
+ public void testConstantScoreNonScoreSort() throws Exception {
+ // consult filterCache because constant score query, but no skip sort
(because sort-by-id)
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true",
"sort", "id asc"));
+ assertMetricCounts(response, false, USE_FILTER_FOR_SORTED_QUERY ? 1 : 0,
1, 0);
+ }
+
+ /**
+ * As {@link #testConstantScoreNonScoreSort} (though an analogous test could
be written corresponding to
+ * {@link #testConstantScoreSortByScore()}, etc...); but with an additional
constant-score clause that causes
+ * the associated DocSet, (if {@link #USE_FILTER_FOR_SORTED_QUERY}==true) to
be cached as equivalent to
+ * MatchAllDocsQuery/liveDocs, _in addition to_ in the filterCache.
+ *
+ * This is an edge case, but it's the behavior we want, and despite there
being two entries, the actual DocSet
+ * will be the same (`==`) in both locations (liveDocs and filterCache)
+ */
+ @Test
+ public void testConstantScoreMatchesAllDocsNonScoreSort() throws Exception {
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY + " OR (str:e*)^=4.0",
"indent", "true", "sort", "id asc"));
+ assertMetricCounts(response, USE_FILTER_FOR_SORTED_QUERY,
USE_FILTER_FOR_SORTED_QUERY ? 1 : 0, 1, 0, ALL_DOCS);
+ }
+
+ @Test
+ public void testMatchAllDocsPlain() throws Exception {
+ // plain request with "score" sort should skip sort even if `rows`
requested
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true"));
+ assertMetricCounts(response, true, 0, 0, 1);
+ }
+
+ @Test
+ public void testMatchAllDocsFlScore() throws Exception {
+ // explicitly requesting scores should unconditionally disable all cache
consultation and sort optimization
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"rows", "0", "fl", "id,score", "sort", (random().nextBoolean() ? "id asc" :
"score desc")));
+ // NOTE: pretend we're not MatchAllDocs ...
+ assertMetricCounts(response, false, 0, 1, 0, ALL_DOCS);
+ }
+
+ @Test
+ public void testMatchAllDocsZeroRows() throws Exception {
+ // plain request should _always_ skip sort when `rows=0`
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"rows", "0", "sort", "id asc"));
+ assertMetricCounts(response, true, 0, 0, 1);
+ }
+
+ @Test
+ public void testMatchAllDocsNonScoreSort() throws Exception {
+ // plain request _with_ rows and non-score sort should consult cache, but
not skip sort
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"sort", "id asc"));
+ assertMetricCounts(response, true, 0, 1, 0);
+ }
+
+ @Test
+ public void testCursorMark() throws Exception {
+ String q = pickRandom(ALL_QUERIES);
+ boolean includeScoreInSort = random().nextBoolean();
+ String response = JQ(req("q", q, "indent", "true", "cursorMark", "*",
"sort", includeScoreInSort ? "score desc,id asc" : "id asc"));
+ final int expectNumFound = MATCH_ALL_DOCS_QUERY.equals(q) ? ALL_DOCS :
MOST_DOCS;
+ final boolean consultMatchAllDocs;
+ final boolean insertFilterCache;
+ if (includeScoreInSort) {
+ consultMatchAllDocs = false;
+ insertFilterCache = false;
+ } else if (MATCH_ALL_DOCS_QUERY.equals(q)) {
+ consultMatchAllDocs = true;
+ insertFilterCache = false;
+ } else {
+ consultMatchAllDocs = false;
+ insertFilterCache = USE_FILTER_FOR_SORTED_QUERY;
+ }
+ assertMetricCounts(response, consultMatchAllDocs, insertFilterCache ? 1 :
0, 1, 0, expectNumFound);
+ }
+
+ @Test
+ public void testCursorMarkZeroRows() throws Exception {
+ String q = pickRandom(ALL_QUERIES);
+ String response = JQ(req("q", q, "indent", "true", "cursorMark", "*",
"rows", "0", "sort", random().nextBoolean() ? "id asc" : "score desc,id asc"));
+ final boolean consultMatchAllDocs;
+ final boolean insertFilterCache;
+ final boolean skipSort;
+ if (MATCH_ALL_DOCS_QUERY.equals(q)) {
+ consultMatchAllDocs = true;
+ insertFilterCache = false;
+ skipSort = true;
+ } else {
+ consultMatchAllDocs = false;
+ insertFilterCache = USE_FILTER_FOR_SORTED_QUERY;
+ skipSort = USE_FILTER_FOR_SORTED_QUERY;
+ }
+ assertMetricCounts(response, consultMatchAllDocs, insertFilterCache ? 1 :
0, skipSort ? 0 : 1, skipSort ? 1 : 0);
+ }
+
+ private static void assertMetricCounts(String response, boolean
matchAllDocs, int expectFilterCacheInsertCount, int expectFullSortCount, int
expectSkipSortCount) {
+ assertMetricCounts(response, matchAllDocs, expectFilterCacheInsertCount,
expectFullSortCount, expectSkipSortCount, matchAllDocs ? ALL_DOCS : MOST_DOCS);
+ }
+
+ private static void assertMetricCounts(String response, boolean
matchAllDocs, int expectFilterCacheInsertCount,
+ int expectFullSortCount, int
expectSkipSortCount, int expectNumFound) {
+ Map<?, ?> res = (Map<?, ?>) fromJSONString(response);
+ Map<?, ?> body = (Map<?, ?>) (res.get("response"));
+ SolrCore core = h.getCore();
+ assertEquals("Bad matchAllDocs insert count", (matchAllDocs ? 1 : 0),
coreToMatchAllDocsInsertCount(core));
+ assertEquals("Bad filterCache insert count", expectFilterCacheInsertCount,
coreToInserts(core));
+ assertEquals("Bad full sort count", expectFullSortCount,
coreToSortCount(core, "full"));
+ assertEquals("Bad skip sort count", expectSkipSortCount,
coreToSortCount(core, "skip"));
+ assertEquals("Should have exactly " + expectNumFound, expectNumFound,
(long) (body.get("numFound"))); // sanity check
+ }
+
+ @Test
+ public void testConcurrentMatchAllDocsInitialization() throws Exception {
+ final int nThreads = 20;
+ final ExecutorService executor =
ExecutorUtil.newMDCAwareFixedThreadPool(nThreads, new
SolrNamedThreadFactory(getTestName()));
+ final Future<?>[] followup = new Future<?>[nThreads];
+ for (int i = 0; i < nThreads; i++) {
+ final int myI = i;
+ followup[i] = executor.submit(() -> {
+ try {
+ // NOTE: we use cursorMark=* here because it prevents consulting the
queryResultCache, which can interfere
+ // with DocSet fetching (which is what we care about in this test).
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "request_id",
Integer.toString(myI), "cursorMark", "*", "sort", "id asc"));
+ Map<?, ?> res = (Map<?, ?>) fromJSONString(response);
+ Map<?, ?> body = (Map<?, ?>) (res.get("response"));
+ assertEquals("Should have exactly " + ALL_DOCS, ALL_DOCS, (long)
(body.get("numFound"))); // sanity check
+ } catch (Exception ex) {
+ throw new RuntimeException(ex);
+ }
+ });
+ }
+ try {
+ for (Future<?> f : followup) {
+ f.get(); // to access exceptions/errors
+ }
+ } finally {
+ executor.shutdown();
+ assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS)); // tasks
should already have completed
+ }
+ final SolrCore core = h.getCore();
+ Map<String, Object> liveDocsCacheMetrics =
coreToLiveDocsCacheMetrics(core);
+ long inserts = (long) liveDocsCacheMetrics.get("inserts"); // the one and
only liveDocs computation
+ long hits = (long) liveDocsCacheMetrics.get("hits"); // hits during the
initial phase
+ long asyncHits = (long) liveDocsCacheMetrics.get("asyncHits");
Review comment:
use the constants in SolrCache instead of strings here?
##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -119,10 +128,18 @@
private final int queryResultMaxDocsCached;
private final boolean useFilterForSortedQuery;
+ /**
+ * Special-case cache to handle the lazy-init of {@link #liveDocs}.
+ */
+ private final SolrCache<MatchAllDocsQuery,BitDocSet> liveDocsCache;
Review comment:
Using a CaffeineCache for a single element here seems heavy. Would it be
simpler to use a single CompletableFuture? Or do we take advantage of the stats
on the cache here? I guess that doesn't work with offerLiveDocs?
##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -854,20 +880,92 @@ private DocSet getAndCacheDocSet(Query query) throws
IOException {
return filterCache.computeIfAbsent(query, q -> getDocSetNC(q, null));
}
- private static Query matchAllDocsQuery = new MatchAllDocsQuery();
- private volatile BitDocSet liveDocs;
+ private static final MatchAllDocsQuery MATCH_ALL_DOCS_QUERY = new
MatchAllDocsQuery();
+
+ /**
+ * A naively cached canonical `liveDocs` DocSet. This does not need to be
volatile. It may be set multiple times,
+ * but should always be set to the same value, as all set values should pass
through `liveDocsCache.computeIfAbsent`
+ */
+ private BitDocSet liveDocs;
+ private final IOFunction<MatchAllDocsQuery, BitDocSet> computeLiveDocs =
this::computeLiveDocs;
+
+ private static final BitDocSet EMPTY = new BitDocSet(new FixedBitSet(0), 0);
+
+ private BitDocSet computeLiveDocs(Query q) {
+ assert q == MATCH_ALL_DOCS_QUERY;
Review comment:
This is a strange signature/assertion. Why not do
`computeLiveDocs(MatchAllDocsQuery q)`? Actually, the query isn't even used at
all, so why does it matter?
##########
File path: solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
##########
@@ -0,0 +1,313 @@
+/*
+ * 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.solr.search;
+
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.solr.common.util.Utils.fromJSONString;
+
+/**
+ * Verify caching interactions between main query and filterCache
+ */
+public class TestMainQueryCaching extends SolrTestCaseJ4 {
+
+ private static final int MOST_DOCS = 100;
+ private static final int ALL_DOCS = MOST_DOCS + 1;
+ private static final String TEST_UFFSQ_PROPNAME =
"solr.test.useFilterForSortedQuery";
+ static String RESTORE_UFFSQ_PROP;
+ static boolean USE_FILTER_FOR_SORTED_QUERY;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // TODO: figure out why the line below (accepting this property as
overridden on test invocation) isn't working
+ // as expected.
+ final String uffsq = System.getProperty(TEST_UFFSQ_PROPNAME,
Boolean.toString(random().nextBoolean()));
+ USE_FILTER_FOR_SORTED_QUERY = Boolean.parseBoolean(uffsq);
+ RESTORE_UFFSQ_PROP = System.setProperty(TEST_UFFSQ_PROPNAME, uffsq);
+ initCore("solrconfig-deeppaging.xml", "schema-sorts.xml");
+ createIndex();
+ }
+
+ @AfterClass
+ public static void afterClass() throws Exception {
+ if (RESTORE_UFFSQ_PROP == null) {
+ System.clearProperty(TEST_UFFSQ_PROPNAME);
+ } else {
+ System.setProperty(TEST_UFFSQ_PROPNAME, RESTORE_UFFSQ_PROP);
+ }
+ }
Review comment:
The test framework takes care of this for us already, but it's fine to
leave it here.
##########
File path: solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
##########
@@ -0,0 +1,313 @@
+/*
+ * 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.solr.search;
+
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.solr.common.util.Utils.fromJSONString;
+
+/**
+ * Verify caching interactions between main query and filterCache
+ */
+public class TestMainQueryCaching extends SolrTestCaseJ4 {
+
+ private static final int MOST_DOCS = 100;
+ private static final int ALL_DOCS = MOST_DOCS + 1;
+ private static final String TEST_UFFSQ_PROPNAME =
"solr.test.useFilterForSortedQuery";
+ static String RESTORE_UFFSQ_PROP;
+ static boolean USE_FILTER_FOR_SORTED_QUERY;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // TODO: figure out why the line below (accepting this property as
overridden on test invocation) isn't working
+ // as expected.
+ final String uffsq = System.getProperty(TEST_UFFSQ_PROPNAME,
Boolean.toString(random().nextBoolean()));
+ USE_FILTER_FOR_SORTED_QUERY = Boolean.parseBoolean(uffsq);
+ RESTORE_UFFSQ_PROP = System.setProperty(TEST_UFFSQ_PROPNAME, uffsq);
+ initCore("solrconfig-deeppaging.xml", "schema-sorts.xml");
+ createIndex();
+ }
+
+ @AfterClass
+ public static void afterClass() throws Exception {
+ if (RESTORE_UFFSQ_PROP == null) {
+ System.clearProperty(TEST_UFFSQ_PROPNAME);
+ } else {
+ System.setProperty(TEST_UFFSQ_PROPNAME, RESTORE_UFFSQ_PROP);
+ }
+ }
+
+ public static void createIndex() {
+ for (int i = 0; i < MOST_DOCS; i++) {
+ assertU(adoc("id", Integer.toString(i), "str", "d" + i));
+ if (random().nextInt(MOST_DOCS) == 0) {
+ assertU(commit()); // sometimes make multiple segments
+ }
+ }
+ // add an extra doc to distinguish scoring query from `*:*`
+ assertU(adoc("id", Integer.toString(MOST_DOCS), "str", "e" + MOST_DOCS));
+ assertU(commit());
+ }
+
+ @Before
+ public void beforeTest() throws Exception {
+ // testing caching, it's far simpler to just reload the core every time to
prevent
+ // subsequent requests from affecting each other
+ h.reload();
+ }
+
+ private static long coreToInserts(SolrCore core) {
+ return (long)((MetricsMap)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("CACHE.searcher.filterCache")).getGauge())
+ .getValue().get("inserts");
+ }
+
+ private static long coreToSortCount(SolrCore core, String skipOrFull) {
+ return (long)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("SEARCHER.searcher." +
skipOrFull + "SortCount")).getGauge()
+ .getValue();
+ }
+
+ private static long coreToLiveDocsNaiveCacheHitCount(SolrCore core) {
+ return (long)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("SEARCHER.searcher.liveDocsNaiveCacheHitCount")).getGauge()
+ .getValue();
+ }
+
+ private static long coreToMatchAllDocsInsertCount(SolrCore core) {
+ return (long) coreToLiveDocsCacheMetrics(core).get("inserts");
+ }
+
+ private static Map<String, Object> coreToLiveDocsCacheMetrics(SolrCore core)
{
+ return
((MetricsMap)((SolrMetricManager.GaugeWrapper<?>)core.getCoreMetricManager().getRegistry()
+
.getMetrics().get("CACHE.searcher.liveDocsCache")).getGauge()).getValue();
+ }
+ private static final String SCORING_QUERY = "str:d*";
+ private static final String CONSTANT_SCORE_QUERY = "(" + SCORING_QUERY +
")^=1.0"; // wrapped as a ConstantScoreQuery
+ private static final String MATCH_ALL_DOCS_QUERY = "*:*";
+
+ private static final String[] ALL_QUERIES = new String[] { SCORING_QUERY,
CONSTANT_SCORE_QUERY, MATCH_ALL_DOCS_QUERY };
+
+ @Test
+ public void testScoringQuery() throws Exception {
+ // plain request should have no caching or sorting optimization
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true"));
+ assertMetricCounts(response, false, 0, 1, 0);
+ }
+
+ @Test
+ public void testConstantScoreFlScore() throws Exception {
+ // explicitly requesting scores should unconditionally disable caching and
sorting optimizations
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true",
"rows", "0", "fl", "id,score", "sort", (random().nextBoolean() ? "id asc" :
"score desc")));
+ assertMetricCounts(response, false, 0, 1, 0);
+ }
+
+ @Test
+ public void testScoringQueryNonScoreSort() throws Exception {
+ // plain request with no score in sort should consult filterCache, but
need full sorting
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true", "sort", "id
asc"));
+ assertMetricCounts(response, false, USE_FILTER_FOR_SORTED_QUERY ? 1 : 0,
1, 0);
+ }
+
+ @Test
+ public void testScoringQueryZeroRows() throws Exception {
+ // always hit cache, optimize sort because rows=0
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true", "rows",
"0", "sort", (random().nextBoolean() ? "id asc" : "score desc")));
+ final int insertAndSkipCount = USE_FILTER_FOR_SORTED_QUERY ? 1 : 0;
+ assertMetricCounts(response, false, insertAndSkipCount,
USE_FILTER_FOR_SORTED_QUERY ? 0 : 1, insertAndSkipCount);
+ }
+
+ @Test
+ public void testConstantScoreSortByScore() throws Exception {
+ // hit cache and skip sort because constant score query
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true"));
+ final int insertAndSkipCount = USE_FILTER_FOR_SORTED_QUERY ? 1 : 0;
+ assertMetricCounts(response, false, insertAndSkipCount,
USE_FILTER_FOR_SORTED_QUERY ? 0 : 1, insertAndSkipCount);
+ }
+
+ @Test
+ public void testConstantScoreNonScoreSort() throws Exception {
+ // consult filterCache because constant score query, but no skip sort
(because sort-by-id)
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true",
"sort", "id asc"));
+ assertMetricCounts(response, false, USE_FILTER_FOR_SORTED_QUERY ? 1 : 0,
1, 0);
+ }
+
+ /**
+ * As {@link #testConstantScoreNonScoreSort} (though an analogous test could
be written corresponding to
+ * {@link #testConstantScoreSortByScore()}, etc...); but with an additional
constant-score clause that causes
+ * the associated DocSet, (if {@link #USE_FILTER_FOR_SORTED_QUERY}==true) to
be cached as equivalent to
+ * MatchAllDocsQuery/liveDocs, _in addition to_ in the filterCache.
+ *
+ * This is an edge case, but it's the behavior we want, and despite there
being two entries, the actual DocSet
+ * will be the same (`==`) in both locations (liveDocs and filterCache)
+ */
+ @Test
+ public void testConstantScoreMatchesAllDocsNonScoreSort() throws Exception {
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY + " OR (str:e*)^=4.0",
"indent", "true", "sort", "id asc"));
+ assertMetricCounts(response, USE_FILTER_FOR_SORTED_QUERY,
USE_FILTER_FOR_SORTED_QUERY ? 1 : 0, 1, 0, ALL_DOCS);
+ }
+
+ @Test
+ public void testMatchAllDocsPlain() throws Exception {
+ // plain request with "score" sort should skip sort even if `rows`
requested
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true"));
+ assertMetricCounts(response, true, 0, 0, 1);
+ }
+
+ @Test
+ public void testMatchAllDocsFlScore() throws Exception {
+ // explicitly requesting scores should unconditionally disable all cache
consultation and sort optimization
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"rows", "0", "fl", "id,score", "sort", (random().nextBoolean() ? "id asc" :
"score desc")));
+ // NOTE: pretend we're not MatchAllDocs ...
Review comment:
What does this mean? Did you want to do an exists query that happens to
match all docs, like `str:*` (or `id:*`)
##########
File path: solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
##########
@@ -0,0 +1,313 @@
+/*
+ * 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.solr.search;
+
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.solr.common.util.Utils.fromJSONString;
+
+/**
+ * Verify caching interactions between main query and filterCache
+ */
+public class TestMainQueryCaching extends SolrTestCaseJ4 {
+
+ private static final int MOST_DOCS = 100;
+ private static final int ALL_DOCS = MOST_DOCS + 1;
+ private static final String TEST_UFFSQ_PROPNAME =
"solr.test.useFilterForSortedQuery";
+ static String RESTORE_UFFSQ_PROP;
+ static boolean USE_FILTER_FOR_SORTED_QUERY;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // TODO: figure out why the line below (accepting this property as
overridden on test invocation) isn't working
+ // as expected.
Review comment:
What's the issue here? I'm not entirely sure what I'm looking for.
##########
File path: solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
##########
@@ -0,0 +1,313 @@
+/*
+ * 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.solr.search;
+
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.solr.common.util.Utils.fromJSONString;
+
+/**
+ * Verify caching interactions between main query and filterCache
+ */
+public class TestMainQueryCaching extends SolrTestCaseJ4 {
+
+ private static final int MOST_DOCS = 100;
+ private static final int ALL_DOCS = MOST_DOCS + 1;
+ private static final String TEST_UFFSQ_PROPNAME =
"solr.test.useFilterForSortedQuery";
+ static String RESTORE_UFFSQ_PROP;
+ static boolean USE_FILTER_FOR_SORTED_QUERY;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // TODO: figure out why the line below (accepting this property as
overridden on test invocation) isn't working
+ // as expected.
+ final String uffsq = System.getProperty(TEST_UFFSQ_PROPNAME,
Boolean.toString(random().nextBoolean()));
+ USE_FILTER_FOR_SORTED_QUERY = Boolean.parseBoolean(uffsq);
+ RESTORE_UFFSQ_PROP = System.setProperty(TEST_UFFSQ_PROPNAME, uffsq);
+ initCore("solrconfig-deeppaging.xml", "schema-sorts.xml");
+ createIndex();
+ }
+
+ @AfterClass
+ public static void afterClass() throws Exception {
+ if (RESTORE_UFFSQ_PROP == null) {
+ System.clearProperty(TEST_UFFSQ_PROPNAME);
+ } else {
+ System.setProperty(TEST_UFFSQ_PROPNAME, RESTORE_UFFSQ_PROP);
+ }
+ }
+
+ public static void createIndex() {
+ for (int i = 0; i < MOST_DOCS; i++) {
+ assertU(adoc("id", Integer.toString(i), "str", "d" + i));
+ if (random().nextInt(MOST_DOCS) == 0) {
+ assertU(commit()); // sometimes make multiple segments
+ }
+ }
+ // add an extra doc to distinguish scoring query from `*:*`
+ assertU(adoc("id", Integer.toString(MOST_DOCS), "str", "e" + MOST_DOCS));
+ assertU(commit());
+ }
+
+ @Before
+ public void beforeTest() throws Exception {
+ // testing caching, it's far simpler to just reload the core every time to
prevent
+ // subsequent requests from affecting each other
+ h.reload();
+ }
+
+ private static long coreToInserts(SolrCore core) {
+ return (long)((MetricsMap)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("CACHE.searcher.filterCache")).getGauge())
+ .getValue().get("inserts");
+ }
+
+ private static long coreToSortCount(SolrCore core, String skipOrFull) {
+ return (long)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("SEARCHER.searcher." +
skipOrFull + "SortCount")).getGauge()
+ .getValue();
+ }
+
+ private static long coreToLiveDocsNaiveCacheHitCount(SolrCore core) {
+ return (long)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("SEARCHER.searcher.liveDocsNaiveCacheHitCount")).getGauge()
+ .getValue();
+ }
+
+ private static long coreToMatchAllDocsInsertCount(SolrCore core) {
+ return (long) coreToLiveDocsCacheMetrics(core).get("inserts");
+ }
+
+ private static Map<String, Object> coreToLiveDocsCacheMetrics(SolrCore core)
{
+ return
((MetricsMap)((SolrMetricManager.GaugeWrapper<?>)core.getCoreMetricManager().getRegistry()
+
.getMetrics().get("CACHE.searcher.liveDocsCache")).getGauge()).getValue();
+ }
+ private static final String SCORING_QUERY = "str:d*";
+ private static final String CONSTANT_SCORE_QUERY = "(" + SCORING_QUERY +
")^=1.0"; // wrapped as a ConstantScoreQuery
+ private static final String MATCH_ALL_DOCS_QUERY = "*:*";
+
+ private static final String[] ALL_QUERIES = new String[] { SCORING_QUERY,
CONSTANT_SCORE_QUERY, MATCH_ALL_DOCS_QUERY };
+
+ @Test
+ public void testScoringQuery() throws Exception {
+ // plain request should have no caching or sorting optimization
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true"));
+ assertMetricCounts(response, false, 0, 1, 0);
+ }
+
+ @Test
+ public void testConstantScoreFlScore() throws Exception {
+ // explicitly requesting scores should unconditionally disable caching and
sorting optimizations
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true",
"rows", "0", "fl", "id,score", "sort", (random().nextBoolean() ? "id asc" :
"score desc")));
+ assertMetricCounts(response, false, 0, 1, 0);
+ }
+
+ @Test
+ public void testScoringQueryNonScoreSort() throws Exception {
+ // plain request with no score in sort should consult filterCache, but
need full sorting
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true", "sort", "id
asc"));
+ assertMetricCounts(response, false, USE_FILTER_FOR_SORTED_QUERY ? 1 : 0,
1, 0);
+ }
+
+ @Test
+ public void testScoringQueryZeroRows() throws Exception {
+ // always hit cache, optimize sort because rows=0
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true", "rows",
"0", "sort", (random().nextBoolean() ? "id asc" : "score desc")));
+ final int insertAndSkipCount = USE_FILTER_FOR_SORTED_QUERY ? 1 : 0;
+ assertMetricCounts(response, false, insertAndSkipCount,
USE_FILTER_FOR_SORTED_QUERY ? 0 : 1, insertAndSkipCount);
+ }
+
+ @Test
+ public void testConstantScoreSortByScore() throws Exception {
+ // hit cache and skip sort because constant score query
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true"));
+ final int insertAndSkipCount = USE_FILTER_FOR_SORTED_QUERY ? 1 : 0;
+ assertMetricCounts(response, false, insertAndSkipCount,
USE_FILTER_FOR_SORTED_QUERY ? 0 : 1, insertAndSkipCount);
+ }
+
+ @Test
+ public void testConstantScoreNonScoreSort() throws Exception {
+ // consult filterCache because constant score query, but no skip sort
(because sort-by-id)
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true",
"sort", "id asc"));
+ assertMetricCounts(response, false, USE_FILTER_FOR_SORTED_QUERY ? 1 : 0,
1, 0);
+ }
+
+ /**
+ * As {@link #testConstantScoreNonScoreSort} (though an analogous test could
be written corresponding to
+ * {@link #testConstantScoreSortByScore()}, etc...); but with an additional
constant-score clause that causes
+ * the associated DocSet, (if {@link #USE_FILTER_FOR_SORTED_QUERY}==true) to
be cached as equivalent to
+ * MatchAllDocsQuery/liveDocs, _in addition to_ in the filterCache.
+ *
+ * This is an edge case, but it's the behavior we want, and despite there
being two entries, the actual DocSet
+ * will be the same (`==`) in both locations (liveDocs and filterCache)
+ */
+ @Test
+ public void testConstantScoreMatchesAllDocsNonScoreSort() throws Exception {
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY + " OR (str:e*)^=4.0",
"indent", "true", "sort", "id asc"));
+ assertMetricCounts(response, USE_FILTER_FOR_SORTED_QUERY,
USE_FILTER_FOR_SORTED_QUERY ? 1 : 0, 1, 0, ALL_DOCS);
+ }
+
+ @Test
+ public void testMatchAllDocsPlain() throws Exception {
+ // plain request with "score" sort should skip sort even if `rows`
requested
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true"));
+ assertMetricCounts(response, true, 0, 0, 1);
+ }
+
+ @Test
+ public void testMatchAllDocsFlScore() throws Exception {
+ // explicitly requesting scores should unconditionally disable all cache
consultation and sort optimization
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"rows", "0", "fl", "id,score", "sort", (random().nextBoolean() ? "id asc" :
"score desc")));
+ // NOTE: pretend we're not MatchAllDocs ...
+ assertMetricCounts(response, false, 0, 1, 0, ALL_DOCS);
+ }
+
+ @Test
+ public void testMatchAllDocsZeroRows() throws Exception {
+ // plain request should _always_ skip sort when `rows=0`
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"rows", "0", "sort", "id asc"));
+ assertMetricCounts(response, true, 0, 0, 1);
+ }
+
+ @Test
+ public void testMatchAllDocsNonScoreSort() throws Exception {
+ // plain request _with_ rows and non-score sort should consult cache, but
not skip sort
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"sort", "id asc"));
+ assertMetricCounts(response, true, 0, 1, 0);
+ }
+
+ @Test
+ public void testCursorMark() throws Exception {
+ String q = pickRandom(ALL_QUERIES);
+ boolean includeScoreInSort = random().nextBoolean();
+ String response = JQ(req("q", q, "indent", "true", "cursorMark", "*",
"sort", includeScoreInSort ? "score desc,id asc" : "id asc"));
+ final int expectNumFound = MATCH_ALL_DOCS_QUERY.equals(q) ? ALL_DOCS :
MOST_DOCS;
+ final boolean consultMatchAllDocs;
+ final boolean insertFilterCache;
+ if (includeScoreInSort) {
+ consultMatchAllDocs = false;
+ insertFilterCache = false;
+ } else if (MATCH_ALL_DOCS_QUERY.equals(q)) {
+ consultMatchAllDocs = true;
+ insertFilterCache = false;
+ } else {
+ consultMatchAllDocs = false;
+ insertFilterCache = USE_FILTER_FOR_SORTED_QUERY;
+ }
+ assertMetricCounts(response, consultMatchAllDocs, insertFilterCache ? 1 :
0, 1, 0, expectNumFound);
+ }
+
+ @Test
+ public void testCursorMarkZeroRows() throws Exception {
+ String q = pickRandom(ALL_QUERIES);
+ String response = JQ(req("q", q, "indent", "true", "cursorMark", "*",
"rows", "0", "sort", random().nextBoolean() ? "id asc" : "score desc,id asc"));
+ final boolean consultMatchAllDocs;
+ final boolean insertFilterCache;
+ final boolean skipSort;
+ if (MATCH_ALL_DOCS_QUERY.equals(q)) {
+ consultMatchAllDocs = true;
+ insertFilterCache = false;
+ skipSort = true;
+ } else {
+ consultMatchAllDocs = false;
+ insertFilterCache = USE_FILTER_FOR_SORTED_QUERY;
+ skipSort = USE_FILTER_FOR_SORTED_QUERY;
+ }
+ assertMetricCounts(response, consultMatchAllDocs, insertFilterCache ? 1 :
0, skipSort ? 0 : 1, skipSort ? 1 : 0);
+ }
+
+ private static void assertMetricCounts(String response, boolean
matchAllDocs, int expectFilterCacheInsertCount, int expectFullSortCount, int
expectSkipSortCount) {
+ assertMetricCounts(response, matchAllDocs, expectFilterCacheInsertCount,
expectFullSortCount, expectSkipSortCount, matchAllDocs ? ALL_DOCS : MOST_DOCS);
+ }
+
+ private static void assertMetricCounts(String response, boolean
matchAllDocs, int expectFilterCacheInsertCount,
+ int expectFullSortCount, int
expectSkipSortCount, int expectNumFound) {
+ Map<?, ?> res = (Map<?, ?>) fromJSONString(response);
+ Map<?, ?> body = (Map<?, ?>) (res.get("response"));
+ SolrCore core = h.getCore();
+ assertEquals("Bad matchAllDocs insert count", (matchAllDocs ? 1 : 0),
coreToMatchAllDocsInsertCount(core));
+ assertEquals("Bad filterCache insert count", expectFilterCacheInsertCount,
coreToInserts(core));
+ assertEquals("Bad full sort count", expectFullSortCount,
coreToSortCount(core, "full"));
+ assertEquals("Bad skip sort count", expectSkipSortCount,
coreToSortCount(core, "skip"));
+ assertEquals("Should have exactly " + expectNumFound, expectNumFound,
(long) (body.get("numFound"))); // sanity check
+ }
+
+ @Test
+ public void testConcurrentMatchAllDocsInitialization() throws Exception {
+ final int nThreads = 20;
+ final ExecutorService executor =
ExecutorUtil.newMDCAwareFixedThreadPool(nThreads, new
SolrNamedThreadFactory(getTestName()));
+ final Future<?>[] followup = new Future<?>[nThreads];
+ for (int i = 0; i < nThreads; i++) {
+ final int myI = i;
+ followup[i] = executor.submit(() -> {
+ try {
+ // NOTE: we use cursorMark=* here because it prevents consulting the
queryResultCache, which can interfere
+ // with DocSet fetching (which is what we care about in this test).
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "request_id",
Integer.toString(myI), "cursorMark", "*", "sort", "id asc"));
+ Map<?, ?> res = (Map<?, ?>) fromJSONString(response);
+ Map<?, ?> body = (Map<?, ?>) (res.get("response"));
+ assertEquals("Should have exactly " + ALL_DOCS, ALL_DOCS, (long)
(body.get("numFound"))); // sanity check
+ } catch (Exception ex) {
+ throw new RuntimeException(ex);
+ }
+ });
+ }
+ try {
+ for (Future<?> f : followup) {
+ f.get(); // to access exceptions/errors
Review comment:
`assertNull` here
##########
File path: solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
##########
@@ -0,0 +1,313 @@
+/*
+ * 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.solr.search;
+
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.apache.solr.common.util.Utils.fromJSONString;
+
+/**
+ * Verify caching interactions between main query and filterCache
+ */
+public class TestMainQueryCaching extends SolrTestCaseJ4 {
+
+ private static final int MOST_DOCS = 100;
+ private static final int ALL_DOCS = MOST_DOCS + 1;
+ private static final String TEST_UFFSQ_PROPNAME =
"solr.test.useFilterForSortedQuery";
+ static String RESTORE_UFFSQ_PROP;
+ static boolean USE_FILTER_FOR_SORTED_QUERY;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // TODO: figure out why the line below (accepting this property as
overridden on test invocation) isn't working
+ // as expected.
+ final String uffsq = System.getProperty(TEST_UFFSQ_PROPNAME,
Boolean.toString(random().nextBoolean()));
+ USE_FILTER_FOR_SORTED_QUERY = Boolean.parseBoolean(uffsq);
+ RESTORE_UFFSQ_PROP = System.setProperty(TEST_UFFSQ_PROPNAME, uffsq);
+ initCore("solrconfig-deeppaging.xml", "schema-sorts.xml");
+ createIndex();
+ }
+
+ @AfterClass
+ public static void afterClass() throws Exception {
+ if (RESTORE_UFFSQ_PROP == null) {
+ System.clearProperty(TEST_UFFSQ_PROPNAME);
+ } else {
+ System.setProperty(TEST_UFFSQ_PROPNAME, RESTORE_UFFSQ_PROP);
+ }
+ }
+
+ public static void createIndex() {
+ for (int i = 0; i < MOST_DOCS; i++) {
+ assertU(adoc("id", Integer.toString(i), "str", "d" + i));
+ if (random().nextInt(MOST_DOCS) == 0) {
+ assertU(commit()); // sometimes make multiple segments
+ }
+ }
+ // add an extra doc to distinguish scoring query from `*:*`
+ assertU(adoc("id", Integer.toString(MOST_DOCS), "str", "e" + MOST_DOCS));
+ assertU(commit());
+ }
+
+ @Before
+ public void beforeTest() throws Exception {
+ // testing caching, it's far simpler to just reload the core every time to
prevent
+ // subsequent requests from affecting each other
+ h.reload();
+ }
+
+ private static long coreToInserts(SolrCore core) {
+ return (long)((MetricsMap)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("CACHE.searcher.filterCache")).getGauge())
+ .getValue().get("inserts");
+ }
+
+ private static long coreToSortCount(SolrCore core, String skipOrFull) {
+ return (long)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("SEARCHER.searcher." +
skipOrFull + "SortCount")).getGauge()
+ .getValue();
+ }
+
+ private static long coreToLiveDocsNaiveCacheHitCount(SolrCore core) {
+ return (long)((SolrMetricManager.GaugeWrapper<?>)core
+
.getCoreMetricManager().getRegistry().getMetrics().get("SEARCHER.searcher.liveDocsNaiveCacheHitCount")).getGauge()
+ .getValue();
+ }
+
+ private static long coreToMatchAllDocsInsertCount(SolrCore core) {
+ return (long) coreToLiveDocsCacheMetrics(core).get("inserts");
+ }
+
+ private static Map<String, Object> coreToLiveDocsCacheMetrics(SolrCore core)
{
+ return
((MetricsMap)((SolrMetricManager.GaugeWrapper<?>)core.getCoreMetricManager().getRegistry()
+
.getMetrics().get("CACHE.searcher.liveDocsCache")).getGauge()).getValue();
+ }
+ private static final String SCORING_QUERY = "str:d*";
+ private static final String CONSTANT_SCORE_QUERY = "(" + SCORING_QUERY +
")^=1.0"; // wrapped as a ConstantScoreQuery
+ private static final String MATCH_ALL_DOCS_QUERY = "*:*";
+
+ private static final String[] ALL_QUERIES = new String[] { SCORING_QUERY,
CONSTANT_SCORE_QUERY, MATCH_ALL_DOCS_QUERY };
+
+ @Test
+ public void testScoringQuery() throws Exception {
+ // plain request should have no caching or sorting optimization
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true"));
+ assertMetricCounts(response, false, 0, 1, 0);
+ }
+
+ @Test
+ public void testConstantScoreFlScore() throws Exception {
+ // explicitly requesting scores should unconditionally disable caching and
sorting optimizations
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true",
"rows", "0", "fl", "id,score", "sort", (random().nextBoolean() ? "id asc" :
"score desc")));
+ assertMetricCounts(response, false, 0, 1, 0);
+ }
+
+ @Test
+ public void testScoringQueryNonScoreSort() throws Exception {
+ // plain request with no score in sort should consult filterCache, but
need full sorting
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true", "sort", "id
asc"));
+ assertMetricCounts(response, false, USE_FILTER_FOR_SORTED_QUERY ? 1 : 0,
1, 0);
+ }
+
+ @Test
+ public void testScoringQueryZeroRows() throws Exception {
+ // always hit cache, optimize sort because rows=0
+ String response = JQ(req("q", SCORING_QUERY, "indent", "true", "rows",
"0", "sort", (random().nextBoolean() ? "id asc" : "score desc")));
+ final int insertAndSkipCount = USE_FILTER_FOR_SORTED_QUERY ? 1 : 0;
+ assertMetricCounts(response, false, insertAndSkipCount,
USE_FILTER_FOR_SORTED_QUERY ? 0 : 1, insertAndSkipCount);
+ }
+
+ @Test
+ public void testConstantScoreSortByScore() throws Exception {
+ // hit cache and skip sort because constant score query
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true"));
+ final int insertAndSkipCount = USE_FILTER_FOR_SORTED_QUERY ? 1 : 0;
+ assertMetricCounts(response, false, insertAndSkipCount,
USE_FILTER_FOR_SORTED_QUERY ? 0 : 1, insertAndSkipCount);
+ }
+
+ @Test
+ public void testConstantScoreNonScoreSort() throws Exception {
+ // consult filterCache because constant score query, but no skip sort
(because sort-by-id)
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY, "indent", "true",
"sort", "id asc"));
+ assertMetricCounts(response, false, USE_FILTER_FOR_SORTED_QUERY ? 1 : 0,
1, 0);
+ }
+
+ /**
+ * As {@link #testConstantScoreNonScoreSort} (though an analogous test could
be written corresponding to
+ * {@link #testConstantScoreSortByScore()}, etc...); but with an additional
constant-score clause that causes
+ * the associated DocSet, (if {@link #USE_FILTER_FOR_SORTED_QUERY}==true) to
be cached as equivalent to
+ * MatchAllDocsQuery/liveDocs, _in addition to_ in the filterCache.
+ *
+ * This is an edge case, but it's the behavior we want, and despite there
being two entries, the actual DocSet
+ * will be the same (`==`) in both locations (liveDocs and filterCache)
+ */
+ @Test
+ public void testConstantScoreMatchesAllDocsNonScoreSort() throws Exception {
+ String response = JQ(req("q", CONSTANT_SCORE_QUERY + " OR (str:e*)^=4.0",
"indent", "true", "sort", "id asc"));
+ assertMetricCounts(response, USE_FILTER_FOR_SORTED_QUERY,
USE_FILTER_FOR_SORTED_QUERY ? 1 : 0, 1, 0, ALL_DOCS);
+ }
+
+ @Test
+ public void testMatchAllDocsPlain() throws Exception {
+ // plain request with "score" sort should skip sort even if `rows`
requested
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true"));
+ assertMetricCounts(response, true, 0, 0, 1);
+ }
+
+ @Test
+ public void testMatchAllDocsFlScore() throws Exception {
+ // explicitly requesting scores should unconditionally disable all cache
consultation and sort optimization
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"rows", "0", "fl", "id,score", "sort", (random().nextBoolean() ? "id asc" :
"score desc")));
+ // NOTE: pretend we're not MatchAllDocs ...
+ assertMetricCounts(response, false, 0, 1, 0, ALL_DOCS);
+ }
+
+ @Test
+ public void testMatchAllDocsZeroRows() throws Exception {
+ // plain request should _always_ skip sort when `rows=0`
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"rows", "0", "sort", "id asc"));
+ assertMetricCounts(response, true, 0, 0, 1);
+ }
+
+ @Test
+ public void testMatchAllDocsNonScoreSort() throws Exception {
+ // plain request _with_ rows and non-score sort should consult cache, but
not skip sort
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "indent", "true",
"sort", "id asc"));
+ assertMetricCounts(response, true, 0, 1, 0);
+ }
+
+ @Test
+ public void testCursorMark() throws Exception {
+ String q = pickRandom(ALL_QUERIES);
+ boolean includeScoreInSort = random().nextBoolean();
+ String response = JQ(req("q", q, "indent", "true", "cursorMark", "*",
"sort", includeScoreInSort ? "score desc,id asc" : "id asc"));
+ final int expectNumFound = MATCH_ALL_DOCS_QUERY.equals(q) ? ALL_DOCS :
MOST_DOCS;
+ final boolean consultMatchAllDocs;
+ final boolean insertFilterCache;
+ if (includeScoreInSort) {
+ consultMatchAllDocs = false;
+ insertFilterCache = false;
+ } else if (MATCH_ALL_DOCS_QUERY.equals(q)) {
+ consultMatchAllDocs = true;
+ insertFilterCache = false;
+ } else {
+ consultMatchAllDocs = false;
+ insertFilterCache = USE_FILTER_FOR_SORTED_QUERY;
+ }
+ assertMetricCounts(response, consultMatchAllDocs, insertFilterCache ? 1 :
0, 1, 0, expectNumFound);
+ }
+
+ @Test
+ public void testCursorMarkZeroRows() throws Exception {
+ String q = pickRandom(ALL_QUERIES);
+ String response = JQ(req("q", q, "indent", "true", "cursorMark", "*",
"rows", "0", "sort", random().nextBoolean() ? "id asc" : "score desc,id asc"));
+ final boolean consultMatchAllDocs;
+ final boolean insertFilterCache;
+ final boolean skipSort;
+ if (MATCH_ALL_DOCS_QUERY.equals(q)) {
+ consultMatchAllDocs = true;
+ insertFilterCache = false;
+ skipSort = true;
+ } else {
+ consultMatchAllDocs = false;
+ insertFilterCache = USE_FILTER_FOR_SORTED_QUERY;
+ skipSort = USE_FILTER_FOR_SORTED_QUERY;
+ }
+ assertMetricCounts(response, consultMatchAllDocs, insertFilterCache ? 1 :
0, skipSort ? 0 : 1, skipSort ? 1 : 0);
+ }
+
+ private static void assertMetricCounts(String response, boolean
matchAllDocs, int expectFilterCacheInsertCount, int expectFullSortCount, int
expectSkipSortCount) {
+ assertMetricCounts(response, matchAllDocs, expectFilterCacheInsertCount,
expectFullSortCount, expectSkipSortCount, matchAllDocs ? ALL_DOCS : MOST_DOCS);
+ }
+
+ private static void assertMetricCounts(String response, boolean
matchAllDocs, int expectFilterCacheInsertCount,
+ int expectFullSortCount, int
expectSkipSortCount, int expectNumFound) {
+ Map<?, ?> res = (Map<?, ?>) fromJSONString(response);
+ Map<?, ?> body = (Map<?, ?>) (res.get("response"));
+ SolrCore core = h.getCore();
+ assertEquals("Bad matchAllDocs insert count", (matchAllDocs ? 1 : 0),
coreToMatchAllDocsInsertCount(core));
+ assertEquals("Bad filterCache insert count", expectFilterCacheInsertCount,
coreToInserts(core));
+ assertEquals("Bad full sort count", expectFullSortCount,
coreToSortCount(core, "full"));
+ assertEquals("Bad skip sort count", expectSkipSortCount,
coreToSortCount(core, "skip"));
+ assertEquals("Should have exactly " + expectNumFound, expectNumFound,
(long) (body.get("numFound"))); // sanity check
+ }
+
+ @Test
+ public void testConcurrentMatchAllDocsInitialization() throws Exception {
+ final int nThreads = 20;
+ final ExecutorService executor =
ExecutorUtil.newMDCAwareFixedThreadPool(nThreads, new
SolrNamedThreadFactory(getTestName()));
+ final Future<?>[] followup = new Future<?>[nThreads];
+ for (int i = 0; i < nThreads; i++) {
+ final int myI = i;
+ followup[i] = executor.submit(() -> {
+ try {
+ // NOTE: we use cursorMark=* here because it prevents consulting the
queryResultCache, which can interfere
+ // with DocSet fetching (which is what we care about in this test).
+ String response = JQ(req("q", MATCH_ALL_DOCS_QUERY, "request_id",
Integer.toString(myI), "cursorMark", "*", "sort", "id asc"));
+ Map<?, ?> res = (Map<?, ?>) fromJSONString(response);
+ Map<?, ?> body = (Map<?, ?>) (res.get("response"));
+ assertEquals("Should have exactly " + ALL_DOCS, ALL_DOCS, (long)
(body.get("numFound"))); // sanity check
+ } catch (Exception ex) {
+ throw new RuntimeException(ex);
+ }
+ });
+ }
+ try {
+ for (Future<?> f : followup) {
+ f.get(); // to access exceptions/errors
+ }
+ } finally {
+ executor.shutdown();
+ assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS)); // tasks
should already have completed
+ }
+ final SolrCore core = h.getCore();
+ Map<String, Object> liveDocsCacheMetrics =
coreToLiveDocsCacheMetrics(core);
+ long inserts = (long) liveDocsCacheMetrics.get("inserts"); // the one and
only liveDocs computation
+ long hits = (long) liveDocsCacheMetrics.get("hits"); // hits during the
initial phase
+ long asyncHits = (long) liveDocsCacheMetrics.get("asyncHits");
+ long naiveHits = coreToLiveDocsNaiveCacheHitCount(core);
+
+ assertEquals(1, inserts);
+ assertEquals(nThreads - 1, hits + naiveHits);
+ assertTrue(asyncHits <= hits);
+
+ // NOTE: The assertion below is commented out because, although it may
_often_ be true, it is dependent
+ // on timing/thread scheduling; in practice it happens that not
infrequently `asyncHits == 0` (e.g., if matchAllDocs
+ // computation happens quickly, and/or if subsequent threads were delayed).
+ //
+ // It seems that the assertion below more frequently succeeds when this
test is run in isolation; e.g.:
+ // `gradlew :solr:core:test --tests
"org.apache.solr.search.TestMainQueryCaching.testConcurrentMatchAllDocsInitialization"`
+
+ //assertTrue("expected asyncHits > 0; found asyncHits=" + asyncHits,
asyncHits > 0);
Review comment:
We could introduce delay at some point to cause threads to stall via
TestInjection, but I'm fine without it.
##########
File path: solr/core/src/java/org/apache/solr/search/QueryUtils.java
##########
@@ -45,6 +46,39 @@ public static boolean isNegative(Query q) {
return true;
}
+ /**
+ * Recursively unwraps the specified query to determine whether it is
capable of producing a score
+ * that varies across different documents. Returns true if this query is not
capable of producing a
+ * varying score (i.e., it is a constant score query).
+ */
+ public static boolean isConstantScoreQuery(Query q) {
+ for (;;) {
+ if (q instanceof BoostQuery) {
+ q = ((BoostQuery) q).getQuery();
+ } else if (q instanceof WrappedQuery) {
+ q = ((WrappedQuery) q).getWrappedQuery();
+ } else if (q instanceof ConstantScoreQuery) {
+ return true;
+ } else if (q instanceof MatchAllDocsQuery) {
+ return true;
+ } else if (q instanceof MatchNoDocsQuery) {
+ return true;
+ } else if (q instanceof Filter || q instanceof SolrConstantScoreQuery) {
+ // TODO: this clause will be replaced with `q instanceof DocSetQuery`,
pending SOLR-12336
Review comment:
This has been merged :)
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
