From c50bd903d66b016fde31091d33649b26e89356c8 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Thu, 16 May 2024 15:02:20 -0700 Subject: [PATCH 1/7] Test updates demoing problems --- .../search/DistributedReRankExplainTest.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java b/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java index a366ba2e67a..7cb145927bf 100644 --- a/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java +++ b/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java @@ -30,6 +30,7 @@ import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.SolrParams; import org.junit.BeforeClass; import org.junit.Test; import org.slf4j.Logger; @@ -60,10 +61,7 @@ public static void setupCluster() throws Exception { .process(cluster.getSolrClient()); cluster.waitForActiveCollection(collection, 2, 2); - } - @Test - public void testReRankExplain() throws Exception { CloudSolrClient client = cluster.getSolrClient(); UpdateRequest updateRequest = new UpdateRequest(); for (int i = 0; i < 100; i++) { @@ -75,34 +73,56 @@ public void testReRankExplain() throws Exception { updateRequest.process(client, COLLECTIONORALIAS); client.commit(COLLECTIONORALIAS); - String[] debugParams = {CommonParams.DEBUG, CommonParams.DEBUG_QUERY}; + } + + @Test + public void testDebugTrue() throws Exception { + doTestReRankExplain(params(CommonParams.DEBUG_QUERY, "true")); + doTestReRankExplain(params(CommonParams.DEBUG, "true")); + } + + @Test + public void testDebugAll() throws Exception { + doTestReRankExplain(params(CommonParams.DEBUG, "all")); + } + + @Test + public void testDebugResults() throws Exception { + doTestReRankExplain(params(CommonParams.DEBUG, "results")); + } + + private void doTestReRankExplain(final SolrParams debugParams) throws Exception { + CloudSolrClient client = cluster.getSolrClient(); Random random = random(); ModifiableSolrParams solrParams = new ModifiableSolrParams(); String reRank = "{!rerank reRankDocs=10 reRankMainScale=0-10 reRankQuery='test_s:hello'}"; solrParams .add("q", "test_s:hello") - .add(debugParams[random.nextInt(2)], "true") + .add("fl", "id,test_s,score") .add(CommonParams.RQ, reRank); - QueryRequest queryRequest = new QueryRequest(solrParams); + QueryRequest queryRequest = new QueryRequest(SolrParams.wrapDefaults(solrParams, debugParams)); + QueryResponse queryResponse = queryRequest.process(client, COLLECTIONORALIAS); Map debug = queryResponse.getDebugMap(); assertNotNull(debug); String explain = debug.get("explain").toString(); assertTrue( explain.contains("5.0101576 = combined scaled first and unscaled second pass score ")); + assertNotNull(queryResponse.getResults().get(0).getFieldValue("test_s")); solrParams = new ModifiableSolrParams(); reRank = "{!rerank reRankDocs=10 reRankScale=0-10 reRankQuery='test_s:hello'}"; solrParams .add("q", "test_s:hello") - .add(debugParams[random.nextInt(2)], "true") + .add("fl", "id,test_s,score") .add(CommonParams.RQ, reRank); - queryRequest = new QueryRequest(solrParams); + queryRequest = new QueryRequest(SolrParams.wrapDefaults(solrParams, debugParams)); queryResponse = queryRequest.process(client, COLLECTIONORALIAS); debug = queryResponse.getDebugMap(); assertNotNull(debug); explain = debug.get("explain").toString(); assertTrue( explain.contains("10.005078 = combined unscaled first and scaled second pass score ")); + assertNotNull(queryResponse.getResults().get(0).getFieldValue("test_s")); } } From 31660e7a525923d3e0994353cc6c7384e74724b8 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Thu, 16 May 2024 15:06:28 -0700 Subject: [PATCH 2/7] Remove singlePassExplain logic This gets us back to the simple case of all tests failing with NPE --- .../handler/component/QueryComponent.java | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java index 4bc2a867ab2..a421dcc42df 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java +++ b/solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java @@ -89,7 +89,6 @@ import org.apache.solr.search.QueryResult; import org.apache.solr.search.QueryUtils; import org.apache.solr.search.RankQuery; -import org.apache.solr.search.ReRankQParserPlugin; import org.apache.solr.search.ReturnFields; import org.apache.solr.search.SolrIndexSearcher; import org.apache.solr.search.SolrReturnFields; @@ -782,7 +781,6 @@ protected void createMainQuery(ResponseBuilder rb) { boolean distribSinglePass = rb.req.getParams().getBool(ShardParams.DISTRIB_SINGLE_PASS, false); if (distribSinglePass - || singlePassExplain(rb.req.getParams()) || (fields != null && fields.wantsField(keyFieldName) && fields.getRequestedFieldNames() != null @@ -864,36 +862,6 @@ protected void createMainQuery(ResponseBuilder rb) { rb.addRequest(this, sreq); } - private boolean singlePassExplain(SolrParams params) { - - /* - * Currently there is only one explain that requires a single pass - * and that is the reRank when scaling is used. - */ - - String rankQuery = params.get(CommonParams.RQ); - if (rankQuery != null) { - if (rankQuery.contains(ReRankQParserPlugin.RERANK_MAIN_SCALE) - || rankQuery.contains(ReRankQParserPlugin.RERANK_SCALE)) { - boolean debugQuery = params.getBool(CommonParams.DEBUG_QUERY, false); - if (debugQuery) { - return true; - } else { - String[] debugParams = params.getParams(CommonParams.DEBUG); - if (debugParams != null) { - for (String debugParam : debugParams) { - if (debugParam.equals("true")) { - return true; - } - } - } - } - } - } - - return false; - } - protected boolean addFL(StringBuilder fl, String field, boolean additionalAdded) { if (additionalAdded) fl.append(","); fl.append(field); From 3e0c03dd64c7d5b3fadfc07cd8669594a2b50e91 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Thu, 16 May 2024 16:17:12 -0700 Subject: [PATCH 3/7] Fix ReRankQParser to get an accurate indication if 'explain results' is nequested --- .../solr/search/ReRankQParserPlugin.java | 65 +++++++++++++------ .../org/apache/solr/search/ReRankScaler.java | 10 +-- 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java index e9597c3dd86..60242f5739f 100644 --- a/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java @@ -25,7 +25,9 @@ import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.StrUtils; +import org.apache.solr.handler.component.ResponseBuilder; import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.request.SolrRequestInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -63,6 +65,42 @@ public QParser createParser( private static class ReRankQParser extends QParser { + private boolean isExplainResults() { + final SolrRequestInfo ri = SolrRequestInfo.getRequestInfo(); + if (null != ri) { + final ResponseBuilder rb = ri.getResponseBuilder(); + if (null != rb) { + return rb.isDebugResults(); + } + } + + // HACK: The code below should not be used. It is preserved for backcompat + // on the slim remote chance that someone is using ReRankQParserPlugin + // w/o using SearchHandler+ResponseBuilder + // + // (It's also wrong, and doesn't account for thigns like debug=true + // or debug=all ... but as stated: it's for esoteric backcompat purposes + // only, so we're not going to change it and start returning "true" + // if existing code doesn't expect it + + boolean debugQuery = params.getBool(CommonParams.DEBUG_QUERY, false); + + if (!debugQuery) { + String[] debugParams = params.getParams(CommonParams.DEBUG); + if (debugParams != null) { + for (String debugParam : debugParams) { + if ("true".equals(debugParam)) { + debugQuery = true; + break; + } + } + } + } + return debugQuery; + + + } + public ReRankQParser( String query, SolrParams localParams, SolrParams params, SolrQueryRequest req) { super(query, localParams, params, req); @@ -88,19 +126,8 @@ public Query parse() throws SyntaxError { String mainScale = localParams.get(RERANK_MAIN_SCALE); String reRankScale = localParams.get(RERANK_SCALE); - boolean debugQuery = params.getBool(CommonParams.DEBUG_QUERY, false); - if (!debugQuery) { - String[] debugParams = params.getParams(CommonParams.DEBUG); - if (debugParams != null) { - for (String debugParam : debugParams) { - if ("true".equals(debugParam)) { - debugQuery = true; - break; - } - } - } - } + final boolean explainResults = isExplainResults(); double reRankScaleWeight = reRankWeight; @@ -111,7 +138,7 @@ public Query parse() throws SyntaxError { reRankScaleWeight, reRankOperator, new ReRankQueryRescorer(reRankQuery, 1, ReRankOperator.REPLACE), - debugQuery); + explainResults); if (reRankScaler.scaleScores()) { // Scaler applies the weighting instead of the rescorer @@ -119,7 +146,7 @@ public Query parse() throws SyntaxError { } return new ReRankQuery( - reRankQuery, reRankDocs, reRankWeight, reRankOperator, reRankScaler, debugQuery); + reRankQuery, reRankDocs, reRankWeight, reRankOperator, reRankScaler, explainResults); } } @@ -165,7 +192,7 @@ protected float combine( private static final class ReRankQuery extends AbstractReRankQuery { private final Query reRankQuery; private final double reRankWeight; - private final boolean debugQuery; + private final boolean explainResults; @Override public int hashCode() { @@ -198,7 +225,7 @@ public ReRankQuery( double reRankWeight, ReRankOperator reRankOperator, ReRankScaler reRankScaler, - boolean debugQuery) { + boolean explainResults) { super( defaultQuery, reRankDocs, @@ -207,7 +234,7 @@ public ReRankQuery( reRankOperator); this.reRankQuery = reRankQuery; this.reRankWeight = reRankWeight; - this.debugQuery = debugQuery; + this.explainResults = explainResults; } @Override @@ -246,13 +273,13 @@ public String toString(String s) { @Override protected Query rewrite(Query rewrittenMainQuery) throws IOException { return new ReRankQuery( - reRankQuery, reRankDocs, reRankWeight, reRankOperator, reRankScaler, debugQuery) + reRankQuery, reRankDocs, reRankWeight, reRankOperator, reRankScaler, explainResults) .wrap(rewrittenMainQuery); } @Override public boolean getCache() { - if (reRankScaler.scaleScores() && debugQuery) { + if (reRankScaler.scaleScores() && explainResults) { // Caching breaks explain when reRankScaling is used. return false; } else { diff --git a/solr/core/src/java/org/apache/solr/search/ReRankScaler.java b/solr/core/src/java/org/apache/solr/search/ReRankScaler.java index 686faa4ae1d..7e269654933 100644 --- a/solr/core/src/java/org/apache/solr/search/ReRankScaler.java +++ b/solr/core/src/java/org/apache/solr/search/ReRankScaler.java @@ -32,7 +32,7 @@ public class ReRankScaler { protected int mainQueryMax = -1; protected int reRankQueryMin = -1; protected int reRankQueryMax = -1; - protected boolean debugQuery; + protected boolean explainResults; protected ReRankOperator reRankOperator; protected ReRankScalerExplain reRankScalerExplain; private QueryRescorer replaceRescorer; @@ -45,11 +45,11 @@ public ReRankScaler( double reRankScaleWeight, ReRankOperator reRankOperator, QueryRescorer replaceRescorer, - boolean debugQuery) + boolean explainResults) throws SyntaxError { this.reRankScaleWeight = reRankScaleWeight; - this.debugQuery = debugQuery; + this.explainResults = explainResults; this.reRankScalerExplain = new ReRankScalerExplain(mainScale, reRankScale); this.replaceRescorer = replaceRescorer; if (reRankOperator != ReRankOperator.ADD @@ -171,12 +171,12 @@ public ScoreDoc[] scaleScores(ScoreDoc[] originalDocs, ScoreDoc[] rescoredDocs, scaledOriginalScoreMap = originalScoreMap; } - this.reRankSet = debugQuery ? new HashSet<>() : null; + this.reRankSet = explainResults ? new HashSet<>() : null; for (int i = 0; i < howMany; i++) { ScoreDoc rescoredDoc = rescoredDocs[i]; int doc = rescoredDoc.doc; - if (debugQuery) { + if (explainResults) { reRankSet.add(doc); } float score = rescoredDoc.score; From 5b85b95fc9dabd930559313b1bbf7fd890741097 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Thu, 16 May 2024 17:13:27 -0700 Subject: [PATCH 4/7] change ReRankScaler to be clear when it doesn't have the data it needs to fully populate the explain And updated the test to expect this explanation, but assert we get accurate explanation if 'distrib.singlePass' is used --- .../org/apache/solr/search/ReRankScaler.java | 9 +- .../search/DistributedReRankExplainTest.java | 104 ++++++++++++------ 2 files changed, 81 insertions(+), 32 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/ReRankScaler.java b/solr/core/src/java/org/apache/solr/search/ReRankScaler.java index 7e269654933..c1c0c6c92e1 100644 --- a/solr/core/src/java/org/apache/solr/search/ReRankScaler.java +++ b/solr/core/src/java/org/apache/solr/search/ReRankScaler.java @@ -345,7 +345,14 @@ public Explanation explain( int doc, Explanation mainQueryExplain, Explanation reRankQueryExplain) { float reRankScore = reRankQueryExplain.getDetails()[1].getValue().floatValue(); float mainScore = mainQueryExplain.getValue().floatValue(); - if (reRankSet.contains(doc)) { + if (null == reRankSet) { + // we don't have the data needed to accurately report scaling, + // probably due to distributed request + return Explanation.match( + reRankScore, + "ReRank Scaling effects unkown, see SOLR-XXX (consider distrib.singlePass=true)", // nocommit: file new jira w/details + reRankQueryExplain); + } else if (reRankSet.contains(doc)) { if (scaleMainScores() && scaleReRankScores()) { if (reRankScore > 0) { MinMaxExplain mainScaleExplain = reRankScalerExplain.getMainScaleExplain(); diff --git a/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java b/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java index 7cb145927bf..8f630dc2e2f 100644 --- a/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java +++ b/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java @@ -30,12 +30,16 @@ import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.params.SolrParams; import org.junit.BeforeClass; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + @SolrTestCaseJ4.SuppressSSL public class DistributedReRankExplainTest extends SolrCloudTestCase { @@ -88,41 +92,79 @@ public void testDebugAll() throws Exception { @Test public void testDebugResults() throws Exception { - doTestReRankExplain(params(CommonParams.DEBUG, "results")); + doTestReRankExplain(params(CommonParams.DEBUG, CommonParams.RESULTS)); } private void doTestReRankExplain(final SolrParams debugParams) throws Exception { - CloudSolrClient client = cluster.getSolrClient(); - Random random = random(); - ModifiableSolrParams solrParams = new ModifiableSolrParams(); - String reRank = "{!rerank reRankDocs=10 reRankMainScale=0-10 reRankQuery='test_s:hello'}"; - solrParams - .add("q", "test_s:hello") - .add("fl", "id,test_s,score") - .add(CommonParams.RQ, reRank); - QueryRequest queryRequest = new QueryRequest(SolrParams.wrapDefaults(solrParams, debugParams)); - - QueryResponse queryResponse = queryRequest.process(client, COLLECTIONORALIAS); - Map debug = queryResponse.getDebugMap(); - assertNotNull(debug); - String explain = debug.get("explain").toString(); - assertTrue( - explain.contains("5.0101576 = combined scaled first and unscaled second pass score ")); - assertNotNull(queryResponse.getResults().get(0).getFieldValue("test_s")); + final String reRankMainScale = "{!rerank reRankDocs=10 reRankMainScale=0-10 reRankQuery='test_s:hello'}"; + final String reRankScale = "{!rerank reRankDocs=10 reRankScale=0-10 reRankQuery='test_s:hello'}"; + + { // multi-pass reRankMainScale + final QueryResponse queryResponse = + doQueryAndCommonChecks(SolrParams.wrapDefaults + (params(CommonParams.RQ, reRankMainScale), + debugParams)); + final Map debug = queryResponse.getDebugMap(); + assertNotNull(debug); + final String explain = debug.get("explain").toString(); + assertThat(explain, + containsString("ReRank Scaling effects unkown")); + } + + { // single-pass reRankMainScale + final QueryResponse queryResponse = + doQueryAndCommonChecks(SolrParams.wrapDefaults + (params(CommonParams.RQ, reRankMainScale, + ShardParams.DISTRIB_SINGLE_PASS, "true"), + debugParams)); + final Map debug = queryResponse.getDebugMap(); + assertNotNull(debug); + final String explain = debug.get("explain").toString(); + assertThat(explain, + containsString("5.0101576 = combined scaled first and unscaled second pass score ")); + assertThat( + explain, + not(containsString("ReRank Scaling effects unkown"))); + } + + { // multi-pass reRankMainScale + final QueryResponse queryResponse = + doQueryAndCommonChecks(SolrParams.wrapDefaults + (params(CommonParams.RQ, reRankScale), + debugParams)); + final Map debug = queryResponse.getDebugMap(); + assertNotNull(debug); + final String explain = debug.get("explain").toString(); + assertThat(explain, + containsString("ReRank Scaling effects unkown")); + } + + { // single-pass reRankMainScale + final QueryResponse queryResponse = + doQueryAndCommonChecks(SolrParams.wrapDefaults + (params(CommonParams.RQ, reRankScale, + ShardParams.DISTRIB_SINGLE_PASS, "true"), + debugParams)); + final Map debug = queryResponse.getDebugMap(); + assertNotNull(debug); + final String explain = debug.get("explain").toString(); + assertThat( + explain, + containsString("10.005078 = combined unscaled first and scaled second pass score ")); + assertThat( + explain, + not(containsString("ReRank Scaling effects unkown"))); + } + } + + private QueryResponse doQueryAndCommonChecks(final SolrParams params) throws Exception { + final CloudSolrClient client = cluster.getSolrClient(); + final QueryRequest queryRequest = + new QueryRequest(SolrParams.wrapDefaults(params, params(CommonParams.Q, "test_s:hello", + "fl", "id,test_s,score"))); - solrParams = new ModifiableSolrParams(); - reRank = "{!rerank reRankDocs=10 reRankScale=0-10 reRankQuery='test_s:hello'}"; - solrParams - .add("q", "test_s:hello") - .add("fl", "id,test_s,score") - .add(CommonParams.RQ, reRank); - queryRequest = new QueryRequest(SolrParams.wrapDefaults(solrParams, debugParams)); - queryResponse = queryRequest.process(client, COLLECTIONORALIAS); - debug = queryResponse.getDebugMap(); - assertNotNull(debug); - explain = debug.get("explain").toString(); - assertTrue( - explain.contains("10.005078 = combined unscaled first and scaled second pass score ")); + final QueryResponse queryResponse = queryRequest.process(client, COLLECTIONORALIAS); assertNotNull(queryResponse.getResults().get(0).getFieldValue("test_s")); + return queryResponse; } } From 153669fe4b5053c783bb7e2971669f34a9adb509 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Thu, 16 May 2024 17:17:15 -0700 Subject: [PATCH 5/7] tidy --- .../solr/search/ReRankQParserPlugin.java | 4 +- .../search/DistributedReRankExplainTest.java | 85 +++++++++---------- 2 files changed, 40 insertions(+), 49 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java index 60242f5739f..5bf5ce4067d 100644 --- a/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java @@ -97,10 +97,8 @@ private boolean isExplainResults() { } } return debugQuery; - - } - + public ReRankQParser( String query, SolrParams localParams, SolrParams params, SolrQueryRequest req) { super(query, localParams, params, req); diff --git a/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java b/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java index 8f630dc2e2f..a0c99cb7f4f 100644 --- a/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java +++ b/solr/core/src/test/org/apache/solr/search/DistributedReRankExplainTest.java @@ -17,9 +17,11 @@ package org.apache.solr.search; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + import java.lang.invoke.MethodHandles; import java.util.Map; -import java.util.Random; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; @@ -29,7 +31,6 @@ import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.CommonParams; -import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.params.SolrParams; import org.junit.BeforeClass; @@ -37,9 +38,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; - @SolrTestCaseJ4.SuppressSSL public class DistributedReRankExplainTest extends SolrCloudTestCase { @@ -76,7 +74,6 @@ public static void setupCluster() throws Exception { } updateRequest.process(client, COLLECTIONORALIAS); client.commit(COLLECTIONORALIAS); - } @Test @@ -84,84 +81,80 @@ public void testDebugTrue() throws Exception { doTestReRankExplain(params(CommonParams.DEBUG_QUERY, "true")); doTestReRankExplain(params(CommonParams.DEBUG, "true")); } - + @Test public void testDebugAll() throws Exception { doTestReRankExplain(params(CommonParams.DEBUG, "all")); } - + @Test public void testDebugResults() throws Exception { doTestReRankExplain(params(CommonParams.DEBUG, CommonParams.RESULTS)); } - + private void doTestReRankExplain(final SolrParams debugParams) throws Exception { - final String reRankMainScale = "{!rerank reRankDocs=10 reRankMainScale=0-10 reRankQuery='test_s:hello'}"; - final String reRankScale = "{!rerank reRankDocs=10 reRankScale=0-10 reRankQuery='test_s:hello'}"; + final String reRankMainScale = + "{!rerank reRankDocs=10 reRankMainScale=0-10 reRankQuery='test_s:hello'}"; + final String reRankScale = + "{!rerank reRankDocs=10 reRankScale=0-10 reRankQuery='test_s:hello'}"; { // multi-pass reRankMainScale - final QueryResponse queryResponse = - doQueryAndCommonChecks(SolrParams.wrapDefaults - (params(CommonParams.RQ, reRankMainScale), - debugParams)); + final QueryResponse queryResponse = + doQueryAndCommonChecks( + SolrParams.wrapDefaults(params(CommonParams.RQ, reRankMainScale), debugParams)); final Map debug = queryResponse.getDebugMap(); assertNotNull(debug); final String explain = debug.get("explain").toString(); - assertThat(explain, - containsString("ReRank Scaling effects unkown")); + assertThat(explain, containsString("ReRank Scaling effects unkown")); } - + { // single-pass reRankMainScale - final QueryResponse queryResponse = - doQueryAndCommonChecks(SolrParams.wrapDefaults - (params(CommonParams.RQ, reRankMainScale, - ShardParams.DISTRIB_SINGLE_PASS, "true"), - debugParams)); + final QueryResponse queryResponse = + doQueryAndCommonChecks( + SolrParams.wrapDefaults( + params(CommonParams.RQ, reRankMainScale, ShardParams.DISTRIB_SINGLE_PASS, "true"), + debugParams)); final Map debug = queryResponse.getDebugMap(); assertNotNull(debug); final String explain = debug.get("explain").toString(); - assertThat(explain, - containsString("5.0101576 = combined scaled first and unscaled second pass score ")); assertThat( - explain, - not(containsString("ReRank Scaling effects unkown"))); + explain, + containsString("5.0101576 = combined scaled first and unscaled second pass score ")); + assertThat(explain, not(containsString("ReRank Scaling effects unkown"))); } { // multi-pass reRankMainScale - final QueryResponse queryResponse = - doQueryAndCommonChecks(SolrParams.wrapDefaults - (params(CommonParams.RQ, reRankScale), - debugParams)); + final QueryResponse queryResponse = + doQueryAndCommonChecks( + SolrParams.wrapDefaults(params(CommonParams.RQ, reRankScale), debugParams)); final Map debug = queryResponse.getDebugMap(); assertNotNull(debug); final String explain = debug.get("explain").toString(); - assertThat(explain, - containsString("ReRank Scaling effects unkown")); + assertThat(explain, containsString("ReRank Scaling effects unkown")); } - + { // single-pass reRankMainScale - final QueryResponse queryResponse = - doQueryAndCommonChecks(SolrParams.wrapDefaults - (params(CommonParams.RQ, reRankScale, - ShardParams.DISTRIB_SINGLE_PASS, "true"), - debugParams)); + final QueryResponse queryResponse = + doQueryAndCommonChecks( + SolrParams.wrapDefaults( + params(CommonParams.RQ, reRankScale, ShardParams.DISTRIB_SINGLE_PASS, "true"), + debugParams)); final Map debug = queryResponse.getDebugMap(); assertNotNull(debug); final String explain = debug.get("explain").toString(); assertThat( - explain, - containsString("10.005078 = combined unscaled first and scaled second pass score ")); - assertThat( - explain, - not(containsString("ReRank Scaling effects unkown"))); + explain, + containsString("10.005078 = combined unscaled first and scaled second pass score ")); + assertThat(explain, not(containsString("ReRank Scaling effects unkown"))); } } private QueryResponse doQueryAndCommonChecks(final SolrParams params) throws Exception { final CloudSolrClient client = cluster.getSolrClient(); final QueryRequest queryRequest = - new QueryRequest(SolrParams.wrapDefaults(params, params(CommonParams.Q, "test_s:hello", - "fl", "id,test_s,score"))); + new QueryRequest( + SolrParams.wrapDefaults( + params, params(CommonParams.Q, "test_s:hello", "fl", "id,test_s,score"))); final QueryResponse queryResponse = queryRequest.process(client, COLLECTIONORALIAS); assertNotNull(queryResponse.getResults().get(0).getFieldValue("test_s")); From 002f30042fc65eb09cd6e1db1d0c28158817e5f2 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Fri, 17 May 2024 13:48:43 -0700 Subject: [PATCH 6/7] Add link to forward looking jira in Explanation details --- solr/core/src/java/org/apache/solr/search/ReRankScaler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/search/ReRankScaler.java b/solr/core/src/java/org/apache/solr/search/ReRankScaler.java index c1c0c6c92e1..8410467ddf8 100644 --- a/solr/core/src/java/org/apache/solr/search/ReRankScaler.java +++ b/solr/core/src/java/org/apache/solr/search/ReRankScaler.java @@ -350,7 +350,7 @@ public Explanation explain( // probably due to distributed request return Explanation.match( reRankScore, - "ReRank Scaling effects unkown, see SOLR-XXX (consider distrib.singlePass=true)", // nocommit: file new jira w/details + "ReRank Scaling effects unkown, consider using distrib.singlePass=true (see https://issues.apache.org/jira/browse/SOLR-17299)", reRankQueryExplain); } else if (reRankSet.contains(doc)) { if (scaleMainScores() && scaleReRankScores()) { From 171db2b99477b821cd16f62daced176ef8fd54f8 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Fri, 17 May 2024 14:45:19 -0700 Subject: [PATCH 7/7] CHANGES entry in new 9.6.1 section --- solr/CHANGES.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 1d90fa57df0..227f13c50f6 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -147,6 +147,11 @@ Other Changes * GITHUB#2454: Refactor preparePutOrPost method in HttpJdkSolrClient (Andy Webb) +================== 9.6.1 ================== +Bug Fixes +--------------------- +* SOLR-17296: Remove (broken) singlePass attempt when reRankScale + debug is used, and fix underlying NPE. (hossman) + ================== 9.6.0 ================== New Features ---------------------