Solr
  1. Solr
  2. SOLR-1427

SearchComponents aren't listed on registry.jsp

    Details

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

      Description

      SearchComponent implements SolrInfoMBean using getCategory() of "OTHER" but they aren't listed on the registry.jsp display of loaded plugins.

      This may be a one-of-glitch because of the way SearchComponents get loaded, or it may indicate some other problem with the infoRegistry.

      1. SOLR-1427.afterlatch.patch
        5 kB
        Hoss Man
      2. SOLR-1427.patch
        1 kB
        Grant Ingersoll
      3. SOLR-1427.patch
        6 kB
        Grant Ingersoll
      4. SOLR-1427.patch
        5 kB
        Grant Ingersoll

        Activity

        Hide
        Grant Ingersoll added a comment -

        Yeah, doesn't look like the get registered w/ the infoRegistry on the core.

        Some other things I noticed:
        1. Should ProcessorChains implement SolrInfoMBean too? That could be a 1.5 thing.
        2. Seems like custom code that implements SolrInfoMBean is not going to be registered either. Again, I think that can be handled for 1.5.

        I'll put up a patch shortly on fixing the SearchComponents one (or at least the default ones for now)

        Show
        Grant Ingersoll added a comment - Yeah, doesn't look like the get registered w/ the infoRegistry on the core. Some other things I noticed: 1. Should ProcessorChains implement SolrInfoMBean too? That could be a 1.5 thing. 2. Seems like custom code that implements SolrInfoMBean is not going to be registered either. Again, I think that can be handled for 1.5. I'll put up a patch shortly on fixing the SearchComponents one (or at least the default ones for now)
        Hide
        Grant Ingersoll added a comment -

        Fixes the issue, including handling registering beans loaded by the SolrResourceLoader. Added test to SolrCoreTest. Will commit later today or tomorrow.

        Show
        Grant Ingersoll added a comment - Fixes the issue, including handling registering beans loaded by the SolrResourceLoader. Added test to SolrCoreTest. Will commit later today or tomorrow.
        Hide
        Grant Ingersoll added a comment -

        Adds in a section for highlighting.

        Show
        Grant Ingersoll added a comment - Adds in a section for highlighting.
        Hide
        Grant Ingersoll added a comment -

        Committed revision 815587.

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

        Reopening - these changes are implicated in deadlocks on startup.
        We need to find a way to make it bullet proof, or revert for now.

        Show
        Yonik Seeley added a comment - Reopening - these changes are implicated in deadlocks on startup. We need to find a way to make it bullet proof, or revert for now.
        Hide
        Grant Ingersoll added a comment -

        Hmm, I thought they would be fine given I handled them like the SolrCoreAware stuff. It seems like all it is doing is adding to a map, no?

        Show
        Grant Ingersoll added a comment - Hmm, I thought they would be fine given I handled them like the SolrCoreAware stuff. It seems like all it is doing is adding to a map, no?
        Hide
        Hoss Man added a comment -

        Comments from email thread...

        I haven't tried to reproduce the problem, and i don't even have a concrete
        theory as to what the problem is ... but i did want to point out something
        that smells fishy...

        looking over the patch in SOLR-1427, i notice that the patch adds new code
        so that SolrResourceLoader adds everything it instantiates to the registry
        map – but there are no lines in that patch removing any code that was
        already adding stuff to the infoRegistry ... which means any bean
        types that were already getting instantiated by the ResourceLoader, and explicitly
        added to hte registry are probably now getting added to the registry twice
        ... for a normal Map that's not a big deal, but perhaps the JMX backed map
        has some problems with that? (it's not likely something that we ever
        tested extensively)

        Alternately: maybe the problem isn't with adding to the JMX backed map
        twice ... maybe the issue is what state the beans are in each time we add
        them to the map ... the first time requesthandlers are added to the
        infoRegistry it's prior to the inform(SolrCore) call so they havne't been
        fully initalized yet – but the way SOlrResourceLoader adds them now (the
        second time) is after inform(SolrCore) has been called and in between
        the creation/countDown of the CountDownLatch for the searchExecutor ...

        ...hmmm, actually this starting to sound like a concrete theory,
        ReplicationHandler.getStatistics has very different behavior before/after
        inform(SolrCore) has been called....

        Maybe moving the resourceLoader.inform(infoRegistry) call PRIOR to
        resourceLoader.inform( this ) or AFTER latch.countDown() would solve
        this problem?

        followup comment...

        If this really is the problem, then a more general purpose solution to
        future proof us against similar problems down the road would probably be
        to get rid of the current "if (config.jmxConfig.enabled)" logic for
        initializing SolrCore.infoRegistry, and instead us something like...

        infoRegistry = new ConcurrentHashMap<String, SolrInfoMBean>();
        ...
        // do all initialization, add things to infoRegistry as needed
        ...
        // done with all initilation, core and all MBeans are fully functional
        if (config.jmxConfig.enabled)

        Unknown macro: { Map tmp = infoRegistry; infoRegistry = new JmxMonitoredMap<String, SolrInfoMBean>(name, config.jmxConfig); infoRegistry.putAll(tmp); }

        else

        Unknown macro: { log.info("JMX monitoring not detected for core}

        ...that way we wouldn't have to worry about any other type of resource
        contention that might happen from the JMX monitor wanting ot get info from
        the MBeans as soon as they are added to the registry.

        Show
        Hoss Man added a comment - Comments from email thread... I haven't tried to reproduce the problem, and i don't even have a concrete theory as to what the problem is ... but i did want to point out something that smells fishy... looking over the patch in SOLR-1427 , i notice that the patch adds new code so that SolrResourceLoader adds everything it instantiates to the registry map – but there are no lines in that patch removing any code that was already adding stuff to the infoRegistry ... which means any bean types that were already getting instantiated by the ResourceLoader, and explicitly added to hte registry are probably now getting added to the registry twice ... for a normal Map that's not a big deal, but perhaps the JMX backed map has some problems with that? (it's not likely something that we ever tested extensively) Alternately: maybe the problem isn't with adding to the JMX backed map twice ... maybe the issue is what state the beans are in each time we add them to the map ... the first time requesthandlers are added to the infoRegistry it's prior to the inform(SolrCore) call so they havne't been fully initalized yet – but the way SOlrResourceLoader adds them now (the second time) is after inform(SolrCore) has been called and in between the creation/countDown of the CountDownLatch for the searchExecutor ... ...hmmm, actually this starting to sound like a concrete theory, ReplicationHandler.getStatistics has very different behavior before/after inform(SolrCore) has been called.... Maybe moving the resourceLoader.inform(infoRegistry) call PRIOR to resourceLoader.inform( this ) or AFTER latch.countDown() would solve this problem? followup comment... If this really is the problem, then a more general purpose solution to future proof us against similar problems down the road would probably be to get rid of the current "if (config.jmxConfig.enabled)" logic for initializing SolrCore.infoRegistry, and instead us something like... infoRegistry = new ConcurrentHashMap<String, SolrInfoMBean>(); ... // do all initialization, add things to infoRegistry as needed ... // done with all initilation, core and all MBeans are fully functional if (config.jmxConfig.enabled) Unknown macro: { Map tmp = infoRegistry; infoRegistry = new JmxMonitoredMap<String, SolrInfoMBean>(name, config.jmxConfig); infoRegistry.putAll(tmp); } else Unknown macro: { log.info("JMX monitoring not detected for core} ...that way we wouldn't have to worry about any other type of resource contention that might happen from the JMX monitor wanting ot get info from the MBeans as soon as they are added to the registry.
        Hide
        Grant Ingersoll added a comment -

        I'm guessing the problem is most likely in loading the SearchComponents, not in the SolrResourceLoader. The reason being what Yonik said in that the core is not ready yet at that point.

        Also, need to address the possible double loading in SolrResourceLoader.

        Show
        Grant Ingersoll added a comment - I'm guessing the problem is most likely in loading the SearchComponents, not in the SolrResourceLoader. The reason being what Yonik said in that the core is not ready yet at that point. Also, need to address the possible double loading in SolrResourceLoader.
        Hide
        Grant Ingersoll added a comment -

        Hoss, where in the SolrResourceLoader do you see other puts into the infoRegistry happening?

        Show
        Grant Ingersoll added a comment - Hoss, where in the SolrResourceLoader do you see other puts into the infoRegistry happening?
        Hide
        Grant Ingersoll added a comment -

        Patch that defers registering the components until later. I can't reproduce the problem, so this is just a educated guess.

        Show
        Grant Ingersoll added a comment - Patch that defers registering the components until later. I can't reproduce the problem, so this is just a educated guess.
        Hide
        Yonik Seeley added a comment -

        I've reverted the majority of this patch (except for adding a highlight section to registry.xsl
        We probably should have done that immediately when this hang was detected since it turned a minor bug into a critical bug.

        Show
        Yonik Seeley added a comment - I've reverted the majority of this patch (except for adding a highlight section to registry.xsl We probably should have done that immediately when this hang was detected since it turned a minor bug into a critical bug.
        Hide
        Yonik Seeley added a comment -

        update: given one more try, I was able to reproduce the hang in tomcat before the patch was reverted.

        • I used java6
        • enabled replication as master
        • export JAVA_OPTS="-Dcom.sun.management.jmxremote" (I don't think this is necessary w/ Java6)
        • bin/catalina.sh start
        Show
        Yonik Seeley added a comment - update: given one more try, I was able to reproduce the hang in tomcat before the patch was reverted. I used java6 enabled replication as master export JAVA_OPTS="-Dcom.sun.management.jmxremote" (I don't think this is necessary w/ Java6) bin/catalina.sh start
        Hide
        Hoss Man added a comment -

        The more I think about it, and the more I look at the threaddumps from people reporting a problem after this patch...

        http://www.nabble.com/Latest-trunk--locks-execution-thread-in-SolrCore.getSearcher%28%29-to25483788.html#a25496516

        ...the more certain i am that my previous guess was correct.

        Some background...

        • SolrCore.getSearcher() explicitly says it can't be called during SolrCoreAware.inform(SolrCore) ... there is a "latch" that blocks getSearcher arround the inform calls.
        • ReplicationHandler implements SolrCoreAware, and obey's this contract but stores a permanent reference to the core.
        • ReplicationHandler.getStatistics uses that core to call getSearcher()

        Now here's where our problem comes from...

        • the previous patch resulted in SOlrResoureceLoader adding SOlrMBeans into the infoRegsitry map (which could be a JmxMonitoredMap) during the "latch" that causes blocks on getSearcher
        • JmxMonitoredMap.put registeres the MBeans with the JMX server.
        • based on the threaddumps provided there are evidently some JMX implementations in which JmxMBeanServer.registerMBean blocks until rDynamicMBean.getMBeanInfo returns.
        • In a SolrDynamicMBean.getMBeanInfo this method calls getStatistics

        ... hence the problem.

        I'm attaching a patch that is essentially a "redo" of what Yonik reverted, but it moves the line adding items to the infoRegistry until after the latch has been released.

        I'd appreciate it if anyone who was able to reproduce the problem that caused the revert could try this patch (with JMX turned on as it is in the example solrconfig.xml)

        Show
        Hoss Man added a comment - The more I think about it, and the more I look at the threaddumps from people reporting a problem after this patch... http://www.nabble.com/Latest-trunk--locks-execution-thread-in-SolrCore.getSearcher%28%29-to25483788.html#a25496516 ...the more certain i am that my previous guess was correct. Some background... SolrCore.getSearcher() explicitly says it can't be called during SolrCoreAware.inform(SolrCore) ... there is a "latch" that blocks getSearcher arround the inform calls. ReplicationHandler implements SolrCoreAware, and obey's this contract but stores a permanent reference to the core. ReplicationHandler.getStatistics uses that core to call getSearcher() Now here's where our problem comes from... the previous patch resulted in SOlrResoureceLoader adding SOlrMBeans into the infoRegsitry map (which could be a JmxMonitoredMap) during the "latch" that causes blocks on getSearcher JmxMonitoredMap.put registeres the MBeans with the JMX server. based on the threaddumps provided there are evidently some JMX implementations in which JmxMBeanServer.registerMBean blocks until rDynamicMBean.getMBeanInfo returns. In a SolrDynamicMBean.getMBeanInfo this method calls getStatistics ... hence the problem. I'm attaching a patch that is essentially a "redo" of what Yonik reverted, but it moves the line adding items to the infoRegistry until after the latch has been released. I'd appreciate it if anyone who was able to reproduce the problem that caused the revert could try this patch (with JMX turned on as it is in the example solrconfig.xml)
        Hide
        Hoss Man added a comment -

        Hoss, where in the SolrResourceLoader do you see other puts into the infoRegistry happening?

        I didn't say i saw other puts in SolrResourceLoader, I said now there are puts in SolrResourceLoader, but none of the existing puts (in other files) were removed even though in some cases they are now redundent.

        One Example...

        RequestHandlers.initHandlersFromConfig calls core.createRequestHandler which uses SolrResourceLoader to create every RequestHandler, and then initHandlersFromConfig also calls RequestHandlers.register which adds the handler to core.getInfoRegistry().

        But we problem shouldn't just remove any redundent looking puts on core.getInfoRegistry(), because they night not be redundant in all cases (RequestHandler.register could conceivable be used to register a Handler that some other plugin created w/o using SOlrResourceLoader) ... so the redundancy isn't necessarily bad, it just raised a red flag – and explains where the thread hang came from: registering RequestHandlers twice didn't cause the hang, it was the fact that the second registration happened during the latch on the searchExecutor.

        Show
        Hoss Man added a comment - Hoss, where in the SolrResourceLoader do you see other puts into the infoRegistry happening? I didn't say i saw other puts in SolrResourceLoader, I said now there are puts in SolrResourceLoader, but none of the existing puts (in other files) were removed even though in some cases they are now redundent. One Example... RequestHandlers.initHandlersFromConfig calls core.createRequestHandler which uses SolrResourceLoader to create every RequestHandler, and then initHandlersFromConfig also calls RequestHandlers.register which adds the handler to core.getInfoRegistry(). But we problem shouldn't just remove any redundent looking puts on core.getInfoRegistry(), because they night not be redundant in all cases (RequestHandler.register could conceivable be used to register a Handler that some other plugin created w/o using SOlrResourceLoader) ... so the redundancy isn't necessarily bad, it just raised a red flag – and explains where the thread hang came from: registering RequestHandlers twice didn't cause the hang, it was the fact that the second registration happened during the latch on the searchExecutor.
        Hide
        Hoss Man added a comment -

        got feedback on solr-user from two who had thread locks with SOLR-1427.patch indicating that SOLR-1427.afterlatch.patch worked fine for them, so....

        hossman@coaster:~/lucene/solr$ svn commit -m "SOLR-1427: redo of Grant's previous commit that yonik rolled back - but this time with the registration postponed until the latch is released"
        ...
        Committed revision 817499.

        Show
        Hoss Man added a comment - got feedback on solr-user from two who had thread locks with SOLR-1427 .patch indicating that SOLR-1427 .afterlatch.patch worked fine for them, so.... hossman@coaster:~/lucene/solr$ svn commit -m " SOLR-1427 : redo of Grant's previous commit that yonik rolled back - but this time with the registration postponed until the latch is released" ... Committed revision 817499.
        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:
            Hoss Man
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development