You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ma...@apache.org on 2022/03/29 18:22:35 UTC
[solr] branch branch_9x updated: SOLR-16002: Avoid redundant `FilterQuery` caching, e.g. via `filter($query)` syntax (#624)
This is an automated email from the ASF dual-hosted git repository.
magibney pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new f90af2a SOLR-16002: Avoid redundant `FilterQuery` caching, e.g. via `filter($query)` syntax (#624)
f90af2a is described below
commit f90af2aa0c522055abec4eac346c562afc38bd3a
Author: Michael Gibney <mi...@michaelgibney.net>
AuthorDate: Tue Mar 29 14:16:30 2022 -0400
SOLR-16002: Avoid redundant `FilterQuery` caching, e.g. via `filter($query)` syntax (#624)
(cherry picked from commit 534696cf0e83c8671a1d3ef31e2ba2e6a93d7ec0)
---
solr/CHANGES.txt | 2 +
.../java/org/apache/solr/query/FilterQuery.java | 28 +++
.../solr/search/TestFiltersQueryCaching.java | 197 +++++++++++++++++++++
3 files changed, 227 insertions(+)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index fd1e407..8a8c610 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -16,6 +16,8 @@ Improvements
* SOLR-15652: Add Slack channel for community support to Admin UI footer, update links. (Eric Pugh)
+* SOLR-16002: Avoid redundant `FilterQuery` caching, e.g. via `filter($query)` syntax. (Michael Gibney, David Smiley)
+
Optimizations
---------------------
(No changes)
diff --git a/solr/core/src/java/org/apache/solr/query/FilterQuery.java b/solr/core/src/java/org/apache/solr/query/FilterQuery.java
index 28cae88..ce49583 100644
--- a/solr/core/src/java/org/apache/solr/query/FilterQuery.java
+++ b/solr/core/src/java/org/apache/solr/query/FilterQuery.java
@@ -46,6 +46,34 @@ public class FilterQuery extends ExtendedQueryBase {
this.q = q;
}
+ @Override
+ public final void setCache(boolean cache) {
+ /*
+ NOTE: at the implementation level, we silently ignore explicit `setCache` directives. But at a higher level
+ (i.e., from the client's perspective) by ignoring at the implementation level, we in fact respect the semantics
+ both of explicit `{!cache=false}filter(q)` and `{!cache=true}filter(q)`. Since the purpose of FilterQuery
+ is to handle caching _internal_ to the query, external `cache=false` _should_ have no effect. Slightly
+ less intuitive: the `cache=true` case should be interpreted as directing "ensure that we consult the
+ filterCache for this query" -- and indeed because caching is handled internally, the essence of the
+ top-level `cache=true` directive is most appropriately respected by having `getCache()` continue to return
+ `false` at the level of the FilterQuery _per se_.
+ */
+ }
+
+ @Override
+ public final boolean getCache() {
+ return false;
+ /*
+ Paradoxically, this _is_ what we want. The FilterQuery wrapper is designed to ensure that its
+ inner query always consults the filterCache, regardless of the context in which it's called.
+ FilterQuery internally calls SolrIndexSearcher.getDocSet with its _wrapped_ query, so the caching
+ happens at that level, and we want _not_ to consult the filterCache with the FilterQuery wrapper
+ per se. Allowing `getCache()=true` here can result in double-entry in the filterCache (e.g. when
+ using `fq=filter({!term f=field v=term})`, or caching separate clauses of a BooleanQuery via
+ `fq={!bool should='filter($q1)' should='filter($q2)'}`).
+ */
+ }
+
public Query getQuery() {
return q;
}
diff --git a/solr/core/src/test/org/apache/solr/search/TestFiltersQueryCaching.java b/solr/core/src/test/org/apache/solr/search/TestFiltersQueryCaching.java
new file mode 100644
index 0000000..cf797fe
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/search/TestFiltersQueryCaching.java
@@ -0,0 +1,197 @@
+/*
+ * 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 org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** Verify caching impacts of FiltersQParser and FilterQuery */
+public class TestFiltersQueryCaching extends SolrTestCaseJ4 {
+
+ private static final int NUM_DOCS = 20;
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ initCore("solrconfig.xml", "schema_latest.xml");
+ createIndex();
+ }
+
+ public static void createIndex() {
+ for (int i = 0; i < NUM_DOCS; i++) {
+ assertU(adoc("id", Integer.toString(i), "field_s", "d" + i));
+ if (random().nextInt(NUM_DOCS) == 0) {
+ assertU(commit()); // sometimes make multiple segments
+ }
+ }
+ assertU(commit());
+ }
+
+ private static Map<String, Object> lookupFilterCacheMetrics(SolrCore core) {
+ return ((MetricsMap)
+ ((SolrMetricManager.GaugeWrapper<?>)
+ core.getCoreMetricManager()
+ .getRegistry()
+ .getMetrics()
+ .get("CACHE.searcher.filterCache"))
+ .getGauge())
+ .getValue();
+ }
+
+ private static long lookupFilterCacheInserts(SolrCore core) {
+ return (long)
+ ((MetricsMap)
+ ((SolrMetricManager.GaugeWrapper<?>)
+ core.getCoreMetricManager()
+ .getRegistry()
+ .getMetrics()
+ .get("CACHE.searcher.filterCache"))
+ .getGauge())
+ .getValue()
+ .get("inserts");
+ }
+
+ @Test
+ public void testRecursiveFilter() throws Exception {
+ final String termQuery = "{!term f=field_s v='d0'}";
+ final String filterTermQuery = "filter(" + termQuery + ")";
+ final int expectNumFound = 1;
+ final String expectNumFoundXPath = "/response/numFound==" + expectNumFound;
+
+ h.reload();
+ assertJQ(req("q", termQuery, "indent", "true"), expectNumFoundXPath);
+ assertEquals(0, lookupFilterCacheInserts(h.getCore()));
+
+ h.reload();
+ assertJQ(req("q", filterTermQuery, "indent", "true"), expectNumFoundXPath);
+ assertEquals(1, lookupFilterCacheInserts(h.getCore()));
+
+ h.reload();
+ assertJQ(
+ req("q", "*:*", "indent", "true", "fq", "{!cache=false}field_s:d0"), expectNumFoundXPath);
+ assertEquals(0, lookupFilterCacheInserts(h.getCore()));
+
+ h.reload();
+ assertJQ(
+ req("q", "*:*", "indent", "true", "fq", "{!cache=true}field_s:d0"), expectNumFoundXPath);
+ assertEquals(1, lookupFilterCacheInserts(h.getCore()));
+
+ h.reload();
+ assertJQ(
+ req("q", "*:*", "indent", "true", "fq", "{!cache=false}filter(field_s:d0)"),
+ expectNumFoundXPath);
+ assertEquals(1, lookupFilterCacheInserts(h.getCore()));
+
+ h.reload();
+ assertJQ(
+ req("q", "*:*", "indent", "true", "fq", "{!cache=true}filter(field_s:d0)"),
+ expectNumFoundXPath);
+ assertEquals(1, lookupFilterCacheInserts(h.getCore()));
+
+ h.reload();
+ assertJQ(req("q", "*:*", "indent", "true", "fq", termQuery), expectNumFoundXPath);
+ assertEquals(1, lookupFilterCacheInserts(h.getCore()));
+
+ h.reload();
+ assertJQ(req("q", "*:*", "indent", "true", "fq", filterTermQuery), expectNumFoundXPath);
+ assertEquals(1, lookupFilterCacheInserts(h.getCore()));
+
+ h.reload();
+ SolrCore core = h.getCore();
+ Map<String, Object> filterCacheMetrics;
+ final String termQuery2 = "{!term f=field_s v='d1'}";
+ final String filterTermQuery2 = "filter(" + termQuery2 + ")";
+ assertJQ(
+ req(
+ "q",
+ "*:*",
+ "indent",
+ "true",
+ "fq",
+ "{!bool cache=false should=$ftq should=$ftq2}",
+ "ftq",
+ filterTermQuery,
+ "ftq2",
+ filterTermQuery2),
+ "/response/numFound==2");
+ assertEquals(2, lookupFilterCacheInserts(core));
+ JQ(
+ req(
+ "q",
+ "*:*",
+ "indent",
+ "true",
+ "fq",
+ random().nextBoolean() ? termQuery : filterTermQuery));
+ filterCacheMetrics = lookupFilterCacheMetrics(core);
+ assertEquals(2, (long) filterCacheMetrics.get("inserts")); // unchanged
+ assertEquals(1, (long) filterCacheMetrics.get("hits"));
+ JQ(
+ req(
+ "q",
+ "*:*",
+ "indent",
+ "true",
+ "fq",
+ random().nextBoolean() ? termQuery2 : filterTermQuery2));
+ filterCacheMetrics = lookupFilterCacheMetrics(core);
+ assertEquals(2, (long) filterCacheMetrics.get("inserts")); // unchanged
+ assertEquals(2, (long) filterCacheMetrics.get("hits"));
+ JQ(
+ req(
+ "q",
+ "*:*",
+ "indent",
+ "true",
+ "fq",
+ "{!bool cache=false should=$ftq should=$ftq2}",
+ "ftq",
+ filterTermQuery,
+ "ftq2",
+ filterTermQuery2,
+ "cursorMark",
+ "*",
+ "sort",
+ "id asc"));
+ filterCacheMetrics = lookupFilterCacheMetrics(core);
+ assertEquals(2, (long) filterCacheMetrics.get("inserts")); // unchanged
+ assertEquals(4, (long) filterCacheMetrics.get("hits"));
+ JQ(
+ req(
+ "q",
+ "*:*",
+ "indent",
+ "true",
+ "fq",
+ "{!bool should=$ftq should=$ftq2}",
+ "ftq",
+ filterTermQuery,
+ "ftq2",
+ filterTermQuery2,
+ "cursorMark",
+ "*",
+ "sort",
+ "id asc"));
+ filterCacheMetrics = lookupFilterCacheMetrics(core);
+ assertEquals(3, (long) filterCacheMetrics.get("inserts")); // added top-level
+ assertEquals(6, (long) filterCacheMetrics.get("hits"));
+ }
+}