[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
