You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2015/05/18 23:41:23 UTC
Review Request 34373: Exposing task reconciliation stats.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34373/
-----------------------------------------------------------
Review request for Aurora, Ben Mahler and Zameer Manji.
Bugs: AURORA-1047
https://issues.apache.org/jira/browse/AURORA-1047
Repository: aurora
Description
-------
Adding counters for the new `StatusChangeResult` values.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java f1e5dd2d47af2570f404d198b3fbdd754174206e
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f4631c1cec4386d813d5869c28289207acf8523f
Diff: https://reviews.apache.org/r/34373/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 34373: Exposing task reconciliation stats.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34373/#review84236
-----------------------------------------------------------
Ship it!
Master (920263b) 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 May 18, 2015, 10:42 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34373/
> -----------------------------------------------------------
>
> (Updated May 18, 2015, 10:42 p.m.)
>
>
> Review request for Aurora, Ben Mahler and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding counters for the new `StatusChangeResult` values.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java f1e5dd2d47af2570f404d198b3fbdd754174206e
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f4631c1cec4386d813d5869c28289207acf8523f
>
> Diff: https://reviews.apache.org/r/34373/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34373: Exposing task reconciliation stats.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On May 19, 2015, 9:46 p.m., Ben Mahler wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, lines 214-217
> > <https://reviews.apache.org/r/34373/diff/2/?file=963238#file963238line214>
> >
> > Any reason not to pass the '`reason`' directly, rather than passing the whole `TaskStatus`?
Just making the helper method more useful by offloading all name extracting logic into it making the consumer code cleaner.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34373/#review84394
-----------------------------------------------------------
On May 18, 2015, 10:42 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34373/
> -----------------------------------------------------------
>
> (Updated May 18, 2015, 10:42 p.m.)
>
>
> Review request for Aurora, Ben Mahler and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding counters for the new `StatusChangeResult` values.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java f1e5dd2d47af2570f404d198b3fbdd754174206e
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f4631c1cec4386d813d5869c28289207acf8523f
>
> Diff: https://reviews.apache.org/r/34373/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34373: Exposing task reconciliation stats.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34373/#review84394
-----------------------------------------------------------
Looks good to me. :)
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/34373/#comment135623>
Any reason not to pass the '`reason`' directly, rather than passing the whole `TaskStatus`?
- Ben Mahler
On May 18, 2015, 10:42 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34373/
> -----------------------------------------------------------
>
> (Updated May 18, 2015, 10:42 p.m.)
>
>
> Review request for Aurora, Ben Mahler and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding counters for the new `StatusChangeResult` values.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java f1e5dd2d47af2570f404d198b3fbdd754174206e
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f4631c1cec4386d813d5869c28289207acf8523f
>
> Diff: https://reviews.apache.org/r/34373/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34373: Exposing task reconciliation stats.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34373/#review84231
-----------------------------------------------------------
Ship it!
Ship It!
- Zameer Manji
On May 18, 2015, 3:42 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34373/
> -----------------------------------------------------------
>
> (Updated May 18, 2015, 3:42 p.m.)
>
>
> Review request for Aurora, Ben Mahler and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding counters for the new `StatusChangeResult` values.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java f1e5dd2d47af2570f404d198b3fbdd754174206e
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f4631c1cec4386d813d5869c28289207acf8523f
>
> Diff: https://reviews.apache.org/r/34373/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34373: Exposing task reconciliation stats.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34373/
-----------------------------------------------------------
(Updated May 18, 2015, 10:42 p.m.)
Review request for Aurora, Ben Mahler and Zameer Manji.
Changes
-------
Zameer's comments.
Bugs: AURORA-1047
https://issues.apache.org/jira/browse/AURORA-1047
Repository: aurora
Description
-------
Adding counters for the new `StatusChangeResult` values.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java f1e5dd2d47af2570f404d198b3fbdd754174206e
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f4631c1cec4386d813d5869c28289207acf8523f
Diff: https://reviews.apache.org/r/34373/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 34373: Exposing task reconciliation stats.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34373/#review84224
-----------------------------------------------------------
Ship it!
Master (920263b) 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 May 18, 2015, 9:41 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34373/
> -----------------------------------------------------------
>
> (Updated May 18, 2015, 9:41 p.m.)
>
>
> Review request for Aurora, Ben Mahler and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding counters for the new `StatusChangeResult` values.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java f1e5dd2d47af2570f404d198b3fbdd754174206e
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f4631c1cec4386d813d5869c28289207acf8523f
>
> Diff: https://reviews.apache.org/r/34373/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34373: Exposing task reconciliation stats.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On May 18, 2015, 10:05 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 204
> > <https://reviews.apache.org/r/34373/diff/1/?file=963203#file963203line204>
> >
> > Special-casing the reconsiliation reason and not the others smells like trouble to me. Why can't we break out stats for all of the reasons?
Thanks, had a similar thought but decided to skip as we already expose reason counters (task_exit_*). It certainly does not hurt to know the whole reason/result matrix. Done.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34373/#review84226
-----------------------------------------------------------
On May 18, 2015, 9:41 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34373/
> -----------------------------------------------------------
>
> (Updated May 18, 2015, 9:41 p.m.)
>
>
> Review request for Aurora, Ben Mahler and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding counters for the new `StatusChangeResult` values.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java f1e5dd2d47af2570f404d198b3fbdd754174206e
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f4631c1cec4386d813d5869c28289207acf8523f
>
> Diff: https://reviews.apache.org/r/34373/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34373: Exposing task reconciliation stats.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34373/#review84226
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/34373/#comment135370>
Special-casing the reconsiliation reason and not the others smells like trouble to me. Why can't we break out stats for all of the reasons?
- Zameer Manji
On May 18, 2015, 2:41 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34373/
> -----------------------------------------------------------
>
> (Updated May 18, 2015, 2:41 p.m.)
>
>
> Review request for Aurora, Ben Mahler and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding counters for the new `StatusChangeResult` values.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java f1e5dd2d47af2570f404d198b3fbdd754174206e
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java f4631c1cec4386d813d5869c28289207acf8523f
>
> Diff: https://reviews.apache.org/r/34373/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>