solr
solr copied to clipboard
SOLR-6962: bin/solr stop -a should complain about missing parameter
https://issues.apache.org/jira/browse/SOLR-6962
Description
make sure if there is a -a, there is a following parameter.
There IS an argument that maybe the -a should be used instead as a short cut for --all, as that is more likely to be typed. However we can decide that in a separate ticket.
Solution
add check
Tests
manually
Checklist
Please review the following and check all that apply:
- [ ] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
- [ ] I have developed this patch against the
mainbranch. - [ ] I have run
./gradlew check. - [ ] I have added tests for my changes.
- [ ] I have added documentation for the Reference Guide
@rahulgoswami I would love your review of the windows changes...
@rahulgoswami I would love your review of the windows changes...
Reviewing this now...
I tried reproducing the issue. Built the main branch and did "solr start -e techproducts". Thereafter tried "solr stop -a" but I get a nice message shown below instead of the hanging behavior the Jira talks about.
I see the branch is still unmerged. What am I missing ? How can I reproduce this ?
solr stop -a
ERROR: Must specify the port when trying to stop Solr, or use --all to stop all running nodes on this host.
Probably my most used feature with -a option is passing the jdwp parameters for remote debugging
solr start -p 8983 -s "C:\Work\Git\ApacheSolr\solr\solr\packaging\build\dev\example\techproducts\solr" -a "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=18983"
Tried this without providing any value for -a , but that doesn't hang either (and runs fine in fact). Could this be a Linux specific issue?!
This could be a linux specific issue. The stop command is written all in script (not in java code), so the implmentation on linux is parallel to windows.
Could you also try this branch? I wonder if the edits I made to bin/solr.cmd are actually not needed?
I tried main as well as the tag for the latest 9.x release "releases/solr/9.7.0" on Windows. I am observing the same behavior in both cases where "solr stop -a" doesn't hang and gives a meaningful message. Also tried this branch and it seems fine, although with a different message as per the changes made in this PR. I think the solr.cmd changes are not required.
thanks @rahulgoswami I am going to back out the solr.cmd changes!
Okaym, I dug more into this, and I think I see another problem! It turns out that we parse the args for the start/stop/restart commands separate from actually invoking the command.
This means you don't get an error message for passing a start arg, like --jvm-opts to a stop commmand. That's annoying.. and a reason to move these commands to Java code and use commons-cli more!
Okay, I just did some testing of bin/solr and this is what I get (and expect):
dev git:(SOLR-6962) ✗ bin/solr stop -a
ERROR: Jvm options are required when using the -a option!
Usage: solr stop [-k key] [-p port] [-V]
and for fun:
➜ dev git:(SOLR-6962) bin/solr stop --jvm-opts
ERROR: Jvm options are required when using the --jvm-opts option!
Usage: solr stop [-k key] [-p port] [-V]
@rahulgoswami can you take one more looksee at the change I made to bin/solr.cmd and we can call this done! Thanks for your help!
Opened PR https://github.com/epugh/solr/pull/10 against this branch.
currently below usage of -a in solr start doesn't raise any alert and starts solr successfully:
solr start -a -p 8983 -s "C:\Work\Git\ApacheSolr\solr\solr\packaging\build\dev\example\techproducts\solr"
PR will make the behaviour consistent with Linux. Now displays "ERROR: JVM options are required when using the -a or --jvm-opts option"
Also I have a different opinion on solr stop behavior. In the new implementation on Linux, solr stop -a would say "Jvm options are required when using the -a option!" . But truly what the user's intent is to stop all instances .
On Windows, the user gets a message "ERROR: Must specify the port when trying to stop Solr, or use --all to stop all running nodes on this host." which imo guides them better than the Linux implementation. For this reason I have retained this Windows message in my PR. Thoughts?
Happy to make the change for Linux too, but would need your help for validation. :)