You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ep...@apache.org on 2024/09/28 19:50:39 UTC
(solr) branch branch_9_7 updated: Fix bin/solr script for bugs in -p, -V, and healthcheck tool
This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch branch_9_7
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9_7 by this push:
new 21a6b4c5bf3 Fix bin/solr script for bugs in -p, -V, and healthcheck tool
21a6b4c5bf3 is described below
commit 21a6b4c5bf30b414a3bab5fa8ac84f7e78a08725
Author: Eric Pugh <ep...@opensourceconnections.com>
AuthorDate: Sat Sep 28 15:50:33 2024 -0400
Fix bin/solr script for bugs in -p, -V, and healthcheck tool
* Re-introduce support for -p and -port
* Now supporting the deprecated -V (versus -v)
* fix a bug in HealthcheckTool itself and how we pass parameters in
---------
Co-authored-by: Christos Malliaridis <c....@gmail.com>
---
solr/bin/solr | 2 +-
solr/bin/solr.cmd | 2 +-
.../src/java/org/apache/solr/cli/CreateTool.java | 55 +++++++++++++++-------
.../src/java/org/apache/solr/cli/DeleteTool.java | 42 +++++++++++++----
.../java/org/apache/solr/cli/HealthcheckTool.java | 14 +++++-
.../core/src/java/org/apache/solr/cli/SolrCLI.java | 18 ++++++-
.../src/java/org/apache/solr/cli/ToolBase.java | 11 ++++-
solr/packaging/test/test_create.bats | 23 +++++++++
solr/packaging/test/test_delete_collection.bats | 8 ++++
solr/packaging/test/test_healthcheck.bats | 5 +-
solr/packaging/test/test_zk.bats | 2 +-
11 files changed, 146 insertions(+), 36 deletions(-)
diff --git a/solr/bin/solr b/solr/bin/solr
index bf23db848ad..b04a68eb3b0 100755
--- a/solr/bin/solr
+++ b/solr/bin/solr
@@ -946,7 +946,7 @@ if [ "$SCRIPT_CMD" == "healthcheck" ]; then
exit 1
fi
- run_tool healthcheck -zkHost "$ZK_HOST" -collection "$HEALTHCHECK_COLLECTION" $VERBOSE
+ run_tool healthcheck -z "$ZK_HOST" -c "$HEALTHCHECK_COLLECTION" $VERBOSE
exit $?
fi
diff --git a/solr/bin/solr.cmd b/solr/bin/solr.cmd
index 4294f1cc017..081489d7e4c 100755
--- a/solr/bin/solr.cmd
+++ b/solr/bin/solr.cmd
@@ -1548,7 +1548,7 @@ echo ZK_HOST: !ZK_HOST!
"%JAVA%" %SOLR_SSL_OPTS% %AUTHC_OPTS% %SOLR_ZK_CREDS_AND_ACLS% -Dsolr.install.dir="%SOLR_TIP%" ^
-Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^
-classpath "%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^
- org.apache.solr.cli.SolrCLI healthcheck -collection !HEALTHCHECK_COLLECTION! -zkHost !ZK_HOST! %HEALTHCHECK_VERBOSE%
+ org.apache.solr.cli.SolrCLI healthcheck -c !HEALTHCHECK_COLLECTION! -z !ZK_HOST! %HEALTHCHECK_VERBOSE%
goto done
:run_solrcli
diff --git a/solr/core/src/java/org/apache/solr/cli/CreateTool.java b/solr/core/src/java/org/apache/solr/cli/CreateTool.java
index 6f869e990e5..1f67bb02b83 100644
--- a/solr/core/src/java/org/apache/solr/cli/CreateTool.java
+++ b/solr/core/src/java/org/apache/solr/cli/CreateTool.java
@@ -45,6 +45,7 @@ import org.apache.solr.cloud.ZkConfigSetService;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CollectionAdminParams;
import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.EnvUtils;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.core.ConfigSetService;
import org.noggit.CharArr;
@@ -148,16 +149,36 @@ public class CreateTool extends ToolBase {
.required(false)
.desc("Configuration name; default is the collection name.")
.build(),
+ Option.builder("p")
+ .longOpt("port")
+ .deprecated(
+ DeprecatedAttributes.builder()
+ .setForRemoval(true)
+ .setSince("9.7")
+ .setDescription("Use --solr-url instead")
+ .get())
+ .argName("PORT")
+ .hasArg()
+ .required(false)
+ .desc("Port of a local Solr instance where you want to create the new core.")
+ .build(),
SolrCLI.OPTION_SOLRURL,
SolrCLI.OPTION_SOLRURL_DEPRECATED,
SolrCLI.OPTION_ZKHOST,
- SolrCLI.OPTION_ZKHOST_DEPRECATED);
+ SolrCLI.OPTION_ZKHOST_DEPRECATED,
+ SolrCLI.OPTION_VERBOSE_DEPRECATED);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
SolrCLI.raiseLogLevelUnlessVerbose(cli);
+ // If a port is provided, inject it into the environment variables so
+ // that {@link SolrCLI#getDefaultSolrUrl()} can use it instead of the default 8983.
+ if (cli.hasOption("p")) {
+ EnvUtils.setProperty("jetty.port", cli.getOptionValue("p"));
+ }
+
try (var solrClient = SolrCLI.getSolrClient(cli)) {
if (SolrCLI.isCloudMode(solrClient)) {
createCollection(cli);
@@ -224,12 +245,12 @@ public class CreateTool extends ToolBase {
try {
CoreAdminResponse res = CoreAdminRequest.createCore(coreName, coreName, solrClient);
- if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+ if (isVerbose()) {
echo(res.jsonStr());
echo("\n");
- } else {
- echo(String.format(Locale.ROOT, "\nCreated new core '%s'", coreName));
}
+ echo(String.format(Locale.ROOT, "\nCreated new core '%s'", coreName));
+
} catch (Exception e) {
/* create-core failed, cleanup the copied configset before propagating the error. */
PathUtils.deleteDirectory(coreInstanceDir);
@@ -349,25 +370,25 @@ public class CreateTool extends ToolBase {
"Failed to create collection '" + collectionName + "' due to: " + sse.getMessage());
}
- if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+ if (isVerbose()) {
// pretty-print the response to stdout
CharArr arr = new CharArr();
new JSONWriter(arr, 2).write(response.asMap());
echo(arr.toString());
- } else {
- String endMessage =
- String.format(
- Locale.ROOT,
- "Created collection '%s' with %d shard(s), %d replica(s)",
- collectionName,
- numShards,
- replicationFactor);
- if (confName != null && !confName.trim().isEmpty()) {
- endMessage += String.format(Locale.ROOT, " with config-set '%s'", confName);
- }
+ }
- echo(endMessage);
+ String endMessage =
+ String.format(
+ Locale.ROOT,
+ "Created collection '%s' with %d shard(s), %d replica(s)",
+ collectionName,
+ numShards,
+ replicationFactor);
+ if (confName != null && !confName.trim().isEmpty()) {
+ endMessage += String.format(Locale.ROOT, " with config-set '%s'", confName);
}
+
+ echo(endMessage);
}
private Path getFullConfDir(Path solrInstallDir, Path confDirName) {
diff --git a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java
index d5bf4234858..32e6f6d7c16 100644
--- a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java
+++ b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java
@@ -22,6 +22,7 @@ import static org.apache.solr.common.params.CommonParams.SYSTEM_INFO_PATH;
import java.io.PrintStream;
import java.lang.invoke.MethodHandles;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -40,6 +41,7 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.CoreAdminRequest;
import org.apache.solr.client.solrj.request.GenericSolrRequest;
import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.EnvUtils;
import org.apache.solr.common.util.NamedList;
import org.noggit.CharArr;
import org.noggit.JSONWriter;
@@ -122,16 +124,36 @@ public class DeleteTool extends ToolBase {
.desc(
"Skip safety checks when deleting the configuration directory used by a collection.")
.build(),
+ Option.builder("p")
+ .longOpt("port")
+ .deprecated(
+ DeprecatedAttributes.builder()
+ .setForRemoval(true)
+ .setSince("9.7")
+ .setDescription("Use --solr-url instead")
+ .get())
+ .argName("PORT")
+ .hasArg()
+ .required(false)
+ .desc("Port of a local Solr instance where you want to create the new core.")
+ .build(),
SolrCLI.OPTION_SOLRURL,
SolrCLI.OPTION_SOLRURL_DEPRECATED,
SolrCLI.OPTION_ZKHOST,
- SolrCLI.OPTION_ZKHOST_DEPRECATED);
+ SolrCLI.OPTION_ZKHOST_DEPRECATED,
+ SolrCLI.OPTION_VERBOSE_DEPRECATED);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
SolrCLI.raiseLogLevelUnlessVerbose(cli);
+ // If a port is provided, inject it into the environment variables so
+ // that {@link SolrCLI#getDefaultSolrUrl()} can use it instead of the default 8983.
+ if (cli.hasOption("p")) {
+ EnvUtils.setProperty("jetty.port", cli.getOptionValue("p"));
+ }
+
try (var solrClient = SolrCLI.getSolrClient(cli)) {
Map<String, Object> systemInfo =
solrClient
@@ -248,21 +270,22 @@ public class DeleteTool extends ToolBase {
}
}
- if (response != null) {
- // pretty-print the response to stdout
- CharArr arr = new CharArr();
- new JSONWriter(arr, 2).write(response.asMap());
- echo(arr.toString());
- echo("\n");
+ if (isVerbose()) {
+ if (response != null) {
+ // pretty-print the response to stdout
+ CharArr arr = new CharArr();
+ new JSONWriter(arr, 2).write(response.asMap());
+ echo(arr.toString());
+ }
}
- echo("Deleted collection '" + collectionName + "' using CollectionAdminRequest");
+ echo(String.format(Locale.ROOT, "\nDeleted collection '%s'", collectionName));
}
protected void deleteCore(CommandLine cli, SolrClient solrClient) throws Exception {
String coreName = cli.getOptionValue(NAME);
- echo("\nDeleting core '" + coreName + "' using CoreAdminRequest\n");
+ echoIfVerbose("\nDeleting core '" + coreName + "'", cli);
NamedList<Object> response;
try {
@@ -281,5 +304,6 @@ public class DeleteTool extends ToolBase {
echoIfVerbose((String) response.get("response"), cli);
echoIfVerbose("\n", cli);
}
+ echo(String.format(Locale.ROOT, "\nDeleted core '%s'", coreName));
}
}
diff --git a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java
index 94cc006ff8d..62ad942cbf5 100644
--- a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java
+++ b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java
@@ -28,12 +28,14 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.TimeUnit;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Option;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.client.solrj.request.GenericSolrRequest;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.cloud.ClusterState;
@@ -92,8 +94,8 @@ public class HealthcheckTool extends ToolBase {
CLIO.err("Healthcheck tool only works in Solr Cloud mode.");
System.exit(1);
}
+ echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
try (CloudHttp2SolrClient cloudSolrClient = SolrCLI.getCloudHttp2SolrClient(zkHost)) {
- echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
cloudSolrClient.connect();
runCloudTool(cloudSolrClient, cli);
}
@@ -170,7 +172,15 @@ public class HealthcheckTool extends ToolBase {
q = new SolrQuery("*:*");
q.setRows(0);
q.set(DISTRIB, "false");
- try (var solrClientForCollection = SolrCLI.getSolrClient(coreUrl)) {
+
+ // SolrCLI.getSolrClient converts the coreUrl back into a root Solr url, but we need to
+ // talk to specific core so manually creating the client.
+ // try (var solrClientForCollection = SolrCLI.getSolrClient(coreUrl)) {
+ try (var solrClientForCollection =
+ new Http2SolrClient.Builder(coreUrl)
+ .withMaxConnectionsPerHost(32)
+ .withKeyStoreReloadInterval(-1, TimeUnit.SECONDS)
+ .build()) {
qr = solrClientForCollection.query(q);
numDocs = qr.getResults().getNumFound();
try (var solrClient = SolrCLI.getSolrClient(replicaCoreProps.getBaseUrl())) {
diff --git a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
index 2dc25b03556..91e66d409eb 100755
--- a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
+++ b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
@@ -150,6 +150,17 @@ public class SolrCLI implements CLIO {
.required(false)
.desc("Enable verbose command output.")
.build();
+ public static final Option OPTION_VERBOSE_DEPRECATED =
+ Option.builder("V")
+ .deprecated(
+ DeprecatedAttributes.builder()
+ .setForRemoval(true)
+ .setSince("9.7")
+ .setDescription("Use -v instead")
+ .get())
+ .required(false)
+ .desc("Enable verbose command output.")
+ .build();
public static final Option OPTION_HELP =
Option.builder("h").longOpt("help").required(false).desc("Print this message.").build();
@@ -294,7 +305,8 @@ public class SolrCLI implements CLIO {
}
public static void raiseLogLevelUnlessVerbose(CommandLine cli) {
- if (!cli.hasOption(OPTION_VERBOSE.getOpt())) {
+ if (!(cli.hasOption(OPTION_VERBOSE.getOpt())
+ || cli.hasOption(OPTION_VERBOSE_DEPRECATED.getOpt()))) {
StartupLoggingUtils.changeLogLevel("WARN");
}
}
@@ -519,6 +531,10 @@ public class SolrCLI implements CLIO {
* <p>Note that from Solr 10.0 Solr will use / as base URL to support /api and other contexts, so
* handling of hostContext differs between Solr 9.x and 10.x.
*
+ * <p>Note that this should not be used with individual core specific urls such as
+ * "http://127.0.0.1:51428/solr/bob_shard1_replica_n1/" as it removes the core name by converting
+ * the url to "http://127.0.0.1:51428/" before creating the client.
+ *
* @param solrUrl the base URL of Solr
* @return a SolrClient initialized with correct hostContext, default '/solr'
*/
diff --git a/solr/core/src/java/org/apache/solr/cli/ToolBase.java b/solr/core/src/java/org/apache/solr/cli/ToolBase.java
index 43a42f77d7b..a668ed22b5e 100644
--- a/solr/core/src/java/org/apache/solr/cli/ToolBase.java
+++ b/solr/core/src/java/org/apache/solr/cli/ToolBase.java
@@ -34,18 +34,25 @@ public abstract class ToolBase implements Tool {
}
protected void echoIfVerbose(final String msg, CommandLine cli) {
- if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+ if (verbose) {
echo(msg);
}
}
+ /** Is this tool being run in a verbose mode? */
+ protected boolean isVerbose() {
+ return verbose;
+ }
+
protected void echo(final String msg) {
stdout.println(msg);
}
@Override
public int runTool(CommandLine cli) throws Exception {
- verbose = cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt());
+ verbose =
+ cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())
+ || cli.hasOption(SolrCLI.OPTION_VERBOSE_DEPRECATED);
int toolExitStatus = 0;
try {
diff --git a/solr/packaging/test/test_create.bats b/solr/packaging/test/test_create.bats
index e59a9055852..2fed6fb6a73 100644
--- a/solr/packaging/test/test_create.bats
+++ b/solr/packaging/test/test_create.bats
@@ -34,8 +34,31 @@ teardown() {
assert_output --partial "Created new core 'COLL_NAME'"
}
+@test "create_core works" {
+ run solr start
+ run solr create_core -c COLL_NAME
+ assert_output --partial "Created new core 'COLL_NAME'"
+}
+
@test "create for cloud mode" {
run solr start -c
run solr create -c COLL_NAME
assert_output --partial "Created collection 'COLL_NAME'"
}
+
+@test "ensure -p port parameter is supported" {
+ solr start -p ${SOLR2_PORT}
+ solr assert --not-started http://localhost:${SOLR_PORT} --timeout 5000
+ solr assert --started http://localhost:${SOLR2_PORT} --timeout 5000
+
+ run solr create -c COLL_NAME -p ${SOLR2_PORT}
+ assert_output --partial "Created new core 'COLL_NAME'"
+}
+
+@test "ensure -V verbose parameter is supported" {
+ solr start
+ solr assert --started http://localhost:${SOLR_PORT} --timeout 5000
+
+ run solr create -c COLL_NAME -V
+ assert_output --partial "Creating new core 'COLL_NAME' using CoreAdminRequest"
+}
diff --git a/solr/packaging/test/test_delete_collection.bats b/solr/packaging/test/test_delete_collection.bats
index 6fec21283e6..68be61ce9eb 100644
--- a/solr/packaging/test/test_delete_collection.bats
+++ b/solr/packaging/test/test_delete_collection.bats
@@ -77,3 +77,11 @@ teardown() {
solr delete -c "COLL_NAME" --delete-config false
assert config_exists "COLL_NAME"
}
+
+@test "ensure -p port parameter is supported" {
+ run solr create -c COLL_NAME -p ${SOLR_PORT}
+ assert_output --partial "Created collection 'COLL_NAME'"
+
+ run solr delete -c COLL_NAME --delete-config false -p ${SOLR_PORT} -V
+ assert_output --partial "Deleted collection 'COLL_NAME'"
+}
diff --git a/solr/packaging/test/test_healthcheck.bats b/solr/packaging/test/test_healthcheck.bats
index 9a0768b284e..0aa6db6fb1f 100644
--- a/solr/packaging/test/test_healthcheck.bats
+++ b/solr/packaging/test/test_healthcheck.bats
@@ -29,8 +29,9 @@ teardown() {
}
@test "healthcheck on cloud solr" {
- solr start -c -e films
- run solr healthcheck -c films
+ solr start -c -e films -p ${SOLR_PORT}
+ run solr healthcheck -c films -zkhost localhost:${ZK_PORT} -V
refute_output --partial 'error'
+ assert_output --partial '"collection":"films"'
}
diff --git a/solr/packaging/test/test_zk.bats b/solr/packaging/test/test_zk.bats
index be7e2c25d1d..cccddeabf5c 100644
--- a/solr/packaging/test/test_zk.bats
+++ b/solr/packaging/test/test_zk.bats
@@ -89,7 +89,7 @@ teardown() {
touch myfile2.txt
run solr zk cp myfile2.txt zk:myfile2.txt -z localhost:${ZK_PORT}
sleep 1
- run solr zk ls / -z localhost:${ZK_PORT}
+ run solr zk ls / -z localhost:${ZK_PORT} -V
assert_output --partial "myfile2.txt"
touch myfile3.txt