Solr
  1. Solr
  2. SOLR-6766

Switch o.a.s.store.blockcache.Metrics to use JMX

    Details

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

      Description

      The Metrics class currently reports to hadoop metrics, but it would be better to report to JMX.

      1. SOLR-6766.patch
        14 kB
        Mark Miller
      2. SOLR-6766.patch
        14 kB
        Mike Drob
      3. SOLR-6766.patch
        14 kB
        Mike Drob

        Issue Links

          Activity

          Hide
          Mike Drob added a comment -

          Mark Miller - Continuing discussion from SOLR-6752...

          I fired up Solr on HDFS with JMX enabled and took a look at the exported mbeans with JConsole. I did not see anything for the block cache.

          I'd look at how SolrResourceLoader adds the plugins that it loads to the JmxMonitoredMap.

          Been digging deeper into this... metrics are tracked on a per-core basis. Each core has an infoRegistry that is populated in the constructor either directly or from beans that the SolrResourceLoader had previously created. So instead of creating a new Metrics object directly, we will need to create one through the SolrResourceLoader.newInstance(), which is I think what you were suggesting.

          The trick here is that we need to create the bean before the SolrCore finishes constructing, but after the HdfsDirectoryFactory (HDF) exists to make sure that it gets registered in time. So basically, in the no-arg HDF constructor is our only option. The problem is that HDF (or any implementation of DirectoryFactory) is not aware of the resource loader or even a SolrConfig to be able to acquire a reference to the resource loader. I'm hesitant to add a setResourceLoader method or similar on DirectoryFactory because that is starting to feel very intrusive, but I also don't see another way to plumb this through.

          Show
          Mike Drob added a comment - Mark Miller - Continuing discussion from SOLR-6752 ... I fired up Solr on HDFS with JMX enabled and took a look at the exported mbeans with JConsole. I did not see anything for the block cache. I'd look at how SolrResourceLoader adds the plugins that it loads to the JmxMonitoredMap. Been digging deeper into this... metrics are tracked on a per-core basis. Each core has an infoRegistry that is populated in the constructor either directly or from beans that the SolrResourceLoader had previously created. So instead of creating a new Metrics object directly, we will need to create one through the SolrResourceLoader.newInstance() , which is I think what you were suggesting. The trick here is that we need to create the bean before the SolrCore finishes constructing, but after the HdfsDirectoryFactory (HDF) exists to make sure that it gets registered in time. So basically, in the no-arg HDF constructor is our only option. The problem is that HDF (or any implementation of DirectoryFactory ) is not aware of the resource loader or even a SolrConfig to be able to acquire a reference to the resource loader. I'm hesitant to add a setResourceLoader method or similar on DirectoryFactory because that is starting to feel very intrusive, but I also don't see another way to plumb this through.
          Hide
          Mark Miller added a comment -

          The block cache should normally just be one now that the global block cache is enabled by default. The per directory version should be special case at best. Perhaps the HdfsDirectoryFactory could simply track all the Metrics objects and spit out the stats for the caches (normally 1) that it tracks. If everything is keyed on HdfsDirectoryFactory as the info bean, nothing else has to be changed. Just spitballing though.

          Show
          Mark Miller added a comment - The block cache should normally just be one now that the global block cache is enabled by default. The per directory version should be special case at best. Perhaps the HdfsDirectoryFactory could simply track all the Metrics objects and spit out the stats for the caches (normally 1) that it tracks. If everything is keyed on HdfsDirectoryFactory as the info bean, nothing else has to be changed. Just spitballing though.
          Hide
          Mike Drob added a comment -

          This patch depends on the one on SOLR-6752 being applied first.

          Make HdfsDirctoryFactory implement SolrInfoMBean and expose metrics that way. I think it works for both the global cache and local cache options because it uses the same Metrics object for everything.

          Show
          Mike Drob added a comment - This patch depends on the one on SOLR-6752 being applied first. Make HdfsDirctoryFactory implement SolrInfoMBean and expose metrics that way. I think it works for both the global cache and local cache options because it uses the same Metrics object for everything.
          Hide
          Mark Miller added a comment -

          I think it works for both the global cache and local cache options because it uses the same Metrics object for everything.

          Oh cool, well that simplifies things. I'll take a look at this shortly.

          Show
          Mark Miller added a comment - I think it works for both the global cache and local cache options because it uses the same Metrics object for everything. Oh cool, well that simplifies things. I'll take a look at this shortly.
          Hide
          Mike Drob added a comment -

          I didn't rename any of the metrics yet because that should be a fairly easy change to make. Trying to get the structure of the changeset correct, first.

          Show
          Mike Drob added a comment - I didn't rename any of the metrics yet because that should be a fairly easy change to make. Trying to get the structure of the changeset correct, first.
          Hide
          Mark Miller added a comment -

          +1 - shaping up nicely.

          Show
          Mark Miller added a comment - +1 - shaping up nicely.
          Hide
          Mike Drob added a comment -

          Renamed some of the metrics, based on what the other caches use. Could not figure out what would be tracking "inserts" into the block cache.

          The other caches also track cumulative metrics, should we track those here as well?

          Show
          Mike Drob added a comment - Renamed some of the metrics, based on what the other caches use. Could not figure out what would be tracking "inserts" into the block cache. The other caches also track cumulative metrics, should we track those here as well?
          Hide
          Otis Gospodnetic added a comment -

          Is this aimed at 4.10.3?

          Show
          Otis Gospodnetic added a comment - Is this aimed at 4.10.3?
          Hide
          Mike Drob added a comment -

          I build the patch against trunk, but it could probably go into branch-5. I think it applies pretty cleanly to branch-4 as well, but I haven't looked at it that closely.

          Show
          Mike Drob added a comment - I build the patch against trunk, but it could probably go into branch-5. I think it applies pretty cleanly to branch-4 as well, but I haven't looked at it that closely.
          Hide
          Mark Miller added a comment -

          Is this aimed at 4.10.3?

          4.10.3 is a bug fix release - the fact that the code exists but is not hooked up could allow one to try and stretch it to 4.10.3 - along with the idea that there will be no more 4.x releases. No plans on it right now though.

          Show
          Mark Miller added a comment - Is this aimed at 4.10.3? 4.10.3 is a bug fix release - the fact that the code exists but is not hooked up could allow one to try and stretch it to 4.10.3 - along with the idea that there will be no more 4.x releases. No plans on it right now though.
          Hide
          Mark Miller added a comment -

          Here is a patch cleaning up a couple small things. Looks good overall - I just have to do a manual check to make sure everything shows up in the jmx output as expected and do a brief check of all the key names.

          Show
          Mark Miller added a comment - Here is a patch cleaning up a couple small things. Looks good overall - I just have to do a manual check to make sure everything shows up in the jmx output as expected and do a brief check of all the key names.
          Hide
          ASF subversion and git services added a comment -

          Commit 1649939 from Mark Miller in branch 'dev/trunk'
          [ https://svn.apache.org/r1649939 ]

          SOLR-6766: Expose HdfsDirectoryFactory Block Cache statistics via JMX.

          Show
          ASF subversion and git services added a comment - Commit 1649939 from Mark Miller in branch 'dev/trunk' [ https://svn.apache.org/r1649939 ] SOLR-6766 : Expose HdfsDirectoryFactory Block Cache statistics via JMX.
          Hide
          ASF subversion and git services added a comment -

          Commit 1649941 from Mark Miller in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1649941 ]

          SOLR-6766: Expose HdfsDirectoryFactory Block Cache statistics via JMX.

          Show
          ASF subversion and git services added a comment - Commit 1649941 from Mark Miller in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1649941 ] SOLR-6766 : Expose HdfsDirectoryFactory Block Cache statistics via JMX.
          Hide
          Mark Miller added a comment -

          Thanks Mike!

          Show
          Mark Miller added a comment - Thanks Mike!
          Hide
          Anshum Gupta added a comment -

          Bulk close after 5.0 release.

          Show
          Anshum Gupta added a comment - Bulk close after 5.0 release.

            People

            • Assignee:
              Mark Miller
              Reporter:
              Mike Drob
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development