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/03/28 04:32:20 UTC

Review Request 32597: Improving async preemptor efficiency.

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

Review request for Aurora, Bill Farner and Zameer Manji.


Bugs: AURORA-1219
    https://issues.apache.org/jira/browse/AURORA-1219


Repository: aurora


Description
-------

This is addressing the preemption efficiency loss due to making preemptor async. Summary:
- Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
- Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
- Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.

Will not apply cleanly. Diffed against https://reviews.apache.org/r/32352/


Diffs
-----

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 

Diff: https://reviews.apache.org/r/32597/diff/


Testing
-------

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review78744
-----------------------------------------------------------


Master (a7b95d9) is red with this patch.
  ./build-support/jenkins/build.sh

:check
:build
:api:assemble
:api:compileTestJava UP-TO-DATE
:api:processTestResources UP-TO-DATE
:api:testClasses UP-TO-DATE
:api:test UP-TO-DATE
:api:check UP-TO-DATE
:api:build
:buildSrc:compileJava UP-TO-DATE
:buildSrc:processResources UP-TO-DATE
:buildSrc:classes UP-TO-DATE
:buildSrc:jar
:buildSrc:assemble
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build

BUILD SUCCESSFUL

Total time: 4 mins 1.746 secs
+ export PIP_DEFAULT_TIMEOUT=60
+ PIP_DEFAULT_TIMEOUT=60
+ mkdir -p third_party
+ pip install -d third_party -r /dev/fd/63
++ grep -v mesos.native 3rdparty/python/requirements.txt
Downloading/unpacking bottle==0.11.6 (from -r /dev/fd/63 (line 1))
  Saved ./third_party/bottle-0.11.6-py2.py3-none-any.whl
Downloading/unpacking CherryPy==3.6.0 (from -r /dev/fd/63 (line 2))

pip can't proceed with requirement 'CherryPy==3.6.0 (from -r /dev/fd/63 (line 2))' due to a pre-existing build directory.
 location: /tmp/user/2395/pip_build_jenkins/CherryPy
This is likely due to a previous installation that failed.
pip is being responsible and not assuming it can delete this.
Please delete it and try again.

Storing debug log for failure in /home/jenkins/.pip/pip.log


I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On April 2, 2015, 11:39 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 2, 2015, 11:39 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
> - Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review78947
-----------------------------------------------------------

Ship it!


Inverting the loop is a great idea.


src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java
<https://reviews.apache.org/r/32597/#comment128063>

    A ticket should be filed to ensure we do not lose track of this improvement.


- Zameer Manji


On April 3, 2015, 1:04 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 3, 2015, 1:04 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
> - Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On April 16, 2015, 11:08 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 183
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line183>
> >
> >     Don't we want `pendingGroups.remove(group)`?

This will only remove a single occurence while we want to delete all of them.


- Maxim


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


On April 16, 2015, 1:39 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 16, 2015, 1:39 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review80396
-----------------------------------------------------------

Ship it!



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130285>

    Don't we want `pendingGroups.remove(group)`?


- Zameer Manji


On April 15, 2015, 6:39 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 6:39 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Bill Farner <wf...@apache.org>.

> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, lines 142-144
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line142>
> >
> >     ```
> >     Set<String> allSlaves = Sets.newHashSet(Iterables.concat(
> >         slavesToOffers.keySet(),
> >         slavesToActiveTasks.keySet()));
> >     ```
> 
> Maxim Khutornenko wrote:
>     Not opposed to the change but how is this better exactly? The way it's currently written will iterate over all available slaves only once whereas the proposed change will have to do it twice (at least the way I read it).

Are you assuming that `Iterables.concat` eagerly iterates over the inputs?  If so, that is false:

http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/collect/Iterables.html#concat(java.lang.Iterable...)

I prefer the snippet i've posted since it uses a single statement for a single logical entity.  Of course, if that has the side-effect of signifcantly worse performance, it is not worth it.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java, line 96
> > <https://reviews.apache.org/r/32597/diff/5/?file=931276#file931276line96>
> >
> >     Should have brought this up in a previous review, but i'm uncomfortable with using a mock to control the behavior of a data structure.  It really feels like the test knows way too much about the internals of the class at this point, given that this is internally-managed state.  Is there any particular reason we shouldn't use a concrete `BiCache` instance here?
> 
> Maxim Khutornenko wrote:
>     Refactored to use concrete instance.

