Solr
  1. Solr
  2. SOLR-6453

Stop throwing an error message from Overseer on Solr exit

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0, 6.0
    • Component/s: SolrCloud
    • Labels:
      None

      Description

      SOLR-5859 adds a leadership check every time Overseer exits loop. This however gets triggered even when Solr really is exiting, causing a spurious error. Here's a one-liner to stop that from happening.

      1. SOLR-6453.patch
        0.7 kB
        Noble Paul

        Activity

        Hide
        ASF GitHub Bot added a comment -

        GitHub user andyetitmoves opened a pull request:

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

        Stop attempting to check Overseer leadership on exit

        Patch for SOLR-6453

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

        $ git pull https://github.com/bloomberg/lucene-solr stop-exception-exit

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

        https://github.com/apache/lucene-solr/pull/89.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 #89


        commit bb3e78b402e0d316cee6ec09494e28db02e2743c
        Author: Ramkumar Aiyengar <raiyengar@bloomberg.net>
        Date: 2014-08-29T18:59:25Z

        Stop attempting to check Overseer leadership on exit


        Show
        ASF GitHub Bot added a comment - GitHub user andyetitmoves opened a pull request: https://github.com/apache/lucene-solr/pull/89 Stop attempting to check Overseer leadership on exit Patch for SOLR-6453 You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/lucene-solr stop-exception-exit Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/89.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 #89 commit bb3e78b402e0d316cee6ec09494e28db02e2743c Author: Ramkumar Aiyengar <raiyengar@bloomberg.net> Date: 2014-08-29T18:59:25Z Stop attempting to check Overseer leadership on exit
        Hide
        Noble Paul added a comment -

        I guess this is the right fix

        Show
        Noble Paul added a comment - I guess this is the right fix
        Hide
        Ramkumar Aiyengar added a comment -

        Thanks for picking this up, Noble! Just so that I understand, could you explain the difference?

        I see that the controller closes the Overseer and then the client, but aren't we coupling the two implementations tightly by assuming that? In hindsight, it doesn't make sense to use the zkClient when it is closed, so I should have probably used if (zkClient.isClosed()) instead of just isClosed. Checking just zkController does make sense for the final operation of rejoining election (which is what we are checking in the finally block anyway), but checking just that assumes that the state reader passed to us is from the same controller. May be we should check both, i.e. the controller is active (if not, we can't rejoin anyway), and the client is active..

        Show
        Ramkumar Aiyengar added a comment - Thanks for picking this up, Noble! Just so that I understand, could you explain the difference? I see that the controller closes the Overseer and then the client, but aren't we coupling the two implementations tightly by assuming that? In hindsight, it doesn't make sense to use the zkClient when it is closed, so I should have probably used if (zkClient.isClosed()) instead of just isClosed . Checking just zkController does make sense for the final operation of rejoining election (which is what we are checking in the finally block anyway), but checking just that assumes that the state reader passed to us is from the same controller. May be we should check both, i.e. the controller is active (if not, we can't rejoin anyway), and the client is active..
        Hide
        Noble Paul added a comment -

        The objective of that method is to handle the overseer "QUIT" command. So , when a QUIT command is received, the node is still running, (corecontainer.isShutdown() returns false and isClosed() returns true) and the current overseer should do the cleanup and exit gracefully

        Show
        Noble Paul added a comment - The objective of that method is to handle the overseer "QUIT" command. So , when a QUIT command is received, the node is still running, (corecontainer.isShutdown() returns false and isClosed() returns true) and the current overseer should do the cleanup and exit gracefully
        Hide
        Ramkumar Aiyengar added a comment - - edited

        Ah, okay, makes sense.. Could you add the zkClient.isClosed() check though as per my comment above?

        Also, OverseerTest passed for me with this change, probably shouldn't have?

        Show
        Ramkumar Aiyengar added a comment - - edited Ah, okay, makes sense.. Could you add the zkClient.isClosed() check though as per my comment above? Also, OverseerTest passed for me with this change, probably shouldn't have?
        Hide
        Noble Paul added a comment -

        Also, OverseerTest passed for me with this change, probably shouldn't have?

        The cleanup is usually not required . So, the tests should normally have no impact . Under load it might behave differently

        Show
        Noble Paul added a comment - Also, OverseerTest passed for me with this change, probably shouldn't have? The cleanup is usually not required . So, the tests should normally have no impact . Under load it might behave differently
        Hide
        ASF subversion and git services added a comment -

        Commit 1627343 from Noble Paul in branch 'dev/trunk'
        [ https://svn.apache.org/r1627343 ]

        SOLR-6453

        Show
        ASF subversion and git services added a comment - Commit 1627343 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1627343 ] SOLR-6453
        Hide
        ASF subversion and git services added a comment -

        Commit 1627344 from Noble Paul in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1627344 ]

        SOLR-6453

        Show
        ASF subversion and git services added a comment - Commit 1627344 from Noble Paul in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1627344 ] SOLR-6453
        Hide
        ASF GitHub Bot added a comment -

        Github user andyetitmoves closed the pull request at:

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

        Show
        ASF GitHub Bot added a comment - Github user andyetitmoves closed the pull request at: https://github.com/apache/lucene-solr/pull/89
        Hide
        Anshum Gupta added a comment -

        Bulk close after 5.0 release.

        Show
        Anshum Gupta added a comment - Bulk close after 5.0 release.

          People

          • Assignee:
            Noble Paul
            Reporter:
            Ramkumar Aiyengar
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development