Re: Review Request 34373: Exposing task reconciliation stats.

2015-05-19 Thread Maxim Khutornenko 


> On May 19, 2015, 9:46 p.m., Ben Mahler wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, lines 
> > 214-217
> > 
> >
> > 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.

2015-05-19 Thread Ben Mahler 

---
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


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.

2015-05-18 Thread Aurora ReviewBot 

---
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.

2015-05-18 Thread Zameer Manji 

---
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.

2015-05-18 Thread Maxim Khutornenko 

---
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.

2015-05-18 Thread Maxim Khutornenko 


> On May 18, 2015, 10:05 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 204
> > 
> >
> > 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.

2015-05-18 Thread Zameer Manji 

---
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


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
> 
>

Re: Review Request 34373: Exposing task reconciliation stats.

2015-05-18 Thread Aurora ReviewBot 

---
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
> 
>

Review Request 34373: Exposing task reconciliation stats.

2015-05-18 Thread Maxim Khutornenko 

---
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