Thanks!!


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 184
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line184>
> >
> >     It's probably not worth changing code, but might be worth noting that this breaks round-robin, since the position is reset.
> 
> Maxim Khutornenko wrote:
>     Not sure what you mean. The whole reason to have a consuming iterator here is to ensure the position is not reset when unsatisfiable groups are removed.

Aha, thanks - it slipped my mind that the consuming iterator and removeAll were affecting the same data.  You're right.


- Bill


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


On April 21, 2015, 1:12 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 21, 2015, 1:12 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, lines 142-144
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line142>
> >
> >     ```
> >     Set<String> allSlaves = Sets.newHashSet(Iterables.concat(
> >         slavesToOffers.keySet(),
> >         slavesToActiveTasks.keySet()));
> >     ```

Not opposed to the change but how is this better exactly? The way it's currently written will iterate over all available slaves only once whereas the proposed change will have to do it twice (at least the way I read it).


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 149
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line149>
> >
> >     "Reservations are made for at most one task group per slave."
> >     
> >     This reads as though you might make multiple reservations on a slave, so long as they are from different task groups.

Rephrased.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 150
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line150>
> >
> >     "neither of the slaves"  what does that mean?

Rephrased.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 184
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line184>
> >
> >     It's probably not worth changing code, but might be worth noting that this breaks round-robin, since the position is reset.

Not sure what you mean. The whole reason to have a consuming iterator here is to ensure the position is not reset when unsatisfiable groups are removed.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 193
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line193>
> >
> >     Given that this is all private/local code, consider `HashMultiset.create(Iterable)` here and avoid the immutable copy -> mutable copy dance.

Makes sense. Done.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 215
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line215>
> >
> >     Less efficient when there are many task groups, but i find this easier to read:
> >     
> >     ```
> >     List<TaskGroupKey> instructions = Lists.newLinkedList();
> >     Set<TaskGroupKey> keys = ImmutableSet.copyOf(groups.elementSet());
> >     while (!groups.isEmpty()) {
> >       for (TaskGroupKey key : keys) {
> >         if (groups.contains(key)) {
> >           instructions.add(key);
> >           // Removes a single instance of key.
> >           groups.remove(key);
> >         }
> >       }
> >     }
> >     ```

Sure, done.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java, line 23
> > <https://reviews.apache.org/r/32597/diff/5/?file=931269#file931269line23>
> >
> >     a small doc would be nice here

Added.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java, line 92
> > <https://reviews.apache.org/r/32597/diff/5/?file=931271#file931271line92>
> >
> >     `slaveId`
> >     
> >     https://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s5.3-camel-case

No idea how it got like that. Changed.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java, line 294
> > <https://reviews.apache.org/r/32597/diff/5/?file=931276#file931276line294>
> >
> >     `@Nullable String slaveId`

Added.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java, line 298
> > <https://reviews.apache.org/r/32597/diff/5/?file=931276#file931276line298>
> >
> >     I'd like to avoid randomness in tests, as trivial as it may seem here.  It's reasonable to assume that calling `makeTask()` with identical arguments should at least produce an `equals()` object.  I'd rather see a task id argument to make that true.

Changed.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java, line 80
> > <https://reviews.apache.org/r/32597/diff/5/?file=931277#file931277line80>
> >
> >     I don't think anyObject() is what you want, that's for matching arguments.  This returns `null`.

Done.


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java, line 96
> > <https://reviews.apache.org/r/32597/diff/5/?file=931276#file931276line96>
> >
> >     Should have brought this up in a previous review, but i'm uncomfortable with using a mock to control the behavior of a data structure.  It really feels like the test knows way too much about the internals of the class at this point, given that this is internally-managed state.  Is there any particular reason we shouldn't use a concrete `BiCache` instance here?

Refactored to use concrete instance.


- Maxim


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


