[ https://issues.apache.org/jira/browse/SOLR-11389?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16199093#comment-16199093 ]
Christine Poerschke commented on SOLR-11389: -------------------------------------------- {{solr/core}} unit test suite passes. Questions, comments and/or code reviews welcome as usual. Hoping to commit these changes early next week. > call registerReporter after Solr(Shard|Cluster)Reporter.setCore[Container] > -------------------------------------------------------------------------- > > Key: SOLR-11389 > URL: https://issues.apache.org/jira/browse/SOLR-11389 > Project: Solr > Issue Type: Task > Components: metrics > Reporter: Christine Poerschke > Assignee: Christine Poerschke > Priority: Minor > Attachments: SOLR-11389.patch > > > Currently > [SolrMetricManager.loadReporter|https://github.com/apache/lucene-solr/blob/releases/lucene-solr/7.0.0/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java#L823-L844] > 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 > {code} > - 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; > } > {code} > * SolrShardReporter.java > {code} > + @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); > ... > } > {code} > * SolrClusterReporter.java > {code} > + @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); > ... > } > {code} > 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. -- This message was sent by Atlassian JIRA (v6.4.14#64029) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org