You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by md...@apache.org on 2022/04/04 17:30:49 UTC

[solr] branch branch_9x updated (c1c8527847c -> 570d9a67821)

This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a change to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git


    from c1c8527847c Add distDir as a gradle input for integration tests (#787)
     new 912d9157271 SOLR-16097 Batch add docs when starting tests (#744)
     new c347fd6b5c5 SOLR-16127 Ordered Executor orphans locks (#779)
     new 570d9a67821 SOLR-16134: Failing integration tests should fail the build (#785)

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 solr/bin/solr                                      | 51 +++++++++++-----------
 .../java/org/apache/solr/util/OrderedExecutor.java |  9 +++-
 .../apache/solr/cloud/DistribCursorPagingTest.java | 21 ++++-----
 .../component/DistributedFacetPivotLargeTest.java  |  4 +-
 .../TestDistributedStatsComponentCardinality.java  | 51 +++++++++++++---------
 .../test/org/apache/solr/update/PeerSyncTest.java  | 32 ++++++--------
 .../solr/update/TestInPlaceUpdatesDistrib.java     |  2 +-
 .../org/apache/solr/util/OrderedExecutorTest.java  | 51 ++++++++++++++++++++++
 solr/packaging/build.gradle                        |  1 +
 solr/packaging/test/README.md                      |  2 +
 solr/packaging/test/bats_helper.bash               |  1 +
 .../test/{test_start_solr.bats => test_bats.bats}  | 26 +++++++----
 solr/packaging/test/test_create_collection.bats    |  4 +-
 solr/packaging/test/test_delete_collection.bats    |  4 +-
 solr/packaging/test/test_start_solr.bats           |  4 +-
 .../apache/solr/BaseDistributedSearchTestCase.java | 49 ++++++++++++++++++---
 .../cloud/AbstractBasicDistributedZkTestBase.java  |  2 +-
 .../solr/cloud/AbstractFullDistribZkTestBase.java  |  4 +-
 18 files changed, 215 insertions(+), 103 deletions(-)
 copy solr/packaging/test/{test_start_solr.bats => test_bats.bats} (61%)


[solr] 03/03: SOLR-16134: Failing integration tests should fail the build (#785)

Posted by md...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git

commit 570d9a6782198a835914c46642edc7ab2e8dfd42
Author: Mike Drob <md...@apache.org>
AuthorDate: Mon Apr 4 11:38:32 2022 -0500

    SOLR-16134: Failing integration tests should fail the build (#785)
    
    Address a few pipe-fail cases in bin/solr
    
    (cherry picked from commit 3ed17c2737db038c2908a1475099647fb4ef5b38)
---
 solr/bin/solr                                      | 51 +++++++++++-----------
 solr/packaging/build.gradle                        |  1 +
 solr/packaging/test/README.md                      |  2 +
 solr/packaging/test/bats_helper.bash               |  1 +
 .../test/{test_start_solr.bats => test_bats.bats}  | 26 +++++++----
 solr/packaging/test/test_create_collection.bats    |  4 +-
 solr/packaging/test/test_delete_collection.bats    |  4 +-
 solr/packaging/test/test_start_solr.bats           |  4 +-
 8 files changed, 54 insertions(+), 39 deletions(-)

diff --git a/solr/bin/solr b/solr/bin/solr
index e0f0dc26ceb..c1ef2af1e00 100755
--- a/solr/bin/solr
+++ b/solr/bin/solr
@@ -64,6 +64,7 @@ fi
 
 # This helps with debugging when running bats tests but not the whole script is compliant yet
 # set -u
+# set -o pipefail
 
 # Resolve symlinks to this script
 while [ -h "$SOLR_SCRIPT" ] ; do
@@ -848,8 +849,8 @@ function stop_solr() {
         # Check if a process is running with the specified PID.
         # -o stat will output the STAT, where Z indicates a zombie
         # stat='' removes the header (--no-headers isn't supported on all platforms)
-        STAT=`ps -o stat='' $SOLR_PID | tr -d ' '`
-        if [[ "$STAT" != "" && "$STAT" != "Z" ]]; then
+        STAT=`(ps -o stat='' $SOLR_PID || :) | tr -d ' '`
+        if [[ "${STAT:-Z}" != "Z" ]]; then
           slept=$((loops * 2))
           if [ $slept -lt $SOLR_STOP_WAIT ]; then
             sleep 2
@@ -868,9 +869,9 @@ function stop_solr() {
     exit 0
   fi
 
-  STAT=`ps -o stat='' $SOLR_PID | tr -d ' '`
-  if [[ "$STAT" != "" && "$STAT" != "Z" ]]; then
-    if [ "$JSTACK" != "" ]; then
+  STAT=`(ps -o stat='' $SOLR_PID || :) | tr -d ' '`
+  if [[ "${STAT:-Z}" != "Z" ]]; then
+    if [ -n "{$JSTACK:-}" ]; then
       echo -e "Solr process $SOLR_PID is still running; jstacking it now."
       $JSTACK $SOLR_PID
     elif [ "$JATTACH" != "" ]; then
@@ -884,13 +885,13 @@ function stop_solr() {
     sleep 10
   fi
 
-  STAT=`ps -o stat='' $SOLR_PID | tr -d ' '`
-  if [ "$STAT" == "Z" ]; then
+  STAT=`(ps -o stat='' $SOLR_PID || :) | tr -d ' '`
+  if [ "${STAT:-}" == "Z" ]; then
     # This can happen if, for example, you are running Solr inside a docker container with multiple processes
     # rather than running it is as the only service. The --init flag on docker avoids that particular problem.
     echo -e "Solr process $SOLR_PID has terminated abnormally. Solr has exited but a zombie process entry remains."
     exit 1
-  elif [ "$STAT" != "" ]; then
+  elif [ -n "${STAT:-}" ]; then
     echo "ERROR: Failed to kill previous Solr Java process $SOLR_PID ... script fails."
     exit 1
   fi
@@ -1905,12 +1906,12 @@ if [[ "$SCRIPT_CMD" == "start" ]]; then
   # see if Solr is already running
   SOLR_PID=`solr_pid_by_port "$SOLR_PORT"`
 
-  if [ -z "$SOLR_PID" ]; then
+  if [ -z "${SOLR_PID:-}" ]; then
     # not found using the pid file ... but use ps to ensure not found
-    SOLR_PID=`ps auxww | grep start\.jar | grep -w "\-Djetty\.port=$SOLR_PORT" | grep -v grep | awk '{print $2}' | sort -r`
+    SOLR_PID=`ps auxww | grep start\.jar | awk "/\-Djetty\.port=$SOLR_PORT/"' {print $2}' | sort -r`
   fi
 
-  if [ "$SOLR_PID" != "" ]; then
+  if [ -n "${SOLR_PID:-}" ]; then
     echo -e "\nPort $SOLR_PORT is already being used by another process (pid: $SOLR_PID)\nPlease choose a different port using the -p option.\n"
     exit 1
   fi
@@ -1920,7 +1921,7 @@ else
   SOLR_PID=`solr_pid_by_port "$SOLR_PORT"`
   if [ -z "$SOLR_PID" ]; then
     # not found using the pid file ... but use ps to ensure not found
-    SOLR_PID=`ps auxww | grep start\.jar | grep -w "\-Djetty\.port=$SOLR_PORT" | grep -v grep | awk '{print $2}' | sort -r`
+    SOLR_PID=`ps auxww | grep start\.jar | awk "/\-Djetty\.port=$SOLR_PORT/"' {print $2}' | sort -r`
   fi
   if [ "$SOLR_PID" != "" ]; then
     stop_solr "$SOLR_SERVER_DIR" "$SOLR_PORT" "$STOP_KEY" "$SOLR_PID"
@@ -2189,7 +2190,7 @@ function start_solr() {
     echo -e "    JAVA            = $JAVA"
     echo -e "    SOLR_SERVER_DIR = $SOLR_SERVER_DIR"
     echo -e "    SOLR_HOME       = $SOLR_HOME"
-    echo -e "    SOLR_HOST       = $SOLR_HOST"
+    echo -e "    SOLR_HOST       = ${SOLR_HOST:-}"
     echo -e "    SOLR_PORT       = $SOLR_PORT"
     echo -e "    STOP_PORT       = $STOP_PORT"
     echo -e "    JAVA_MEM_OPTS   = ${JAVA_MEM_OPTS[@]}"
@@ -2201,27 +2202,27 @@ function start_solr() {
       echo -e "    CLOUD_MODE_OPTS = ${CLOUD_MODE_OPTS[@]}"
     fi
 
-    if [ "$SOLR_OPTS" != "" ]; then
+    if [ -n "${SOLR_OPTS:-}" ]; then
       echo -e "    SOLR_OPTS       = ${SOLR_OPTS[@]}"
     fi
 
-    if [ "$SOLR_ADDL_ARGS" != "" ]; then
+    if [ -n "${SOLR_ADDL_ARGS:-}" ]; then
       echo -e "    SOLR_ADDL_ARGS  = $SOLR_ADDL_ARGS"
     fi
 
-    if [ "$ENABLE_REMOTE_JMX_OPTS" == "true" ]; then
-      echo -e "    RMI_PORT        = $RMI_PORT"
+    if [ "${ENABLE_REMOTE_JMX_OPTS:-false}" == "true" ]; then
+      echo -e "    RMI_PORT        = ${RMI_PORT:-}"
       echo -e "    REMOTE_JMX_OPTS = ${REMOTE_JMX_OPTS[@]}"
     fi
 
-    if [ "$SOLR_LOG_LEVEL" != "" ]; then
+    if [ -n "${SOLR_LOG_LEVEL:-}" ]; then
       echo -e "    SOLR_LOG_LEVEL  = $SOLR_LOG_LEVEL"
     fi
 
-    if [ "$SOLR_DATA_HOME" != "" ]; then
+    if [ -n "${SOLR_DATA_HOME:-}" ]; then
       echo -e "    SOLR_DATA_HOME  = $SOLR_DATA_HOME"
     fi
-    echo -e "\n"
+    echo
   fi
 
   # need to launch solr from the server dir
@@ -2274,9 +2275,9 @@ function start_solr() {
       (loops=0
       while true
       do
-        running=$(lsof -t -PniTCP:$SOLR_PORT -sTCP:LISTEN)
-        if [ -z "$running" ]; then
-	  slept=$((loops * 2))
+        running=$(lsof -t -PniTCP:$SOLR_PORT -sTCP:LISTEN || :)
+        if [ -z "${running:-}" ]; then
+          slept=$((loops * 2))
           if [ $slept -lt $SOLR_START_WAIT ]; then
             sleep 2
             loops=$[$loops+1]
@@ -2286,7 +2287,7 @@ function start_solr() {
             exit # subshell!
           fi
         else
-          SOLR_PID=`ps auxww | grep start\.jar | grep -w "\-Djetty\.port=$SOLR_PORT" | grep -v grep | awk '{print $2}' | sort -r`
+          SOLR_PID=`ps auxww | grep start\.jar | awk "/\-Djetty\.port=$SOLR_PORT/"' {print $2}' | sort -r`
           echo -e "\nStarted Solr server on port $SOLR_PORT (pid=$SOLR_PID). Happy searching!\n"
           exit # subshell!
         fi
@@ -2295,7 +2296,7 @@ function start_solr() {
     else
       echo -e "NOTE: Please install lsof as this script needs it to determine if Solr is listening on port $SOLR_PORT."
       sleep 10
-      SOLR_PID=`ps auxww | grep start\.jar | grep -w "\-Djetty\.port=$SOLR_PORT" | grep -v grep | awk '{print $2}' | sort -r`
+      SOLR_PID=`ps auxww | grep start\.jar | awk "/\-Djetty\.port=$SOLR_PORT/"' {print $2}' | sort -r`
       echo -e "\nStarted Solr server on port $SOLR_PORT (pid=$SOLR_PID). Happy searching!\n"
       return;
     fi
diff --git a/solr/packaging/build.gradle b/solr/packaging/build.gradle
index 67cdf561556..04279e56f75 100644
--- a/solr/packaging/build.gradle
+++ b/solr/packaging/build.gradle
@@ -218,6 +218,7 @@ class BatsTask extends Exec {
     executable "$project.ext.nodeProjectDir/node_modules/bats/bin/bats"
 
     // Note: tests to run must be listed after all other arguments
+    // Additional debugging output: -x, --verbose-run
     setArgs(['--print-output-on-failure'] + (testFiles.empty ? testDir : testFiles))
 
     super.exec()
diff --git a/solr/packaging/test/README.md b/solr/packaging/test/README.md
index 91fdac223a7..93599e3b3fe 100644
--- a/solr/packaging/test/README.md
+++ b/solr/packaging/test/README.md
@@ -49,6 +49,8 @@ Test are defined as `@test "description of the test" { ... }`
 Some tests will start clusters or create collections,
  please take care to delete any resources that you create.
  They will not be cleaned for you automatically.
+ The example `test_bats.bats` shows how to properly do setup and teardown,
+ along with debug advice and explanations of some subtle traps to avoid.
 
 It is recommended that you install and run `shellcheck` to verify your test scripts and catch common mistakes before committing your changes.
 
diff --git a/solr/packaging/test/bats_helper.bash b/solr/packaging/test/bats_helper.bash
index 88a6cc90c1a..da52b973fc7 100644
--- a/solr/packaging/test/bats_helper.bash
+++ b/solr/packaging/test/bats_helper.bash
@@ -27,6 +27,7 @@ common_setup() {
     load "${BATS_LIB_PREFIX}/bats-assert/load.bash"
 
     PATH="${SOLR_TIP:-.}/bin:$PATH"
+    export SOLR_ULIMIT_CHECKS=false
 }
 
 delete_all_collections() {
diff --git a/solr/packaging/test/test_start_solr.bats b/solr/packaging/test/test_bats.bats
similarity index 61%
copy from solr/packaging/test/test_start_solr.bats
copy to solr/packaging/test/test_bats.bats
index ff0ba356a6d..53c09544c19 100644
--- a/solr/packaging/test/test_start_solr.bats
+++ b/solr/packaging/test/test_bats.bats
@@ -15,19 +15,29 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# This file has stubs for setup and teardown of a cluster and debugging hints
+
 load bats_helper
 
-setup() {
+setup_file() {
+  # set up paths and helpers
   common_setup
+
+  solr start -c -V
+  # echo $output >&3
 }
 
-teardown() {
-  solr stop -all >/dev/null 2>&1
+teardown_file() {
+  # set up paths, not preserved from setup
+  common_setup
+
+  # Conversely, on shutdown, we do need this to execute strictly
+  # because using "run" will eat filing test exit codes
+  solr stop -all
+  # DEBUG : (echo -n "# " ; solr stop -V -all) >&3
 }
 
-@test "SOLR11740 check f" {
-  run -0 solr start
-  run -0 solr start -p 7574
-  run bash -c 'solr stop -all 2>&1'
-  refute_output --partial 'forcefully killing'
+@test "nothing" {
+  # hint: if we need to demonstrate a failing test, change this line to 'false'
+  true
 }
diff --git a/solr/packaging/test/test_create_collection.bats b/solr/packaging/test/test_create_collection.bats
index 7d01ba45f56..8430992af43 100644
--- a/solr/packaging/test/test_create_collection.bats
+++ b/solr/packaging/test/test_create_collection.bats
@@ -19,7 +19,7 @@ load bats_helper
 
 setup_file() {
   common_setup
-  run solr start -c
+  solr start -c
 
   local source_configset_dir="$SOLR_TIP/server/solr/configsets/sample_techproducts_configs"
   test -d $source_configset_dir
@@ -28,7 +28,7 @@ setup_file() {
 
 teardown_file() {
   common_setup
-  run solr stop -all
+  solr stop -all
 }
 
 setup() {
diff --git a/solr/packaging/test/test_delete_collection.bats b/solr/packaging/test/test_delete_collection.bats
index 1951a9812ba..5d3afd9cd84 100644
--- a/solr/packaging/test/test_delete_collection.bats
+++ b/solr/packaging/test/test_delete_collection.bats
@@ -19,12 +19,12 @@ load bats_helper
 
 setup_file() {
   common_setup
-  run solr start -c
+  solr start -c
 }
 
 teardown_file() {
   common_setup
-  run solr stop -all
+  solr stop -all
 }
 
 setup() {
diff --git a/solr/packaging/test/test_start_solr.bats b/solr/packaging/test/test_start_solr.bats
index ff0ba356a6d..048f99fc9e1 100644
--- a/solr/packaging/test/test_start_solr.bats
+++ b/solr/packaging/test/test_start_solr.bats
@@ -26,8 +26,8 @@ teardown() {
 }
 
 @test "SOLR11740 check f" {
-  run -0 solr start
-  run -0 solr start -p 7574
+  solr start
+  solr start -p 7574
   run bash -c 'solr stop -all 2>&1'
   refute_output --partial 'forcefully killing'
 }


[solr] 01/03: SOLR-16097 Batch add docs when starting tests (#744)

Posted by md...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git

commit 912d9157271fdec2b22261ed086eaeea1b4079f5
Author: Mike Drob <md...@apache.org>
AuthorDate: Wed Mar 30 09:40:59 2022 -0500

    SOLR-16097 Batch add docs when starting tests (#744)
    
    Co-authored-by: Christine Poerschke <cp...@apache.org>
---
 .../apache/solr/cloud/DistribCursorPagingTest.java | 21 ++++-----
 .../component/DistributedFacetPivotLargeTest.java  |  4 +-
 .../TestDistributedStatsComponentCardinality.java  | 51 +++++++++++++---------
 .../test/org/apache/solr/update/PeerSyncTest.java  | 32 ++++++--------
 .../solr/update/TestInPlaceUpdatesDistrib.java     |  2 +-
 .../apache/solr/BaseDistributedSearchTestCase.java | 49 ++++++++++++++++++---
 .../cloud/AbstractBasicDistributedZkTestBase.java  |  2 +-
 .../solr/cloud/AbstractFullDistribZkTestBase.java  |  4 +-
 8 files changed, 102 insertions(+), 63 deletions(-)

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 47ec9c34459..b26644009f6 100644
--- a/solr/core/src/test/org/apache/solr/cloud/DistribCursorPagingTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/DistribCursorPagingTest.java
@@ -28,6 +28,7 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
+import java.util.stream.IntStream;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.lucene.util.LuceneTestCase.Slow;
 import org.apache.lucene.util.SentinelIntSet;
@@ -41,7 +42,6 @@ import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.SolrDocumentList;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
-import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.CursorMarkParams;
 import org.apache.solr.common.params.GroupParams;
@@ -581,13 +581,10 @@ public class DistribCursorPagingTest extends AbstractFullDistribZkTestBase {
 
     // start with a smallish number of documents, and test that we can do a full walk using a
     // sort on *every* field in the schema...
-
-    List<SolrInputDocument> initialDocs = new ArrayList<>();
-    for (int i = 1; i <= numInitialDocs; i++) {
-      SolrInputDocument doc = CursorPagingTest.buildRandomDocument(i);
-      initialDocs.add(doc);
-      indexDoc(doc);
-    }
+    indexDocs(
+        IntStream.rangeClosed(1, numInitialDocs)
+            .mapToObj(CursorPagingTest::buildRandomDocument)
+            .iterator());
     commit();
 
     for (String f : allFieldNames) {
@@ -618,10 +615,10 @@ public class DistribCursorPagingTest extends AbstractFullDistribZkTestBase {
     }
 
     // now add a lot more docs, and test a handful of randomized multi-level sorts
-    for (int i = numInitialDocs + 1; i <= totalDocs; i++) {
-      SolrInputDocument doc = CursorPagingTest.buildRandomDocument(i);
-      indexDoc(doc);
-    }
+    indexDocs(
+        IntStream.rangeClosed(numInitialDocs + 1, totalDocs)
+            .mapToObj(CursorPagingTest::buildRandomDocument)
+            .iterator());
     commit();
 
     final int numRandomSorts = atLeast(3);
diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedFacetPivotLargeTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedFacetPivotLargeTest.java
index ad2df43b775..e3110ec1951 100644
--- a/solr/core/src/test/org/apache/solr/handler/component/DistributedFacetPivotLargeTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/component/DistributedFacetPivotLargeTest.java
@@ -1574,13 +1574,13 @@ public class DistributedFacetPivotLargeTest extends BaseDistributedSearchTestCas
    * Builds up a SolrInputDocument using the specified fields, then adds it to the specified client
    * as well as the control client
    *
-   * @see #indexDoc(org.apache.solr.client.solrj.SolrClient,SolrParams,SolrInputDocument...)
+   * @see #indexDoc(SolrClient, SolrParams, SolrInputDocument)
    * @see #sdoc
    */
   private void addPivotDoc(SolrClient client, Object... fields)
       throws IOException, SolrServerException {
 
-    indexDoc(client, params(), sdoc(fields));
+    indexDocs(client, null, List.of(sdoc(fields)));
   }
 
   private int docNumber = 0;
diff --git a/solr/core/src/test/org/apache/solr/handler/component/TestDistributedStatsComponentCardinality.java b/solr/core/src/test/org/apache/solr/handler/component/TestDistributedStatsComponentCardinality.java
index 4262346b20b..b41cfc02633 100644
--- a/solr/core/src/test/org/apache/solr/handler/component/TestDistributedStatsComponentCardinality.java
+++ b/solr/core/src/test/org/apache/solr/handler/component/TestDistributedStatsComponentCardinality.java
@@ -22,14 +22,18 @@ import java.lang.invoke.MethodHandles;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.stream.IntStream;
 import org.apache.lucene.util.LuceneTestCase.Slow;
 import org.apache.lucene.util.TestUtil;
 import org.apache.solr.BaseDistributedSearchTestCase;
 import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
 import org.apache.solr.client.solrj.response.FieldStatsInfo;
 import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.util.LogLevel;
@@ -77,27 +81,32 @@ public class TestDistributedStatsComponentCardinality extends BaseDistributedSea
     log.info("Building an index of {} docs", NUM_DOCS);
 
     // we want a big spread in the long values we use, decrement by BIG_PRIME as we index
-    long longValue = MAX_LONG;
-
-    for (int i = 1; i <= NUM_DOCS; i++) {
-      // with these values, we know that every doc indexed has a unique value in all of the
-      // fields we will compute cardinality against.
-      // which means the number of docs matching a query is the true cardinality for each field
-
-      final String strValue = "s" + longValue;
-      indexDoc(
-          sdoc(
-              "id", "" + i,
-              "int_i", "" + i,
-              "int_i_prehashed_l", "" + HASHER.hashInt(i).asLong(),
-              "long_l", "" + longValue,
-              "long_l_prehashed_l", "" + HASHER.hashLong(longValue).asLong(),
-              "string_s", strValue,
-              "string_s_prehashed_l",
-                  "" + HASHER.hashString(strValue, StandardCharsets.UTF_8).asLong()));
-
-      longValue -= BIG_PRIME;
-    }
+    final AtomicLong longValue = new AtomicLong(MAX_LONG);
+
+    Iterator<SolrInputDocument> docs =
+        IntStream.rangeClosed(1, NUM_DOCS)
+            .mapToObj(
+                i -> {
+                  long currentLong = longValue.getAndAccumulate(BIG_PRIME, (x, y) -> x - y);
+
+                  final String strValue = "s" + currentLong;
+
+                  // with these values, we know that every doc indexed has a unique value in all of
+                  // the fields we will compute cardinality against. which means the number of docs
+                  // matching a query is the true cardinality for each field
+                  return sdoc(
+                      "id", Integer.toString(i),
+                      "int_i", Integer.toString(i),
+                      "int_i_prehashed_l", Long.toString(HASHER.hashInt(i).asLong()),
+                      "long_l", Long.toString(currentLong),
+                      "long_l_prehashed_l", Long.toString(HASHER.hashLong(currentLong).asLong()),
+                      "string_s", strValue,
+                      "string_s_prehashed_l",
+                          Long.toString(
+                              HASHER.hashString(strValue, StandardCharsets.UTF_8).asLong()));
+                })
+            .iterator();
+    indexDocs(docs);
 
     commit();
   }
diff --git a/solr/core/src/test/org/apache/solr/update/PeerSyncTest.java b/solr/core/src/test/org/apache/solr/update/PeerSyncTest.java
index 3e5e03faada..ba3a7ba58bc 100644
--- a/solr/core/src/test/org/apache/solr/update/PeerSyncTest.java
+++ b/solr/core/src/test/org/apache/solr/update/PeerSyncTest.java
@@ -20,6 +20,7 @@ import static org.apache.solr.update.processor.DistributingUpdateProcessorFactor
 import static org.hamcrest.core.StringContains.containsString;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashSet;
@@ -87,7 +88,7 @@ public class PeerSyncTest extends BaseDistributedSearchTestCase {
     SolrClient client1 = clients.get(1);
     SolrClient client2 = clients.get(2);
 
-    long v = 0;
+    int v = 0;
     add(client0, seenLeader, sdoc("id", "1", "_version_", ++v));
 
     // this fails because client0 has no context (i.e. no updates of its own to judge if applying
@@ -213,26 +214,23 @@ public class PeerSyncTest extends BaseDistributedSearchTestCase {
 
     // now lets check fingerprinting causes appropriate fails
     v = 4000;
-    add(client0, seenLeader, sdoc("id", Integer.toString((int) v), "_version_", v));
+    add(client0, seenLeader, sdoc("id", Integer.toString(v), "_version_", v));
     docsAdded.add(4000);
     int toAdd = numVersions + 10;
-    for (int i = 0; i < toAdd; i++) {
-      add(
-          client0,
-          seenLeader,
-          sdoc("id", Integer.toString((int) v + i + 1), "_version_", v + i + 1));
-      add(
-          client1,
-          seenLeader,
-          sdoc("id", Integer.toString((int) v + i + 1), "_version_", v + i + 1));
-      docsAdded.add((int) v + i + 1);
+
+    List<SolrInputDocument> additionalDocs = new ArrayList<>(toAdd);
+    for (int i = v + 1; i < v + toAdd + 1; i++) {
+      additionalDocs.add(sdoc("id", Integer.toString(i), "_version_", i));
+      docsAdded.add(i);
     }
+    add(client0, seenLeader, additionalDocs);
+    add(client1, seenLeader, additionalDocs);
 
     // client0 now has an additional add beyond our window and the fingerprint should cause this to
     // fail
     assertSync(client1, numVersions, false, shardsArr[0]);
 
-    // if we turn of fingerprinting, it should succeed
+    // if we turn off fingerprinting, it should succeed
     System.setProperty("solr.disableFingerprint", "true");
     try {
       assertSync(client1, numVersions, true, shardsArr[0]);
@@ -241,15 +239,13 @@ public class PeerSyncTest extends BaseDistributedSearchTestCase {
     }
 
     // lets add the missing document and verify that order doesn't matter
-    add(client1, seenLeader, sdoc("id", Integer.toString((int) v), "_version_", v));
+    add(client1, seenLeader, sdoc("id", Integer.toString(v), "_version_", v));
     assertSync(client1, numVersions, true, shardsArr[0]);
 
     // lets do some overwrites to ensure that repeated updates and maxDoc don't matter
     for (int i = 0; i < 10; i++) {
-      add(
-          client0,
-          seenLeader,
-          sdoc("id", Integer.toString((int) v + i + 1), "_version_", v + i + 1));
+      // add individually instead of in batch to create more writes
+      add(client0, seenLeader, sdoc("id", Integer.toString(v + i + 1), "_version_", v + i + 1));
     }
     assertSync(client1, numVersions, true, shardsArr[0]);
 
diff --git a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
index 4dd45e6eaa6..e7b3408cad6 100644
--- a/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
+++ b/solr/core/src/test/org/apache/solr/update/TestInPlaceUpdatesDistrib.java
@@ -614,7 +614,7 @@ public class TestInPlaceUpdatesDistrib extends AbstractFullDistribZkTestBase {
     nonexistentDocUpdate.setField("id", 2);
     nonexistentDocUpdate.setField("inplace_updatable_float", map("set", "50"));
 
-    SolrInputDocument docs[] = new SolrInputDocument[] {existingDocUpdate, nonexistentDocUpdate};
+    List<SolrInputDocument> docs = List.of(existingDocUpdate, nonexistentDocUpdate);
 
     SolrClient solrClient = clients.get(random().nextInt(clients.size()));
     add(solrClient, null, docs);
diff --git a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java b/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
index 24409fe114b..08bc1a2ef8d 100644
--- a/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
+++ b/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
@@ -31,6 +31,7 @@ import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -569,23 +570,49 @@ public abstract class BaseDistributedSearchTestCase extends SolrTestCaseJ4 {
     indexDoc(doc);
   }
 
+  private SolrClient clientFor(SolrInputDocument doc) {
+    return clients.get((doc.getField(id).toString().hashCode() & 0x7fffffff) % clients.size());
+  }
+
   /** Indexes the document in both the control client, and a randomly selected client */
   protected void indexDoc(SolrInputDocument doc) throws IOException, SolrServerException {
+    indexDoc(clientFor(doc), null, doc);
+  }
+
+  protected void indexDoc(SolrClient client, SolrParams params, SolrInputDocument doc)
+      throws IOException, SolrServerException {
     controlClient.add(doc);
     if (shardCount == 0) { // mostly for temp debugging
       return;
     }
-    int which = (doc.getField(id).toString().hashCode() & 0x7fffffff) % clients.size();
-    SolrClient client = clients.get(which);
     client.add(doc);
   }
 
+  /**
+   * Indexes the stream of documents in both the control client and randomly selected clients (per
+   * batch)
+   */
+  public void indexDocs(Iterator<SolrInputDocument> docs) throws SolrServerException, IOException {
+    int batchSize = 100; // TODO make configurable?
+    List<SolrInputDocument> batch = new ArrayList<>(batchSize);
+    while (docs.hasNext()) {
+      batch.add(docs.next());
+      if (batch.size() == batchSize) {
+        indexDocs(clientFor(batch.get(0)), null, batch);
+        batch.clear();
+      }
+    }
+    if (!batch.isEmpty()) {
+      indexDocs(clientFor(batch.get(0)), null, batch);
+    }
+  }
+
   /**
    * Indexes the document in both the control client and the specified client asserting that the
    * responses are equivalent
    */
-  protected UpdateResponse indexDoc(
-      SolrClient client, SolrParams params, SolrInputDocument... sdocs)
+  protected UpdateResponse indexDocs(
+      SolrClient client, SolrParams params, Iterable<SolrInputDocument> sdocs)
       throws IOException, SolrServerException {
     UpdateResponse controlRsp = add(controlClient, params, sdocs);
     UpdateResponse specificRsp = add(client, params, sdocs);
@@ -593,7 +620,19 @@ public abstract class BaseDistributedSearchTestCase extends SolrTestCaseJ4 {
     return specificRsp;
   }
 
-  protected UpdateResponse add(SolrClient client, SolrParams params, SolrInputDocument... sdocs)
+  /**
+   * This should not be called in a loop, use {@link #add(SolrClient, SolrParams, Iterable)} instead
+   */
+  protected UpdateResponse add(SolrClient client, SolrParams params, SolrInputDocument sdoc)
+      throws IOException, SolrServerException {
+    UpdateRequest ureq = new UpdateRequest();
+    ureq.setParams(new ModifiableSolrParams(params));
+    ureq.add(sdoc);
+    return ureq.process(client);
+  }
+
+  protected UpdateResponse add(
+      SolrClient client, SolrParams params, Iterable<SolrInputDocument> sdocs)
       throws IOException, SolrServerException {
     UpdateRequest ureq = new UpdateRequest();
     ureq.setParams(new ModifiableSolrParams(params));
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractBasicDistributedZkTestBase.java b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractBasicDistributedZkTestBase.java
index 3f517ba79ac..281cb9a888c 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractBasicDistributedZkTestBase.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractBasicDistributedZkTestBase.java
@@ -710,7 +710,7 @@ public abstract class AbstractBasicDistributedZkTestBase extends AbstractFullDis
 
     ModifiableSolrParams params = new ModifiableSolrParams();
     params.set("commitWithin", 10);
-    add(cloudClient, params, getDoc("id", 300), getDoc("id", 301));
+    add(cloudClient, params, List.of(getDoc("id", 300), getDoc("id", 301)));
 
     newSearcherHook.waitForSearcher(DEFAULT_COLLECTION, 2, 20000, false);
 
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java
index b138b902c67..b607e063da9 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java
@@ -2226,9 +2226,7 @@ public abstract class AbstractFullDistribZkTestBase extends AbstractDistribZkTes
   }
 
   public static SolrInputDocument getDoc(Object... fields) throws Exception {
-    SolrInputDocument doc = new SolrInputDocument();
-    addFields(doc, fields);
-    return doc;
+    return sdoc(fields);
   }
 
   private String checkCollectionExpectations(


[solr] 02/03: SOLR-16127 Ordered Executor orphans locks (#779)

Posted by md...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git

commit c347fd6b5c594cc4d996a4435d8057e7fd8c66ce
Author: Mike Drob <md...@apache.org>
AuthorDate: Fri Apr 1 15:23:32 2022 -0500

    SOLR-16127 Ordered Executor orphans locks (#779)
    
    (cherry picked from commit 18a837296edee768bc9b8506af2efbf0f9a03b21)
---
 .../java/org/apache/solr/util/OrderedExecutor.java |  9 +++-
 .../org/apache/solr/util/OrderedExecutorTest.java  | 51 ++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/solr/core/src/java/org/apache/solr/util/OrderedExecutor.java b/solr/core/src/java/org/apache/solr/util/OrderedExecutor.java
index 15c1072e729..923560a8ac6 100644
--- a/solr/core/src/java/org/apache/solr/util/OrderedExecutor.java
+++ b/solr/core/src/java/org/apache/solr/util/OrderedExecutor.java
@@ -103,7 +103,14 @@ public class OrderedExecutor implements Executor {
         // myLock was successfully inserted
       }
       // won the lock
-      sizeSemaphore.acquire();
+      try {
+        sizeSemaphore.acquire();
+      } catch (InterruptedException e) {
+        if (t != null) {
+          map.remove(t).countDown();
+        }
+        throw e;
+      }
     }
 
     public void remove(T t) {
diff --git a/solr/core/src/test/org/apache/solr/util/OrderedExecutorTest.java b/solr/core/src/test/org/apache/solr/util/OrderedExecutorTest.java
index 24bb7c3f3a2..01f79978d43 100644
--- a/solr/core/src/test/org/apache/solr/util/OrderedExecutorTest.java
+++ b/solr/core/src/test/org/apache/solr/util/OrderedExecutorTest.java
@@ -230,4 +230,55 @@ public class OrderedExecutorTest extends SolrTestCase {
   private static class IntBox {
     int value;
   }
+
+  @Test
+  public void testMaxSize() throws InterruptedException {
+    OrderedExecutor orderedExecutor =
+        new OrderedExecutor(1, ExecutorUtil.newMDCAwareCachedThreadPool("single"));
+
+    CountDownLatch isRunning = new CountDownLatch(1);
+    CountDownLatch blockingLatch = new CountDownLatch(1);
+
+    try {
+      orderedExecutor.execute(
+          () -> {
+            // This will aquire and hold the single max size semaphore permit
+            try {
+              isRunning.countDown();
+              blockingLatch.await();
+            } catch (InterruptedException e) {
+              e.printStackTrace();
+            }
+          });
+
+      isRunning.await(2, TimeUnit.SECONDS);
+
+      // Add another task in a background thread so that we can interrupt it
+      // This _should_ be blocked on the first task because there is only one execution slot
+      CountDownLatch taskTwoFinished = new CountDownLatch(1);
+      Thread t = new Thread(() -> orderedExecutor.execute(2, taskTwoFinished::countDown));
+      t.start();
+      // Interrupt the thread now, but it won't throw until it calls acquire()
+      t.interrupt();
+      // It should complete gracefully from here
+      t.join();
+
+      // Release the first thread
+      assertFalse("Did not expect task #2 to complete", taskTwoFinished.await(0, TimeUnit.SECONDS));
+      blockingLatch.countDown();
+
+      // Tasks without a lock can safely execute again
+      orderedExecutor.execute(() -> {});
+
+      // New threads for lock #2 should be able to execute as well
+      t = new Thread(() -> orderedExecutor.execute(2, () -> {}));
+      t.start();
+
+      // This will also get caught by ThreadLeakControl if it fails
+      t.join(TimeUnit.SECONDS.toMillis(2));
+      assertFalse("Task should have completed", t.isAlive());
+    } finally {
+      orderedExecutor.shutdownAndAwaitTermination();
+    }
+  }
 }