Solr
  1. Solr
  2. SOLR-1237

firstSearcher and newSearcher should identify themselves via the parameter set passed in

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4
    • Component/s: None
    • Labels:
      None

      Description

      The firstSearcher and newSearcher events call the regular search component chain, but are special cases. They should identify themselves by passing in a parameter indicating their type. This way, for instance, the sharding component could know to ignore the &shards parameter, which currently causes Solr to hang if it is present in the firstSearcher query string.

      See http://www.lucidimagination.com/search/document/b2b9c39a8eb4e563/firstsearcher_and_newsearcher_events

      1. SOLR-1237.patch
        11 kB
        Grant Ingersoll
      2. SOLR-1237.patch
        11 kB
        Grant Ingersoll

        Issue Links

          Activity

          Hide
          Grant Ingersoll added a comment -

          The problem I'm seeing here is that anyone can implement a SolrEventListener. How do we enfoce that the parameter is sent along?

          Show
          Grant Ingersoll added a comment - The problem I'm seeing here is that anyone can implement a SolrEventListener. How do we enfoce that the parameter is sent along?
          Hide
          Hoss Man added a comment -

          i think we make QuerySenderListener pass in a special param (init=newSearcher or init=firstSearcher), and i think we document that param, and i think we let people who implement their own SolrEventListeners worry about sending that param themselves if they want/need to...

          If we do it right, we can leave it so people can explicitly override the param in the solrconfig.xml (ie: init=none) so people can hide the fact that it's a warming query if they really, really, want to.

          Show
          Hoss Man added a comment - i think we make QuerySenderListener pass in a special param (init=newSearcher or init=firstSearcher), and i think we document that param, and i think we let people who implement their own SolrEventListeners worry about sending that param themselves if they want/need to... If we do it right, we can leave it so people can explicitly override the param in the solrconfig.xml (ie: init=none) so people can hide the fact that it's a warming query if they really, really, want to.
          Hide
          Grant Ingersoll added a comment -

          Adds CommonParams.EVENT value to specify an event and constants for NEW_SEARCHER and FIRST_SEARCHER. Implements test for checking this (it's "mocking" ugly)

          Also added documentation

          Show
          Grant Ingersoll added a comment - Adds CommonParams.EVENT value to specify an event and constants for NEW_SEARCHER and FIRST_SEARCHER. Implements test for checking this (it's "mocking" ugly) Also added documentation
          Hide
          Grant Ingersoll added a comment -

          I plan on committing in a couple of days.

          Show
          Grant Ingersoll added a comment - I plan on committing in a couple of days.
          Hide
          Erik Hatcher added a comment - - edited

          Just a quick and petty comment:

          + public static final String NEW_SEARCHER = "newSrchr";
          + public static final String FIRST_SEARCHER = "frstSrchr";

          No need to abbreviate so obtusely! "newSearcher" and "firstSearcher" pretty please.

          Show
          Erik Hatcher added a comment - - edited Just a quick and petty comment: + public static final String NEW_SEARCHER = "newSrchr"; + public static final String FIRST_SEARCHER = "frstSrchr"; No need to abbreviate so obtusely! "newSearcher" and "firstSearcher" pretty please.
          Hide
          Noble Paul added a comment - - edited

          No need to abbreviate so obtusely! "newSearcher" and "firstSearcher" pretty please.

          +1

          and why not "evt" changed to "event'

          Show
          Noble Paul added a comment - - edited No need to abbreviate so obtusely! "newSearcher" and "firstSearcher" pretty please. +1 and why not "evt" changed to "event'
          Hide
          Hoss Man added a comment -

          1. i don't know if these params are really common enough to belong in common .. the need to pay attention to them actually seems fairly uncommon, so i would suggest putting them in their own namespace.
          1. skimming the patch it wasn't clear why you needed to write MockQuerySenderListener ... can't you just have a request handler that records whether it got newSearcher & firstSearcher flagged requests and then ask it (ask it once after startup, then send a commit and ask it again)?

          Show
          Hoss Man added a comment - 1. i don't know if these params are really common enough to belong in common .. the need to pay attention to them actually seems fairly uncommon, so i would suggest putting them in their own namespace. 1. skimming the patch it wasn't clear why you needed to write MockQuerySenderListener ... can't you just have a request handler that records whether it got newSearcher & firstSearcher flagged requests and then ask it (ask it once after startup, then send a commit and ask it again)?
          Hide
          Grant Ingersoll added a comment -

          No need to abbreviate so obtusely! "newSearcher" and "firstSearcher" pretty please.

          Dang! That's fine by me, but there seems to be two camps in Solr-land, those who abbreviate and those who don't.

          Show
          Grant Ingersoll added a comment - No need to abbreviate so obtusely! "newSearcher" and "firstSearcher" pretty please. Dang! That's fine by me, but there seems to be two camps in Solr-land, those who abbreviate and those who don't.
          Hide
          Grant Ingersoll added a comment -

          skimming the patch it wasn't clear why you needed to write MockQuerySenderListener ... can't you just have a request handler that records whether it got newSearcher & firstSearcher flagged requests and then ask it (ask it once after startup, then send a commit and ask it again)?

          Actually, this is a good point, I need to modify the addEventParams() method to make a copy of the named list and then add the value, because the values get stored on the NamedList.

          Show
          Grant Ingersoll added a comment - skimming the patch it wasn't clear why you needed to write MockQuerySenderListener ... can't you just have a request handler that records whether it got newSearcher & firstSearcher flagged requests and then ask it (ask it once after startup, then send a commit and ask it again)? Actually, this is a good point, I need to modify the addEventParams() method to make a copy of the named list and then add the value, because the values get stored on the NamedList.
          Hide
          Grant Ingersoll added a comment -

          Fixed the names. Plan on committing in a day or two.

          Show
          Grant Ingersoll added a comment - Fixed the names. Plan on committing in a day or two.
          Hide
          Grant Ingersoll added a comment -

          Committed revision 801417.

          Show
          Grant Ingersoll added a comment - Committed revision 801417.
          Hide
          Grant Ingersoll added a comment -

          Bulk close for Solr 1.4

          Show
          Grant Ingersoll added a comment - Bulk close for Solr 1.4

            People

            • Assignee:
              Grant Ingersoll
              Reporter:
              Grant Ingersoll
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development