This is an automated email from the ASF dual-hosted git repository.
psalagnac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new e049d156ab7 SOLR-17889: Fix noisy TestMainQueryCaching and
TestFiltersQueryCaching, and cleanups (#3660)
e049d156ab7 is described below
commit e049d156ab709e8470c82c0622dbc48b5da94c7a
Author: Pierre Salagnac <[email protected]>
AuthorDate: Thu Sep 18 12:28:24 2025 +0200
SOLR-17889: Fix noisy TestMainQueryCaching and TestFiltersQueryCaching, and
cleanups (#3660)
This change includes some code cleanups in the code that opens a new
searcher (with no behavior changes) mostly to remove some annoying
@SuppressWarnings("unchecked") annotations. No functional changes there.
---
.../java/org/apache/solr/cloud/ZkController.java | 3 +-
.../src/java/org/apache/solr/core/SolrCore.java | 12 +++---
.../java/org/apache/solr/handler/IndexFetcher.java | 3 +-
.../java/org/apache/solr/handler/RestoreCore.java | 3 +-
.../apache/solr/update/DirectUpdateHandler2.java | 5 +--
.../org/apache/solr/update/SolrIndexSplitter.java | 3 +-
.../solr/search/TestFiltersQueryCaching.java | 44 ++++++++++++----------
.../org/apache/solr/search/TestIndexSearcher.java | 3 +-
.../apache/solr/search/TestMainQueryCaching.java | 7 ++++
9 files changed, 44 insertions(+), 39 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
index df00ee0dcaf..688ce428501 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -3080,8 +3080,7 @@ public class ZkController implements Closeable {
}
registeredSearcher.decref();
} else {
- @SuppressWarnings("unchecked")
- Future<Void>[] waitSearcher = (Future<Void>[])
Array.newInstance(Future.class, 1);
+ Future<?>[] waitSearcher = (Future<?>[])
Array.newInstance(Future.class, 1);
if (log.isInfoEnabled()) {
log.info(
"No registered searcher found for core: {}, waiting until a
searcher is registered before publishing as active",
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java
b/solr/core/src/java/org/apache/solr/core/SolrCore.java
index ce0b94aa220..dbfe8f7ee0a 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -2068,8 +2068,8 @@ public class SolrCore implements SolrInfoBean, Closeable {
private int onDeckSearchers; // number of searchers preparing
// Lock ordering: one can acquire the openSearcherLock and then the
searcherLock, but not
// vice-versa.
- private Object searcherLock = new Object(); // the sync object for the
searcher
- private ReentrantLock openSearcherLock =
+ private final Object searcherLock = new Object(); // the sync object for the
searcher
+ private final ReentrantLock openSearcherLock =
new ReentrantLock(true); // used to serialize opens/reopens for absolute
ordering
private final int maxWarmingSearchers; // max number of on-deck searchers
allowed
private final int slowQueryThresholdMillis; // threshold above which a query
is considered slow
@@ -2269,7 +2269,7 @@ public class SolrCore implements SolrInfoBean, Closeable {
}
public RefCounted<SolrIndexSearcher> getSearcher(
- boolean forceNew, boolean returnSearcher, final Future<Void>[]
waitSearcher) {
+ boolean forceNew, boolean returnSearcher, final Future<?>[]
waitSearcher) {
return getSearcher(forceNew, returnSearcher, waitSearcher, false);
}
@@ -2506,7 +2506,7 @@ public class SolrCore implements SolrInfoBean, Closeable {
public RefCounted<SolrIndexSearcher> getSearcher(
boolean forceNew,
boolean returnSearcher,
- final Future<Void>[] waitSearcher,
+ final Future<?>[] waitSearcher,
boolean updateHandlerReopens) {
// it may take some time to open an index.... we may need to make
// sure that two threads aren't trying to open one at the same time
@@ -2574,7 +2574,6 @@ public class SolrCore implements SolrInfoBean, Closeable {
}
// a signal to decrement onDeckSearchers if something goes wrong.
- final boolean[] decrementOnDeckCount = new boolean[] {true};
RefCounted<SolrIndexSearcher> currSearcherHolder = null; // searcher we
are autowarming from
RefCounted<SolrIndexSearcher> searchHolder = null;
boolean success = false;
@@ -2599,7 +2598,6 @@ public class SolrCore implements SolrInfoBean, Closeable {
// want to register this one before warming is complete instead of
waiting.
if (solrConfig.useColdSearcher) {
registerSearcher(newSearchHolder);
- decrementOnDeckCount[0] = false;
alreadyRegistered = true;
}
} else {
@@ -2612,7 +2610,7 @@ public class SolrCore implements SolrInfoBean, Closeable {
final SolrIndexSearcher currSearcher =
currSearcherHolder == null ? null : currSearcherHolder.get();
- Future<Void> future = null;
+ Future<?> future = null;
// if the underlying searcher has not changed, no warming is needed
if (newSearcher != currSearcher) {
diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
index 58e19057da1..7eb563d1629 100644
--- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
+++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
@@ -1019,8 +1019,7 @@ public class IndexFetcher {
if (core == null) {
return; // core closed, presumably
}
- @SuppressWarnings("unchecked")
- Future<Void>[] waitSearcher = (Future<Void>[])
Array.newInstance(Future.class, 1);
+ Future<?>[] waitSearcher = (Future<?>[]) Array.newInstance(Future.class,
1);
RefCounted<SolrIndexSearcher> searcher = core.getSearcher(true, true,
waitSearcher, true);
try {
if (waitSearcher[0] != null) {
diff --git a/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
b/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
index 9bbfa4b9049..7f4abc18ffd 100644
--- a/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
+++ b/solr/core/src/java/org/apache/solr/handler/RestoreCore.java
@@ -196,8 +196,7 @@ public class RestoreCore implements Callable<Boolean> {
}
private void openNewSearcher() throws Exception {
- @SuppressWarnings("unchecked")
- Future<Void>[] waitSearcher = (Future<Void>[])
Array.newInstance(Future.class, 1);
+ Future<?>[] waitSearcher = (Future<?>[]) Array.newInstance(Future.class,
1);
core.getSearcher(true, false, waitSearcher, true);
if (waitSearcher[0] != null) {
waitSearcher[0].get();
diff --git
a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
index 60dbb9c4f28..f990f3cf58f 100644
--- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
+++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
@@ -729,9 +729,8 @@ public class DirectUpdateHandler2 extends UpdateHandler
if (cmd.expungeDeletes) expungeDeleteCommands.mark();
}
- @SuppressWarnings("unchecked")
- Future<Void>[] waitSearcher =
- cmd.waitSearcher ? (Future<Void>[]) Array.newInstance(Future.class, 1)
: null;
+ Future<?>[] waitSearcher =
+ cmd.waitSearcher ? (Future<?>[]) Array.newInstance(Future.class, 1) :
null;
boolean error = true;
try {
diff --git a/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java
b/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java
index 2b9d8252440..9312b831e75 100644
--- a/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java
+++ b/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java
@@ -512,8 +512,7 @@ public class SolrIndexSplitter {
}
private void openNewSearcher(SolrCore core) throws Exception {
- @SuppressWarnings("unchecked")
- Future<Void>[] waitSearcher = (Future<Void>[])
Array.newInstance(Future.class, 1);
+ Future<?>[] waitSearcher = (Future<?>[]) Array.newInstance(Future.class,
1);
core.getSearcher(true, false, waitSearcher, true);
if (waitSearcher[0] != null) {
waitSearcher[0].get();
diff --git
a/solr/core/src/test/org/apache/solr/search/TestFiltersQueryCaching.java
b/solr/core/src/test/org/apache/solr/search/TestFiltersQueryCaching.java
index 0319b1be6d0..48733cca532 100644
--- a/solr/core/src/test/org/apache/solr/search/TestFiltersQueryCaching.java
+++ b/solr/core/src/test/org/apache/solr/search/TestFiltersQueryCaching.java
@@ -16,11 +16,13 @@
*/
package org.apache.solr.search;
+import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.Future;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.core.SolrCore;
import org.apache.solr.metrics.MetricsMap;
@@ -49,6 +51,19 @@ public class TestFiltersQueryCaching extends SolrTestCaseJ4 {
assertU(commit());
}
+ /** Reload the core to reset non-cumulative cache metrics. */
+ private void reloadAndWait() throws Exception {
+ h.reload();
+
+ // Make sure the new searcher (after reload) is fully initialized. This is
to avoid races
+ // between queries submitted by the test and cache metrics warmup of the
new searcher.
+ // Another option could be to not reload the core and use cumulative
metrics, but the test
+ // would be harder to understand.
+ Future<?>[] waitSearcher = (Future<?>[]) Array.newInstance(Future.class,
1);
+ h.getCore().getSearcher(true, false, waitSearcher, true);
+ waitSearcher[0].get();
+ }
+
private static Map<String, Object> lookupFilterCacheMetrics(SolrCore core) {
return ((MetricsMap)
((SolrMetricManager.GaugeWrapper<?>)
@@ -61,16 +76,7 @@ public class TestFiltersQueryCaching extends SolrTestCaseJ4 {
}
private static long lookupFilterCacheInserts(SolrCore core) {
- return (long)
- ((MetricsMap)
- ((SolrMetricManager.GaugeWrapper<?>)
- core.getCoreMetricManager()
- .getRegistry()
- .getMetrics()
- .get("CACHE.searcher.filterCache"))
- .getGauge())
- .getValue()
- .get("inserts");
+ return (long) lookupFilterCacheMetrics(core).get("inserts");
}
@Test
@@ -80,45 +86,45 @@ public class TestFiltersQueryCaching extends SolrTestCaseJ4
{
final int expectNumFound = 1;
final String expectNumFoundXPath = "/response/numFound==" + expectNumFound;
- h.reload();
+ reloadAndWait();
assertJQ(req("q", termQuery, "indent", "true"), expectNumFoundXPath);
assertEquals(0, lookupFilterCacheInserts(h.getCore()));
- h.reload();
+ reloadAndWait();
assertJQ(req("q", filterTermQuery, "indent", "true"), expectNumFoundXPath);
assertEquals(1, lookupFilterCacheInserts(h.getCore()));
- h.reload();
+ reloadAndWait();
assertJQ(
req("q", "*:*", "indent", "true", "fq", "{!cache=false}field_s:d0"),
expectNumFoundXPath);
assertEquals(0, lookupFilterCacheInserts(h.getCore()));
- h.reload();
+ reloadAndWait();
assertJQ(
req("q", "*:*", "indent", "true", "fq", "{!cache=true}field_s:d0"),
expectNumFoundXPath);
assertEquals(1, lookupFilterCacheInserts(h.getCore()));
- h.reload();
+ reloadAndWait();
assertJQ(
req("q", "*:*", "indent", "true", "fq",
"{!cache=false}filter(field_s:d0)"),
expectNumFoundXPath);
assertEquals(1, lookupFilterCacheInserts(h.getCore()));
- h.reload();
+ reloadAndWait();
assertJQ(
req("q", "*:*", "indent", "true", "fq",
"{!cache=true}filter(field_s:d0)"),
expectNumFoundXPath);
assertEquals(1, lookupFilterCacheInserts(h.getCore()));
- h.reload();
+ reloadAndWait();
assertJQ(req("q", "*:*", "indent", "true", "fq", termQuery),
expectNumFoundXPath);
assertEquals(1, lookupFilterCacheInserts(h.getCore()));
- h.reload();
+ reloadAndWait();
assertJQ(req("q", "*:*", "indent", "true", "fq", filterTermQuery),
expectNumFoundXPath);
assertEquals(1, lookupFilterCacheInserts(h.getCore()));
- h.reload();
+ reloadAndWait();
SolrCore core = h.getCore();
Map<String, Object> filterCacheMetrics;
final String termQuery2 = "{!term f=field_s v='d1'}";
diff --git a/solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java
b/solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java
index 25975ce80e8..649d7bbd914 100644
--- a/solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java
+++ b/solr/core/src/test/org/apache/solr/search/TestIndexSearcher.java
@@ -259,8 +259,7 @@ public class TestIndexSearcher extends SolrTestCaseJ4 {
addDummyDoc(newCore);
// Open a new searcher, this should call the newSearcherListeners
- @SuppressWarnings("unchecked")
- Future<Void>[] future = (Future<Void>[]) Array.newInstance(Future.class,
1);
+ Future<?>[] future = (Future<?>[]) Array.newInstance(Future.class, 1);
newCore.getSearcher(true, false, future);
future[0].get();
diff --git
a/solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
b/solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
index 8a298be2cb1..2cebca1ffe1 100644
--- a/solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
+++ b/solr/core/src/test/org/apache/solr/search/TestMainQueryCaching.java
@@ -18,6 +18,7 @@ package org.apache.solr.search;
import static org.apache.solr.common.util.Utils.fromJSONString;
+import java.lang.reflect.Array;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.ExecutorService;
@@ -86,6 +87,12 @@ public class TestMainQueryCaching extends SolrTestCaseJ4 {
// testing caching, it's far simpler to just reload the core every time to
prevent
// subsequent requests from affecting each other
h.reload();
+
+ // Make sure the new searcher (after reload) is fully initialized. This is
to avoid races
+ // between queries submitted by the test and metrics warmup of the new
searcher.
+ Future<?>[] waitSearcher = (Future<?>[]) Array.newInstance(Future.class,
1);
+ h.getCore().getSearcher(true, false, waitSearcher, true);
+ waitSearcher[0].get();
}
private static long coreToInserts(SolrCore core, String cacheName) {
