Solr
  1. Solr
  2. SOLR-4991

Register QParserPlugin's as SolrInfoMBean's, allowing them to be visible externally like other plugins

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.4, 5.0
    • Component/s: query parsers
    • Labels:
      None

      Description

      QParserPlugins currently cannot be seen externally as official plugins*. Let's register them as SolrInfoMBeans so they can be seen remotely.

      • Yes, solrconfig.xml itself could be retrieved and parsed, but many other similar plugins are available as MBeans and so should be query parsers.
      1. SOLR-4991.patch
        4 kB
        Erik Hatcher
      2. SOLR-4991.patch
        14 kB
        Erik Hatcher
      3. SOLR-4991.patch
        3 kB
        Erik Hatcher

        Activity

        Hide
        Erik Hatcher added a comment -

        Here's a patch that exposes all registered qparser plugins, including default built-in standard ones, as SolrInfoMBeans.

        Show
        Erik Hatcher added a comment - Here's a patch that exposes all registered qparser plugins, including default built-in standard ones, as SolrInfoMBeans.
        Hide
        Erik Hatcher added a comment -

        Here's an updated patch that adds numRequests to QParserPlugin to be returned in the mbean stats (this is congruent to the way highlighting components work, so I figured why not qparsers too), and adds incrementing numRequests in every current QParserPlugin.

        Also added an icon for the plugin UI.

        Show
        Erik Hatcher added a comment - Here's an updated patch that adds numRequests to QParserPlugin to be returned in the mbean stats (this is congruent to the way highlighting components work, so I figured why not qparsers too), and adds incrementing numRequests in every current QParserPlugin. Also added an icon for the plugin UI.
        Hide
        Erik Hatcher added a comment -

        Any objections? With or without the numRequests addition? This all should be backwards compatible, but if anyone sees a problem let me know (custom query parsers will have numRequests=0 until they upgrade Solr and add numRequests++ in their createParser() method).

        Show
        Erik Hatcher added a comment - Any objections? With or without the numRequests addition? This all should be backwards compatible, but if anyone sees a problem let me know (custom query parsers will have numRequests=0 until they upgrade Solr and add numRequests++ in their createParser() method).
        Hide
        Shalin Shekhar Mangar added a comment -

        +1 for the issue and the patch without numRequests. -0 for the patch with numRequests.

        Just wondering why would a query parser (or a highlighter for that matter) need numRequests? Correctness cannot be guaranteed for custom impls because they might forget to increment it. It works for handlers because RequestHandlerBase increments the counter. But if we decide to go this way, we should change it to be an AtomicLong.

        Show
        Shalin Shekhar Mangar added a comment - +1 for the issue and the patch without numRequests. -0 for the patch with numRequests. Just wondering why would a query parser (or a highlighter for that matter) need numRequests? Correctness cannot be guaranteed for custom impls because they might forget to increment it. It works for handlers because RequestHandlerBase increments the counter. But if we decide to go this way, we should change it to be an AtomicLong.
        Hide
        Yonik Seeley added a comment -

        QParserPlugins currently cannot be seen externally as official plugins

        Why do we need to see these? Any builtin plugins we should be able to just rely on them being there and this just adds more noise and startup costs. Perhaps instead we need a way (if we don't have one already) for anyone (i.e. any kind of plugin) to add externally visible statistics?

        Show
        Yonik Seeley added a comment - QParserPlugins currently cannot be seen externally as official plugins Why do we need to see these? Any builtin plugins we should be able to just rely on them being there and this just adds more noise and startup costs. Perhaps instead we need a way (if we don't have one already) for anyone (i.e. any kind of plugin) to add externally visible statistics?
        Hide
        Erik Hatcher added a comment - - edited

        Why do we need to see these?

        For apps to be able to see what query parsers are available.

        Any builtin plugins we should be able to just rely on them being there and this just adds more noise and startup costs. Perhaps instead we need a way (if we don't have one already) for anyone (i.e. any kind of plugin) to add externally visible statistics?

        It's not really about the statistics for my initial use case - it's about an app pointed at a Solr and being able to discern what query parsers are available.

        The main point here, forgetting about numRequests, is simply to have a way for plugins to be discoverable from the outside. This fits in the longer term goal to RESTify all of these things, at least in my opinion, but this is an easy and useful capability in the mean time.

        Just wondering why would a query parser (or a highlighter for that matter) need numRequests?

        Good question, I wondered that myself when I saw HighlightingPluginBase do this. But I can imagine it'd be useful to see which query parser(s) are being used. One might even want to monitor for inadvertent, or even suspicious, uses of other query parsers.

        Show
        Erik Hatcher added a comment - - edited Why do we need to see these? For apps to be able to see what query parsers are available. Any builtin plugins we should be able to just rely on them being there and this just adds more noise and startup costs. Perhaps instead we need a way (if we don't have one already) for anyone (i.e. any kind of plugin) to add externally visible statistics? It's not really about the statistics for my initial use case - it's about an app pointed at a Solr and being able to discern what query parsers are available. The main point here, forgetting about numRequests, is simply to have a way for plugins to be discoverable from the outside. This fits in the longer term goal to RESTify all of these things, at least in my opinion, but this is an easy and useful capability in the mean time. Just wondering why would a query parser (or a highlighter for that matter) need numRequests? Good question, I wondered that myself when I saw HighlightingPluginBase do this. But I can imagine it'd be useful to see which query parser(s) are being used. One might even want to monitor for inadvertent, or even suspicious, uses of other query parsers.
        Hide
        Erik Hatcher added a comment -

        Correctness cannot be guaranteed for custom impls because they might forget to increment it.

        Same with any subclasses of HighlightingPluginBase too.

        It works for handlers because RequestHandlerBase increments the counter. But if we decide to go this way, we should change it to be an AtomicLong.

        I just followed HighlightingPluginBase using a long.

        But perhaps best to remove numRequests altogether here and just get the query parsers showing up as plugins minus any stats.

        Show
        Erik Hatcher added a comment - Correctness cannot be guaranteed for custom impls because they might forget to increment it. Same with any subclasses of HighlightingPluginBase too. It works for handlers because RequestHandlerBase increments the counter. But if we decide to go this way, we should change it to be an AtomicLong. I just followed HighlightingPluginBase using a long. But perhaps best to remove numRequests altogether here and just get the query parsers showing up as plugins minus any stats.
        Hide
        Erik Hatcher added a comment -

        Here's a trunk-based patch without the numRequests. I'll commit to trunk and 4x later today.

        Is this fair to commit to 4.4 branch as well? It'd be good to get this in there if possible.

        Show
        Erik Hatcher added a comment - Here's a trunk-based patch without the numRequests. I'll commit to trunk and 4x later today. Is this fair to commit to 4.4 branch as well? It'd be good to get this in there if possible.
        Hide
        Steve Rowe added a comment -

        Is this fair to commit to 4.4 branch as well? It'd be good to get this in there if possible.

        +1

        Show
        Steve Rowe added a comment - Is this fair to commit to 4.4 branch as well? It'd be good to get this in there if possible. +1
        Hide
        ASF subversion and git services added a comment -

        Commit 1501863 from Erik Hatcher
        [ https://svn.apache.org/r1501863 ]

        SOLR-4991: Register QParserPlugins as SolrInfoMBeans

        Show
        ASF subversion and git services added a comment - Commit 1501863 from Erik Hatcher [ https://svn.apache.org/r1501863 ] SOLR-4991 : Register QParserPlugins as SolrInfoMBeans
        Hide
        ASF subversion and git services added a comment -

        Commit 1501871 from Erik Hatcher
        [ https://svn.apache.org/r1501871 ]

        SOLR-4991: Register QParserPlugins as SolrInfoMBeans (merged from r1501863)

        Show
        ASF subversion and git services added a comment - Commit 1501871 from Erik Hatcher [ https://svn.apache.org/r1501871 ] SOLR-4991 : Register QParserPlugins as SolrInfoMBeans (merged from r1501863)
        Hide
        ASF subversion and git services added a comment -

        Commit 1501874 from Erik Hatcher
        [ https://svn.apache.org/r1501874 ]

        SOLR-4991: Register QParserPlugins as SolrInfoMBeans (merged from r1501863)

        Show
        ASF subversion and git services added a comment - Commit 1501874 from Erik Hatcher [ https://svn.apache.org/r1501874 ] SOLR-4991 : Register QParserPlugins as SolrInfoMBeans (merged from r1501863)
        Hide
        Steve Rowe added a comment -

        Bulk close resolved 4.4 issues

        Show
        Steve Rowe added a comment - Bulk close resolved 4.4 issues

          People

          • Assignee:
            Erik Hatcher
            Reporter:
            Erik Hatcher
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development