Solr
  1. Solr
  2. SOLR-7169

init.d status command has incorrect return value

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.0
    • Fix Version/s: 5.4
    • Component/s: scripts and tools
    • Labels:
      None
    • Environment:

      Debian stable

      Description

      /etc/init.d/solr status returns 0 if Solr is not running, but according to the LSB 0 means that the service is running (http://refspecs.linuxfoundation.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html).

      In my situation this causes problems when managing the service with Puppet which uses the status command to determine if a service should be started. Puppet simply doesn't start the service because it thinks its already running. Any other script depending on the result of the status command will suffer similar problems.

      Workaround for other Puppet users:
      Setting hasstatus => false and pattern => "solr.solr.home" in the service definition works for me.

        Issue Links

          Activity

          Hide
          Dominik Siebel added a comment -

          Experiencing the same problem with orchestration via Saltstack currently.

          Show
          Dominik Siebel added a comment - Experiencing the same problem with orchestration via Saltstack currently.
          Hide
          Dominik Siebel added a comment -

          Since this task seems to be "IN PROGRESS" already: I have a patched, working version of the solr run script that I use in our setup. Anybody interested in that?
          Tim Potter, Martin Skøtt

          Show
          Dominik Siebel added a comment - Since this task seems to be "IN PROGRESS" already: I have a patched, working version of the solr run script that I use in our setup. Anybody interested in that? Tim Potter , Martin Skøtt
          Hide
          Ishan Chattopadhyaya added a comment -

          I think it is always a good idea to have patches. It'll be nice if you can contribute your patch so that others can review. Thanks.
          This page has info on how to start creating patches, https://wiki.apache.org/solr/HowToContribute

          Show
          Ishan Chattopadhyaya added a comment - I think it is always a good idea to have patches. It'll be nice if you can contribute your patch so that others can review. Thanks. This page has info on how to start creating patches, https://wiki.apache.org/solr/HowToContribute
          Hide
          Dominik Siebel added a comment -

          Ishan Chattopadhyaya Thanks, it's not my first change
          Is it already possible to provide patches via GitHub Pull Requests?
          I saw a couple of them being opened and integrated by committers manually...

          Show
          Dominik Siebel added a comment - Ishan Chattopadhyaya Thanks, it's not my first change Is it already possible to provide patches via GitHub Pull Requests? I saw a couple of them being opened and integrated by committers manually...
          Hide
          Upayavira added a comment -

          The Solr development community does not make much collective use of github - its workflow is based upon JIRA, and patches uploaded to JIRA. Deviations from that norm may reduce the likelihood of a patch being reviewed (but not necessarily prevent it). If you mention this ticket (i.e. SOLR-7169) in your PR message, it should automatically be included in this JIRA, which will make review easier.

          Show
          Upayavira added a comment - The Solr development community does not make much collective use of github - its workflow is based upon JIRA, and patches uploaded to JIRA. Deviations from that norm may reduce the likelihood of a patch being reviewed (but not necessarily prevent it). If you mention this ticket (i.e. SOLR-7169 ) in your PR message, it should automatically be included in this JIRA, which will make review easier.
          Hide
          ASF GitHub Bot added a comment -

          GitHub user dsiebel opened a pull request:

          https://github.com/apache/lucene-solr/pull/212

          SOLR-7169: Use proper exit code in `status` command to signal process status

          As reported in SOLR-7169(https://issues.apache.org/jira/browse/SOLR-7169) the solr run script always return `0` as exit code. This causes provisioners like Puppet or Saltstack assume that he process is already running.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/dsiebel/lucene-solr SOLR-7169-proper-exit-code-for-runscript

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/lucene-solr/pull/212.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #212


          commit b6d279ae19bf8d4c0cca8a0bcf0be50aa15ca3ec
          Author: dsiebel <dominik.siebel@trivago.com>
          Date: 2015-11-23T13:01:10Z

          SOLR-7169 use proper exit code in `status` command to signal process status


          Show
          ASF GitHub Bot added a comment - GitHub user dsiebel opened a pull request: https://github.com/apache/lucene-solr/pull/212 SOLR-7169 : Use proper exit code in `status` command to signal process status As reported in SOLR-7169 ( https://issues.apache.org/jira/browse/SOLR-7169 ) the solr run script always return `0` as exit code. This causes provisioners like Puppet or Saltstack assume that he process is already running. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsiebel/lucene-solr SOLR-7169 -proper-exit-code-for-runscript Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/212.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #212 commit b6d279ae19bf8d4c0cca8a0bcf0be50aa15ca3ec Author: dsiebel <dominik.siebel@trivago.com> Date: 2015-11-23T13:01:10Z SOLR-7169 use proper exit code in `status` command to signal process status
          Hide
          Dominik Siebel added a comment -

          Seems like it works

          Show
          Dominik Siebel added a comment - Seems like it works
          Hide
          Dominik Siebel added a comment -

          As it turns out following the LSB standard and returning 3 if the service is not running does not fix the problem with saltstack orchestration (returning 1 however does).
          Does it fix the problem for Puppet? Any chance you could test the patch, Martin Skøtt?

          Show
          Dominik Siebel added a comment - As it turns out following the LSB standard and returning 3 if the service is not running does not fix the problem with saltstack orchestration (returning 1 however does). Does it fix the problem for Puppet? Any chance you could test the patch, Martin Skøtt ?
          Hide
          Ishan Chattopadhyaya added a comment -

          Thanks Dominik.
          I tried to take a look at your patch. There are lots of whitespace changes. Is it please possible for you to post another patch that contains just the required change? It would be easier to review the change that way.
          As Upayavira mentioned, an SVN patch is more likely to be reviewed here than a git patch, but I can volunteer to convert your git patch to an SVN one for now for ease of reviewing.

          Show
          Ishan Chattopadhyaya added a comment - Thanks Dominik. I tried to take a look at your patch. There are lots of whitespace changes. Is it please possible for you to post another patch that contains just the required change? It would be easier to review the change that way. As Upayavira mentioned, an SVN patch is more likely to be reviewed here than a git patch, but I can volunteer to convert your git patch to an SVN one for now for ease of reviewing.
          Hide
          Dominik Siebel added a comment -

          Ishan Chattopadhyaya Sure, I will give it another try.
          I also realized that it's not yet working in all cases because the script forks subshells to read the pid files for example.
          I will provide another changeset that hopefully covers all of them.

          Show
          Dominik Siebel added a comment - Ishan Chattopadhyaya Sure, I will give it another try. I also realized that it's not yet working in all cases because the script forks subshells to read the pid files for example. I will provide another changeset that hopefully covers all of them.
          Hide
          Dominik Siebel added a comment - - edited

          New patch: https://github.com/apache/lucene-solr/pull/212.patch

          I threw away all previous changes and combined everything into one commit.
          The subshell problem seems to be solved but there is still one minor issue remaining:
          If there are running Solr instances the return value of get_info will always be the return value of the last run_tool status-call since all available pid files / running processes are processed in a loop.
          This is fine for single-node setups but might lead to side effects on multi-node setups (multiple instances / services running on one machine).

          Show
          Dominik Siebel added a comment - - edited New patch: https://github.com/apache/lucene-solr/pull/212.patch I threw away all previous changes and combined everything into one commit. The subshell problem seems to be solved but there is still one minor issue remaining: If there are running Solr instances the return value of get_info will always be the return value of the last run_tool status -call since all available pid files / running processes are processed in a loop. This is fine for single-node setups but might lead to side effects on multi-node setups (multiple instances / services running on one machine).
          Hide
          ASF subversion and git services added a comment -

          Commit 1716516 from Timothy Potter in branch 'dev/trunk'
          [ https://svn.apache.org/r1716516 ]

          SOLR-7169: bin/solr status should return exit code 3 no Solr is running instead of 0

          Show
          ASF subversion and git services added a comment - Commit 1716516 from Timothy Potter in branch 'dev/trunk' [ https://svn.apache.org/r1716516 ] SOLR-7169 : bin/solr status should return exit code 3 no Solr is running instead of 0
          Hide
          ASF subversion and git services added a comment -

          Commit 1716517 from Timothy Potter in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1716517 ]

          SOLR-7169: bin/solr status should return exit code 3 no Solr is running instead of 0

          Show
          ASF subversion and git services added a comment - Commit 1716517 from Timothy Potter in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1716517 ] SOLR-7169 : bin/solr status should return exit code 3 no Solr is running instead of 0
          Hide
          ASF subversion and git services added a comment -

          Commit 1716518 from Timothy Potter in branch 'dev/branches/lucene_solr_5_4'
          [ https://svn.apache.org/r1716518 ]

          SOLR-7169: bin/solr status should return exit code 3 no Solr is running instead of 0

          Show
          ASF subversion and git services added a comment - Commit 1716518 from Timothy Potter in branch 'dev/branches/lucene_solr_5_4' [ https://svn.apache.org/r1716518 ] SOLR-7169 : bin/solr status should return exit code 3 no Solr is running instead of 0
          Hide
          Timothy Potter added a comment -

          Thanks Dominik

          Show
          Timothy Potter added a comment - Thanks Dominik
          Show
          ASF GitHub Bot added a comment - Github user dsiebel commented on the pull request: https://github.com/apache/lucene-solr/pull/212#issuecomment-159714390 Changes merged via JIRA/SVN: trunk: https://svn.apache.org/viewvc?view=revision&revision=r1716516 branch_5x: https://svn.apache.org/viewvc?view=revision&revision=r1716517 lucene_solr_5_4: https://svn.apache.org/viewvc?view=revision&revision=r1716518
          Hide
          ASF GitHub Bot added a comment -

          Github user dsiebel closed the pull request at:

          https://github.com/apache/lucene-solr/pull/212

          Show
          ASF GitHub Bot added a comment - Github user dsiebel closed the pull request at: https://github.com/apache/lucene-solr/pull/212
          Hide
          Dominik Siebel added a comment -

          Thanks for merging this, Timothy.
          This will then be fixed with 5.4.0 I assume?
          Will there be another 5.3.x before that?

          Show
          Dominik Siebel added a comment - Thanks for merging this, Timothy. This will then be fixed with 5.4.0 I assume? Will there be another 5.3.x before that?
          Hide
          Upayavira added a comment -

          Dominik Siebel We have already started the process of releasing 5.4. First release candidate should be produced middle of next week.

          Show
          Upayavira added a comment - Dominik Siebel We have already started the process of releasing 5.4. First release candidate should be produced middle of next week.
          Hide
          Dominik Siebel added a comment -

          Upayavira Excellent, Thanks!

          Show
          Dominik Siebel added a comment - Upayavira Excellent, Thanks!

            People

            • Assignee:
              Timothy Potter
              Reporter:
              Martin Skøtt
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development