Solr
  1. Solr
  2. SOLR-509

Event Listeners called before request handlers are informed of SolrCore

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3
    • Component/s: search
    • Labels:
      None

      Description

      http://www.nabble.com/Nullpointer-when-using-QuerySenderListener-to16201207.html

      Thijs noticed NullPointerException from SearchHandler on startup when trying to configure some firstSearcher events using QuerySenderListener.

      The problem is the events are getting triggered as soon as the firstSearcher is opened, but the SOlrCore isn't finished being initialized, so inform(SolrCore) hasn't been called on all the Handlers (and some handlers aren't fully initialized and ready to process requests yet.

      We need to more the firstSearcher event handling after inform has been called on all SolrCoreAware objects.

      1. SOLR-509.patch
        3 kB
        Grant Ingersoll
      2. SOLR-509.patch
        2 kB
        Koji Sekiguchi
      3. SOLR-509-fix.patch
        4 kB
        Yonik Seeley
      4. SOLR-509-fix.patch
        2 kB
        Yonik Seeley

        Activity

        Hide
        Koji Sekiguchi added a comment -

        How about this?

        1. move firsrSearcher processing block from getSearcher() to a newly created private method
        2. call the new private method at the end of SolrCore constructor

        Show
        Koji Sekiguchi added a comment - How about this? 1. move firsrSearcher processing block from getSearcher() to a newly created private method 2. call the new private method at the end of SolrCore constructor
        Hide
        Yonik Seeley added a comment -

        firstSearcher should be called whenever a new searcher is being opened and there is no current searcher to base warming on. Right now, that will only happen at startup, but in the future?

        Show
        Yonik Seeley added a comment - firstSearcher should be called whenever a new searcher is being opened and there is no current searcher to base warming on. Right now, that will only happen at startup, but in the future?
        Hide
        Grant Ingersoll added a comment -

        FWIW, applies cleanly and the tests pass. As for the future, I guess I say we wait until it happens. I can add a comment.

        Show
        Grant Ingersoll added a comment - FWIW, applies cleanly and the tests pass. As for the future, I guess I say we wait until it happens. I can add a comment.
        Hide
        Grant Ingersoll added a comment -

        I'll commit this shortly. Added to CHANGES and put in a comment about calls to firstSearcher initialization.

        Show
        Grant Ingersoll added a comment - I'll commit this shortly. Added to CHANGES and put in a comment about calls to firstSearcher initialization.
        Hide
        Thijs Vonk added a comment -

        there is a small typo in your patch

        //TODO: It may not always be the case that this the only time the first searcher event needs to fire.
        doFirstSearcherEvent(getSearcher().get());

        //TODO: It may not always be the case that this is the only time the first searcher event needs to fire.
        doFirstSearcherEvent(getSearcher().get());

        Show
        Thijs Vonk added a comment - there is a small typo in your patch //TODO: It may not always be the case that this the only time the first searcher event needs to fire. doFirstSearcherEvent(getSearcher().get()); //TODO: It may not always be the case that this is the only time the first searcher event needs to fire. doFirstSearcherEvent(getSearcher().get());
        Hide
        Grant Ingersoll added a comment -

        Committed revision 649046.

        Show
        Grant Ingersoll added a comment - Committed revision 649046.
        Hide
        Yonik Seeley added a comment -

        Reopening... This patch introduced a bug that prevents the first searcher opened from ever being closed.
        I'm looking into a fix.

        Show
        Yonik Seeley added a comment - Reopening... This patch introduced a bug that prevents the first searcher opened from ever being closed. I'm looking into a fix.
        Hide
        Yonik Seeley added a comment -

        This is one possible (untested) fix.
        I didn't test it because I'm looking into something that might be a bit better.... just stall the executor until later.

        Show
        Yonik Seeley added a comment - This is one possible (untested) fix. I didn't test it because I'm looking into something that might be a bit better.... just stall the executor until later.
        Hide
        Yonik Seeley added a comment -

        Here's the latest version, which restores firstSearcher event handling to getSearcher, and uses a latch to block any searcher warming events until the end of the SolrCore constructor.

        Show
        Yonik Seeley added a comment - Here's the latest version, which restores firstSearcher event handling to getSearcher, and uses a latch to block any searcher warming events until the end of the SolrCore constructor.
        Hide
        Koji Sekiguchi added a comment -

        I confirmed the fix. I like the second patch (uses a latch version). Thanks Yonik to solve this problem!

        Show
        Koji Sekiguchi added a comment - I confirmed the fix. I like the second patch (uses a latch version). Thanks Yonik to solve this problem!
        Hide
        Yonik Seeley added a comment -

        fix committed.

        Show
        Yonik Seeley added a comment - fix committed.

          People

          • Assignee:
            Yonik Seeley
            Reporter:
            Hoss Man
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development