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


Ship it!




This LGTM.

One issue I always find with these metrics is that they are dynamically 
generated (Based on enum name). However we do not initialize all of these 
values to 0 initially. This means that some of the enum values do not appear 
until at least one action has occured.

Would it be possible to also intitalize these metrics to 0?

To do this, I think all you have to do is iterate of the `.values()` method of 
the enum and prime the cache. That would create the counters.

- Zameer Manji


On Dec. 23, 2016, 12:24 p.m., Mehrdad Nurolahzade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55019/
> -----------------------------------------------------------
> 
> (Updated Dec. 23, 2016, 12:24 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: AURORA-1851
>     https://issues.apache.org/jira/browse/AURORA-1851
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> AURORA-1851 Expose stats on JobUpdateAction transitions
> 
> Introduced new stats that exposes `JobUpdateAction` transitions.
> 
> Refactored away from `CachedCounters` for existing metric; it was dynamically 
> generating new String objects (through concatenation) per stats collection 
> event.
> 
> Fixed for a mistake in a previous changeset 
> (https://reviews.apache.org/r/55003/); removed unnecessary checked 
> `Exception` on `CacheLoader.load()`.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> e0698a3c5bb6c3cea1cc94934fcf303c4f2e692d 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> 7621facdf1efc5d09473e0dc09fbb9c520248c7b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  d01dc3f5aa86775d777232dcda20c1ae787fcd42 
> 
> Diff: https://reviews.apache.org/r/55019/diff/
> 
> 
> Testing
> -------
> 
> ```
> curl 192.168.33.7:8081/vars | grep update_instance_transition
>   % Total    % Received % Xferd  Average Speed   Time    Time     Time  
> Current
>                                  Dload  Upload   Total   Spent    Left  Speed
> 100 45206    0 45206    0     0  6331k      0 --:--:-- --:--:-- --:--:-- 7357k
> update_instance_transition_INSTANCE_UPDATED 2
> update_instance_transition_INSTANCE_UPDATING 2
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Reply via email to