This is an automated email from the ASF dual-hosted git repository.
houston 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 0a50edab233 SOLR-17650: Fix tests for unordered buffered updates
(#3197)
0a50edab233 is described below
commit 0a50edab23350947157731ab973a1f07ec7cf69a
Author: Houston Putman <[email protected]>
AuthorDate: Tue Feb 25 09:16:54 2025 -0600
SOLR-17650: Fix tests for unordered buffered updates (#3197)
---
.../test/org/apache/solr/search/TestRecovery.java | 149 ++++++++++++++++++---
.../src/java/org/apache/solr/SolrTestCaseJ4.java | 58 ++++++++
.../java/org/apache/solr/util/SolrMatchers.java | 97 ++++++++++++++
3 files changed, 286 insertions(+), 18 deletions(-)
diff --git a/solr/core/src/test/org/apache/solr/search/TestRecovery.java
b/solr/core/src/test/org/apache/solr/search/TestRecovery.java
index 36e33b772e2..2c814cf9b5f 100644
--- a/solr/core/src/test/org/apache/solr/search/TestRecovery.java
+++ b/solr/core/src/test/org/apache/solr/search/TestRecovery.java
@@ -18,6 +18,7 @@ package org.apache.solr.search;
import static
org.apache.solr.search.TestRecovery.VersionProvider.getNextVersion;
import static
org.apache.solr.update.processor.DistributingUpdateProcessorFactory.DISTRIB_UPDATE_PARAM;
+import static org.apache.solr.util.SolrMatchers.subListMatches;
import com.codahale.metrics.Gauge;
import com.codahale.metrics.Meter;
@@ -52,6 +53,9 @@ import org.apache.solr.update.UpdateLog;
import
org.apache.solr.update.processor.DistributedUpdateProcessor.DistribPhase;
import org.apache.solr.util.TestInjection;
import org.apache.solr.util.TimeOut;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -686,10 +690,6 @@ public class TestRecovery extends SolrTestCaseJ4 {
String versionListFirstCheck =
String.join(",", v2010_del, v1030, v1020, v1017_del, v1015, v1010);
- String versionListSecondCheck =
- String.join(
- ",", v3000_del, v1080, v1050, v1060, v940_del, v1040, v3,
v2010_del, v1030, v1020,
- v1017_del, v1015, v1010);
// simulate updates from a leader
updateJ(
@@ -736,8 +736,25 @@ public class TestRecovery extends SolrTestCaseJ4 {
assertEquals(6L, applyingBuffered.getCount() - initialApplyingOps);
- assertJQ(
- req("qt", "/get", "getVersions", "6"), "=={'versions':[" +
versionListFirstCheck + "]}");
+ assertThatJQ(
+ req("qt", "/get", "getVersions", "6"),
+ "Incorrect ordering of versions during applyBufferedUpdates",
+ versionsMatch(
+ 6,
+ // These do not have the same id, so they may be in any order
+ subListMatches(
+ 0,
+ 3,
+ Matchers.containsInAnyOrder(
+ Long.parseLong(v1020), Long.parseLong(v1030),
Long.parseLong(v2010_del))),
+ // The deleteByQuery will not be applied in parallel, it will be
in the exact right
+ // place in the list
+ subListMatches(3, 4,
Matchers.contains(Long.parseLong(v1017_del))),
+ // These do not have the same id, so they may be in any order
+ subListMatches(
+ 4,
+ 6,
+ Matchers.containsInAnyOrder(Long.parseLong(v1015),
Long.parseLong(v1010)))));
assertJQ(req("q", "*:*"), "/response/numFound==2");
@@ -776,12 +793,38 @@ public class TestRecovery extends SolrTestCaseJ4 {
"{\"delete\": { \"query\":\"id:B2 OR id:B8\" }}",
params(DISTRIB_UPDATE_PARAM, FROM_LEADER, "_version_", v3000_del));
- assertJQ(
+ assertThatJQ(
req("qt", "/get", "getVersions", "13"),
- "=={'versions':["
- + versionListSecondCheck
- + "]}" // the "3" appears because versions aren't checked while
buffering
- );
+ "Incorrect versions during buffering",
+ versionsMatch(
+ 13,
+ // These buffered updates have not been applied, they should be
in the exact order
+ // the "3" appears because versions aren't checked while
buffering
+ subListMatches(
+ 0,
+ 7,
+ Matchers.contains(
+ Long.parseLong(v3000_del),
+ Long.parseLong(v1080),
+ Long.parseLong(v1050),
+ Long.parseLong(v1060),
+ Long.parseLong(v940_del),
+ Long.parseLong(v1040),
+ Long.parseLong(v3))),
+ // These do not have the same id, so they may be in any order
+ subListMatches(
+ 7,
+ 10,
+ Matchers.containsInAnyOrder(
+ Long.parseLong(v1020), Long.parseLong(v1030),
Long.parseLong(v2010_del))),
+ // The deleteByQuery will not be applied in parallel, it will be
in the exact right
+ // place in the list
+ subListMatches(10, 11,
Matchers.contains(Long.parseLong(v1017_del))),
+ // These do not have the same id, so they may be in any order
+ subListMatches(
+ 11,
+ 13,
+ Matchers.containsInAnyOrder(Long.parseLong(v1015),
Long.parseLong(v1010)))));
logReplay.drainPermits();
rinfoFuture = ulog.applyBufferedUpdates();
@@ -906,7 +949,13 @@ public class TestRecovery extends SolrTestCaseJ4 {
UpdateLog.RecoveryInfo rinfo = rinfoFuture.get();
assertEquals(2, rinfo.adds);
- assertJQ(req("qt", "/get", "getVersions", "2"), "=={'versions':[" + v105
+ "," + v104 + "]}");
+ assertThatJQ(
+ req("qt", "/get", "getVersions", "2"),
+ "Wrong updates after applyBufferedUpdates",
+ versionsMatch(
+ 2,
+ // Buffered Updates might not be in the original order
+ Matchers.containsInAnyOrder(Long.parseLong(v105),
Long.parseLong(v104))));
// this time add some docs first before buffering starts (so tlog won't
be at pos 0)
updateJ(
@@ -961,9 +1010,22 @@ public class TestRecovery extends SolrTestCaseJ4 {
+ "]");
// Note that the v101->v103 are dropped, therefore it does not present
in RTG
- assertJQ(
+ assertThatJQ(
req("qt", "/get", "getVersions", "6"),
- "=={'versions':[" + String.join(",", v206, v205, v201, v200, v105,
v104) + "]}");
+ "Incorrect versions after applyBufferedUpdates",
+ versionsMatch(
+ 6,
+ // Buffered Updates might not be in the original order
+ subListMatches(
+ 0, 2, Matchers.containsInAnyOrder(Long.parseLong(v206),
Long.parseLong(v205))),
+ // These updates were not buffered
+ subListMatches(
+ 2,
+ 4,
+ Matchers.containsInRelativeOrder(Long.parseLong(v201),
Long.parseLong(v200))),
+ // Buffered Updates might not be in the original order
+ subListMatches(
+ 4, 6, Matchers.containsInAnyOrder(Long.parseLong(v105),
Long.parseLong(v104)))));
ulog.bufferUpdates();
assertEquals(UpdateLog.State.BUFFERING, ulog.getState());
@@ -1078,7 +1140,13 @@ public class TestRecovery extends SolrTestCaseJ4 {
UpdateLog.RecoveryInfo rinfo = rinfoFuture.get();
assertEquals(2, rinfo.adds);
- assertJQ(req("qt", "/get", "getVersions", "2"), "=={'versions':[" + v105
+ "," + v104 + "]}");
+ assertThatJQ(
+ req("qt", "/get", "getVersions", "2"),
+ "Wrong updates after applyBufferedUpdates",
+ versionsMatch(
+ 2,
+ // Buffered Updates might not be in the original order
+ Matchers.containsInAnyOrder(Long.parseLong(v105),
Long.parseLong(v104))));
updateJ(
jsonAdd(sdoc("id", "c100", "_version_", v200)),
@@ -1130,12 +1198,24 @@ public class TestRecovery extends SolrTestCaseJ4 {
+ ",{'id':'c106','_version_':"
+ v206
+ "}"
- + ""
+ "]");
- assertJQ(
+ assertThatJQ(
req("qt", "/get", "getVersions", "6"),
- "=={'versions':[" + String.join(",", v206, v205, v201, v200, v105,
v104) + "]}");
+ "Incorrect versions after applyBufferedUpdates",
+ versionsMatch(
+ 6,
+ // Buffered Updates might not be in the original order
+ subListMatches(
+ 0, 2, Matchers.containsInAnyOrder(Long.parseLong(v206),
Long.parseLong(v205))),
+ // These updates were not buffered
+ subListMatches(
+ 2,
+ 4,
+ Matchers.containsInRelativeOrder(Long.parseLong(v201),
Long.parseLong(v200))),
+ // Buffered Updates might not be in the original order
+ subListMatches(
+ 4, 6, Matchers.containsInAnyOrder(Long.parseLong(v105),
Long.parseLong(v104)))));
assertEquals(
UpdateLog.State.ACTIVE, ulog.getState()); // leave each test method
in a good state
@@ -1976,4 +2056,37 @@ public class TestRecovery extends SolrTestCaseJ4 {
return Long.toString(version++);
}
}
+
+ @SafeVarargs
+ public static Matcher<Map<String, List<Long>>> versionsMatch(
+ int numVersions, Matcher<? super List<Long>>... versionsMatchers) {
+ return new VersionsMatcher(numVersions, versionsMatchers);
+ }
+
+ public static class VersionsMatcher extends FeatureMatcher<Map<String,
List<Long>>, List<Long>> {
+
+ @SafeVarargs
+ @SuppressWarnings("varargs")
+ public VersionsMatcher(int numVersions, Matcher<? super List<Long>>...
subMatchers) {
+ super(
+ allOf(Matchers.hasSize(numVersions), subMatchers),
+ "a response with versions list",
+ "versions list");
+ }
+
+ @SafeVarargs
+ @SuppressWarnings("varargs")
+ public static Matcher<? super List<Long>> allOf(
+ Matcher<? super List<Long>> firstMatcher, Matcher<? super
List<Long>>... subMatchers) {
+ List<Matcher<? super List<Long>>> matchers = new
ArrayList<>(subMatchers.length + 1);
+ matchers.add(firstMatcher);
+ matchers.addAll(Arrays.asList(subMatchers));
+ return Matchers.allOf(matchers);
+ }
+
+ @Override
+ protected List<Long> featureValueOf(Map<String, List<Long>> actual) {
+ return actual.get("versions");
+ }
+ }
}
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
index d8a9ac37ed6..7dd7121406e 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
@@ -142,6 +142,7 @@ import org.apache.solr.util.SSLTestConfig;
import org.apache.solr.util.TestHarness;
import org.apache.solr.util.TestInjection;
import org.apache.zookeeper.KeeperException;
+import org.hamcrest.Matcher;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Rule;
@@ -1027,6 +1028,63 @@ public abstract class SolrTestCaseJ4 extends
SolrTestCase {
}
}
+ public static <T> String assertThatJQ(SolrQueryRequest req, Matcher<T> test)
throws Exception {
+ return assertThatJQ(req, "", test);
+ }
+
+ /**
+ * Validates a query completes and, using JSON deserialization, returns an
object that passes the
+ * given Matcher test.
+ *
+ * <p>Please use this with care: this makes it easy to match complete
structures, but doing so can
+ * result in fragile tests if you are matching more than what you want to
test.
+ *
+ * @param req Solr request to execute
+ * @param message Failure message for test
+ * @param test Matcher for the given object returned from deserializing the
response
+ * @return The request response as a JSON String if the test matcher passes
+ */
+ @SuppressWarnings("unchecked")
+ public static <T> String assertThatJQ(SolrQueryRequest req, String message,
Matcher<T> test)
+ throws Exception {
+ final SolrParams params = req.getParams();
+ try {
+ if (!"json".equals(params.get("wt", "xml")) || params.get("indent") ==
null) {
+ ModifiableSolrParams newParams = new ModifiableSolrParams(params);
+ newParams.set("wt", "json");
+ if (params.get("indent") == null) newParams.set("indent", "true");
+ req.setParams(newParams);
+ }
+
+ String response;
+ boolean failed = true;
+ try {
+ response = h.query(req);
+ failed = false;
+ } finally {
+ if (failed) {
+ fail("REQUEST FAILED: " + req.getParamString());
+ }
+ }
+ Object responseObj;
+ failed = true;
+ try {
+ responseObj = Utils.fromJSONString(response);
+ failed = false;
+ } finally {
+ if (failed) {
+ fail("Couldn't deserialize json: " + response);
+ }
+ }
+
+ assertThat(message, (T) responseObj, test);
+
+ return response;
+ } finally {
+ req.setParams(params); // restore in case we changed it
+ }
+ }
+
/** Makes sure a query throws a SolrException with the listed response code
*/
public static void assertQEx(String message, SolrQueryRequest req, int code)
{
try {
diff --git
a/solr/test-framework/src/java/org/apache/solr/util/SolrMatchers.java
b/solr/test-framework/src/java/org/apache/solr/util/SolrMatchers.java
new file mode 100644
index 00000000000..fed3559489b
--- /dev/null
+++ b/solr/test-framework/src/java/org/apache/solr/util/SolrMatchers.java
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import java.util.List;
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.hamcrest.TypeSafeDiagnosingMatcher;
+
+/**
+ * A Utility class for extra Hamcrest {@link Matcher} implementations.
+ *
+ * <p>These may be directly related to Solr, or be filling gaps that the
default Hamcrest Matchers
+ * do not cover.
+ */
+public class SolrMatchers {
+
+ /**
+ * Matches a segment of a list between two indices against a provided
matcher, useful for checking
+ * ordered and unordered sub-sequences in a larger list
+ *
+ * @param fromIndex starting index of the desired sub-list (inclusive)
+ * @param toIndex ending index of the desired sub-list (exclusive)
+ * @param subListMatcher matcher for the resulting sub-list
+ * @return a Matcher for the original super-list
+ * @param <T> The type that the list being matched against will contain
+ */
+ public static <T> Matcher<List<? extends T>> subListMatches(
+ int fromIndex, int toIndex, Matcher<? super List<? super T>>
subListMatcher) {
+ return new SubListMatcher<>(fromIndex, toIndex, subListMatcher);
+ }
+
+ public static class SubListMatcher<T> extends
TypeSafeDiagnosingMatcher<List<? extends T>> {
+ private final int fromIndex;
+ private final int toIndex;
+ private final Matcher<? super List<T>> matchOnSubList;
+
+ public SubListMatcher(
+ int fromIndex, int toIndex, Matcher<? super List<? super T>>
matchOnSubList) {
+ super();
+ this.fromIndex = fromIndex;
+ this.toIndex = toIndex;
+ this.matchOnSubList = matchOnSubList;
+ }
+
+ @Override
+ public void describeTo(Description description) {
+ description
+ .appendText("sub-list from ")
+ .appendValue(fromIndex)
+ .appendText(" to ")
+ .appendValue(toIndex)
+ .appendText(" matching ")
+ .appendDescriptionOf(matchOnSubList);
+ }
+
+ @Override
+ protected boolean matchesSafely(List<? extends T> item, Description
mismatchDescription) {
+ if (item.size() < toIndex) {
+ mismatchDescription
+ .appendText(": expected sub-list endIndex ")
+ .appendValue(toIndex)
+ .appendText(" greater than list size ")
+ .appendValue(item.size())
+ .appendText("\n full-list: ")
+ .appendValue(item);
+ return false;
+ } else {
+ List<? extends T> subList = item.subList(fromIndex, toIndex);
+ if (!matchOnSubList.matches(subList)) {
+ matchOnSubList.describeMismatch(subList, mismatchDescription);
+ mismatchDescription
+ .appendText("\n sub-list: ")
+ .appendValue(subList)
+ .appendText("\n full-list: ")
+ .appendValue(item);
+ return false;
+ }
+ }
+ return true;
+ }
+ }
+}
