You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mk...@apache.org on 2019/06/12 16:10:27 UTC
[lucene-solr] branch master updated: SOLR-13509: add
omitHeader=false for shards requests to avoid NPE on partialResuls check
This is an automated email from the ASF dual-hosted git repository.
mkhl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new 5ba6c0c SOLR-13509: add omitHeader=false for shards requests to avoid NPE on partialResuls check
5ba6c0c is described below
commit 5ba6c0c7a2ed71fef321abee6b0fee13d93ea183
Author: Mikhail Khludnev <mk...@apache.org>
AuthorDate: Wed Jun 12 17:53:16 2019 +0200
SOLR-13509: add omitHeader=false for shards requests to avoid NPE on partialResuls check
---
solr/CHANGES.txt | 2 ++
.../solr/handler/component/QueryComponent.java | 26 +++++++++-------------
.../solr/handler/component/SearchHandler.java | 7 +++---
.../cloud/CloudExitableDirectoryReaderTest.java | 23 ++++++++++++++-----
.../component/DistributedDebugComponentTest.java | 11 ++++-----
.../solr/search/facet/RangeFacetCloudTest.java | 9 ++++++++
.../src/common-query-parameters.adoc | 22 +++++++++++++++++-
7 files changed, 68 insertions(+), 32 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index dabb2d0..6b67773 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -135,6 +135,8 @@ Bug Fixes
* SOLR-12013: collections API CUSTERSTATUS command fails when configset missing (Erick Erickson)
+* SOLR-13509: NPE on omitHeader=true is fixed by sending omitHeader=false to shard searches (Munendra S N, Mikhail Khludnev)
+
Other Changes
----------------------
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 b0b9fb8..868b5c9 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
@@ -849,7 +849,7 @@ public class QueryComponent extends SearchComponent
}
else {
responseHeader = (NamedList<?>)srsp.getSolrResponse().getResponse().get("responseHeader");
- final Object rhste = (responseHeader == null ? null : responseHeader.get(SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY));
+ final Object rhste = responseHeader.get(SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY);
if (rhste != null) {
nl.add(SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY, rhste);
}
@@ -879,20 +879,16 @@ public class QueryComponent extends SearchComponent
}
final boolean thisResponseIsPartial;
- if (responseHeader != null) {
- thisResponseIsPartial = Boolean.TRUE.equals(responseHeader.getBooleanArg(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY));
- thereArePartialResults |= thisResponseIsPartial;
-
- if (!Boolean.TRUE.equals(segmentTerminatedEarly)) {
- final Object ste = responseHeader.get(SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY);
- if (Boolean.TRUE.equals(ste)) {
- segmentTerminatedEarly = Boolean.TRUE;
- } else if (Boolean.FALSE.equals(ste)) {
- segmentTerminatedEarly = Boolean.FALSE;
- }
+ thisResponseIsPartial = Boolean.TRUE.equals(responseHeader.getBooleanArg(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY));
+ thereArePartialResults |= thisResponseIsPartial;
+
+ if (!Boolean.TRUE.equals(segmentTerminatedEarly)) {
+ final Object ste = responseHeader.get(SolrQueryResponse.RESPONSE_HEADER_SEGMENT_TERMINATED_EARLY_KEY);
+ if (Boolean.TRUE.equals(ste)) {
+ segmentTerminatedEarly = Boolean.TRUE;
+ } else if (Boolean.FALSE.equals(ste)) {
+ segmentTerminatedEarly = Boolean.FALSE;
}
- } else {
- thisResponseIsPartial = false;
}
// calculate global maxScore and numDocsFound
@@ -1185,7 +1181,7 @@ public class QueryComponent extends SearchComponent
}
{
NamedList<?> responseHeader = (NamedList<?>)srsp.getSolrResponse().getResponse().get("responseHeader");
- if (responseHeader!=null && Boolean.TRUE.equals(responseHeader.getBooleanArg(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY))) {
+ if (Boolean.TRUE.equals(responseHeader.getBooleanArg(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY))) {
rb.rsp.getResponseHeader().asShallowMap()
.put(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, Boolean.TRUE);
}
diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
index 89cfa2e..96c2200 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
@@ -16,9 +16,6 @@
*/
package org.apache.solr.handler.component;
-import static org.apache.solr.common.params.CommonParams.DISTRIB;
-import static org.apache.solr.common.params.CommonParams.PATH;
-
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.invoke.MethodHandles;
@@ -56,6 +53,9 @@ import org.apache.solr.util.plugin.SolrCoreAware;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.solr.common.params.CommonParams.DISTRIB;
+import static org.apache.solr.common.params.CommonParams.PATH;
+
/**
*
@@ -370,6 +370,7 @@ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware ,
params.set(ShardParams.IS_SHARD, true); // a sub (shard) request
params.set(ShardParams.SHARDS_PURPOSE, sreq.purpose);
params.set(ShardParams.SHARD_URL, shard); // so the shard knows what was asked
+ params.set(CommonParams.OMIT_HEADER, false);
if (rb.requestInfo != null) {
// we could try and detect when this is needed, but it could be tricky
params.set("NOW", Long.toString(rb.requestInfo.getNOW().getTime()));
diff --git a/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java b/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java
index 3a45500..e75d700 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java
@@ -21,13 +21,15 @@ import java.util.LinkedHashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import com.carrotsearch.randomizedtesting.annotations.Repeat;
+import com.codahale.metrics.Metered;
+import com.codahale.metrics.MetricRegistry;
import org.apache.lucene.util.TestUtil;
import org.apache.solr.client.solrj.embedded.JettySolrRunner;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.UpdateRequest;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.cloud.MiniSolrCloudCluster.JettySolrRunnerWithMetrics;
-import static org.apache.solr.cloud.TrollingIndexReaderFactory.*;
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
@@ -40,9 +42,11 @@ import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.carrotsearch.randomizedtesting.annotations.Repeat;
-import com.codahale.metrics.Metered;
-import com.codahale.metrics.MetricRegistry;
+import static org.apache.solr.cloud.TrollingIndexReaderFactory.CheckMethodName;
+import static org.apache.solr.cloud.TrollingIndexReaderFactory.Trap;
+import static org.apache.solr.cloud.TrollingIndexReaderFactory.catchClass;
+import static org.apache.solr.cloud.TrollingIndexReaderFactory.catchCount;
+import static org.apache.solr.cloud.TrollingIndexReaderFactory.catchTrace;
/**
* Distributed test for {@link org.apache.lucene.index.ExitableDirectoryReader}
@@ -193,7 +197,7 @@ public class CloudExitableDirectoryReaderTest extends SolrCloudTestCase {
params( "sort","query($q,1) asc"),
params("rows","0", "facet","true", "facet.method", "enum", "facet.field", "name"),
params("rows","0", "json.facet","{ ids: { type: range, field : num, start : 1, end : 99, gap : 9 }}")
- }; //add more cases here
+ }; // add more cases here
params.add(cases[random().nextInt(cases.length)]);
for (; ; creep*=1.5) {
@@ -218,13 +222,20 @@ public class CloudExitableDirectoryReaderTest extends SolrCloudTestCase {
int numBites = atLeast(100);
for(int bite=0; bite<numBites; bite++) {
int boundary = random().nextInt(creep);
+ boolean omitHeader = random().nextBoolean();
try(Trap catchCount = catchCount(boundary)){
+ params.set("omitHeader", "" + omitHeader);
params.set("boundary", boundary);
QueryResponse rsp = cluster.getSolrClient().query(COLLECTION,
params);
assertEquals(""+rsp, rsp.getStatus(), 0);
assertNo500s(""+rsp);
- assertEquals(""+creep+" ticks were sucessful; trying "+boundary+" yields "+rsp,
+ // without responseHeader, whether the response is partial or not can't be known
+ // omitHeader=true used in request to ensure that no NPE exceptions are thrown
+ if (omitHeader) {
+ continue;
+ }
+ assertEquals("" + creep + " ticks were successful; trying " + boundary + " yields " + rsp,
catchCount.hasCaught(), isPartial(rsp));
}catch(AssertionError ae) {
Trap.dumpLastStackTraces(log);
diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java
index 52c05dd..ffd62a4 100644
--- a/solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java
@@ -117,6 +117,10 @@ public class DistributedDebugComponentTest extends SolrJettyTestBase {
query.set("distrib", "true");
query.setFields("id", "text");
query.set("shards", shard1 + "," + shard2);
+
+ if (random().nextBoolean()) {
+ query.add("omitHeader", Boolean.toString(random().nextBoolean()));
+ }
QueryResponse response = collection1.query(query);
NamedList<Object> track = (NamedList<Object>) response.getDebugMap().get("track");
assertNotNull(track);
@@ -138,13 +142,6 @@ public class DistributedDebugComponentTest extends SolrJettyTestBase {
assertElementsPresent((NamedList<String>)((NamedList<Object>)track.get("GET_FIELDS")).get(shard2),
"QTime", "ElapsedTime", "RequestPurpose", "NumFound", "Response");
- query.add("omitHeader", "true");
- response = collection1.query(query);
- assertNull("QTime is not included in the response when omitHeader is set to true",
- ((NamedList<Object>)response.getDebugMap().get("track")).findRecursive("EXECUTE_QUERY", shard1, "QTime"));
- assertNull("QTime is not included in the response when omitHeader is set to true",
- ((NamedList<Object>)response.getDebugMap().get("track")).findRecursive("GET_FIELDS", shard2, "QTime"));
-
query.setQuery("id:1");
response = collection1.query(query);
track = (NamedList<Object>) response.getDebugMap().get("track");
diff --git a/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java b/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java
index 8e53b4e..b11ff3e 100644
--- a/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java
+++ b/solr/core/src/test/org/apache/solr/search/facet/RangeFacetCloudTest.java
@@ -219,6 +219,15 @@ public class RangeFacetCloudTest extends SolrCloudTestCase {
}
}
}
+
+ public void testStatsWithOmitHeader() throws Exception {
+ // SOLR-13509: no NPE should be thrown when only stats are specified with omitHeader=true
+ SolrQuery solrQuery = new SolrQuery("q", "*:*", "omitHeader", "true",
+ "json.facet", "{unique_foo:\"unique(" + STR_FIELD+ ")\"}");
+ final QueryResponse rsp = cluster.getSolrClient().query(solrQuery);
+ // response shouldn't contain header as omitHeader is set to true
+ assertNull(rsp.getResponseHeader());
+ }
public void testInclude_Upper() throws Exception {
for (boolean doSubFacet : Arrays.asList(false, true)) {
diff --git a/solr/solr-ref-guide/src/common-query-parameters.adoc b/solr/solr-ref-guide/src/common-query-parameters.adoc
index 4994015..aeb77df 100644
--- a/solr/solr-ref-guide/src/common-query-parameters.adoc
+++ b/solr/solr-ref-guide/src/common-query-parameters.adoc
@@ -203,7 +203,27 @@ The default value of this parameter is blank, which causes no extra "explain inf
== timeAllowed Parameter
-This parameter specifies the amount of time, in milliseconds, allowed for a search to complete. If this time expires before the search is complete, any partial results will be returned, but values such as `numFound`, <<faceting.adoc#faceting,facet>> counts, and result <<the-stats-component.adoc#the-stats-component,stats>> may not be accurate for the entire result set.
+This parameter specifies the amount of time, in milliseconds, allowed for a search to complete. If this time expires before the search is complete, any partial results will be returned, but values such as `numFound`, <<faceting.adoc#faceting,facet>> counts, and result <<the-stats-component.adoc#the-stats-component,stats>> may not be accurate for the entire result set. In case of expiration, if `omitHeader` isn't set to `true` the response header contains a special flag called `partialResults`.
+
+[source,json]
+----
+{
+ "responseHeader": {
+ "status": 0,
+ "zkConnected": true,
+ "partialResults": true,
+ "QTime": 20,
+ "params": {
+ "q": "*:*"
+ }
+ },
+ "response": {
+ "numFound": 77,
+ "start": 0,
+ "docs": [ "..." ]
+ }
+}
+----
This value is only checked at the time of: