You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Kelvin Wong (JIRA)" <ji...@apache.org> on 2016/11/23 14:28:58 UTC
[jira] [Comment Edited] (SOLR-8785) Use Metrics library for core
metrics
[ https://issues.apache.org/jira/browse/SOLR-8785?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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 (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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org