[jira] [Commented] (SOLR-11291) Adding Solr Core Reporter
[ https://issues.apache.org/jira/browse/SOLR-11291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16152591#comment-16152591 ] Christine Poerschke commented on SOLR-11291: (Disclosure: Omar and I are in the same team and I've already internally seen and had input into the attached patch.) The motivation for the proposed changes is, as Omar mentioned, to support new reporters that allow metrics to be reported on a per-core basis. I.e. the reporter itself directly 'knows' which core it is reporting metrics for; this is similar to but slightly different from the [Core Metric Registry|https://lucene.apache.org/solr/guide/6_6/metrics-reporting.html#core-solrcore-registry] functionality. Since SOLR-9858 in Solr 7.0 the SolrShardReporter reports selected metrics from replicas to the shard leader. If the {{group="shard"}} attribute is configured then SolrCoreMetricManager calls SolrMetricManager.loadShardReporters which eventually calls {code} ((SolrShardReporter)reporter).setCore(core); {code} so that the SolrShardReporter 'knows' which core it is reporting metrics for. This ticket proposes the creation of an abstract class: {code} abstract public class SolrCoreReporter extends FilteringSolrMetricReporter { protected SolrCore core; ... constructor here ... public void setCore(SolrCore core) { this.core = core; } public SolrCore getCore() { return core; } } {code} SolrCoreReporter is a tentative name e.g. it could be renamed to (say) SolrReplicaReporter or similar if SolrCoreReporter is too close to the Core Metric Registry concept. The existing SolrShardReporter would extend the new abstract class. Omar's patch includes a test example reporter SolrConsoleReporter which extends the new abstract class and via {code} ant test -Dtestcase=SolrConsoleReporterTest {code} always and {code} ant test -Dtestcase=SolrCloudReportersTest {code} part of the time via randomization the SolrConsoleReporter class is used. (Slightly longer than anticipated preamble, oops, now here's a question.) In its current form the patch includes the addition of a new {{SolrInfoBean.Group.replica}} enum choice and SolrMetricManager changes like this: {code} ... + public void loadReplicaReporters(PluginInfo[] pluginInfos, SolrCore core) { +doLoadSolrCoreReporters(pluginInfos, core, SolrInfoBean.Group.replica); + } + public void loadShardReporters(PluginInfo[] pluginInfos, SolrCore core) { +doLoadSolrCoreReporters(pluginInfos, core, SolrInfoBean.Group.shard); + } + + private void doLoadSolrCoreReporters(PluginInfo[] pluginInfos, SolrCore core, SolrInfoBean.Group group) { ... {code} i.e. there is overlap between the existing {{SolrInfoBean.Group.shard}} code and the new {{SolrInfoBean.Group.replica}} code. Might such overlap be confusing from a user's point of view? If it would be confusing and if the new abstract class was called {{SolrReplicaReporter}} then might a solution be to deprecate {{SolrInfoBean.Group.shard}} in favour of {{SolrInfoBean.Group.replica}}? [~ab] - would you have any thoughts on this? > Adding Solr Core Reporter > - > > Key: SOLR-11291 > URL: https://issues.apache.org/jira/browse/SOLR-11291 > Project: Solr > Issue Type: New Feature > Components: metrics >Reporter: Omar Abdelnabi >Priority: Minor > Attachments: SOLR-11291.patch > > > Adds a new reporter, SolrCoreReporter, which allows metrics to be reported on > per-core basis. > Also modifies the SolrMetricManager and SolrCoreMetricManager to take > advantage of this new reporter. > Adds a test/example that uses the SolrCoreReporter. Also adds randomization > to SolrCloudReportersTest. -- 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
[jira] [Commented] (SOLR-11291) Adding Solr Core Reporter
[ https://issues.apache.org/jira/browse/SOLR-11291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16169864#comment-16169864 ] Christine Poerschke commented on SOLR-11291: [~ab] and I spoke about this offline. At present there's the {{SolrInfoBean.Group.shard}} enum choice associated with the {{SolrShardReporter}} class for which {{setCore(core)}} is called. Instead of having the {{SolrInfoBean.Group.shard}} enum choice (and a potential additional {{SolrInfoBean.Group.replica}} enum choice) we should be able to inspect the class and based on that call the {{setCore(core)}} method e.g. something along the lines of {code} if (SolrCoreReporter.class.isAssignableFrom(reporter.getClass())) { ((SolrCoreReporter)reporter).setCore(core); } {code} which then likely also should permit the removal altogether of the {{SolrInfoBean.Group.shard}} enum choice. > Adding Solr Core Reporter > - > > Key: SOLR-11291 > URL: https://issues.apache.org/jira/browse/SOLR-11291 > Project: Solr > Issue Type: New Feature > Components: metrics >Reporter: Omar Abdelnabi >Priority: Minor > Attachments: SOLR-11291.patch > > > Adds a new reporter, SolrCoreReporter, which allows metrics to be reported on > per-core basis. > Also modifies the SolrMetricManager and SolrCoreMetricManager to take > advantage of this new reporter. > Adds a test/example that uses the SolrCoreReporter. Also adds randomization > to SolrCloudReportersTest. -- 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
[jira] [Commented] (SOLR-11291) Adding Solr Core Reporter
[ https://issues.apache.org/jira/browse/SOLR-11291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16176644#comment-16176644 ] Christine Poerschke commented on SOLR-11291: bq. ... Instead of having the {{SolrInfoBean.Group.shard}} enum choice (and a potential additional {{SolrInfoBean.Group.replica}} enum choice) we should be able to ... which then likely also should permit the removal altogether of the {{SolrInfoBean.Group.shard}} enum choice. [~omar_abdelnabi] and I looked into this further and it seems that removing of the {{SolrInfoBean.Group.shard}} enum choice is not a possibility since [SolrMetricManager.prepareCloudPlugins|https://github.com/apache/lucene-solr/blob/releases/lucene-solr/7.0.0/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java#L1017] uses the enum to decide about creating or not creating of a reporter object. In other words, with the removal of that enum and that logic we would unintentionally create extra SolrMetricReporter objects. So, how could we proceed here then? * In SOLR-11389 I delved into the code paths leading to the {{Solr(Shard|Cluster)Reporter.setCore\[Container\]}} method calls. * I propose to merge the two classes' {{setCore\[Container\]}} method logic into {{init(PluginInfo,\[Solr\]Core\[Container\])}} method variants of the existing {{init(PluginInfo)}} method. * The SOLR-11389 change would subtly change existing behaviour, hence the separation from the changes on this ticket here. * After SOLR-11389 this ticket here would then add an abstract {{SolrCoreReporter}} class _and_ an abstract {{SolrCoreContainerReporter}} class; the {{SolrMetricManager.java}} change required would be very small: {code} - if (reporter instanceof SolrShardReporter) { -((SolrShardReporter)reporter).init(pluginInfo, solrCore); + if (reporter instanceof SolrCoreReporter) { +((SolrCoreReporter)reporter).init(pluginInfo, solrCore); - } else if (reporter instanceof SolrClusterReporter) { -((SolrClusterReporter)reporter).init(pluginInfo, coreContainer); + } else if (reporter instanceof SolrCoreContainerReporter) { +((SolrCoreContainerReporter)reporter).init(pluginInfo, coreContainer); } else { reporter.init(pluginInfo); } {code} How does that sound? > Adding Solr Core Reporter > - > > Key: SOLR-11291 > URL: https://issues.apache.org/jira/browse/SOLR-11291 > Project: Solr > Issue Type: New Feature > Components: metrics >Reporter: Omar Abdelnabi >Assignee: Christine Poerschke >Priority: Minor > Attachments: SOLR-11291.patch > > > Adds a new reporter, SolrCoreReporter, which allows metrics to be reported on > per-core basis. > Also modifies the SolrMetricManager and SolrCoreMetricManager to take > advantage of this new reporter. > Adds a test/example that uses the SolrCoreReporter. Also adds randomization > to SolrCloudReportersTest. -- 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
[jira] [Commented] (SOLR-11291) Adding Solr Core Reporter
[ https://issues.apache.org/jira/browse/SOLR-11291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16268955#comment-16268955 ] ASF subversion and git services commented on SOLR-11291: Commit 812db14f278ce8fe03de94d18e0c662d104f62e3 in lucene-solr's branch refs/heads/master from [~cpoerschke] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=812db14 ] SOLR-11291: Factor out abstract metrics/SolrCore[Container]Reporter classes. (Omar Abdelnabi, Christine Poerschke) > Adding Solr Core Reporter > - > > Key: SOLR-11291 > URL: https://issues.apache.org/jira/browse/SOLR-11291 > Project: Solr > Issue Type: New Feature > Components: metrics >Reporter: Omar Abdelnabi >Assignee: Christine Poerschke >Priority: Minor > Attachments: SOLR-11291.patch, SOLR-11291.patch, SOLR-11291.patch, > SOLR-11291.patch > > > Adds a new reporter, SolrCoreReporter, which allows metrics to be reported on > per-core basis. > Also modifies the SolrMetricManager and SolrCoreMetricManager to take > advantage of this new reporter. > Adds a test/example that uses the SolrCoreReporter. Also adds randomization > to SolrCloudReportersTest. -- 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
[jira] [Commented] (SOLR-11291) Adding Solr Core Reporter
[ https://issues.apache.org/jira/browse/SOLR-11291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16274536#comment-16274536 ] ASF subversion and git services commented on SOLR-11291: Commit a8fbff4d1b8aef343b87a78d0be0fb711a46e53b in lucene-solr's branch refs/heads/branch_7x from [~cpoerschke] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=a8fbff4 ] SOLR-11291: Factor out abstract metrics/SolrCore[Container]Reporter classes. (Omar Abdelnabi, Christine Poerschke) > Adding Solr Core Reporter > - > > Key: SOLR-11291 > URL: https://issues.apache.org/jira/browse/SOLR-11291 > Project: Solr > Issue Type: New Feature > Components: metrics >Reporter: Omar Abdelnabi >Assignee: Christine Poerschke >Priority: Minor > Attachments: SOLR-11291.patch, SOLR-11291.patch, SOLR-11291.patch, > SOLR-11291.patch > > > Adds a new reporter, SolrCoreReporter, which allows metrics to be reported on > per-core basis. > Also modifies the SolrMetricManager and SolrCoreMetricManager to take > advantage of this new reporter. > Adds a test/example that uses the SolrCoreReporter. Also adds randomization > to SolrCloudReportersTest. -- 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
[jira] [Commented] (SOLR-11291) Adding Solr Core Reporter
[ https://issues.apache.org/jira/browse/SOLR-11291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16227290#comment-16227290 ] ASF subversion and git services commented on SOLR-11291: Commit d58ec8728cefe803925fc7497c5a722b7b98700f in lucene-solr's branch refs/heads/master from [~cpoerschke] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d58ec87 ] Remove unnecessary casts in SolrCloudReportersTest. Split out from the SOLR-11291 changes. (Omar Abdelnabi via Christine Poerschke) > Adding Solr Core Reporter > - > > Key: SOLR-11291 > URL: https://issues.apache.org/jira/browse/SOLR-11291 > Project: Solr > Issue Type: New Feature > Components: metrics >Reporter: Omar Abdelnabi >Assignee: Christine Poerschke >Priority: Minor > Attachments: SOLR-11291.patch, SOLR-11291.patch > > > Adds a new reporter, SolrCoreReporter, which allows metrics to be reported on > per-core basis. > Also modifies the SolrMetricManager and SolrCoreMetricManager to take > advantage of this new reporter. > Adds a test/example that uses the SolrCoreReporter. Also adds randomization > to SolrCloudReportersTest. -- 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
[jira] [Commented] (SOLR-11291) Adding Solr Core Reporter
[ https://issues.apache.org/jira/browse/SOLR-11291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16227293#comment-16227293 ] ASF subversion and git services commented on SOLR-11291: Commit 4d638faa46ceafb0c57d9faf753a932043dbda9c in lucene-solr's branch refs/heads/branch_7x from [~cpoerschke] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4d638fa ] Remove unnecessary casts in SolrCloudReportersTest. Split out from the SOLR-11291 changes. (Omar Abdelnabi via Christine Poerschke) > Adding Solr Core Reporter > - > > Key: SOLR-11291 > URL: https://issues.apache.org/jira/browse/SOLR-11291 > Project: Solr > Issue Type: New Feature > Components: metrics >Reporter: Omar Abdelnabi >Assignee: Christine Poerschke >Priority: Minor > Attachments: SOLR-11291.patch, SOLR-11291.patch > > > Adds a new reporter, SolrCoreReporter, which allows metrics to be reported on > per-core basis. > Also modifies the SolrMetricManager and SolrCoreMetricManager to take > advantage of this new reporter. > Adds a test/example that uses the SolrCoreReporter. Also adds randomization > to SolrCloudReportersTest. -- 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