[jira] [Commented] (SOLR-4735) Improve Solr metrics reporting

2016-12-02 Thread Kelvin Wong (JIRA) 

[ 
https://issues.apache.org/jira/browse/SOLR-4735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15716698#comment-15716698
 ] 

Kelvin Wong commented on SOLR-4735:
---

Hmm wouldn't these aggregate registries defeat the point of keeping them 
separate in the first place (from a performance perspective)? For example, if a 
user configures a JMXReporter and a GraphiteReporter on all registries, Solr 
would have to basically make two copies of all of its registries.

Perhaps we can just "fake" an aggregate reporter? There can be configuration 
logic so that one reporter is instantiated for each registry that the user 
configured. This might be a bit wasteful but we won't have to deal with 
maintaining an aggregate registry or writing reporters that do the aggregation. 
And to the user, it seems as though they only needed to configure one reporter.

> Improve Solr metrics reporting
> --
>
> Key: SOLR-4735
> URL: https://issues.apache.org/jira/browse/SOLR-4735
> Project: Solr
>  Issue Type: Improvement
>Reporter: Alan Woodward
>Assignee: Andrzej Bialecki 
>Priority: Minor
> Attachments: SOLR-4735.patch, SOLR-4735.patch, SOLR-4735.patch, 
> SOLR-4735.patch
>
>
> Following on from a discussion on the mailing list:
> http://search-lucene.com/m/IO0EI1qdyJF1/codahale=Solr+metrics+in+Codahale+metrics+and+Graphite+
> It would be good to make Solr play more nicely with existing devops 
> monitoring systems, such as Graphite or Ganglia.  Stats monitoring at the 
> moment is poll-only, either via JMX or through the admin stats page.  I'd 
> like to refactor things a bit to make this more pluggable.
> This patch is a start.  It adds a new interface, InstrumentedBean, which 
> extends SolrInfoMBean to return a 
> [[Metrics|http://metrics.codahale.com/manual/core/]] MetricRegistry, and a 
> couple of MetricReporters (which basically just duplicate the JMX and admin 
> page reporting that's there at the moment, but which should be more 
> extensible).  The patch includes a change to RequestHandlerBase showing how 
> this could work.  The idea would be to eventually replace the getStatistics() 
> call on SolrInfoMBean with this instead.
> The next step would be to allow more MetricReporters to be defined in 
> solrconfig.xml.  The Metrics library comes with ganglia and graphite 
> reporting modules, and we can add contrib plugins for both of those.
> There's some more general cleanup that could be done around SolrInfoMBean 
> (we've got two plugin handlers at /mbeans and /plugins that basically do the 
> same thing, and the beans themselves have some weirdly inconsistent data on 
> them - getVersion() returns different things for different impls, and 
> getSource() seems pretty useless), but maybe that's for another issue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

[jira] [Commented] (SOLR-4735) Improve Solr metrics reporting

2016-12-02 Thread Kelvin Wong (JIRA) 

[ 
https://issues.apache.org/jira/browse/SOLR-4735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15715107#comment-15715107
 ] 

Kelvin Wong commented on SOLR-4735:
---

Hi Andrzej, 

{quote}
I added a notion of "group" of metrics, which corresponds to a top-level 
subsystem in a Solr node
{quote}
* Nice! I really like this concept. Will we be instantiating separate reporters 
for each `Group` then? That way, reporting can be more flexibly configured. 
(ex. Jetty goes to JMX and Graphite, JVM goes to only JMX, etc...)

{quote}
I'll look into reusing single global-level reporters when possible, and 
creating new instances only if there are per-collection overrides.
{quote}

* It looks like 
[JmxReporter|https://github.com/dropwizard/metrics/blob/3.2-development/metrics-core/src/main/java/com/codahale/metrics/JmxReporter.java#L701]
 takes only one `MetricRegistry` at a time (and 
[GraphiteReporter|https://github.com/dropwizard/metrics/blob/3.2-development/metrics-graphite/src/main/java/com/codahale/metrics/graphite/GraphiteReporter.java#L145],
 etc. for that matter). Will we need to build some sort of 
`AggregateMetricRegistry` to join each core's registries? Or do you have 
something else in mind?
* On a separate note, it would be nice if we could just specify which registry 
we'd like a reporter to attach to. So for example, we can attach one reporter 
to `collection1`, another to `zookeeper`, and one more to `jvm`. These are at 
different levels in the metrics hierarchy but perhaps we can just pass in the 
registry's name as part of the config for a reporter?


> Improve Solr metrics reporting
> --
>
> Key: SOLR-4735
> URL: https://issues.apache.org/jira/browse/SOLR-4735
> Project: Solr
>  Issue Type: Improvement
>Reporter: Alan Woodward
>Assignee: Andrzej Bialecki 
>Priority: Minor
> Attachments: SOLR-4735.patch, SOLR-4735.patch, SOLR-4735.patch, 
> SOLR-4735.patch
>
>
> Following on from a discussion on the mailing list:
> http://search-lucene.com/m/IO0EI1qdyJF1/codahale=Solr+metrics+in+Codahale+metrics+and+Graphite+
> It would be good to make Solr play more nicely with existing devops 
> monitoring systems, such as Graphite or Ganglia.  Stats monitoring at the 
> moment is poll-only, either via JMX or through the admin stats page.  I'd 
> like to refactor things a bit to make this more pluggable.
> This patch is a start.  It adds a new interface, InstrumentedBean, which 
> extends SolrInfoMBean to return a 
> [[Metrics|http://metrics.codahale.com/manual/core/]] MetricRegistry, and a 
> couple of MetricReporters (which basically just duplicate the JMX and admin 
> page reporting that's there at the moment, but which should be more 
> extensible).  The patch includes a change to RequestHandlerBase showing how 
> this could work.  The idea would be to eventually replace the getStatistics() 
> call on SolrInfoMBean with this instead.
> The next step would be to allow more MetricReporters to be defined in 
> solrconfig.xml.  The Metrics library comes with ganglia and graphite 
> reporting modules, and we can add contrib plugins for both of those.
> There's some more general cleanup that could be done around SolrInfoMBean 
> (we've got two plugin handlers at /mbeans and /plugins that basically do the 
> same thing, and the beans themselves have some weirdly inconsistent data on 
> them - getVersion() returns different things for different impls, and 
> getSource() seems pretty useless), but maybe that's for another issue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

[jira] [Commented] (SOLR-4735) Improve Solr metrics reporting

2016-11-25 Thread Kelvin Wong (JIRA) 

[ 
https://issues.apache.org/jira/browse/SOLR-4735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15695483#comment-15695483
 ] 

Kelvin Wong commented on SOLR-4735:
---

{quote}
Having a reload reset stats is a bad idea. We can provide an explicit API to 
reset stats for a node or a core if required.
{quote}
Right now, each producer creates/manages its own set of metrics. It might make 
sense to have some global object creating/managing all our metrics instead. 
Each producer can then call {{getOrCreate}} so that the same set of metrics are 
used across core reloads. My only concern here is how to namespace the metrics 
so that different producers clash on metric names. Perhaps give each producer 
access to the core name?

{quote}
Perhaps SolrMetricManager should use long-lived MetricRegistry instances that 
are managed in SharedMetricsRegistries?
{quote}
Sounds good. One suggestion I have is to namespace the registries. For example, 
we can have each {{SolrCore}} report on a registry, Jetty report on another, 
JVM on another, etc... Then we can configure reporters for each such registry 
by just specifying its name. This keeps the different sets of metrics nicely 
isolated and gives us flexibility as to how to report each set of metrics?

> Improve Solr metrics reporting
> --
>
> Key: SOLR-4735
> URL: https://issues.apache.org/jira/browse/SOLR-4735
> Project: Solr
>  Issue Type: Improvement
>Reporter: Alan Woodward
>Assignee: Andrzej Bialecki 
>Priority: Minor
> Attachments: SOLR-4735.patch, SOLR-4735.patch, SOLR-4735.patch, 
> SOLR-4735.patch
>
>
> Following on from a discussion on the mailing list:
> http://search-lucene.com/m/IO0EI1qdyJF1/codahale=Solr+metrics+in+Codahale+metrics+and+Graphite+
> It would be good to make Solr play more nicely with existing devops 
> monitoring systems, such as Graphite or Ganglia.  Stats monitoring at the 
> moment is poll-only, either via JMX or through the admin stats page.  I'd 
> like to refactor things a bit to make this more pluggable.
> This patch is a start.  It adds a new interface, InstrumentedBean, which 
> extends SolrInfoMBean to return a 
> [[Metrics|http://metrics.codahale.com/manual/core/]] MetricRegistry, and a 
> couple of MetricReporters (which basically just duplicate the JMX and admin 
> page reporting that's there at the moment, but which should be more 
> extensible).  The patch includes a change to RequestHandlerBase showing how 
> this could work.  The idea would be to eventually replace the getStatistics() 
> call on SolrInfoMBean with this instead.
> The next step would be to allow more MetricReporters to be defined in 
> solrconfig.xml.  The Metrics library comes with ganglia and graphite 
> reporting modules, and we can add contrib plugins for both of those.
> There's some more general cleanup that could be done around SolrInfoMBean 
> (we've got two plugin handlers at /mbeans and /plugins that basically do the 
> same thing, and the beans themselves have some weirdly inconsistent data on 
> them - getVersion() returns different things for different impls, and 
> getSource() seems pretty useless), but maybe that's for another issue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

[jira] [Commented] (SOLR-4735) Improve Solr metrics reporting

2016-11-24 Thread Kelvin Wong (JIRA) 

[ 
https://issues.apache.org/jira/browse/SOLR-4735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15693036#comment-15693036
 ] 

Kelvin Wong commented on SOLR-4735:
---

Thanks Jeff. 

The attached patch really just piggybacks off of Alan's work and tries to flesh 
out the design. For now, only RequestHandlerBase exposes metrics through this 
framework. The idea is to eventually convert other SolrInfoMBeans into 
SolrMetricProducers so they can start providing reportable metrics too. This 
seems to be fairly doable.

Re SharedMetricsRegistries: that's something we can definitely do. My rationale 
for not using it is so that logical groups of metrics can be nicely isolated at 
a per-core level. This ensures that any metric in a MetricManager's registry 
must have been registered through MetricManager::registerMetrics. A nice 
side-effect is that we can also store meta-information about each metric and 
pass that on to the reporters.

I realize that using SharedMetricsRegistries provides a level of flexibility 
that this patch's approach does not. For example, if we wanted to share the 
registry on a CoreContainer level. I think there are ways around this and my 
personal preference is still for this logical grouping of metrics. But perhaps 
there may be use cases I'm neglecting to consider?

Would be interested to hear your thoughts on this. Thanks!

> Improve Solr metrics reporting
> --
>
> Key: SOLR-4735
> URL: https://issues.apache.org/jira/browse/SOLR-4735
> Project: Solr
>  Issue Type: Improvement
>Reporter: Alan Woodward
>Assignee: Alan Woodward
>Priority: Minor
> Attachments: SOLR-4735.patch, SOLR-4735.patch, SOLR-4735.patch, 
> SOLR-4735.patch
>
>
> Following on from a discussion on the mailing list:
> http://search-lucene.com/m/IO0EI1qdyJF1/codahale=Solr+metrics+in+Codahale+metrics+and+Graphite+
> It would be good to make Solr play more nicely with existing devops 
> monitoring systems, such as Graphite or Ganglia.  Stats monitoring at the 
> moment is poll-only, either via JMX or through the admin stats page.  I'd 
> like to refactor things a bit to make this more pluggable.
> This patch is a start.  It adds a new interface, InstrumentedBean, which 
> extends SolrInfoMBean to return a 
> [[Metrics|http://metrics.codahale.com/manual/core/]] MetricRegistry, and a 
> couple of MetricReporters (which basically just duplicate the JMX and admin 
> page reporting that's there at the moment, but which should be more 
> extensible).  The patch includes a change to RequestHandlerBase showing how 
> this could work.  The idea would be to eventually replace the getStatistics() 
> call on SolrInfoMBean with this instead.
> The next step would be to allow more MetricReporters to be defined in 
> solrconfig.xml.  The Metrics library comes with ganglia and graphite 
> reporting modules, and we can add contrib plugins for both of those.
> There's some more general cleanup that could be done around SolrInfoMBean 
> (we've got two plugin handlers at /mbeans and /plugins that basically do the 
> same thing, and the beans themselves have some weirdly inconsistent data on 
> them - getVersion() returns different things for different impls, and 
> getSource() seems pretty useless), but maybe that's for another issue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

[jira] [Commented] (SOLR-8785) Use Metrics library for core metrics

2016-11-23 Thread Kelvin Wong (JIRA) 

[ 
https://issues.apache.org/jira/browse/SOLR-8785?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15691044#comment-15691044
 ] 

Kelvin Wong commented on SOLR-8785:
---

No problem! Great to see this resolved.

> Use Metrics library for core metrics
> 
>
> Key: SOLR-8785
> URL: https://issues.apache.org/jira/browse/SOLR-8785
> Project: Solr
>  Issue Type: Improvement
>Affects Versions: 4.1
>Reporter: Jeff Wartes
>Assignee: Shalin Shekhar Mangar
>  Labels: patch, patch-available
> Fix For: master (7.0), 6.4
>
> Attachments: SOLR-8785-increment.patch, SOLR-8785.patch, 
> SOLR-8785.patch, SOLR-8785.patch, SOLR_8775_rates_per_minute_fix.patch
>
>
> The Metrics library (https://dropwizard.github.io/metrics/3.1.0/) is a 
> well-known way to track metrics about applications. 
> In SOLR-1972, latency percentile tracking was added. The comment list is 
> long, so here’s my synopsis:
> 1. An attempt was made to use the Metrics library
> 2. That attempt failed due to a memory leak in Metrics v2.1.1
> 3. Large parts of Metrics were then copied wholesale into the 
> org.apache.solr.util.stats package space and that was used instead.
> Copy/pasting Metrics code into Solr may have been the correct solution at the 
> time, but I submit that it isn’t correct any more. 
> The leak in Metrics was fixed even before SOLR-1972 was released, and by 
> copy/pasting a subset of the functionality, we miss access to other important 
> things that the Metrics library provides, particularly the concept of a 
> Reporter. (https://dropwizard.github.io/metrics/3.1.0/manual/core/#reporters)
> Further, Metrics v3.0.2 is already packaged with Solr anyway, because it’s 
> used in two contrib modules. (map-reduce and morphines-core)
> I’m proposing that:
> 1. Metrics as bundled with Solr be upgraded to the current v3.1.2
> 2. Most of the org.apache.solr.util.stats package space be deleted outright, 
> or gutted and replaced with simple calls to Metrics. Due to the copy/paste 
> origin, the concepts should mostly map 1:1.
> I’d further recommend a usage pattern like:
> SharedMetricRegistries.getOrCreate(System.getProperty(“solr.metrics.registry”,
>  “solr-registry”))
> There are all kinds of areas in Solr that could benefit from metrics tracking 
> and reporting. This pattern allows diverse areas of code to track metrics 
> within a single, named registry. This well-known-name then becomes a handle 
> you can use to easily attach a Reporter and ship all of those metrics off-box.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

[jira] [Commented] (SOLR-8785) Use Metrics library for core metrics

2016-11-23 Thread Kelvin Wong (JIRA) 

[ 
https://issues.apache.org/jira/browse/SOLR-8785?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15690859#comment-15690859
 ] 

Kelvin Wong commented on SOLR-8785:
---

I think the RequestHandlerBase and AnalyticsStatisticsCollector previously used 
per-second rates; only the OverseerStatusCmd uses per-minute rates. 
Standardizing all rates to per-minute will break backwards compatibility with 
those classes. Apologies for not being clearer earlier.

> Use Metrics library for core metrics
> 
>
> Key: SOLR-8785
> URL: https://issues.apache.org/jira/browse/SOLR-8785
> Project: Solr
>  Issue Type: Improvement
>Affects Versions: 4.1
>Reporter: Jeff Wartes
>Assignee: Shalin Shekhar Mangar
>  Labels: patch, patch-available
> Fix For: master (7.0), 6.4
>
> Attachments: SOLR-8785-increment.patch, SOLR-8785.patch, 
> SOLR-8785.patch, SOLR-8785.patch, SOLR_8775_rates_per_minute_fix.patch
>
>
> The Metrics library (https://dropwizard.github.io/metrics/3.1.0/) is a 
> well-known way to track metrics about applications. 
> In SOLR-1972, latency percentile tracking was added. The comment list is 
> long, so here’s my synopsis:
> 1. An attempt was made to use the Metrics library
> 2. That attempt failed due to a memory leak in Metrics v2.1.1
> 3. Large parts of Metrics were then copied wholesale into the 
> org.apache.solr.util.stats package space and that was used instead.
> Copy/pasting Metrics code into Solr may have been the correct solution at the 
> time, but I submit that it isn’t correct any more. 
> The leak in Metrics was fixed even before SOLR-1972 was released, and by 
> copy/pasting a subset of the functionality, we miss access to other important 
> things that the Metrics library provides, particularly the concept of a 
> Reporter. (https://dropwizard.github.io/metrics/3.1.0/manual/core/#reporters)
> Further, Metrics v3.0.2 is already packaged with Solr anyway, because it’s 
> used in two contrib modules. (map-reduce and morphines-core)
> I’m proposing that:
> 1. Metrics as bundled with Solr be upgraded to the current v3.1.2
> 2. Most of the org.apache.solr.util.stats package space be deleted outright, 
> or gutted and replaced with simple calls to Metrics. Due to the copy/paste 
> origin, the concepts should mostly map 1:1.
> I’d further recommend a usage pattern like:
> SharedMetricRegistries.getOrCreate(System.getProperty(“solr.metrics.registry”,
>  “solr-registry”))
> There are all kinds of areas in Solr that could benefit from metrics tracking 
> and reporting. This pattern allows diverse areas of code to track metrics 
> within a single, named registry. This well-known-name then becomes a handle 
> you can use to easily attach a Reporter and ship all of those metrics off-box.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

[jira] [Comment Edited] (SOLR-8785) Use Metrics library for core metrics

2016-11-23 Thread Kelvin Wong (JIRA) 

[ 
https://issues.apache.org/jira/browse/SOLR-8785?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15690240#comment-15690240
 ] 

Kelvin Wong edited comment on SOLR-8785 at 11/23/16 2:28 PM:
-

Thanks Shalin, Jeff, and Christine!

One problem I noticed is that Codahale Timers report per-second rates by 
default. In the patch above, we've wrongly named these rates as per-minute 
rates in TimerUtils.java. Because we cannot configure the Timer to report on a 
per-minute basis, I propose we standardize the rates to be reported on a 
per-second basis. Otherwise, we would need a utility function to do the 
conversion.

Please see below for a potential fix. What this means now is that:
- Timer rates are reported in a standard per-second basis. This changes 
existing behaviour in OverseerStatusCmd.java which used to report in a 
per-minute basis.
- In RequestHandlerBase.java and AnalyticsStatisticsCollector.java, the naming 
of the rates metrics is changed. For example, from '5minRateReqsPerSecond' to 
'5minRateRequestsPerSecond'. This keeps the naming consistent with the 
'avgRequestsPerSecond'.

solr/core/src/java/org/apache/solr/util/stats/TimerUtils.java
{code}
- lst.add("avgRequestsPerMinute", timer.getMeanRate());
- lst.add("5minRateRequestsPerMinute", timer.getFiveMinuteRate());
- lst.add("15minRateRequestsPerMinute", timer.getFifteenMinuteRate());
+ lst.add("avgRequestsPerSecond", timer.getMeanRate());
+ lst.add("5minRateRequestsPerSecond", timer.getFiveMinuteRate());
+ lst.add("15minRateRequestsPerSecond", timer.getFifteenMinuteRate());
{code}

solr/core/src/test/org/apache/solr/util/stats/TimerUtilsTest.java
{code}
- // cannot test avgRequestsPerMinute directly because mean rate changes as 
time increases!
- // assertEquals(lst.get("avgRequestsPerMinute"), timer.getMeanRate());
- assertEquals(lst.get("5minRateRequestsPerMinute"), timer.getFiveMinuteRate());
- assertEquals(lst.get("15minRateRequestsPerMinute"), 
timer.getFifteenMinuteRate());
+ // cannot test avgRequestsPerSecond directly because mean rate changes as 
time increases!
+ // assertEquals(lst.get("avgRequestsPerSecond"), timer.getMeanRate());
+ assertEquals(lst.get("5minRateRequestsPerSecond"), timer.getFiveMinuteRate());
+ assertEquals(lst.get("15minRateRequestsPerSecond"), 
timer.getFifteenMinuteRate());
{code}


was (Author: kwong494):
Thanks Shalin, Jeff, and Christine!

One problem I noticed is that Codahale Timers report per-second rates by 
default. In the patch above, we've wrongly named these rates as per-minute 
rates in TimerUtils.java. Because we cannot configure the Timer to report on a 
per-minute basis, I propose we standardize the rates to be reported on a 
per-second basis. Otherwise, we would need a utility function to do the 
conversion.

Please see below for a potential fix. What this means now is that:
- Timer rates are reported in a standard per-second basis. This changes 
existing behaviour in OverseerStatusCmd.java which used to report in a 
per-minute basis.
- In RequestHandlerBase.java and AnalyticsStatisticsCollector.java, the naming 
of the rates metrics is changed. For example, from '5minRateReqsPerSecond' to 
'5minRateRequestsPerSecond'. This keeps the naming consistent with the 
'avgRequestsPerSecond'.

solr/core/src/java/org/apache/solr/util/stats/TimerUtils.java
{code}
-  lst.add("avgRequestsPerMinute", timer.getMeanRate());
-  lst.add("5minRateRequestsPerMinute", timer.getFiveMinuteRate());
-  lst.add("15minRateRequestsPerMinute", timer.getFifteenMinuteRate());
+ lst.add("avgRequestsPerSecond", timer.getMeanRate());
+ lst.add("5minRateRequestsPerSecond", timer.getFiveMinuteRate());
+ lst.add("15minRateRequestsPerSecond", timer.getFifteenMinuteRate());
{code}

solr/core/src/test/org/apache/solr/util/stats/TimerUtilsTest.java
{code}
-  // cannot test avgRequestsPerMinute directly because mean rate changes as 
time increases!
-  // assertEquals(lst.get("avgRequestsPerMinute"), timer.getMeanRate());
-  assertEquals(lst.get("5minRateRequestsPerMinute"), 
timer.getFiveMinuteRate());
-  assertEquals(lst.get("15minRateRequestsPerMinute"), 
timer.getFifteenMinuteRate());
+ // cannot test avgRequestsPerSecond directly because mean rate changes as 
time increases!
+ // assertEquals(lst.get("avgRequestsPerSecond"), timer.getMeanRate());
+ assertEquals(lst.get("5minRateRequestsPerSecond"), timer.getFiveMinuteRate());
+ assertEquals(lst.get("15minRateRequestsPerSecond"), 
timer.getFifteenMinuteRate());
{code}

> Use Metrics library for core metrics
> 
>
> Key: SOLR-8785
> URL: https://issues.apache.org/jira/browse/SOLR-8785
> Project: Solr
>  Issue Type: Improvement
>Affects Versions: 4.1
>Reporter: Jeff Wartes
>Assignee: Shalin Shekhar Mangar
>  Labels: patch, patch-available
> Fix For: master

[jira] [Commented] (SOLR-8785) Use Metrics library for core metrics

2016-11-23 Thread Kelvin Wong (JIRA) 

[ 
https://issues.apache.org/jira/browse/SOLR-8785?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15690240#comment-15690240
 ] 

Kelvin Wong commented on SOLR-8785:
---

Thanks Shalin, Jeff, and Christine!

One problem I noticed is that Codahale Timers report per-second rates by 
default. In the patch above, we've wrongly named these rates as per-minute 
rates in TimerUtils.java. Because we cannot configure the Timer to report on a 
per-minute basis, I propose we standardize the rates to be reported on a 
per-second basis. Otherwise, we would need a utility function to do the 
conversion.

Please see below for a potential fix. What this means now is that:
- Timer rates are reported in a standard per-second basis. This changes 
existing behaviour in OverseerStatusCmd.java which used to report in a 
per-minute basis.
- In RequestHandlerBase.java and AnalyticsStatisticsCollector.java, the naming 
of the rates metrics is changed. For example, from '5minRateReqsPerSecond' to 
'5minRateRequestsPerSecond'. This keeps the naming consistent with the 
'avgRequestsPerSecond'.

solr/core/src/java/org/apache/solr/util/stats/TimerUtils.java
{code}
-  lst.add("avgRequestsPerMinute", timer.getMeanRate());
-  lst.add("5minRateRequestsPerMinute", timer.getFiveMinuteRate());
-  lst.add("15minRateRequestsPerMinute", timer.getFifteenMinuteRate());
+ lst.add("avgRequestsPerSecond", timer.getMeanRate());
+ lst.add("5minRateRequestsPerSecond", timer.getFiveMinuteRate());
+ lst.add("15minRateRequestsPerSecond", timer.getFifteenMinuteRate());
{code}

solr/core/src/test/org/apache/solr/util/stats/TimerUtilsTest.java
{code}
-  // cannot test avgRequestsPerMinute directly because mean rate changes as 
time increases!
-  // assertEquals(lst.get("avgRequestsPerMinute"), timer.getMeanRate());
-  assertEquals(lst.get("5minRateRequestsPerMinute"), 
timer.getFiveMinuteRate());
-  assertEquals(lst.get("15minRateRequestsPerMinute"), 
timer.getFifteenMinuteRate());
+ // cannot test avgRequestsPerSecond directly because mean rate changes as 
time increases!
+ // assertEquals(lst.get("avgRequestsPerSecond"), timer.getMeanRate());
+ assertEquals(lst.get("5minRateRequestsPerSecond"), timer.getFiveMinuteRate());
+ assertEquals(lst.get("15minRateRequestsPerSecond"), 
timer.getFifteenMinuteRate());
{code}

> Use Metrics library for core metrics
> 
>
> Key: SOLR-8785
> URL: https://issues.apache.org/jira/browse/SOLR-8785
> Project: Solr
>  Issue Type: Improvement
>Affects Versions: 4.1
>Reporter: Jeff Wartes
>Assignee: Shalin Shekhar Mangar
>  Labels: patch, patch-available
> Fix For: master (7.0), 6.4
>
> Attachments: SOLR-8785-increment.patch, SOLR-8785.patch, 
> SOLR-8785.patch, SOLR-8785.patch
>
>
> The Metrics library (https://dropwizard.github.io/metrics/3.1.0/) is a 
> well-known way to track metrics about applications. 
> In SOLR-1972, latency percentile tracking was added. The comment list is 
> long, so here’s my synopsis:
> 1. An attempt was made to use the Metrics library
> 2. That attempt failed due to a memory leak in Metrics v2.1.1
> 3. Large parts of Metrics were then copied wholesale into the 
> org.apache.solr.util.stats package space and that was used instead.
> Copy/pasting Metrics code into Solr may have been the correct solution at the 
> time, but I submit that it isn’t correct any more. 
> The leak in Metrics was fixed even before SOLR-1972 was released, and by 
> copy/pasting a subset of the functionality, we miss access to other important 
> things that the Metrics library provides, particularly the concept of a 
> Reporter. (https://dropwizard.github.io/metrics/3.1.0/manual/core/#reporters)
> Further, Metrics v3.0.2 is already packaged with Solr anyway, because it’s 
> used in two contrib modules. (map-reduce and morphines-core)
> I’m proposing that:
> 1. Metrics as bundled with Solr be upgraded to the current v3.1.2
> 2. Most of the org.apache.solr.util.stats package space be deleted outright, 
> or gutted and replaced with simple calls to Metrics. Due to the copy/paste 
> origin, the concepts should mostly map 1:1.
> I’d further recommend a usage pattern like:
> SharedMetricRegistries.getOrCreate(System.getProperty(“solr.metrics.registry”,
>  “solr-registry”))
> There are all kinds of areas in Solr that could benefit from metrics tracking 
> and reporting. This pattern allows diverse areas of code to track metrics 
> within a single, named registry. This well-known-name then becomes a handle 
> you can use to easily attach a Reporter and ship all of those metrics off-box.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org