This is an automated email from the ASF dual-hosted git repository.
hossman 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 0897e7ed0bf SOLR-5386: Fix various methods of invoking "local" solr
requests to re-use the same searcher as the original request.
0897e7ed0bf is described below
commit 0897e7ed0bf8fb5504a7066527980d32c57c644a
Author: Chris Hostetter <[email protected]>
AuthorDate: Thu Apr 10 12:57:28 2025 -0700
SOLR-5386: Fix various methods of invoking "local" solr requests to re-use
the same searcher as the original request.
This notably fixes the use of spellcheck.maxCollationTries in firstSearcher
warming queries to prevent deadlock
(cherry picked from commit 17510244b214036ad9365019c5053c1711140da8)
---
solr/CHANGES.txt | 3 ++
.../org/apache/solr/core/QuerySenderListener.java | 12 +----
.../java/org/apache/solr/handler/BlobHandler.java | 3 +-
.../org/apache/solr/handler/SolrConfigHandler.java | 3 +-
.../org/apache/solr/request/SolrQueryRequest.java | 51 ++++++++++++++++++++++
.../org/apache/solr/request/SolrRequestInfo.java | 7 ++-
.../solr/search/CollapsingQParserPlugin.java | 3 +-
.../org/apache/solr/search/SolrIndexSearcher.java | 13 +-----
.../apache/solr/spelling/SpellCheckCollator.java | 3 +-
.../conf/solrconfig-spellcheckcomponent.xml | 21 +++++++--
.../handler/component/SpellCheckComponentTest.java | 12 +++++
11 files changed, 92 insertions(+), 39 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a010d2a4972..16eebdd079e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -75,6 +75,9 @@ Bug Fixes
* SOLR-17692: Core unload/deletion now preempts all forms of ongoing
"recovery", rather than inadvertently waiting for
completion in some cases. (Jason Gerlowski)
+* SOLR-5386: Fix various methods of invoking "local" solr requests to re-use
the same searcher as the original request.
+ This notably fixes the use of spellcheck.maxCollationTries in firstSearcher
warming queries to prevent deadlock (hossman)
+
Dependency Upgrades
---------------------
* SOLR-17471: Upgrade Lucene to 9.12.1. (Pierre Salagnac, Christine Poerschke)
diff --git a/solr/core/src/java/org/apache/solr/core/QuerySenderListener.java
b/solr/core/src/java/org/apache/solr/core/QuerySenderListener.java
index a612b27809a..4857cadf101 100644
--- a/solr/core/src/java/org/apache/solr/core/QuerySenderListener.java
+++ b/solr/core/src/java/org/apache/solr/core/QuerySenderListener.java
@@ -23,7 +23,6 @@ import java.util.ArrayList;
import java.util.List;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.util.NamedList;
-import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrRequestInfo;
import org.apache.solr.response.ResultContext;
@@ -59,16 +58,7 @@ public class QuerySenderListener extends
AbstractSolrEventListener {
if (params.get(DISTRIB) == null) {
params.add(DISTRIB, false);
}
- SolrQueryRequest req =
- new LocalSolrQueryRequest(getCore(), params) {
- @Override
- public SolrIndexSearcher getSearcher() {
- return searcher;
- }
-
- @Override
- public void close() {}
- };
+ SolrQueryRequest req = SolrQueryRequest.wrapSearcher(searcher,
params.toSolrParams());
SolrQueryResponse rsp = new SolrQueryResponse();
SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp));
try {
diff --git a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
index 1db714553a4..37dffbeaceb 100644
--- a/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/BlobHandler.java
@@ -58,7 +58,6 @@ import org.apache.solr.core.SolrCore;
import org.apache.solr.handler.admin.api.GetBlobInfoAPI;
import org.apache.solr.handler.admin.api.ReplicationAPIBase;
import org.apache.solr.handler.admin.api.UploadBlobAPI;
-import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.request.SolrRequestInfo;
@@ -330,7 +329,7 @@ public class BlobHandler extends RequestHandlerBase
// works OK for real-time get (which is all that BlobHandler uses it for).
private static void forward(
SolrQueryRequest req, String handler, SolrParams params,
SolrQueryResponse rsp) {
- LocalSolrQueryRequest r = new LocalSolrQueryRequest(req.getCore(), params);
+ SolrQueryRequest r = req.subRequest(params);
SolrRequestInfo.getRequestInfo().addCloseHook(r); // Close as late as
possible...
req.getCore().getRequestHandler(handler).handleRequest(r, rsp);
}
diff --git a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
index 8a8a0da4425..2e82dd375f9 100644
--- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
@@ -89,7 +89,6 @@ import
org.apache.solr.handler.admin.api.ModifyConfigComponentAPI;
import org.apache.solr.handler.admin.api.ModifyParamSetAPI;
import org.apache.solr.pkg.PackageAPI;
import org.apache.solr.pkg.PackageListeners;
-import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.response.SolrQueryResponse;
@@ -372,7 +371,7 @@ public class SolrConfigHandler extends RequestHandlerBase
}
}
- LocalSolrQueryRequest r = new LocalSolrQueryRequest(req.getCore(),
req.getOriginalParams());
+ SolrQueryRequest r = req.subRequest(req.getOriginalParams());
r.getContext().put(USEPARAM, useParams);
NamedList<?> nl = new PluginInfo(SolrRequestHandler.TYPE,
pluginInfo).initArgs;
SolrPluginUtils.setDefaults(
diff --git a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
index 4dfce550bc0..bf571619e3e 100644
--- a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
+++ b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
@@ -214,4 +214,55 @@ public interface SolrQueryRequest extends AutoCloseable {
wt, SolrCore.DEFAULT_RESPONSE_WRITERS.get("standard"));
}
}
+
+ /**
+ * Returns a new "Sub Request" of the current request.
+ *
+ * <p>This is useful in situations where some code handling an existing
request wishes to invoke a
+ * new request -- as if it came from the same user. The request returned
uses the same {@link
+ * #getSearcher} and {@link #getUserPrincipal} as the current request, and
is initialized using
+ * the same {@link #getSchema()} (but {@link #updateSchemaToLatest} is
handled independently for
+ * the two requests)
+ *
+ * <p>The behavior of a sub-request is undefined if the original request is
closed.
+ */
+ default SolrQueryRequest subRequest(final SolrParams params) {
+ final SolrQueryRequest outerRequest = this;
+ // NOTE: we explicitly do not use DelegatingSolrQueryRequest because we do
not want
+ // any existing (or future) "setter" methods to delegate to the
outerRequest
+ return new SolrQueryRequestBase(outerRequest.getCore(), params) {
+ { // super() implicitly uses core.getLatestSchema(), but we want
+ // whatever outerRequest is currently using
+ this.schema = outerRequest.getSchema();
+ }
+
+ @Override
+ public SolrIndexSearcher getSearcher() {
+ // We do not use/set this.searcherHolder, so that super.close() doesn't
+ // double close
+ return outerRequest.getSearcher();
+ }
+
+ @Override
+ public Principal getUserPrincipal() {
+ return outerRequest.getUserPrincipal();
+ }
+ };
+ }
+
+ /**
+ * Returns a request that explicitly uses the specified
<code>SolrIndexSearcher</code> (even if it
+ * is not registered or fully initialized) in conjunction with the
<code>SolrCore</code>
+ * identified via {@link SolrIndexSearcher#getCore}
+ */
+ static SolrQueryRequest wrapSearcher(final SolrIndexSearcher searcher, final
SolrParams params) {
+ return new SolrQueryRequestBase(searcher.getCore(), params) {
+ @Override
+ public SolrIndexSearcher getSearcher() {
+ // We do not use/set this.searcherHolder, so that super.close() doesn't
+ // double close
+ return searcher;
+ }
+ };
+ }
}
diff --git a/solr/core/src/java/org/apache/solr/request/SolrRequestInfo.java
b/solr/core/src/java/org/apache/solr/request/SolrRequestInfo.java
index 444867b430b..c30d6f2c29e 100644
--- a/solr/core/src/java/org/apache/solr/request/SolrRequestInfo.java
+++ b/solr/core/src/java/org/apache/solr/request/SolrRequestInfo.java
@@ -16,7 +16,6 @@
*/
package org.apache.solr.request;
-import java.io.Closeable;
import java.lang.invoke.MethodHandles;
import java.security.Principal;
import java.util.ArrayDeque;
@@ -55,7 +54,7 @@ public class SolrRequestInfo {
public HttpServletRequest httpRequest;
private TimeZone tz;
private ResponseBuilder rb;
- private List<Closeable> closeHooks;
+ private List<AutoCloseable> closeHooks;
private SolrDispatchFilter.Action action;
private boolean useServerToken = false;
@@ -138,7 +137,7 @@ public class SolrRequestInfo {
}
if (closeHooks != null) {
- for (Closeable hook : closeHooks) {
+ for (AutoCloseable hook : closeHooks) {
try {
hook.close();
} catch (Exception e) {
@@ -218,7 +217,7 @@ public class SolrRequestInfo {
this.rb = rb;
}
- public void addCloseHook(Closeable hook) {
+ public void addCloseHook(AutoCloseable hook) {
// is this better here, or on SolrQueryRequest?
synchronized (this) {
if (isClosed()) {
diff --git
a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
index 221f2c8b06b..d4abe2d0d57 100644
--- a/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
+++ b/solr/core/src/java/org/apache/solr/search/CollapsingQParserPlugin.java
@@ -74,7 +74,6 @@ import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.StrUtils;
import org.apache.solr.handler.component.QueryElevationComponent;
import org.apache.solr.handler.component.ResponseBuilder;
-import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrRequestInfo;
import org.apache.solr.schema.FieldType;
@@ -2131,7 +2130,7 @@ public class CollapsingQParserPlugin extends
QParserPlugin {
minMaxFieldType = searcher.getSchema().getField(text).getType();
} else {
SolrParams params = new ModifiableSolrParams();
- try (SolrQueryRequest request = new
LocalSolrQueryRequest(searcher.getCore(), params)) {
+ try (SolrQueryRequest request =
SolrQueryRequest.wrapSearcher(searcher, params)) {
FunctionQParser functionQParser = new FunctionQParser(text, null,
null, request);
funcQuery = (FunctionQuery) functionQParser.parse();
} catch (SyntaxError e) {
diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
index ec270103a1b..cd33360d221 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
@@ -109,7 +109,6 @@ import org.apache.solr.index.SlowCompositeReaderWrapper;
import org.apache.solr.metrics.MetricsMap;
import org.apache.solr.metrics.SolrMetricManager;
import org.apache.solr.metrics.SolrMetricsContext;
-import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrRequestInfo;
import org.apache.solr.response.SolrQueryResponse;
@@ -2539,17 +2538,7 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
log.debug("autowarming [{}] from [{}]\n\t{}", this, old,
old.cacheList[i]);
}
- final SolrQueryRequest req =
- new LocalSolrQueryRequest(core, params) {
- @Override
- public SolrIndexSearcher getSearcher() {
- return SolrIndexSearcher.this;
- }
-
- @Override
- public void close() {}
- };
-
+ final SolrQueryRequest req =
SolrQueryRequest.wrapSearcher(SolrIndexSearcher.this, params);
final SolrQueryResponse rsp = new SolrQueryResponse();
SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp));
try {
diff --git
a/solr/core/src/java/org/apache/solr/spelling/SpellCheckCollator.java
b/solr/core/src/java/org/apache/solr/spelling/SpellCheckCollator.java
index b65e7d115b2..fd5f465d098 100644
--- a/solr/core/src/java/org/apache/solr/spelling/SpellCheckCollator.java
+++ b/solr/core/src/java/org/apache/solr/spelling/SpellCheckCollator.java
@@ -36,7 +36,6 @@ import org.apache.solr.common.util.SimpleOrderedMap;
import org.apache.solr.handler.component.QueryComponent;
import org.apache.solr.handler.component.ResponseBuilder;
import org.apache.solr.handler.component.SearchComponent;
-import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.search.QueryLimits;
import org.slf4j.Logger;
@@ -150,7 +149,7 @@ public class SpellCheckCollator {
// creating a request here... make sure to close it!
ResponseBuilder checkResponse =
new ResponseBuilder(
- new LocalSolrQueryRequest(ultimateResponse.req.getCore(),
params),
+ ultimateResponse.req.subRequest(params),
new SolrQueryResponse(),
Arrays.asList(queryComponent));
checkResponse.setQparser(ultimateResponse.getQparser());
diff --git
a/solr/core/src/test-files/solr/collection1/conf/solrconfig-spellcheckcomponent.xml
b/solr/core/src/test-files/solr/collection1/conf/solrconfig-spellcheckcomponent.xml
index 0253d91b804..0a92e46115b 100644
---
a/solr/core/src/test-files/solr/collection1/conf/solrconfig-spellcheckcomponent.xml
+++
b/solr/core/src/test-files/solr/collection1/conf/solrconfig-spellcheckcomponent.xml
@@ -28,11 +28,24 @@
<writeLockTimeout>1000</writeLockTimeout>
<commitLockTimeout>10000</commitLockTimeout>
<lockType>${solr.tests.lockType:single}</lockType>
- <query>
- <useColdSearcher>false</useColdSearcher>
- <maxWarmingSearchers>1</maxWarmingSearchers>
- </query>
</indexConfig>
+ <query>
+ <useColdSearcher>false</useColdSearcher>
+ <maxWarmingSearchers>1</maxWarmingSearchers>
+ <listener event="firstSearcher" class="solr.QuerySenderListener">
+ <arr name="queries">
+ <lst>
+ <str name="qt">/spellCheckCompRH</str>
+ <str name="q">documemtsss broens</str>
+ <str name="spellcheck">true</str>
+ <str name="spellcheck.dictionary">direct_lowerfilt</str>
+ <str name="spellcheck.collate">true</str>
+ <str name="spellcheck.maxCollations">10</str>
+ <str name="spellcheck.maxCollationTries">10</str>
+ </lst>
+ </arr>
+ </listener>
+ </query>
<requestHandler name="/select"
class="solr.SearchHandler"></requestHandler>
diff --git
a/solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java
b/solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java
index 4a0eda8e506..f3e64e6e7f1 100644
---
a/solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java
+++
b/solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java
@@ -643,4 +643,16 @@ public class SpellCheckComponentTest extends
SolrTestCaseJ4 {
assertNull(suggestions.get("suggestion"));
assertFalse((Boolean) spellCheck.get("correctlySpelled"));
}
+
+ @Test
+ public void testFirstSearcherWarming() throws Exception {
+
+ final long preRestart = h.getCore().withSearcher(s -> s.getOpenNanoTime());
+
+ h.reload();
+
+ try (SolrCore current = h.getCoreInc()) {
+ assertNotEquals(preRestart, (long) current.withSearcher(s ->
s.getOpenNanoTime()));
+ }
+ }
}
