You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Jordan Ly <jo...@gmail.com> on 2018/04/10 22:47:37 UTC
Review Request 66536: Add more preemption metrics (jobs preempted,
preemptors) and logging statements
-----------------------------------------------------------
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
Re: Review Request 66536: Add more preemption metrics (jobs preempted,
preemptors) and logging statements
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/66536/#comment281825>
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
Posted by Santhosh Kumar Shanmugham <sa...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/66536/#comment281815>
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
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 66536: Add more preemption metrics (jobs preempted,
preemptors) and logging statements
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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
Posted by Jordan Ly <jo...@gmail.com>.
-----------------------------------------------------------
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