[SOLR-11389] call registerReporter after Solr(Shard|Cluster)Reporter.setCore[Container]

• Type: Task
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Fix Version/s: 7.2, 8.0
• Component/s: metrics
• Assignee: Christine Poerschke
• Reporter: Christine Poerschke
• Created: 22/Sep/17 16:03
• Updated: 10/Apr/18 08:34
• Resolved: 24/Oct/17 18:54

Description

Currently SolrMetricManager.loadReporter does four things:

• creates a new SolrMetricReporter instance (object)
• calls reporter.init(pluginInfo); on the object
• calls registerReporter(registry, pluginInfo.name, tag, reporter); for the object
• return reporter; returns the object

For the returned object the SolrMetricManager.loadShardReporters and SolrMetricManager.loadClusterReporters callers of SolrMetricManager.loadReporter then call the ((SolrShardReporter)reporter).setCore(core); or ((SolrClusterReporter)reporter).setCoreContainer(cc); method. This means that registerReporter happened before the SolrShardReporter and SolrClusterReporter objects were fully set up. (I have not yet fully investigated if this might be unintentional-and-not-required or intentional-and-required.)

The changes proposed in this ticket can be summarised as follows:

• SolrMetricReporter.java

  -  public SolrMetricReporter loadReporter(...) throws Exception {
  +  public void               loadReporter(...) throws Exception {
       ...
       try {
  -      reporter.init(pluginInfo);
  +      if (reporter instanceof SolrShardReporter) {
  +        ((SolrShardReporter)reporter).init(pluginInfo, solrCore);
  +      } else if (reporter instanceof SolrClusterReporter) {
  +        ((SolrClusterReporter)reporter).init(pluginInfo, coreContainer);
  +      } else {
  +        reporter.init(pluginInfo);
  +      }
       } catch (IllegalStateException e) {
         throw new IllegalArgumentException("reporter init failed: " + pluginInfo, e);
       }
       registerReporter(registry, pluginInfo.name, tag, reporter);
  -    return reporter;
     }
  

• SolrShardReporter.java

  +  @Override
  +  public void init(PluginInfo pluginInfo) {
  +    throw new UnsupportedOperationException(getClass().getCanonicalName()+".init(PluginInfo) is not supported, use init(PluginInfo,SolrCore) instead.");
  +  }
  
  -  public void setCore(SolrCore core) {
  +  public void init(PluginInfo pluginInfo, SolrCore core) {
  +    super.init(pluginInfo);
       ...
     }
  

• SolrClusterReporter.java

  +  @Override
  +  public void init(PluginInfo pluginInfo) {
  +    throw new UnsupportedOperationException(getClass().getCanonicalName()+".init(PluginInfo) is not supported, use init(PluginInfo,CoreContainer) instead.");
  +  }
  
  -  public void setCoreContainer(CoreContainer cc) {
  +  public void init(PluginInfo pluginInfo, CoreContainer cc) {
  +    super.init(pluginInfo);
       ...
     }
  

Context and motivation for the proposed changes is to support (in SOLR-11291) the factoring out of an abstract SolrCoreReporter class, allowing folks to create new reporters that 'know' the SolrCore they are reporting on.

Issue Links

• relates to: SOLR-11291 Adding Solr Core Reporter