You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ho...@apache.org on 2021/04/05 16:57:31 UTC
[solr] branch main updated: SOLR-11921: Move "cursorMark" logic
from QueryComponent to SearchHandler so it can work with things like
QueryElevationComponent that modify the SortSpec in prepare(),
as well as possible custom "search" components other then QueryComponent
This is an automated email from the ASF dual-hosted git repository.
hossman 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 59a5913 SOLR-11921: Move "cursorMark" logic from QueryComponent to SearchHandler so it can work with things like QueryElevationComponent that modify the SortSpec in prepare(), as well as possible custom "search" components other then QueryComponent
59a5913 is described below
commit 59a59138668e9513060824d85ee375a1723f4bfb
Author: Chris Hostetter <ho...@apache.org>
AuthorDate: Mon Apr 5 09:57:24 2021 -0700
SOLR-11921: Move "cursorMark" logic from QueryComponent to SearchHandler so it can work with things like QueryElevationComponent that modify the SortSpec in prepare(), as well as possible custom "search" components other then QueryComponent
---
solr/CHANGES.txt | 3 +
.../solr/handler/component/QueryComponent.java | 10 +-
.../solr/handler/component/SearchHandler.java | 14 ++
.../java/org/apache/solr/search/CursorMark.java | 17 +-
.../collection1/conf/solrconfig-deeppaging.xml | 15 ++
.../src/test/org/apache/solr/CursorPagingTest.java | 180 +++++++++++++++--
.../solr/TestCursorMarkWithoutUniqueKey.java | 5 +-
.../apache/solr/cloud/DistribCursorPagingTest.java | 220 ++++++++++++++++-----
.../solr/handler/component/BadComponentTest.java | 6 +
.../component/QueryElevationComponentTest.java | 101 ++++++++++
.../org/apache/solr/search/CursorMarkTest.java | 20 +-
11 files changed, 490 insertions(+), 101 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 7ce63ff..7ad7907 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -321,6 +321,9 @@ Bug Fixes
* SOLR-15273: Fix NullPointerException in StoredFieldsShardResponseProcessor that happened when a field name alias is
used for the unique key field in searches with distributed result grouping. (limingnihao via Christine Poerschke)
+* SOLR-11921: Move "cursorMark" logic from QueryComponent to SearchHandler so it can work with things like QueryElevationComponent
+ that modify the SortSpec in prepare(), as well as possible custom "search" components other then QueryComponent. (hossman)
+
Other Changes
---------------------
* SOLR-15118: Deprecate CollectionAdminRequest.getV2Request(). (Jason Gerlowski)
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 a08c41d..6622c30 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
@@ -189,14 +189,6 @@ public class QueryComponent extends SearchComponent
rb.setSortSpec( parser.getSortSpec(true) );
rb.setQparser(parser);
- final String cursorStr = rb.req.getParams().get(CursorMarkParams.CURSOR_MARK_PARAM);
- if (null != cursorStr) {
- final CursorMark cursorMark = new CursorMark(rb.req.getSchema(),
- rb.getSortSpec());
- cursorMark.parseSerializedTotem(cursorStr);
- rb.setCursorMark(cursorMark);
- }
-
String[] fqs = req.getParams().getParams(CommonParams.FQ);
if (fqs!=null && fqs.length!=0) {
List<Query> filters = rb.getFilters();
@@ -240,7 +232,7 @@ public class QueryComponent extends SearchComponent
SolrQueryRequest req = rb.req;
SolrParams params = req.getParams();
- if (null != rb.getCursorMark()) {
+ if (null != params.get(CursorMarkParams.CURSOR_MARK_PARAM)) {
// It's hard to imagine, conceptually, what it would mean to combine
// grouping with a cursor - so for now we just don't allow the combination at all
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Can not use Grouping with " +
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 c0c536f..affbfd6 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
@@ -39,7 +39,9 @@ import org.apache.solr.pkg.PackageListeners;
import org.apache.solr.pkg.PackageLoader;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.search.CursorMark;
import org.apache.solr.search.SolrQueryTimeoutImpl;
+import org.apache.solr.search.SortSpec;
import org.apache.solr.search.facet.FacetModule;
import org.apache.solr.security.AuthorizationContext;
import org.apache.solr.security.PermissionNameProvider;
@@ -342,6 +344,18 @@ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware,
subt.stop();
}
+ { // Once all of our components have been prepared, check if this requset involves a SortSpec.
+ // If it does, and if our request includes a cursorMark param, then parse & init the CursorMark state
+ // (This must happen after the prepare() of all components, because any component may have modified the SortSpec)
+ final SortSpec spec = rb.getSortSpec();
+ final String cursorStr = rb.req.getParams().get(CursorMarkParams.CURSOR_MARK_PARAM);
+ if (null != spec && null != cursorStr) {
+ final CursorMark cursorMark = new CursorMark(rb.req.getSchema(), spec);
+ cursorMark.parseSerializedTotem(cursorStr);
+ rb.setCursorMark(cursorMark);
+ }
+ }
+
if (!rb.isDistrib) {
// a normal non-distributed request
diff --git a/solr/core/src/java/org/apache/solr/search/CursorMark.java b/solr/core/src/java/org/apache/solr/search/CursorMark.java
index 8048471..eb33b40 100644
--- a/solr/core/src/java/org/apache/solr/search/CursorMark.java
+++ b/solr/core/src/java/org/apache/solr/search/CursorMark.java
@@ -101,10 +101,10 @@ public final class CursorMark {
}
if (sort.getSort().length != sortSpec.getSchemaFields().size()) {
- throw new SolrException(ErrorCode.SERVER_ERROR,
- "Cursor SortSpec failure: sort length != SchemaFields: "
- + sort.getSort().length + " != " +
- sortSpec.getSchemaFields().size());
+ throw new SolrException(ErrorCode.SERVER_ERROR,
+ "Cursor SortSpec failure: sort length != SchemaFields: "
+ + sort.getSort().length + " != " +
+ sortSpec.getSchemaFields().size());
}
this.sortSpec = sortSpec;
@@ -144,7 +144,12 @@ public final class CursorMark {
if (null == input) {
this.values = null;
} else {
- assert input.size() == sortSpec.getSort().getSort().length;
+ if (input.size() != sortSpec.getSort().getSort().length) {
+ throw new SolrException(ErrorCode.SERVER_ERROR,
+ "Cursor SortSpec failure: sort values != sort length: "
+ + input.size() + " != " + sortSpec.getSort().getSort().length);
+ }
+
// defensive copy
this.values = new ArrayList<>(input);
}
@@ -204,7 +209,7 @@ public final class CursorMark {
"'"+CURSOR_MARK_NEXT+"' returned by a previous search: "
+ serialized, ex);
}
- assert null != pieces : "pieces wasn't parsed?";
+ assert null != pieces : "pieces wasn't parsed, nor exception thrown?";
if (sortFields.length != pieces.size()) {
throw new SolrException(ErrorCode.BAD_REQUEST,
diff --git a/solr/core/src/test-files/solr/collection1/conf/solrconfig-deeppaging.xml b/solr/core/src/test-files/solr/collection1/conf/solrconfig-deeppaging.xml
index 6825dab..d4b5017 100644
--- a/solr/core/src/test-files/solr/collection1/conf/solrconfig-deeppaging.xml
+++ b/solr/core/src/test-files/solr/collection1/conf/solrconfig-deeppaging.xml
@@ -57,5 +57,20 @@
</query>
<requestHandler name="/select" class="solr.SearchHandler" default="true" />
+
+ <!-- elevation should have no (adverse) impact on deep paging -->
+ <searchComponent name="elevate" class="org.apache.solr.handler.component.QueryElevationComponent" >
+ <!-- NOTE: we don't particularly care about the elevate.xml or queryFieldType used,
+ deep paging tests will only worry about using elevateIds.
+ -->
+ <str name="queryFieldType">str</str>
+ <str name="config-file">elevate.xml</str><!-- neccessary due to SOLR-14970 -->
+ </searchComponent>
+ <requestHandler name="/elevate" class="solr.SearchHandler">
+ <arr name="last-components">
+ <str>elevate</str>
+ </arr>
+ </requestHandler>
+
</config>
diff --git a/solr/core/src/test/org/apache/solr/CursorPagingTest.java b/solr/core/src/test/org/apache/solr/CursorPagingTest.java
index 169aaa7..3375b8e 100644
--- a/solr/core/src/test/org/apache/solr/CursorPagingTest.java
+++ b/solr/core/src/test/org/apache/solr/CursorPagingTest.java
@@ -25,6 +25,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
+import java.util.function.Consumer;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
@@ -46,6 +47,9 @@ import org.apache.solr.util.LogLevel;
import org.junit.After;
import org.junit.BeforeClass;
+import org.apache.commons.lang3.StringUtils;
+
+import static org.apache.solr.common.params.SolrParams.wrapDefaults;
import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_NEXT;
import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_PARAM;
import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_START;
@@ -88,7 +92,7 @@ public class CursorPagingTest extends SolrTestCaseJ4 {
} else {
assertU(commit());
}
- assertU(commit());
+ assertU(commit());
// empty, blank, or bogus cursor
for (String c : new String[] { "", " ", "all the docs please!"}) {
@@ -116,6 +120,30 @@ public class CursorPagingTest extends SolrTestCaseJ4 {
GroupParams.GROUP_FIELD, "str",
CURSOR_MARK_PARAM, CURSOR_MARK_START),
ErrorCode.BAD_REQUEST, "Grouping");
+
+ // if a user specifies a 'bogus' cursorMark param, this should error *only* if some other component
+ // cares about (and parses) a SortSpec in it's prepare() method.
+ // (the existence of a 'sort' param shouldn't make a diff ... unless it makes a diff to a component being used,
+ // which it doesn't for RTG)
+ assertU(adoc("id", "yyy", "str", "y", "float", "3", "int", "-3"));
+ if (random().nextBoolean()) {
+ assertU(commit());
+ }
+ for (SolrParams p : Arrays.asList(params(),
+ params(CURSOR_MARK_PARAM, "gibberish"),
+ params(CURSOR_MARK_PARAM, "gibberish",
+ "sort", "id asc"))) {
+ assertJQ(req(p,
+ "qt","/get",
+ "fl", "id",
+ "id","yyy")
+ , "=={'doc':{'id':'yyy'}}");
+ assertJQ(req(p,
+ "qt","/get",
+ "fl", "id",
+ "id","xxx") // doesn't exist in our collection
+ , "=={'doc':null}");
+ }
}
@@ -648,12 +676,24 @@ public class CursorPagingTest extends SolrTestCaseJ4 {
for (String order : new String[] {" asc", " desc"}) {
String sort = f + order + ("id".equals(f) ? "" : ", id" + order);
String rows = "" + TestUtil.nextInt(random(), 13, 50);
- SentinelIntSet ids = assertFullWalkNoDups(totalDocs,
- params("q", "*:*",
- "fl","id",
- "rows",rows,
- "sort",sort));
+ final SolrParams main = params("q", "*:*",
+ "fl","id",
+ "rows",rows,
+ "sort",sort);
+ final SentinelIntSet ids = assertFullWalkNoDups(totalDocs, main);
assertEquals(initialDocs, ids.size());
+
+ // same query, now with QEC ... verify we get all the same docs, but the (expected) elevated docs are first...
+ final SentinelIntSet elevated = assertFullWalkNoDupsElevated(wrapDefaults(params("qt", "/elevate",
+ "fl","id,[elevated]",
+ "forceElevation","true",
+ "elevateIds", "50,20,80"),
+ main),
+ ids);
+ assertTrue(elevated.exists(50));
+ assertTrue(elevated.exists(20));
+ assertTrue(elevated.exists(80));
+ assertEquals(3, elevated.size());
}
}
@@ -671,16 +711,32 @@ public class CursorPagingTest extends SolrTestCaseJ4 {
final String fl = random().nextBoolean() ? "id" : "id,score";
final boolean matchAll = random().nextBoolean();
final String q = matchAll ? "*:*" : buildRandomQuery();
-
- SentinelIntSet ids = assertFullWalkNoDups(totalDocs,
- params("q", q,
- "fl",fl,
- "rows",rows,
- "sort",sort));
+ final SolrParams main = params("q", q,
+ "fl",fl,
+ "rows",rows,
+ "sort",sort);
+ final SentinelIntSet ids = assertFullWalkNoDups(totalDocs, main);
if (matchAll) {
assertEquals(totalDocs, ids.size());
}
+ // same query, now with QEC ... verify we get all the same docs, but the (expected) elevated docs are first...
+ // first we have to build a set of ids to elevate, from the set of ids known to match query...
+ final int[] expectedElevated = pickElevations(TestUtil.nextInt(random(), 3, 33), ids);
+ final SentinelIntSet elevated = assertFullWalkNoDupsElevated
+ (wrapDefaults(params("qt", "/elevate",
+ "fl", fl + ",[elevated]",
+ // HACK: work around SOLR-15307... same results should match, just not same order
+ "sort", (sort.startsWith("score asc") ? "score desc, " + sort : sort),
+ "forceElevation","true",
+ "elevateIds", StringUtils.join(expectedElevated,',')),
+ main),
+ ids);
+ for (int expected : expectedElevated) {
+ assertTrue(expected + " wasn't elevated even though it should have been",
+ elevated.exists(expected));
+ }
+ assertEquals(expectedElevated.length, elevated.size());
}
}
@@ -730,12 +786,74 @@ public class CursorPagingTest extends SolrTestCaseJ4 {
* and then continuously walks the results using {@link CursorMarkParams#CURSOR_MARK_START} as long
* as a non-0 number of docs ar returned. This method records the the set of all id's
* (must be positive ints) encountered and throws an assertion failure if any id is
+ * encountered more than once, or if an id is encountered which is not expected,
+ * or if an id is <code>[elevated]</code> and comes "after" any ids which were not <code>[elevated]</code>
+ *
+ *
+ * @returns set of all elevated ids encountered in the walk
+ * @see #assertFullWalkNoDups(SolrParams,Consumer)
+ */
+ public SentinelIntSet assertFullWalkNoDupsElevated(final SolrParams params, final SentinelIntSet allExpected)
+ throws Exception {
+
+ final SentinelIntSet ids = new SentinelIntSet(allExpected.size(), -1);
+ final SentinelIntSet idsElevated = new SentinelIntSet(32, -1);
+
+ assertFullWalkNoDups(params, (doc) -> {
+ final int id = Integer.parseInt(doc.get("id").toString());
+ final boolean elevated = Boolean.parseBoolean(doc.getOrDefault("[elevated]","false").toString());
+ assertTrue(id + " is not expected to match query",
+ allExpected.exists(id));
+ assertFalse("walk already seen: " + id,
+ ids.exists(id));
+ if (elevated) {
+ assertEquals("id is elevated, but we've already seen non elevated ids: " + id,
+ idsElevated.size(), ids.size());
+ idsElevated.put(id);
+ }
+ ids.put(id);
+ });
+ assertEquals("total number of ids seen did not match expected",
+ allExpected.size(), ids.size());
+
+ return idsElevated;
+ }
+
+
+ /**
+ * Given a set of params, executes a cursor query using {@link CursorMarkParams#CURSOR_MARK_START}
+ * and then continuously walks the results using {@link CursorMarkParams#CURSOR_MARK_START} as long
+ * as a non-0 number of docs ar returned. This method records the the set of all id's
+ * (must be positive ints) encountered and throws an assertion failure if any id is
* encountered more than once, or if the set grows above maxSize
+ *
+ * @returns set of all ids encountered in the walk
+ * @see #assertFullWalkNoDups(SolrParams,Consumer)
*/
public SentinelIntSet assertFullWalkNoDups(int maxSize, SolrParams params)
throws Exception {
-
- SentinelIntSet ids = new SentinelIntSet(maxSize, -1);
+ final SentinelIntSet ids = new SentinelIntSet(maxSize, -1);
+ assertFullWalkNoDups(params, (doc) -> {
+ int id = Integer.parseInt(doc.get("id").toString());
+ assertFalse("walk already seen: " + id, ids.exists(id));
+ ids.put(id);
+ assertFalse("id set bigger then max allowed ("+maxSize+"): " + ids.size(),
+ maxSize < ids.size());
+
+ });
+ return ids;
+ }
+
+ /**
+ * Given a set of params, executes a cursor query using {@link CursorMarkParams#CURSOR_MARK_START}
+ * and then continuously walks the results using {@link CursorMarkParams#CURSOR_MARK_START} as long
+ * as a non-0 number of docs ar returned. This method does some basic validation of each response, and then
+ * passes each doc encountered (in order returned) to the specified Consumer, which may throw an assertion if
+ * there is a problem.
+ */
+ public void assertFullWalkNoDups(SolrParams params, Consumer<Map<Object,Object>> consumer)
+ throws Exception {
+
String cursorMark = CURSOR_MARK_START;
int docsOnThisPage = Integer.MAX_VALUE;
while (0 < docsOnThisPage) {
@@ -760,15 +878,10 @@ public class CursorPagingTest extends SolrTestCaseJ4 {
cursorMark, nextCursorMark);
}
for (Map<Object,Object> doc : docs) {
- int id = Integer.parseInt(doc.get("id").toString());
- assertFalse("walk already seen: " + id, ids.exists(id));
- ids.put(id);
- assertFalse("id set bigger then max allowed ("+maxSize+"): " + ids.size(),
- maxSize < ids.size());
+ consumer.accept(doc);
}
cursorMark = nextCursorMark;
}
- return ids;
}
/**
@@ -1055,4 +1168,31 @@ public class CursorPagingTest extends SolrTestCaseJ4 {
return result.toString();
}
+ /**
+ * Given a set of id, picks some, semi-randomly, to use for elevation
+ */
+ public static int[] pickElevations(final int numToElevate, final SentinelIntSet ids) {
+ assert numToElevate < ids.size();
+ final int[] results = new int[numToElevate];
+ int slot = 0;
+ for (int key : ids.keys) {
+ if (key != ids.emptyVal) {
+ if (slot < results.length) {
+ // until results is full, take any value we can get in the 'next' slot...
+ results[slot] = key;
+ } else if (numToElevate * 2 < slot) {
+ // once we've done enough (extra) iters, break out with what we've got
+ break;
+ } else {
+ // otherwise, pick a random slot to overwrite .. maybe
+ if (random().nextBoolean()) {
+ results[random().nextInt(results.length)] = key;
+ }
+ }
+ slot++;
+ }
+ }
+ return results;
+ }
+
}
diff --git a/solr/core/src/test/org/apache/solr/TestCursorMarkWithoutUniqueKey.java b/solr/core/src/test/org/apache/solr/TestCursorMarkWithoutUniqueKey.java
index cedc9c8..a98d44f 100644
--- a/solr/core/src/test/org/apache/solr/TestCursorMarkWithoutUniqueKey.java
+++ b/solr/core/src/test/org/apache/solr/TestCursorMarkWithoutUniqueKey.java
@@ -27,14 +27,13 @@ import org.junit.After;
*/
public class TestCursorMarkWithoutUniqueKey extends SolrTestCaseJ4 {
- /** solrconfig.xml file name, shared with other cursor related tests */
- public final static String TEST_SOLRCONFIG_NAME = CursorPagingTest.TEST_SOLRCONFIG_NAME;
-
+ public final static String TEST_SOLRCONFIG_NAME = "solrconfig-minimal.xml";
public final static String TEST_SCHEMAXML_NAME = "schema-minimal.xml";
@Before
public void beforeSetupCore() throws Exception {
System.setProperty("solr.test.useFilterForSortedQuery", Boolean.toString(random().nextBoolean()));
+ System.setProperty("solr.test.useFilterForSortedQuery", Boolean.toString(random().nextBoolean()));
initCore(TEST_SOLRCONFIG_NAME, TEST_SCHEMAXML_NAME);
SchemaField uniqueKeyField = h.getCore().getLatestSchema().getUniqueKeyField();
assertNull("This test requires that the schema not have a uniquekey field -- someone violated that in " + TEST_SCHEMAXML_NAME, uniqueKeyField);
diff --git a/solr/core/src/test/org/apache/solr/cloud/DistribCursorPagingTest.java b/solr/core/src/test/org/apache/solr/cloud/DistribCursorPagingTest.java
index b006145..89b4971 100644
--- a/solr/core/src/test/org/apache/solr/cloud/DistribCursorPagingTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/DistribCursorPagingTest.java
@@ -24,6 +24,7 @@ import org.apache.solr.CursorPagingTest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.request.LukeRequest;
import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.cloud.ZkTestServer;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
@@ -36,6 +37,9 @@ import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.search.CursorMark;
+import org.apache.commons.lang3.StringUtils;
+
+import static org.apache.solr.common.params.SolrParams.wrapDefaults;
import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_PARAM;
import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_NEXT;
import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_START;
@@ -50,6 +54,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
+import java.util.function.Consumer;
/**
* Distributed tests of deep paging using {@link CursorMark} and {@link CursorMarkParams#CURSOR_MARK_PARAM}.
@@ -75,8 +80,17 @@ public class DistribCursorPagingTest extends AbstractFullDistribZkTestBase {
return configString;
}
+ /**
+ * A really obnoxious hack needed to get our elevate.xml into zk ...
+ * But simpler for now then re-writing the whole test case using SolrCloudTestCase.
+ */
+ @Override
+ public void distribSetUp() throws Exception {
+ super.distribSetUp();
+ ZkTestServer.putConfig("conf1", zkServer.getZkClient(), ZkTestServer.SOLRHOME, "elevate.xml");
+ }
+
@Test
- // commented out on: 24-Dec-2018 @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // added 23-Aug-2018
public void test() throws Exception {
boolean testFinished = false;
try {
@@ -139,6 +153,7 @@ public class DistribCursorPagingTest extends AbstractFullDistribZkTestBase {
GroupParams.GROUP_FIELD, "str",
CURSOR_MARK_PARAM, CURSOR_MARK_START),
ErrorCode.BAD_REQUEST, "Grouping");
+
}
private void doSimpleTest() throws Exception {
@@ -538,36 +553,25 @@ public class DistribCursorPagingTest extends AbstractFullDistribZkTestBase {
for (String order : new String[] {" asc", " desc"}) {
String sort = f + order + ("id".equals(f) ? "" : ", id" + order);
String rows = "" + TestUtil.nextInt(random(), 13, 50);
- SentinelIntSet ids = assertFullWalkNoDups(numInitialDocs,
- params("q", "*:*",
- "fl","id,"+f,
- "rows",rows,
- "sort",sort));
- if (numInitialDocs != ids.size()) {
- StringBuilder message = new StringBuilder
- ("Expected " + numInitialDocs + " docs but got " + ids.size() + ". ");
- message.append("sort=");
- message.append(sort);
- message.append(". ");
- if (ids.size() < numInitialDocs) {
- message.append("Missing doc(s): ");
- for (SolrInputDocument doc : initialDocs) {
- int id = Integer.parseInt(doc.getFieldValue("id").toString());
- if ( ! ids.exists(id)) {
- QueryResponse rsp = cloudClient.query(params("q", "id:" + id,
- "rows", "1"));
- if (0 == rsp.getResults().size()) {
- message.append("<NOT RETRIEVABLE>:");
- message.append(doc.values());
- } else {
- message.append(rsp.getResults().get(0).getFieldValueMap().toString());
- }
- message.append("; ");
- }
- }
- }
- fail(message.toString());
- }
+ final SolrParams main = params("q", "*:*",
+ "fl","id,"+f,
+ "rows",rows,
+ "sort",sort);
+ final SentinelIntSet ids = assertFullWalkNoDups(numInitialDocs, main);
+ assertEquals(numInitialDocs, ids.size());
+
+ // same query, now with QEC ... verify we get all the same docs, but the (expected) elevated docs are first...
+ final SentinelIntSet elevated = assertFullWalkNoDupsElevated(wrapDefaults(params("qt", "/elevate",
+ "fl","id,[elevated]",
+ "forceElevation","true",
+ "elevateIds", "50,20,80"),
+ main),
+ ids);
+ assertTrue(elevated.exists(50));
+ assertTrue(elevated.exists(20));
+ assertTrue(elevated.exists(80));
+ assertEquals(3, elevated.size());
+
}
}
@@ -585,15 +589,36 @@ public class DistribCursorPagingTest extends AbstractFullDistribZkTestBase {
final String fl = random().nextBoolean() ? "id" : "id,score";
final boolean matchAll = random().nextBoolean();
final String q = matchAll ? "*:*" : CursorPagingTest.buildRandomQuery();
-
- SentinelIntSet ids = assertFullWalkNoDups(totalDocs,
- params("q", q,
- "fl",fl,
- "rows",rows,
- "sort",sort));
+ final SolrParams main = params("q", q,
+ "fl",fl,
+ "rows",rows,
+ "sort",sort);
+ final SentinelIntSet ids = assertFullWalkNoDups(totalDocs,
+ params("q", q,
+ "fl",fl,
+ "rows",rows,
+ "sort",sort));
if (matchAll) {
assertEquals(totalDocs, ids.size());
}
+
+ // same query, now with QEC ... verify we get all the same docs, but the (expected) elevated docs are first...
+ // first we have to build a set of ids to elevate, from the set of ids known to match query...
+ final int[] expectedElevated = CursorPagingTest.pickElevations(TestUtil.nextInt(random(), 3, 33), ids);
+ final SentinelIntSet elevated = assertFullWalkNoDupsElevated
+ (wrapDefaults(params("qt", "/elevate",
+ "fl", fl + ",[elevated]",
+ // HACK: work around SOLR-15307... same results should match, just not same order
+ "sort", (sort.startsWith("score asc") ? "score desc, " + sort : sort),
+ "forceElevation","true",
+ "elevateIds", StringUtils.join(expectedElevated,',')),
+ main),
+ ids);
+ for (int expected : expectedElevated) {
+ assertTrue(expected + " wasn't elevated even though it should have been",
+ elevated.exists(expected));
+ }
+ assertEquals(expectedElevated.length, elevated.size());
}
@@ -689,6 +714,60 @@ public class DistribCursorPagingTest extends AbstractFullDistribZkTestBase {
}
/**
+ * Given a set of params, executes a cursor query using {@link CursorMarkParams#CURSOR_MARK_START}
+ * and then continuously walks the results using {@link CursorMarkParams#CURSOR_MARK_START} as long
+ * as a non-0 number of docs ar returned. This method records the the set of all id's
+ * (must be positive ints) encountered and throws an assertion failure if any id is
+ * encountered more than once, or if an id is encountered which is not expected,
+ * or if an id is <code>[elevated]</code> and comes "after" any ids which were not <code>[elevated]</code>
+ *
+ *
+ * @returns set of all elevated ids encountered in the walk
+ * @see #assertFullWalkNoDups(SolrParams,Consumer)
+ */
+ public SentinelIntSet assertFullWalkNoDupsElevated(final SolrParams params, final SentinelIntSet allExpected)
+ throws Exception {
+
+ final SentinelIntSet ids = new SentinelIntSet(allExpected.size(), -1);
+ final SentinelIntSet idsElevated = new SentinelIntSet(32, -1);
+
+ assertFullWalkNoDups(params, (doc) -> {
+ final int id = Integer.parseInt(doc.get("id").toString());
+ final boolean elevated = Boolean.parseBoolean(doc.getOrDefault("[elevated]","false").toString());
+ assertTrue(id + " is not expected to match query",
+ allExpected.exists(id));
+
+ if (ids.exists(id)) {
+ String msg = "walk already seen: " + id;
+ try {
+ try {
+ queryAndCompareShards(params("distrib","false",
+ "q","id:"+id));
+ } catch (AssertionError ae) {
+ throw new AssertionError(msg + ", found shard inconsistency that would explain it...", ae);
+ }
+ final QueryResponse rsp = cloudClient.query(params("q","id:"+id));
+ throw new AssertionError(msg + ", don't know why; q=id:"+id+" gives: " + rsp.toString());
+ } catch (Exception e) {
+ throw new AssertionError(msg + ", exception trying to fiture out why...", e);
+ }
+ }
+ if (elevated) {
+ assertEquals("id is elevated, but we've already seen non elevated ids: " + id,
+ idsElevated.size(), ids.size());
+ idsElevated.put(id);
+ }
+ ids.put(id);
+ });
+
+ assertEquals("total number of ids seen did not match expected",
+ allExpected.size(), ids.size());
+
+ return idsElevated;
+ }
+
+
+ /**
* <p>
* Given a set of params, executes a cursor query using {@link CursorMarkParams#CURSOR_MARK_START}
* and then continuously walks the results using {@link CursorMarkParams#CURSOR_MARK_START} as long
@@ -705,9 +784,57 @@ public class DistribCursorPagingTest extends AbstractFullDistribZkTestBase {
* to the control index -- which can affect the sorting in some cases and cause false
* negatives in the response comparisons (even if we don't include "score" in the "fl")
* </p>
+ *
+ * @returns set of all ids encountered in the walk
+ * @see #assertFullWalkNoDups(SolrParams,Consumer)
*/
public SentinelIntSet assertFullWalkNoDups(int maxSize, SolrParams params) throws Exception {
- SentinelIntSet ids = new SentinelIntSet(maxSize, -1);
+ final SentinelIntSet ids = new SentinelIntSet(maxSize, -1);
+ assertFullWalkNoDups(params, (doc) -> {
+ int id = Integer.parseInt(doc.getFieldValue("id").toString());
+ if (ids.exists(id)) {
+ String msg = "walk already seen: " + id;
+ try {
+ try {
+ queryAndCompareShards(params("distrib","false",
+ "q","id:"+id));
+ } catch (AssertionError ae) {
+ throw new AssertionError(msg + ", found shard inconsistency that would explain it...", ae);
+ }
+ final QueryResponse rsp = cloudClient.query(params("q","id:"+id));
+ throw new AssertionError(msg + ", don't know why; q=id:"+id+" gives: " + rsp.toString());
+ } catch (Exception e) {
+ throw new AssertionError(msg + ", exception trying to fiture out why...", e);
+ }
+ }
+ ids.put(id);
+ assertFalse("id set bigger then max allowed ("+maxSize+"): " + ids.size(),
+ maxSize < ids.size());
+ });
+ return ids;
+ }
+
+
+ /**
+ * <p>
+ * Given a set of params, executes a cursor query using {@link CursorMarkParams#CURSOR_MARK_START}
+ * and then continuously walks the results using {@link CursorMarkParams#CURSOR_MARK_START} as long
+ * as a non-0 number of docs ar returned. This method does some basic validation of each response, and then
+ * passes each doc encountered (in order returned) to the specified Consumer, which may throw an assertion if
+ * there is a problem.
+ * </p>
+ *
+ * <p>
+ * Note that this method explicitly uses the "cloudClient" for executing the queries,
+ * instead of relying on the test infrastructure to execute the queries redundently
+ * against both the cloud client as well as a control client. This is because term stat
+ * differences in a sharded setup can result in different scores for documents compared
+ * to the control index -- which can affect the sorting in some cases and cause false
+ * negatives in the response comparisons (even if we don't include "score" in the "fl")
+ * </p>
+ */
+ public void assertFullWalkNoDups(SolrParams params, Consumer<SolrDocument> consumer) throws Exception {
+
String cursorMark = CURSOR_MARK_START;
int docsOnThisPage = Integer.MAX_VALUE;
while (0 < docsOnThisPage) {
@@ -727,25 +854,10 @@ public class DistribCursorPagingTest extends AbstractFullDistribZkTestBase {
}
for (SolrDocument doc : docs) {
- int id = Integer.parseInt(doc.getFieldValue("id").toString());
- if (ids.exists(id)) {
- String msg = "(" + p + ") walk already seen: " + id;
- try {
- queryAndCompareShards(params("distrib","false",
- "q","id:"+id));
- } catch (AssertionError ae) {
- throw new AssertionError(msg + ", found shard inconsistency that would explain it...", ae);
- }
- rsp = cloudClient.query(params("q","id:"+id));
- throw new AssertionError(msg + ", don't know why; q=id:"+id+" gives: " + rsp.toString());
- }
- ids.put(id);
- assertFalse("id set bigger then max allowed ("+maxSize+"): " + ids.size(),
- maxSize < ids.size());
+ consumer.accept(doc);
}
cursorMark = nextCursorMark;
}
- return ids;
}
private SolrParams p(SolrParams params, String... other) {
diff --git a/solr/core/src/test/org/apache/solr/handler/component/BadComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/BadComponentTest.java
index 6de1dd6..c09b5bb 100644
--- a/solr/core/src/test/org/apache/solr/handler/component/BadComponentTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/component/BadComponentTest.java
@@ -18,6 +18,7 @@ package org.apache.solr.handler.component;
import org.apache.solr.SolrTestCaseJ4;
+import org.junit.After;
import org.junit.Test;
/**
@@ -38,4 +39,9 @@ public class BadComponentTest extends SolrTestCaseJ4{
resetExceptionIgnores();
}
}
+
+ @After
+ public void deleteCoreThatFailedToInit() throws Exception {
+ deleteCore();
+ }
}
diff --git a/solr/core/src/test/org/apache/solr/handler/component/QueryElevationComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/QueryElevationComponentTest.java
index d236883..7d158aa 100644
--- a/solr/core/src/test/org/apache/solr/handler/component/QueryElevationComponentTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/component/QueryElevationComponentTest.java
@@ -24,6 +24,7 @@ import java.lang.invoke.MethodHandles;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.Arrays;
+import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
@@ -32,6 +33,7 @@ import org.apache.lucene.index.IndexReader;
import org.apache.lucene.util.BytesRef;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.params.CursorMarkParams;
import org.apache.solr.common.params.GroupParams;
import org.apache.solr.common.params.QueryElevationParams;
import org.apache.solr.common.params.SolrParams;
@@ -49,6 +51,11 @@ import org.junit.rules.TestRule;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_NEXT;
+import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_PARAM;
+import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_START;
+import static org.apache.solr.common.util.Utils.fromJSONString;
+
public class QueryElevationComponentTest extends SolrTestCaseJ4 {
@Rule
@@ -1031,8 +1038,102 @@ public class QueryElevationComponentTest extends SolrTestCaseJ4 {
delete();
}
}
+ @Test
+ public void testCursor() throws Exception {
+ try {
+ init("schema12.xml");
+
+ assertU(adoc("id", "a", "title", "ipod trash trash", "str_s1", "group1"));
+ assertU(adoc("id", "b", "title", "ipod ipod trash", "str_s1", "group2"));
+ assertU(adoc("id", "c", "title", "ipod ipod ipod ", "str_s1", "group2"));
+ assertU(adoc("id", "x", "title", "boosted", "str_s1", "group1"));
+ assertU(adoc("id", "y", "title", "boosted boosted", "str_s1", "group2"));
+ assertU(adoc("id", "z", "title", "boosted boosted boosted", "str_s1", "group2"));
+ assertU(commit());
+
+ final SolrParams baseParams = params("qt", "/elevate",
+ "q", "title:ipod",
+ "sort", "score desc, id asc",
+ "fl", "id",
+ "elevateIds", "x,y,z",
+ "excludeIds", "b");
+
+ // sanity check everything returned w/these elevation options...
+ assertJQ(req(baseParams)
+ , "/response/numFound==5"
+ , "/response/start==0"
+ , "/response/docs==[{'id':'x'},{'id':'y'},{'id':'z'},{'id':'c'},{'id':'a'}]"
+ );
+ // same query using CURSOR_MARK_START should produce a 'next' cursor...
+ assertCursorJQ(req(baseParams, CURSOR_MARK_PARAM, CURSOR_MARK_START)
+ , "/response/numFound==5"
+ , "/response/start==0"
+ , "/response/docs==[{'id':'x'},{'id':'y'},{'id':'z'},{'id':'c'},{'id':'a'}]"
+ );
+
+ // use a cursor w/rows < 5, then fetch next cursor...
+ String nextCursor = null;
+ nextCursor = assertCursorJQ(req(baseParams
+ , CURSOR_MARK_PARAM, CURSOR_MARK_START
+ , "rows", "2"
+ )
+ , "/response/numFound==5"
+ , "/response/start==0"
+ , "/response/docs==[{'id':'x'},{'id':'y'}]"
+ );
+ nextCursor = assertCursorJQ(req(baseParams
+ , CURSOR_MARK_PARAM, nextCursor
+ , "rows", "2"
+ )
+ , "/response/numFound==5"
+ , "/response/start==0"
+ , "/response/docs==[{'id':'z'},{'id':'c'}]"
+ );
+ nextCursor = assertCursorJQ(req(baseParams
+ , CURSOR_MARK_PARAM, nextCursor
+ , "rows", "2"
+ )
+ , "/response/numFound==5"
+ , "/response/start==0"
+ , "/response/docs==[{'id':'a'}]"
+ );
+ final String lastCursor = nextCursor;
+ nextCursor = assertCursorJQ(req(baseParams
+ , CURSOR_MARK_PARAM, nextCursor
+ , "rows", "2"
+ )
+ , "/response/numFound==5"
+ , "/response/start==0"
+ , "/response/docs==[]"
+ );
+ assertEquals(lastCursor, nextCursor);
+
+ } finally {
+ delete();
+ }
+
+ }
private static Set<BytesRef> toIdSet(String... ids) {
return Arrays.stream(ids).map(BytesRef::new).collect(Collectors.toSet());
}
+
+
+ /**
+ * Asserts that the query matches the specified JSON patterns and then returns the
+ * {@link CursorMarkParams#CURSOR_MARK_NEXT} value from the response
+ *
+ * @see #assertJQ
+ */
+ private static String assertCursorJQ(SolrQueryRequest req, String... tests) throws Exception {
+ String json = assertJQ(req, tests);
+ @SuppressWarnings({"rawtypes"})
+ Map rsp = (Map) fromJSONString(json);
+ assertTrue("response doesn't contain "+CURSOR_MARK_NEXT + ": " + json,
+ rsp.containsKey(CURSOR_MARK_NEXT));
+ String next = (String)rsp.get(CURSOR_MARK_NEXT);
+ assertNotNull(CURSOR_MARK_NEXT + " is null", next);
+ return next;
+ }
+
}
diff --git a/solr/core/src/test/org/apache/solr/search/CursorMarkTest.java b/solr/core/src/test/org/apache/solr/search/CursorMarkTest.java
index 19722be..9ae3fe4 100644
--- a/solr/core/src/test/org/apache/solr/search/CursorMarkTest.java
+++ b/solr/core/src/test/org/apache/solr/search/CursorMarkTest.java
@@ -39,6 +39,7 @@ import java.util.Collections;
import java.util.UUID;
import org.junit.BeforeClass;
+import static org.hamcrest.core.StringContains.containsString;
/**
* Primarily a test of parsing and serialization of the CursorMark values.
@@ -71,15 +72,16 @@ public class CursorMarkTest extends SolrTestCaseJ4 {
assertEquals("next values not correct", nextValues, next.getSortValues());
assertEquals("next SortSpec not correct", ss, next.getSortSpec());
- try {
- // append to our random sort string so we know it has wrong num clauses
- final SortSpec otherSort = SortSpecParsing.parseSortSpec(randomSortString+",id asc", req);
- CursorMark trash = previous.createNext(Arrays.<Object>asList
- (buildRandomSortObjects(otherSort)));
- fail("didn't fail on next with incorrect num of sortvalues");
- } catch (AssertionError e) {
- // NOOP: we're happy
- }
+ SolrException e = expectThrows(SolrException.class,
+ "didn't fail on next with incorrect num of sortvalues",
+ () -> {
+ // append to our random sort string so we know it has wrong num clauses
+ final SortSpec otherSort = SortSpecParsing.parseSortSpec(randomSortString+",id asc", req);
+ CursorMark trash = previous.createNext(Arrays.<Object>asList
+ (buildRandomSortObjects(otherSort)));
+ });
+ assertEquals(500, e.code());
+ assertThat(e.getMessage(), containsString("sort values != sort length"));
}
public void testInvalidUsage() {