-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53687/#review155765
-----------------------------------------------------------



Some high level feedback.
* Remove the ticket from the title and the comment and attach it to the review 
by filling out the bugs field.
* Please create a more detailed description. Use 
https://github.com/apache/aurora/commit/793711f56d83d532240cc5ca39404ff504b14430
 as inspiration.
* Please add tests for this code.
* Can you please also show an example stat that shows up in `/vars`?


commons/src/main/java/org/apache/aurora/common/stats/SlidingTimedStats.java 
(line 23)
<https://reviews.apache.org/r/53687/#comment225853>

    Remove `@author` we don't use this in our code base.



commons/src/main/java/org/apache/aurora/common/stats/SlidingTimedStats.java 
(line 28)
<https://reviews.apache.org/r/53687/#comment225854>

    What's the rationale for these values?
    
    I don't think p10 is useful to track for every single method call.



commons/src/main/java/org/apache/aurora/common/stats/SlidingTimedStats.java 
(line 51)
<https://reviews.apache.org/r/53687/#comment225855>

    What's the point of this constructor?
    
    Why would we want to disable percentile reording here?


- Zameer Manji


On Nov. 11, 2016, 5:49 p.m., Reza Motamedi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53687/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2016, 5:49 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> track percentile values for timed sliding stats. AURORA-118
> 
> 
> Diffs
> -----
> 
>   commons/src/main/java/org/apache/aurora/common/inject/TimedInterceptor.java 
> a4c4240b67cafb108ca587ad86d9fb9d9bb1553d 
>   commons/src/main/java/org/apache/aurora/common/stats/SlidingStats.java 
> f7a5ae41e307627fc55157758e9b7cdd861c3268 
>   commons/src/main/java/org/apache/aurora/common/stats/SlidingTimedStats.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53687/diff/
> 
> 
> Testing
> -------
> 
> This is a patch for the request in 
> https://issues.apache.org/jira/browse/AURORA-118.
> 
> SlidingTimedStats extends SlidingStats and contains aPercentile for tracking 
> a finer view of the timing stats. Percentile was already implemented to track 
> RequestStats and I reused it to track any timing stats captured via @Timed 
> annotation.
> 
> 
> Thanks,
> 
> Reza Motamedi
> 
>

Reply via email to