You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Mehrdad Nurolahzade <me...@nurolahzade.com> on 2016/12/23 20:24:26 UTC
Review Request 55019: AURORA-1851 Expose stats on JobUpdateAction
transitions
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55019/
-----------------------------------------------------------
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
Re: Review Request 55019: AURORA-1851 Expose stats on JobUpdateAction
transitions
Posted by Mehrdad Nurolahzade <me...@nurolahzade.com>.
> On Dec. 23, 2016, 2:51 p.m., Zameer Manji wrote:
> > 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.
Done.
- Mehrdad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55019/#review160104
-----------------------------------------------------------
On Dec. 23, 2016, 4:52 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, 4:52 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_transition
> % Total % Received % Xferd Average Speed Time Time Time Current
> Dload Upload Total Spent Left Speed
> 100 44363 0 44363 0 0 8451k 0 --:--:-- --:--:-- --:--:-- 8664k
> update_transition_ABORTED 0
> update_transition_ERROR 0
> update_transition_FAILED 0
> update_transition_ROLLED_BACK 0
> update_transition_ROLLED_FORWARD 1
> update_transition_ROLLING_BACK 0
> update_transition_ROLLING_FORWARD 1
> update_transition_ROLL_BACK_AWAITING_PULSE 0
> update_transition_ROLL_BACK_PAUSED 0
> update_transition_ROLL_FORWARD_AWAITING_PULSE 0
> update_transition_ROLL_FORWARD_PAUSED 0
>
> $ 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 44364 0 44364 0 0 3960k 0 --:--:-- --:--:-- --:--:-- 4332k
> update_instance_transition_INSTANCE_ROLLBACK_FAILED 0
> update_instance_transition_INSTANCE_ROLLED_BACK 0
> update_instance_transition_INSTANCE_ROLLING_BACK 0
> update_instance_transition_INSTANCE_UPDATED 2
> update_instance_transition_INSTANCE_UPDATE_FAILED 0
> update_instance_transition_INSTANCE_UPDATING 2
> ```
>
>
> Thanks,
>
> Mehrdad Nurolahzade
>
>
Re: Review Request 55019: AURORA-1851 Expose stats on JobUpdateAction
transitions
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 55019: AURORA-1851 Expose stats on JobUpdateAction
transitions
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55019/#review160158
-----------------------------------------------------------
Ship it!
Ship It!
- Stephan Erb
On Dec. 24, 2016, 1:52 a.m., Mehrdad Nurolahzade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55019/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2016, 1:52 a.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_transition
> % Total % Received % Xferd Average Speed Time Time Time Current
> Dload Upload Total Spent Left Speed
> 100 44363 0 44363 0 0 8451k 0 --:--:-- --:--:-- --:--:-- 8664k
> update_transition_ABORTED 0
> update_transition_ERROR 0
> update_transition_FAILED 0
> update_transition_ROLLED_BACK 0
> update_transition_ROLLED_FORWARD 1
> update_transition_ROLLING_BACK 0
> update_transition_ROLLING_FORWARD 1
> update_transition_ROLL_BACK_AWAITING_PULSE 0
> update_transition_ROLL_BACK_PAUSED 0
> update_transition_ROLL_FORWARD_AWAITING_PULSE 0
> update_transition_ROLL_FORWARD_PAUSED 0
>
> $ 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 44364 0 44364 0 0 3960k 0 --:--:-- --:--:-- --:--:-- 4332k
> update_instance_transition_INSTANCE_ROLLBACK_FAILED 0
> update_instance_transition_INSTANCE_ROLLED_BACK 0
> update_instance_transition_INSTANCE_ROLLING_BACK 0
> update_instance_transition_INSTANCE_UPDATED 2
> update_instance_transition_INSTANCE_UPDATE_FAILED 0
> update_instance_transition_INSTANCE_UPDATING 2
> ```
>
>
> Thanks,
>
> Mehrdad Nurolahzade
>
>
Re: Review Request 55019: AURORA-1851 Expose stats on JobUpdateAction
transitions
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55019/#review160109
-----------------------------------------------------------
Ship it!
Master (e2ca371) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Dec. 24, 2016, 12:52 a.m., Mehrdad Nurolahzade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55019/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2016, 12:52 a.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_transition
> % Total % Received % Xferd Average Speed Time Time Time Current
> Dload Upload Total Spent Left Speed
> 100 44363 0 44363 0 0 8451k 0 --:--:-- --:--:-- --:--:-- 8664k
> update_transition_ABORTED 0
> update_transition_ERROR 0
> update_transition_FAILED 0
> update_transition_ROLLED_BACK 0
> update_transition_ROLLED_FORWARD 1
> update_transition_ROLLING_BACK 0
> update_transition_ROLLING_FORWARD 1
> update_transition_ROLL_BACK_AWAITING_PULSE 0
> update_transition_ROLL_BACK_PAUSED 0
> update_transition_ROLL_FORWARD_AWAITING_PULSE 0
> update_transition_ROLL_FORWARD_PAUSED 0
>
> $ 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 44364 0 44364 0 0 3960k 0 --:--:-- --:--:-- --:--:-- 4332k
> update_instance_transition_INSTANCE_ROLLBACK_FAILED 0
> update_instance_transition_INSTANCE_ROLLED_BACK 0
> update_instance_transition_INSTANCE_ROLLING_BACK 0
> update_instance_transition_INSTANCE_UPDATED 2
> update_instance_transition_INSTANCE_UPDATE_FAILED 0
> update_instance_transition_INSTANCE_UPDATING 2
> ```
>
>
> Thanks,
>
> Mehrdad Nurolahzade
>
>
Re: Review Request 55019: AURORA-1851 Expose stats on JobUpdateAction
transitions
Posted by Mehrdad Nurolahzade <me...@nurolahzade.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55019/
-----------------------------------------------------------
(Updated Dec. 23, 2016, 4:52 p.m.)
Review request for Aurora, David McLaughlin and Zameer Manji.
Changes
-------
Iterate and initialize enum stats to zero
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 (updated)
-----
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 (updated)
-------
```
$ curl 192.168.33.7:8081/vars | grep update_transition
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 44363 0 44363 0 0 8451k 0 --:--:-- --:--:-- --:--:-- 8664k
update_transition_ABORTED 0
update_transition_ERROR 0
update_transition_FAILED 0
update_transition_ROLLED_BACK 0
update_transition_ROLLED_FORWARD 1
update_transition_ROLLING_BACK 0
update_transition_ROLLING_FORWARD 1
update_transition_ROLL_BACK_AWAITING_PULSE 0
update_transition_ROLL_BACK_PAUSED 0
update_transition_ROLL_FORWARD_AWAITING_PULSE 0
update_transition_ROLL_FORWARD_PAUSED 0
$ 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 44364 0 44364 0 0 3960k 0 --:--:-- --:--:-- --:--:-- 4332k
update_instance_transition_INSTANCE_ROLLBACK_FAILED 0
update_instance_transition_INSTANCE_ROLLED_BACK 0
update_instance_transition_INSTANCE_ROLLING_BACK 0
update_instance_transition_INSTANCE_UPDATED 2
update_instance_transition_INSTANCE_UPDATE_FAILED 0
update_instance_transition_INSTANCE_UPDATING 2
```
Thanks,
Mehrdad Nurolahzade
Re: Review Request 55019: AURORA-1851 Expose stats on JobUpdateAction
transitions
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55019/#review160102
-----------------------------------------------------------
Ship it!
Master (e2ca371) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Dec. 23, 2016, 8: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, 8: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
>
>