On April 16, 2015, 1:39 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 16, 2015, 1:39 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, lines 142-144
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line142>
> >
> >     ```
> >     Set<String> allSlaves = Sets.newHashSet(Iterables.concat(
> >         slavesToOffers.keySet(),
> >         slavesToActiveTasks.keySet()));
> >     ```
> 
> Maxim Khutornenko wrote:
>     Not opposed to the change but how is this better exactly? The way it's currently written will iterate over all available slaves only once whereas the proposed change will have to do it twice (at least the way I read it).
> 
> Bill Farner wrote:
>     Are you assuming that `Iterables.concat` eagerly iterates over the inputs?  If so, that is false:
>     
>     http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/collect/Iterables.html#concat(java.lang.Iterable...)
>     
>     I prefer the snippet i've posted since it uses a single statement for a single logical entity.  Of course, if that has the side-effect of signifcantly worse performance, it is not worth it.

Benchmarked both approaches and could not find any difference in perf. Taking your suggestion.


- Maxim


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


On April 21, 2015, 1:12 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 21, 2015, 1:12 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review80480
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130383>

    ```
    Set<String> allSlaves = Sets.newHashSet(Iterables.concat(
        slavesToOffers.keySet(),
        slavesToActiveTasks.keySet()));
    ```



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130384>

    "Reservations are made for at most one task group per slave."
    
    This reads as though you might make multiple reservations on a slave, so long as they are from different task groups.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130385>

    "neither of the slaves"  what does that mean?



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130388>

    It's probably not worth changing code, but might be worth noting that this breaks round-robin, since the position is reset.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130393>

    Given that this is all private/local code, consider `HashMultiset.create(Iterable)` here and avoid the immutable copy -> mutable copy dance.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130404>

    Less efficient when there are many task groups, but i find this easier to read:
    
    ```
    List<TaskGroupKey> instructions = Lists.newLinkedList();
    Set<TaskGroupKey> keys = ImmutableSet.copyOf(groups.elementSet());
    while (!groups.isEmpty()) {
      for (TaskGroupKey key : keys) {
        if (groups.contains(key)) {
          instructions.add(key);
          // Removes a single instance of key.
          groups.remove(key);
        }
      }
    }
    ```



src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java
<https://reviews.apache.org/r/32597/#comment130405>

    a small doc would be nice here



src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java
<https://reviews.apache.org/r/32597/#comment130406>

    `slaveId`
    
    https://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s5.3-camel-case



src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
<https://reviews.apache.org/r/32597/#comment130413>

    Should have brought this up in a previous review, but i'm uncomfortable with using a mock to control the behavior of a data structure.  It really feels like the test knows way too much about the internals of the class at this point, given that this is internally-managed state.  Is there any particular reason we shouldn't use a concrete `BiCache` instance here?



src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
<https://reviews.apache.org/r/32597/#comment130414>

    `@Nullable String slaveId`



src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
<https://reviews.apache.org/r/32597/#comment130415>

    I'd like to avoid randomness in tests, as trivial as it may seem here.  It's reasonable to assume that calling `makeTask()` with identical arguments should at least produce an `equals()` object.  I'd rather see a task id argument to make that true.



src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
<https://reviews.apache.org/r/32597/#comment130417>

    I don't think anyObject() is what you want, that's for matching arguments.  This returns `null`.



src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
<https://reviews.apache.org/r/32597/#comment130427>

    Related to the `OfferManager.andReturn` mock above, you could return a constant there, and expect the same constant here.  Then you get to drop all the `eq()`s.


- Bill Farner


On April 16, 2015, 1:39 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 16, 2015, 1:39 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review81085
-----------------------------------------------------------

Ship it!


Master (d10d2d1) 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 21, 2015, 9:15 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 21, 2015, 9:15 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
-----------------------------------------------------------

