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 <[email protected]>
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() {
