Solr
  1. Solr
  2. SOLR-5087

CoreAdminHandler.handleMergeAction generating NullPointerException

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.4
    • Fix Version/s: 4.5, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      CoreAdminHandler.handleMergeAction is generating NullPointerException

      If directoryFactory.get(...) in handleMergeAction throws an exception the original error is lost as the finally clause will attempt to clean up and generate an NPE. (notice that "dirsToBeReleased" is pre-allocated with nulls that are not filled in)

      ERROR org.apache.solr.core.SolrCore: java.lang.NullPointerException
      at org.apache.solr.core.CachingDirectoryFactory.release(CachingDirectoryFactory.java:430)
      at org.apache.solr.handler.admin.CoreAdminHandler.handleMergeAction(CoreAdminHandler.java:380)
      at org.apache.solr.handler.admin.CoreAdminHandler.handleRequestBody(CoreAdminHandler.java:180)
      
      1. SOLR-5087.patch
        13 kB
        Patrick Hunt

        Activity

        Hide
        Patrick Hunt added a comment -

        This patch fixes the problem by catching/logging/rethrowing the original problem. I've also made some changes to the code to make it less likely that the cleanup (finally clause) will fail.

        The test I added fails w/o the fix applied.

        This patch applies/passes for me on both trunk and branch4x.

        Show
        Patrick Hunt added a comment - This patch fixes the problem by catching/logging/rethrowing the original problem. I've also made some changes to the code to make it less likely that the cleanup (finally clause) will fail. The test I added fails w/o the fix applied. This patch applies/passes for me on both trunk and branch4x.
        Hide
        Mark Miller added a comment -

        Looks good to me - there is a little back compat breakage in the merge command, but I think that's fine. Just calling it out in case anyone else has a concern there.

        Show
        Mark Miller added a comment - Looks good to me - there is a little back compat breakage in the merge command, but I think that's fine. Just calling it out in case anyone else has a concern there.
        Hide
        Patrick Hunt added a comment -

        Oh, yes. I forgot about that, it seemed like an internal operation though. LMK if it should be reverted. (it was cleaner to push the List usage through, but not critical)

        Show
        Patrick Hunt added a comment - Oh, yes. I forgot about that, it seemed like an internal operation though. LMK if it should be reverted. (it was cleaner to push the List usage through, but not critical)
        Hide
        Mark Miller added a comment -

        it seemed like an internal operation though

        Technically it's part of the UpdatePrcoessor chain user plugin point API's - but we are kind of ad-hoc with back compat in these API's - I think it's rare enough to do something custom with the merge command that I'm not personally worried about it though.

        Show
        Mark Miller added a comment - it seemed like an internal operation though Technically it's part of the UpdatePrcoessor chain user plugin point API's - but we are kind of ad-hoc with back compat in these API's - I think it's rare enough to do something custom with the merge command that I'm not personally worried about it though.
        Hide
        Shalin Shekhar Mangar added a comment -

        there is a little back compat breakage in the merge command, but I think that's fine.

        That should be fine. Patch looks good. Thanks Patrick!

        Show
        Shalin Shekhar Mangar added a comment - there is a little back compat breakage in the merge command, but I think that's fine. That should be fine. Patch looks good. Thanks Patrick!
        Hide
        Erick Erickson added a comment -

        Testing, merging and committing shortly.

        Show
        Erick Erickson added a comment - Testing, merging and committing shortly.
        Hide
        ASF subversion and git services added a comment -

        Commit 1508476 from Erick Erickson in branch 'dev/trunk'
        [ https://svn.apache.org/r1508476 ]

        SOLR-5087, CoreAdminHandler.handleMergeAction generating NullPointerException. Thanks Patrick

        Show
        ASF subversion and git services added a comment - Commit 1508476 from Erick Erickson in branch 'dev/trunk' [ https://svn.apache.org/r1508476 ] SOLR-5087 , CoreAdminHandler.handleMergeAction generating NullPointerException. Thanks Patrick
        Hide
        ASF subversion and git services added a comment -

        Commit 1508491 from Erick Erickson in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1508491 ]

        SOLR-5087, CoreAdminHandler.handleMergeAction generating NullPointerException. Thanks Patrick

        Show
        ASF subversion and git services added a comment - Commit 1508491 from Erick Erickson in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1508491 ] SOLR-5087 , CoreAdminHandler.handleMergeAction generating NullPointerException. Thanks Patrick
        Hide
        Erick Erickson added a comment -

        Thanks Patrick! I forgot CHANGES.txt, I'll add shortly.

        Show
        Erick Erickson added a comment - Thanks Patrick! I forgot CHANGES.txt, I'll add shortly.
        Hide
        ASF subversion and git services added a comment -

        Commit 1508494 from Erick Erickson in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1508494 ]

        Added entry for SOLR-5087

        Show
        ASF subversion and git services added a comment - Commit 1508494 from Erick Erickson in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1508494 ] Added entry for SOLR-5087
        Hide
        Adrien Grand added a comment -

        4.5 release -> bulk close

        Show
        Adrien Grand added a comment - 4.5 release -> bulk close

          People

          • Assignee:
            Erick Erickson
            Reporter:
            Patrick Hunt
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development