(Updated April 21, 2015, 9:15 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Bill's comments.


Bugs: AURORA-1219
    https://issues.apache.org/jira/browse/AURORA-1219


Repository: aurora


Description
-------

This is addressing the preemption efficiency loss due to making preemptor async. Summary:
- Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
- Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 

Diff: https://reviews.apache.org/r/32597/diff/


Testing
-------

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review80873
-----------------------------------------------------------

Ship it!


Master (4b90339) 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 21, 2015, 1:12 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 21, 2015, 1:12 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review81030
-----------------------------------------------------------

Ship it!


LGTM overall, but i'd like to converge on the `Sets.newHashSet` usage before this lands.

- Bill Farner


On April 21, 2015, 1:12 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 21, 2015, 1:12 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
-----------------------------------------------------------

(Updated April 21, 2015, 1:12 a.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Bill's comments.


Bugs: AURORA-1219
    https://issues.apache.org/jira/browse/AURORA-1219


Repository: aurora


Description
-------

This is addressing the preemption efficiency loss due to making preemptor async. Summary:
- Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
- Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 

Diff: https://reviews.apache.org/r/32597/diff/


Testing
-------

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review80292
-----------------------------------------------------------

Ship it!


Master (b18dc44) 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 16, 2015, 1:39 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 16, 2015, 1:39 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
-----------------------------------------------------------

(Updated April 16, 2015, 1:39 a.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Bill's comments.


Bugs: AURORA-1219
    https://issues.apache.org/jira/browse/AURORA-1219


Repository: aurora


Description
-------

This is addressing the preemption efficiency loss due to making preemptor async. Summary:
- Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
- Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 

Diff: https://reviews.apache.org/r/32597/diff/


Testing
-------

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review80253
-----------------------------------------------------------

Ship it!


Master (10e75fc) 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 15, 2015, 7:05 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 7:05 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On April 15, 2015, 7:58 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 140
> > <https://reviews.apache.org/r/32597/diff/4/?file=930944#file930944line140>
> >
> >     How about
> >     ```
> >     ImmutableSet.copyOf(Sets.union(
> >         slavesToOffers.keySet(),
> >         slavesToActiveTasks.keySet()));
> >     ```

I actually need a mutable set here to support slave removal.


> On April 15, 2015, 7:58 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 185
> > <https://reviews.apache.org/r/32597/diff/4/?file=930944#file930944line185>
> >
> >     Did you consider using a `Multiset` instead?
> >     
> >     `ImmutableMultiset.copyOf(FluentIterable...);`

That's a great suggestion! Reduced some complexity in task filtering.


> On April 15, 2015, 7:58 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 198
> > <https://reviews.apache.org/r/32597/diff/4/?file=930944#file930944line198>
> >
> >     Either here or at the call site, please call out the reason we arrange elements this way.

Done.


> On April 15, 2015, 7:58 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 203
> > <https://reviews.apache.org/r/32597/diff/4/?file=930944#file930944line203>
> >
> >     `splice` could be semantically confused with the javascript function: http://www.w3schools.com/jsref/jsref_splice.asp
> >     
> >     Consider a name specific to the use case, like `getPreemptionSequence`.

Done.


> On April 15, 2015, 7:58 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java, line 23
> > <https://reviews.apache.org/r/32597/diff/4/?file=930945#file930945line23>
> >
> >     Since there's already diff churn - do you think `PreemptionProposal` is a better name?

No strong preference. Changed to proposal.


> On April 15, 2015, 7:58 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java, line 134
> > <https://reviews.apache.org/r/32597/diff/4/?file=930946#file930946line134>
> >
> >     This is a pretty gnarly method signature.  Do you think there's opportunity to generalize things so you can hide the distinction between `Optional<HostOffer>` and `Iterable<PreemptionVictim>`?

Well, it may make signature a bit tidier but I think we will certainly lose on readability. Given the multitude of types involved here I'd rather prefer keeping things concrete.


- Maxim


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


On April 15, 2015, 7:05 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 7:05 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review80256
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130058>

    How about
    ```
    ImmutableSet.copyOf(Sets.union(
        slavesToOffers.keySet(),
        slavesToActiveTasks.keySet()));
    ```



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130061>

    This is a point where a higher-level comment explaining the algorithm would be really helpful.  Up to this point, you're gathering requisite state, but now you're clearly making a conscious decision with the consumingIterator.  If you call out some of the goals of the algorithm before this, it will make the implementation decisions more obvious.
    
    To be specific - you should call out things like each preemption round will place at most one task per slave.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130065>

    Did you consider using a `Multiset` instead?
    
    `ImmutableMultiset.copyOf(FluentIterable...);`



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130063>

    Either here or at the call site, please call out the reason we arrange elements this way.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment130064>

    `splice` could be semantically confused with the javascript function: http://www.w3schools.com/jsref/jsref_splice.asp
    
    Consider a name specific to the use case, like `getPreemptionSequence`.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java
<https://reviews.apache.org/r/32597/#comment130066>

    Since there's already diff churn - do you think `PreemptionProposal` is a better name?



src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java
<https://reviews.apache.org/r/32597/#comment130067>

    This is a pretty gnarly method signature.  Do you think there's opportunity to generalize things so you can hide the distinction between `Optional<HostOffer>` and `Iterable<PreemptionVictim>`?


- Bill Farner


On April 15, 2015, 7:05 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 15, 2015, 7:05 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
-----------------------------------------------------------

(Updated April 15, 2015, 7:05 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Reworked algorithm to take advantage of generalized TaskGroup pending pool and get rid of circular buffer.


Bugs: AURORA-1219
    https://issues.apache.org/jira/browse/AURORA-1219


Repository: aurora


Description (updated)
-------

This is addressing the preemption efficiency loss due to making preemptor async. Summary:
- Implemented fair task processing by evaluating pending task groups in round-robin fashion (e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
- Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 00919b7910704c5025465e1071378a978e5e60a3 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java f16f964f56f0f9da523950891293083f1bd86780 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java eed2de99a145dd2124b7f2b4d401214f1d8adf2e 

Diff: https://reviews.apache.org/r/32597/diff/


Testing
-------

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review78828
-----------------------------------------------------------

Ship it!


Master (9e46dd8) 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 3, 2015, 8:04 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 3, 2015, 8:04 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
> - Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
-----------------------------------------------------------

(Updated April 3, 2015, 8:04 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Bill's comments.


Bugs: AURORA-1219
    https://issues.apache.org/jira/browse/AURORA-1219


Repository: aurora


Description
-------

This is addressing the preemption efficiency loss due to making preemptor async. Summary:
- Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
- Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
- Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 

Diff: https://reviews.apache.org/r/32597/diff/


Testing
-------

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Zameer Manji <zm...@apache.org>.

> On April 2, 2015, 8:37 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java, line 68
> > <https://reviews.apache.org/r/32597/diff/2/?file=914223#file914223line68>
> >
> >     What's the motivation behind the ordering?  It also seems somewhat contradictory along with the class doc "fair evaluation order of all task groups regardless of their task count".
> 
> Maxim Khutornenko wrote:
>     The idea was to give larger jobs a bigger chance to succeed. E.g. in case tasks from different groups equally qualify for a slave, a task from a larger job wins. I am not entirely sold on that idea though. Perhaps we can drop sorting alltogether for simplicity?

+1 I think we should also drop this sorting for simplicity. I also don't think we prefer large jobs anywhere else in our scheduling so we should not prefer them here as well/


- Zameer


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


On April 3, 2015, 1:04 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 3, 2015, 1:04 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
> - Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.

> On April 3, 2015, 3:37 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java, line 39
> > <https://reviews.apache.org/r/32597/diff/2/?file=914223#file914223line39>
> >
> >     `{@link TaskGroupKey}`

Done.


> On April 3, 2015, 3:37 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java, lines 44-51
> > <https://reviews.apache.org/r/32597/diff/2/?file=914223#file914223line44>
> >
> >     Javadoc does not preserve this formatting.  You'll want to wrap in a `{@code xxx}`
> >     
> >     or `<pre>`.

Done.


> On April 3, 2015, 3:37 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java, line 68
> > <https://reviews.apache.org/r/32597/diff/2/?file=914223#file914223line68>
> >
> >     What's the motivation behind the ordering?  It also seems somewhat contradictory along with the class doc "fair evaluation order of all task groups regardless of their task count".

The idea was to give larger jobs a bigger chance to succeed. E.g. in case tasks from different groups equally qualify for a slave, a task from a larger job wins. I am not entirely sold on that idea though. Perhaps we can drop sorting alltogether for simplicity?


> On April 3, 2015, 3:37 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java, line 90
> > <https://reviews.apache.org/r/32597/diff/2/?file=914223#file914223line90>
> >
> >     s/java.lang//, here and elsewhere

Done.


> On April 3, 2015, 3:37 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 138
> > <https://reviews.apache.org/r/32597/diff/2/?file=914224#file914224line138>
> >
> >     I can't think of a downside if the outer loop were over pending tasks rather than slaves.  The upside would be that we could avoid the `AttributeAggregate` cachine added in this diff.
> >     
> >     Is there something i'm not thinking about?
> >     
> >     *note* If you act on this comment, the `Iterable` comment is no longer relevant.

This approach simlifies the algorithm and imporoves efficiency as the outer loop is traversed only once. If we are to switch loops we would have to maintain some kind of circular buffer for both loops to avoid restarting slave iteration from head when a task/slave match is detected. 

Also, there will be extra lookup required to avoid repeatedly matching a slave that has already yielded a slot in previous iterations.


> On April 3, 2015, 3:37 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, lines 144-150
> > <https://reviews.apache.org/r/32597/diff/2/?file=914224#file914224line144>
> >
> >     Could this be simplified by making `PeekingTaskIterator` an `Iterable`, and have each `Iterator` pass through the groups exactly once?  i.e. perhaps a circular buffer isn't the right model here, since you're having to work around it.
> >     
> >     This would also allow the code here to be ignorant of `TaskGroupKey`, since you could support `Iterator#remove()`.

The main reason for this is fairness. If we always restart from head, some tail groups may starve. The solution is to maintain iteration state within a circular buffer to give all groups a similar share.


- Maxim


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


On April 2, 2015, 11:39 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 2, 2015, 11:39 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
> - Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review78759
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java
<https://reviews.apache.org/r/32597/#comment127784>

    `{@link TaskGroupKey}`



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java
<https://reviews.apache.org/r/32597/#comment127783>

    Javadoc does not preserve this formatting.  You'll want to wrap in a `{@code xxx}`
    
    or `<pre>`.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java
<https://reviews.apache.org/r/32597/#comment127785>

    What's the motivation behind the ordering?  It also seems somewhat contradictory along with the class doc "fair evaluation order of all task groups regardless of their task count".



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java
<https://reviews.apache.org/r/32597/#comment127786>

    s/java.lang//, here and elsewhere



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment127788>

    I can't think of a downside if the outer loop were over pending tasks rather than slaves.  The upside would be that we could avoid the `AttributeAggregate` cachine added in this diff.
    
    Is there something i'm not thinking about?
    
    *note* If you act on this comment, the `Iterable` comment is no longer relevant.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
<https://reviews.apache.org/r/32597/#comment127787>

    Could this be simplified by making `PeekingTaskIterator` an `Iterable`, and have each `Iterator` pass through the groups exactly once?  i.e. perhaps a circular buffer isn't the right model here, since you're having to work around it.
    
    This would also allow the code here to be ignorant of `TaskGroupKey`, since you could support `Iterator#remove()`.


- Bill Farner


On April 2, 2015, 11:39 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 2, 2015, 11:39 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
> - Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
-----------------------------------------------------------

(Updated April 2, 2015, 11:39 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Bugs: AURORA-1219
    https://issues.apache.org/jira/browse/AURORA-1219


Repository: aurora


Description (updated)
-------

This is addressing the preemption efficiency loss due to making preemptor async. Summary:
- Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
- Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
- Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.


Diffs
-----

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 

Diff: https://reviews.apache.org/r/32597/diff/


Testing
-------

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
-----------------------------------------------------------

(Updated April 2, 2015, 11:39 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Rebased.


Bugs: AURORA-1219
    https://issues.apache.org/jira/browse/AURORA-1219


Repository: aurora


Description
-------

This is addressing the preemption efficiency loss due to making preemptor async. Summary:
- Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
- Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
- Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.

Will not apply cleanly. Diffed against https://reviews.apache.org/r/32352/


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 427c0de205c28540b217e817bdbab10b4af5aee4 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 77617ec11119a2bcd062d5b80cd2b4c58dc80514 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 1092c055588363794b37a965fb2f17a6e50d92d7 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
  src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 

Diff: https://reviews.apache.org/r/32597/diff/


Testing
-------

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32597: Improving async preemptor efficiency.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review78150
-----------------------------------------------------------


This patch does not apply cleanly on master (3ab8a9e), do you need to rebase?

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On March 28, 2015, 3:32 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated March 28, 2015, 3:32 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending tasks in the order of their TaskGroupKeys (largest count first)
> - Inverted the traversal of pending tasks to better reuse loop invariants. Every slave is sized up against all pending tasks until a match is found
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The rest is refactoring to adjust to the new traversal approach.
> 
> Will not apply cleanly. Diffed against https://reviews.apache.org/r/32352/
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>