You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2023/01/12 22:24:38 UTC
[GitHub] [solr] ThePrez opened a new pull request, #1287: SOLR-16619: Fix `solr` script for IBM i
ThePrez opened a new pull request, #1287:
URL: https://github.com/apache/solr/pull/1287
https://issues.apache.org/jira/browse/SOLR-16619
# Description
The `solr` script runs an invocation of `ps` that is incompatible with the `ps` that ships on IBM i
# Solution
When running on IBM i, a simplified invocation is needed, namely `ps -f -p "$SOLR_PID"`
# Tests
Manual tests on IBM i have verified that the change causes Solr to behave correctly when using OpenJDK.
"System Java," also known as "JV1," will still fail, but there are likely several contributing factors beyond the startup script issue identified here.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- ~~[ ] I have run `./gradlew check`. (N/A)~~
- ~~[ ] I have added tests for my changes. (N/A)~~
- ~~[ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide) (N/A)~~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] colvinco commented on pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1287:
URL: https://github.com/apache/solr/pull/1287#issuecomment-1607664817
Sorry I've not had a lot of time for Solr recently. Seems okay for now, cleaning up all the different uses of `ps` can still happen later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ThePrez commented on pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "ThePrez (via GitHub)" <gi...@apache.org>.
ThePrez commented on PR #1287:
URL: https://github.com/apache/solr/pull/1287#issuecomment-1612042252
> @ThePrez How do you want to be credited in `CHANGES.txt`????
These changes are so small attribution isn't necessary, but if you choose, you can use my real name: Jesse Gorzinski.
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ThePrez commented on a diff in pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "ThePrez (via GitHub)" <gi...@apache.org>.
ThePrez commented on code in PR #1287:
URL: https://github.com/apache/solr/pull/1287#discussion_r1107504662
##########
solr/bin/solr:
##########
@@ -716,7 +716,11 @@ function solr_pid_by_port() {
# extract the value of the -Djetty.port parameter from a running Solr process
function jetty_port() {
SOLR_PID="$1"
- SOLR_PROC=$(ps -o command='' -p "$SOLR_PID" | grep start\.jar | grep jetty\.port)
+ if [ "OS400" == "$THIS_OS" ]; then
+ SOLR_PROC=`ps -f -p "$SOLR_PID" | grep start\.jar | grep jetty\.port`
Review Comment:
Sorry for the delay. Fixed at latest commit/force-push. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh merged pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh merged PR #1287:
URL: https://github.com/apache/solr/pull/1287
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] ThePrez commented on pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "ThePrez (via GitHub)" <gi...@apache.org>.
ThePrez commented on PR #1287:
URL: https://github.com/apache/solr/pull/1287#issuecomment-1606258339
@colvinco was doing some more in-depth investigation, as documented in Jira.
It may be a good idea to revisit the overall usage of `ps` as was started, but I think it'd be safe to merge this in the meantime (deferring to maintainers for decision though).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1287:
URL: https://github.com/apache/solr/pull/1287#issuecomment-1607914308
I am on MacOS, so I can test that... Assuming it passes, then I'll merge. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1287:
URL: https://github.com/apache/solr/pull/1287#issuecomment-1529021783
What do we think? Comittable? (Someday we may need to think about how to test across many types of JVM's ;-( )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1287:
URL: https://github.com/apache/solr/pull/1287#issuecomment-1611927569
@ThePrez How do you want to be credited in `CHANGES.txt`????
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by GitBox <gi...@apache.org>.
janhoy commented on code in PR #1287:
URL: https://github.com/apache/solr/pull/1287#discussion_r1069521742
##########
solr/bin/solr:
##########
@@ -716,7 +716,11 @@ function solr_pid_by_port() {
# extract the value of the -Djetty.port parameter from a running Solr process
function jetty_port() {
SOLR_PID="$1"
- SOLR_PROC=$(ps -o command='' -p "$SOLR_PID" | grep start\.jar | grep jetty\.port)
+ if [ "OS400" == "$THIS_OS" ]; then
+ SOLR_PROC=`ps -f -p "$SOLR_PID" | grep start\.jar | grep jetty\.port`
Review Comment:
Please stick to the `$(foo)` syntax
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1287:
URL: https://github.com/apache/solr/pull/1287#issuecomment-1611895320
I ran the bats test and the `bin/solr start -e example` with four nodes, and it all seemed to work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1287:
URL: https://github.com/apache/solr/pull/1287#issuecomment-1607840074
@colvinco if you think this "LGTM" then I'm happy to shepherd committing this ;-). I've been periodically reviewing this JIRA and dreading fixing it ;-). Shell scripting magic isn't an area I'm super comofrotable with. Is there any enhancement to our BATS test that would validate/verify this fix?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] colvinco commented on pull request #1287: SOLR-16619: Fix `solr` script for IBM i
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1287:
URL: https://github.com/apache/solr/pull/1287#issuecomment-1607912098
Thanks @epugh. It looks like test_status.bats should fail if something goes wrong here, but as ever the pain is coming from having different OSes and versions of `ps`.
I can't actually see any mention of someone having checked with a macos, so that's the risky bit. I don't have a mac to test with myself. It would be great if PR checks with changes to the bash scripts ran on macos and linux, but in the meantime it would best if someone could verify `-f` works as expected for mac users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org