Re: Review Request 66536: Add more preemption metrics (jobs preempted, preemptors) and logging statements

2018-04-11 Thread Aurora ReviewBot 

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


Ship it!




Master (2d6108b) 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 April 11, 2018, 1:36 p.m., Jordan Ly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66536/
> ---
> 
> (Updated April 11, 2018, 1:36 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and 
> Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added additional metrics:
> ```
> 1. preemptor_tasks_preempted_[JOB_NAME] - The number of times [JOB_NAME] has 
> been preempted for another task.
> 2. preemptor_tasks_preemptor_[JOB_NAME] - The number of times [JOB_NAME] has 
> preempted another task.
> 3. preemptor_slot_search_[success|failed]_for_[JOB_NAME] - The number of 
> times [JOB_NAME] has or hasn't found a slot for preemption.
> 4. preemptor_slot_validation_[success|failed]_for_[JOB_NAME] - The number of 
> times [JOB_NAME] succeeded to or failed to validate a slot before preemption.
> ```
> 
> Additionally, added some `LOG.info` statements for better visibility into 
> preemption/preemption slot finding.
> 
> Did a little bit of code refactoring as well.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessor.java 
> ef06471d007b1d36300eea30cdea059c1ba231b0 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  569cfe6b04e6b7bf0dca7625b00698e9d8e47daf 
>   src/main/java/org/apache/aurora/scheduler/preemptor/Preemptor.java 
> 293d106eee383dd5352a629780b897d58c9dd439 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorMetrics.java 
> 87305774db0ce6fb7ebed060ab4dc99be6c2df4c 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImpl.java 
> edab03dfd7fdbb24891565ba755212f03d6ea3b8 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  ba775f4688dc57504e2def0dc4b5dcd00da448e1 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b3ffb0d4fc9b9b52bb49225765bd14fb8105169a 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
> 0ef29d598784ce529bcaac7017dc0f2cc5055938 
> 
> 
> Diff: https://reviews.apache.org/r/66536/diff/3/
> 
> 
> Testing
> ---
> 
> Added unit tests, `./gradlew test` passes.
> Manually ensured new metrics are exported.
> Tested at scale.
> 
> 
> Thanks,
> 
> Jordan Ly
> 
>

Re: Review Request 66536: Add more preemption metrics (jobs preempted, preemptors) and logging statements

2018-04-11 Thread Jordan Ly 

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

(Updated April 11, 2018, 8:36 p.m.)


Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and 
Stephan Erb.


Changes
---

Removed extraneous comment, log candidates chosen for preemption.


Repository: aurora


Description
---

Added additional metrics:
```
1. preemptor_tasks_preempted_[JOB_NAME] - The number of times [JOB_NAME] has 
been preempted for another task.
2. preemptor_tasks_preemptor_[JOB_NAME] - The number of times [JOB_NAME] has 
preempted another task.
3. preemptor_slot_search_[success|failed]_for_[JOB_NAME] - The number of times 
[JOB_NAME] has or hasn't found a slot for preemption.
4. preemptor_slot_validation_[success|failed]_for_[JOB_NAME] - The number of 
times [JOB_NAME] succeeded to or failed to validate a slot before preemption.
```

Additionally, added some `LOG.info` statements for better visibility into 
preemption/preemption slot finding.

Did a little bit of code refactoring as well.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessor.java 
ef06471d007b1d36300eea30cdea059c1ba231b0 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
569cfe6b04e6b7bf0dca7625b00698e9d8e47daf 
  src/main/java/org/apache/aurora/scheduler/preemptor/Preemptor.java 
293d106eee383dd5352a629780b897d58c9dd439 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorMetrics.java 
87305774db0ce6fb7ebed060ab4dc99be6c2df4c 
  src/main/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImpl.java 
edab03dfd7fdbb24891565ba755212f03d6ea3b8 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
 ba775f4688dc57504e2def0dc4b5dcd00da448e1 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 b3ffb0d4fc9b9b52bb49225765bd14fb8105169a 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
0ef29d598784ce529bcaac7017dc0f2cc5055938 


Diff: https://reviews.apache.org/r/66536/diff/2/

Changes: https://reviews.apache.org/r/66536/diff/1-2/


Testing
---

Added unit tests, `./gradlew test` passes.
Manually ensured new metrics are exported.
Tested at scale.


Thanks,

Jordan Ly

Re: Review Request 66536: Add more preemption metrics (jobs preempted, preemptors) and logging statements

2018-04-11 Thread Stephan Erb 

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


Ship it!




Looks useful, thanks!


src/main/java/org/apache/aurora/scheduler/preemptor/Preemptor.java
Line 86 (original), 88 (patched)


Witht the removal of the iterator this is a bit outdated.


- Stephan Erb


On April 11, 2018, 12:47 a.m., Jordan Ly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66536/
> ---
> 
> (Updated April 11, 2018, 12:47 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and 
> Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added additional metrics:
> ```
> 1. preemptor_tasks_preempted_[JOB_NAME] - The number of times [JOB_NAME] has 
> been preempted for another task.
> 2. preemptor_tasks_preemptor_[JOB_NAME] - The number of times [JOB_NAME] has 
> preempted another task.
> 3. preemptor_slot_search_[success|failed]_for_[JOB_NAME] - The number of 
> times [JOB_NAME] has or hasn't found a slot for preemption.
> 4. preemptor_slot_validation_[success|failed]_for_[JOB_NAME] - The number of 
> times [JOB_NAME] succeeded to or failed to validate a slot before preemption.
> ```
> 
> Additionally, added some `LOG.info` statements for better visibility into 
> preemption/preemption slot finding.
> 
> Did a little bit of code refactoring as well.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessor.java 
> ef06471d007b1d36300eea30cdea059c1ba231b0 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  569cfe6b04e6b7bf0dca7625b00698e9d8e47daf 
>   src/main/java/org/apache/aurora/scheduler/preemptor/Preemptor.java 
> 293d106eee383dd5352a629780b897d58c9dd439 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorMetrics.java 
> 87305774db0ce6fb7ebed060ab4dc99be6c2df4c 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImpl.java 
> edab03dfd7fdbb24891565ba755212f03d6ea3b8 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  ba775f4688dc57504e2def0dc4b5dcd00da448e1 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b3ffb0d4fc9b9b52bb49225765bd14fb8105169a 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
> 0ef29d598784ce529bcaac7017dc0f2cc5055938 
> 
> 
> Diff: https://reviews.apache.org/r/66536/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit tests, `./gradlew test` passes.
> Manually ensured new metrics are exported.
> Tested at scale.
> 
> 
> Thanks,
> 
> Jordan Ly
> 
>

Re: Review Request 66536: Add more preemption metrics (jobs preempted, preemptors) and logging statements

2018-04-11 Thread Santhosh Kumar Shanmugham 

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


Ship it!





src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessor.java
Lines 194 (patched)


Include candidates as well.


- Santhosh Kumar Shanmugham


On April 10, 2018, 3:47 p.m., Jordan Ly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66536/
> ---
> 
> (Updated April 10, 2018, 3:47 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and 
> Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added additional metrics:
> ```
> 1. preemptor_tasks_preempted_[JOB_NAME] - The number of times [JOB_NAME] has 
> been preempted for another task.
> 2. preemptor_tasks_preemptor_[JOB_NAME] - The number of times [JOB_NAME] has 
> preempted another task.
> 3. preemptor_slot_search_[success|failed]_for_[JOB_NAME] - The number of 
> times [JOB_NAME] has or hasn't found a slot for preemption.
> 4. preemptor_slot_validation_[success|failed]_for_[JOB_NAME] - The number of 
> times [JOB_NAME] succeeded to or failed to validate a slot before preemption.
> ```
> 
> Additionally, added some `LOG.info` statements for better visibility into 
> preemption/preemption slot finding.
> 
> Did a little bit of code refactoring as well.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessor.java 
> ef06471d007b1d36300eea30cdea059c1ba231b0 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  569cfe6b04e6b7bf0dca7625b00698e9d8e47daf 
>   src/main/java/org/apache/aurora/scheduler/preemptor/Preemptor.java 
> 293d106eee383dd5352a629780b897d58c9dd439 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorMetrics.java 
> 87305774db0ce6fb7ebed060ab4dc99be6c2df4c 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImpl.java 
> edab03dfd7fdbb24891565ba755212f03d6ea3b8 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  ba775f4688dc57504e2def0dc4b5dcd00da448e1 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b3ffb0d4fc9b9b52bb49225765bd14fb8105169a 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
> 0ef29d598784ce529bcaac7017dc0f2cc5055938 
> 
> 
> Diff: https://reviews.apache.org/r/66536/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit tests, `./gradlew test` passes.
> Manually ensured new metrics are exported.
> Tested at scale.
> 
> 
> Thanks,
> 
> Jordan Ly
> 
>

Re: Review Request 66536: Add more preemption metrics (jobs preempted, preemptors) and logging statements

2018-04-10 Thread Aurora ReviewBot 

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


Ship it!




Master (2d6108b) 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 April 10, 2018, 10:47 p.m., Jordan Ly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66536/
> ---
> 
> (Updated April 10, 2018, 10:47 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and 
> Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Added additional metrics:
> ```
> 1. preemptor_tasks_preempted_[JOB_NAME] - The number of times [JOB_NAME] has 
> been preempted for another task.
> 2. preemptor_tasks_preemptor_[JOB_NAME] - The number of times [JOB_NAME] has 
> preempted another task.
> 3. preemptor_slot_search_[success|failed]_for_[JOB_NAME] - The number of 
> times [JOB_NAME] has or hasn't found a slot for preemption.
> 4. preemptor_slot_validation_[success|failed]_for_[JOB_NAME] - The number of 
> times [JOB_NAME] succeeded to or failed to validate a slot before preemption.
> ```
> 
> Additionally, added some `LOG.info` statements for better visibility into 
> preemption/preemption slot finding.
> 
> Did a little bit of code refactoring as well.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessor.java 
> ef06471d007b1d36300eea30cdea059c1ba231b0 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  569cfe6b04e6b7bf0dca7625b00698e9d8e47daf 
>   src/main/java/org/apache/aurora/scheduler/preemptor/Preemptor.java 
> 293d106eee383dd5352a629780b897d58c9dd439 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorMetrics.java 
> 87305774db0ce6fb7ebed060ab4dc99be6c2df4c 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImpl.java 
> edab03dfd7fdbb24891565ba755212f03d6ea3b8 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  ba775f4688dc57504e2def0dc4b5dcd00da448e1 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b3ffb0d4fc9b9b52bb49225765bd14fb8105169a 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
> 0ef29d598784ce529bcaac7017dc0f2cc5055938 
> 
> 
> Diff: https://reviews.apache.org/r/66536/diff/1/
> 
> 
> Testing
> ---
> 
> Added unit tests, `./gradlew test` passes.
> Manually ensured new metrics are exported.
> Tested at scale.
> 
> 
> Thanks,
> 
> Jordan Ly
> 
>

Review Request 66536: Add more preemption metrics (jobs preempted, preemptors) and logging statements

2018-04-10 Thread Jordan Ly 

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

Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and 
Stephan Erb.


Repository: aurora


Description
---

Added additional metrics:
```
1. preemptor_tasks_preempted_[JOB_NAME] - The number of times [JOB_NAME] has 
been preempted for another task.
2. preemptor_tasks_preemptor_[JOB_NAME] - The number of times [JOB_NAME] has 
preempted another task.
3. preemptor_slot_search_[success|failed]_for_[JOB_NAME] - The number of times 
[JOB_NAME] has or hasn't found a slot for preemption.
4. preemptor_slot_validation_[success|failed]_for_[JOB_NAME] - The number of 
times [JOB_NAME] succeeded to or failed to validate a slot before preemption.
```

Additionally, added some `LOG.info` statements for better visibility into 
preemption/preemption slot finding.

Did a little bit of code refactoring as well.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessor.java 
ef06471d007b1d36300eea30cdea059c1ba231b0 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
569cfe6b04e6b7bf0dca7625b00698e9d8e47daf 
  src/main/java/org/apache/aurora/scheduler/preemptor/Preemptor.java 
293d106eee383dd5352a629780b897d58c9dd439 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorMetrics.java 
87305774db0ce6fb7ebed060ab4dc99be6c2df4c 
  src/main/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImpl.java 
edab03dfd7fdbb24891565ba755212f03d6ea3b8 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
 ba775f4688dc57504e2def0dc4b5dcd00da448e1 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 b3ffb0d4fc9b9b52bb49225765bd14fb8105169a 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
0ef29d598784ce529bcaac7017dc0f2cc5055938 


Diff: https://reviews.apache.org/r/66536/diff/1/


Testing
---

Added unit tests, `./gradlew test` passes.
Manually ensured new metrics are exported.
Tested at scale.


Thanks,

Jordan Ly