You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Zhitao Li <zh...@gmail.com> on 2016/01/13 02:34:16 UTC
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 1:34 a.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
Better code and test organized by Will's comments, making all lists in the review immutable.
Also updatd review summyar and description.
Summary (updated)
-----------------
New class to allocate resources of multiple roles from offer.
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description (updated)
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by John Sirois <js...@apache.org>.
> On Jan. 12, 2016, 6:49 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/Resources.java, line 50
> > <https://reviews.apache.org/r/42126/diff/3/?file=1195149#file1195149line50>
> >
> > Looks like an overly-aggressive find/replace. I've been endlessly frustrated by intellij doing this when you forget to untick the "search text and comments" option (which is always ticked).
> >
> > Please skim through the patch to find other spots impacted.
>
> John Sirois wrote:
> In IntelliJ settings, search for "Find Usages Settings" - I map this to `ctrl-alt-shift-7` - pops up dialog with scope and seach in text or not and other options.
Oh - oops, you're issue is having to untick. I managed to set things up so its unticked by default, I'll dig.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114127
-----------------------------------------------------------
On Jan. 12, 2016, 6:34 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 6:34 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by John Sirois <js...@apache.org>.
> On Jan. 12, 2016, 6:49 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/Resources.java, line 50
> > <https://reviews.apache.org/r/42126/diff/3/?file=1195149#file1195149line50>
> >
> > Looks like an overly-aggressive find/replace. I've been endlessly frustrated by intellij doing this when you forget to untick the "search text and comments" option (which is always ticked).
> >
> > Please skim through the patch to find other spots impacted.
In IntelliJ settings, search for "Find Usages Settings" - I map this to `ctrl-alt-shift-7` - pops up dialog with scope and seach in text or not and other options.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114127
-----------------------------------------------------------
On Jan. 12, 2016, 6:34 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 6:34 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114127
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/Resources.java (line 50)
<https://reviews.apache.org/r/42126/#comment174937>
Looks like an overly-aggressive find/replace. I've been endlessly frustrated by intellij doing this when you forget to untick the "search text and comments" option (which is always ticked).
Please skim through the patch to find other spots impacted.
- Bill Farner
On Jan. 12, 2016, 5:34 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 5:34 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114241
-----------------------------------------------------------
Ship it!
Master (952ef6d) 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 Jan. 13, 2016, 5:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 5:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114280
-----------------------------------------------------------
Ship it!
Please also add a NEWS entry so we are sure to highlight this feature in the next release!
- Bill Farner
On Jan. 13, 2016, 9:50 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 9:50 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114303
-----------------------------------------------------------
This patch does not apply cleanly against master (952ef6d), do you need to rebase?
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 13, 2016, 10:02 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 10:02 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS e2b26d9513c15451afb0766fa0aa6f534f6afe49
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114379
-----------------------------------------------------------
Ship it!
Master (952ef6d) 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 Jan. 14, 2016, 1:39 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 1:39 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114374
-----------------------------------------------------------
@ReviewBot retry
- Zhitao Li
On Jan. 14, 2016, 1:39 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 1:39 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 14, 2016, 2:45 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java, line 216
> > <https://reviews.apache.org/r/42126/diff/9/?file=1196221#file1196221line216>
> >
> > I assume this line will be restored once `AcceptedOffer` accommodates for floating point errors?
Yes, with a slightly different `Offer` object which uses `ResourceSlot` from `SOME_OVERHEAD_EXECUTOR` instead of `THERMOS_EXECUTOR`.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114478
-----------------------------------------------------------
On Jan. 14, 2016, 2:46 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 2:46 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114478
-----------------------------------------------------------
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java
<https://reviews.apache.org/r/42126/#comment175329>
I assume this line will be restored once `AcceptedOffer` accommodates for floating point errors?
- Bill Farner
On Jan. 13, 2016, 5:39 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 5:39 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 14, 2016, 6:14 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java, line 95
> > <https://reviews.apache.org/r/42126/diff/10/?file=1196938#file1196938line95>
> >
> > This is still not technically correct. If role is not set Mesos will not send role with resources at all. All resources will be with role=Null in that case. I'd just drop (role="*") from help.
Fixed.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114519
-----------------------------------------------------------
On Jan. 14, 2016, 6:23 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 6:23 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114519
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java (line 95)
<https://reviews.apache.org/r/42126/#comment175372>
This is still not technically correct. If role is not set Mesos will not send role with resources at all. All resources will be with role=Null in that case. I'd just drop (role="*") from help.
- Maxim Khutornenko
On Jan. 14, 2016, 4:32 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 4:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114535
-----------------------------------------------------------
Ship it!
Master (952ef6d) 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 Jan. 14, 2016, 6:23 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 6:23 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 14, 2016, 6:31 p.m., Maxim Khutornenko wrote:
> > Ship It!
Waiting for Dmitriy to sign off before committing it to master.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114529
-----------------------------------------------------------
On Jan. 14, 2016, 6:23 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 6:23 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114529
-----------------------------------------------------------
Ship it!
Ship It!
- Maxim Khutornenko
On Jan. 14, 2016, 6:23 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 6:23 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114532
-----------------------------------------------------------
Ship it!
Ship It!
- Dmitriy Shirchenko
On Jan. 14, 2016, 6:23 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 6:23 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 14, 2016, 6:23 p.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
Clearer helper message as requested by Maxim
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs (updated)
-----
NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114496
-----------------------------------------------------------
Ship it!
Master (952ef6d) 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 Jan. 14, 2016, 4:32 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 4:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 14, 2016, 4:32 p.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
Use a `nearZero()` function to avoid comparison floating point number with zero, and restore a test case using `SOME_OVERHEAD_EXECUTOR`
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs (updated)
-----
NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 14, 2016, 6:46 a.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
Tweaked review summary in preparation for commit.
-wfarner
Summary (updated)
-----------------
Accept resource offers from multiple framework roles.
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs
-----
NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114372
-----------------------------------------------------------
Master (952ef6d) is red with this patch.
./build-support/jenkins/build.sh
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=1)[0m
[1m [0m
[1m assert hct._total_latency == 0[0m
[1m assert hct.metrics.sample()['total_latency_secs'] == 0[0m
[1m [0m
[1m # start the health check (during health check it is still 0)[0m
[1m epsilon = 0.001[0m
[1m self._clock.tick(1.0 + epsilon)[0m
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=0.5)[0m
[1m assert hct._total_latency == 0[0m
[1m assert hct.metrics.sample()['total_latency_secs'] == 0[0m
[1m assert hct.metrics.sample()['checks'] == 0[0m
[1m [0m
[1m # finish the health check[0m
[1m self._clock.tick(0.5 + epsilon)[0m
[1m self._clock.converge(threads=[hct.threaded_health_checker])[0m
[1m self._clock.assert_waiting(hct.threaded_health_checker, amount=1) # interval_secs[0m
[1m> assert hct._total_latency == 0.5[0m
[1m[31mE AssertionError: assert 0.5009999999999999 == 0.5[0m
[1m[31mE + where 0.5009999999999999 = <apache.aurora.executor.common.health_checker.HealthChecker object at 0x7fc2a9a57210>._total_latency[0m
src/test/python/apache/aurora/executor/common/test_health_checker.py:174: AssertionError
-------------- Captured stderr call --------------
[<twitter.common.testing.clock.ThreadedClock object at 0x7fc2a9a57550>] Time now: 0.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7fc2a9a57550>] Time now: 0.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7fc2a9a57550>] Time now: 1.0
[<twitter.common.testing.clock.ThreadedClock object at 0x7fc2a9a57550>] Time now: 1.001
[<twitter.common.testing.clock.ThreadedClock object at 0x7fc2a9a57550>] Time now: 1.001
[<twitter.common.testing.clock.ThreadedClock object at 0x7fc2a9a57550>] Time now: 1.501
[<twitter.common.testing.clock.ThreadedClock object at 0x7fc2a9a57550>] Time now: 1.502
generated xml file: /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/src.test.python.apache.aurora.executor.common.common.xml
[1m[31m= 1 failed, 42 passed, 2 skipped in 3.47 seconds =[0m
FAILURE
01:51:26 05:22 [complete][31m
FAILURE[0m
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 14, 2016, 1:39 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 1:39 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 14, 2016, 1:39 a.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
1. Better helper message;
2. Remove commented out test code;
3. Move all `static final` things to top.
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs (updated)
-----
NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 14, 2016, 1:06 a.m., Dmitriy Shirchenko wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java, lines 216-217
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196146#file1196146line216>
> >
> > Is this intentionally commented out?
Yes this is intentially removed. Old behavior of changing this `config` here resulted in `resources` in static final object `OFFER` not sufficient to be allocated, and I don't see any point to use a different executor for this test case, so I've deleted this code.
If any reviewer thinks there is a reason to test this with a different executor, I can reconstruct an `Offer` object with correct resource and add this back.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114353
-----------------------------------------------------------
On Jan. 14, 2016, 1:39 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 1:39 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: Accept resource offers from multiple
framework roles.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 14, 2016, 1:06 a.m., Dmitriy Shirchenko wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java, lines 216-217
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196146#file1196146line216>
> >
> > Is this intentionally commented out?
>
> Zhitao Li wrote:
> Yes this is intentially removed. Old behavior of changing this `config` here resulted in `resources` in static final object `OFFER` not sufficient to be allocated, and I don't see any point to use a different executor for this test case, so I've deleted this code.
>
> If any reviewer thinks there is a reason to test this with a different executor, I can reconstruct an `Offer` object with correct resource and add this back.
>
> Bill Farner wrote:
> This is slightly suspicious...are you sure resource accounting wasn't impacted in some way? I would expect old test cases to logically work with this feature.
>
> Zhitao Li wrote:
> I recalled why I commented this out in the first pass now (this was a bit tricky and I mistakenly thought it's logical bug while it's due to floating point precision).
>
> In TaskExecutors.java, `SOME_OVERHEAD_EXECUTOR` uses `0.01` cpus:
> ```
> public static final ExecutorSettings SOME_OVERHEAD_EXECUTOR =
> TestExecutorSettings.thermosOnlyWithOverhead(
> new ResourceSlot(0.01, Amount.of(256L, Data.MB), Amount.of(0L, Data.MB), 0));
> ```
> Unfortunately, `5.01 - 5 - 0.01` **is not zero** in floating point math of Java, so the test keeps throwing `InsufficientResourcesException` even if I constructed an `Offer` whose resource is correctly equivalent to `ResourceSlot.from(TASK_CONFIG).add(TaskExecutors.SOME_OVERHEAD_EXECUTOR.getExecutorOverhead())`.
>
> **My proposal is to change all comparison with zero in `AcceptedOffer` class (3 places in current patch) to compare with an `EPSILON` whose value is `1e-6` or something even smaller.** I don't think there is any reasonable usage of resource unit smaller than this granularity, and the floating point math in the class would be more robust. We can document this behavior somewhere if it sounds interesting.
>
> The default resource usage for `THERMOS_EXECUTOR` is 0.25 so I don't encounter this elsewhere.
>
> Some alternatives which I liked less:
> 1. Change `TaskExecutors.SOME_OVERHEAD_EXECUTOR`'s cpus to a value which could be precisely represented by floating point (0.125 or something similar);
> 2. Manually add a bit more cpu resource to the `Offer` object to trick the test to pass.
>
> If the above proposed solution sounds fine, I can send an update to the patch.
>
> Bill Farner wrote:
> I'm in favor of using an EPSILON value. Please be sure to extract a function to handle the equivalence check.
>
> It's too bad mesos doesn't use fixed point for scalar resources to avoid these issues. Looks like there's been recent momentum to make that change: https://issues.apache.org/jira/browse/MESOS-3997
>
> They recently addressed an instance of this issue with this change:
> https://github.com/apache/mesos/commit/5b5dd566aea71f654c51b2706a958ca1b5cb07a3
>
> ```
> CHECK_NEAR(result.cpus().get(), cpus().get(), MIN_CPUS);
> ```
>
> For reference, `MIN_CPUS` is 0.01.
Will do with a `nearZero()` helper function.
For the value of `ESPILON`, I took a quick look at related Mesos code, and the minimum gradunalar which could make a difference is `MIN_CPUS` / `CPU_SHARES_PER_CPU` (1024) in cgroup based isolation, which is still larger than `1e-6`, so using the latter value should ensure Aurora user won't lose precision even at extreme scheduling condition.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114353
-----------------------------------------------------------
On Jan. 14, 2016, 2:46 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 2:46 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 13, 2016, 5:06 p.m., Dmitriy Shirchenko wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java, lines 216-217
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196146#file1196146line216>
> >
> > Is this intentionally commented out?
>
> Zhitao Li wrote:
> Yes this is intentially removed. Old behavior of changing this `config` here resulted in `resources` in static final object `OFFER` not sufficient to be allocated, and I don't see any point to use a different executor for this test case, so I've deleted this code.
>
> If any reviewer thinks there is a reason to test this with a different executor, I can reconstruct an `Offer` object with correct resource and add this back.
This is slightly suspicious...are you sure resource accounting wasn't impacted in some way? I would expect old test cases to logically work with this feature.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114353
-----------------------------------------------------------
On Jan. 13, 2016, 5:39 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 5:39 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 13, 2016, 5:06 p.m., Dmitriy Shirchenko wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java, lines 216-217
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196146#file1196146line216>
> >
> > Is this intentionally commented out?
>
> Zhitao Li wrote:
> Yes this is intentially removed. Old behavior of changing this `config` here resulted in `resources` in static final object `OFFER` not sufficient to be allocated, and I don't see any point to use a different executor for this test case, so I've deleted this code.
>
> If any reviewer thinks there is a reason to test this with a different executor, I can reconstruct an `Offer` object with correct resource and add this back.
>
> Bill Farner wrote:
> This is slightly suspicious...are you sure resource accounting wasn't impacted in some way? I would expect old test cases to logically work with this feature.
>
> Zhitao Li wrote:
> I recalled why I commented this out in the first pass now (this was a bit tricky and I mistakenly thought it's logical bug while it's due to floating point precision).
>
> In TaskExecutors.java, `SOME_OVERHEAD_EXECUTOR` uses `0.01` cpus:
> ```
> public static final ExecutorSettings SOME_OVERHEAD_EXECUTOR =
> TestExecutorSettings.thermosOnlyWithOverhead(
> new ResourceSlot(0.01, Amount.of(256L, Data.MB), Amount.of(0L, Data.MB), 0));
> ```
> Unfortunately, `5.01 - 5 - 0.01` **is not zero** in floating point math of Java, so the test keeps throwing `InsufficientResourcesException` even if I constructed an `Offer` whose resource is correctly equivalent to `ResourceSlot.from(TASK_CONFIG).add(TaskExecutors.SOME_OVERHEAD_EXECUTOR.getExecutorOverhead())`.
>
> **My proposal is to change all comparison with zero in `AcceptedOffer` class (3 places in current patch) to compare with an `EPSILON` whose value is `1e-6` or something even smaller.** I don't think there is any reasonable usage of resource unit smaller than this granularity, and the floating point math in the class would be more robust. We can document this behavior somewhere if it sounds interesting.
>
> The default resource usage for `THERMOS_EXECUTOR` is 0.25 so I don't encounter this elsewhere.
>
> Some alternatives which I liked less:
> 1. Change `TaskExecutors.SOME_OVERHEAD_EXECUTOR`'s cpus to a value which could be precisely represented by floating point (0.125 or something similar);
> 2. Manually add a bit more cpu resource to the `Offer` object to trick the test to pass.
>
> If the above proposed solution sounds fine, I can send an update to the patch.
I'm in favor of using an EPSILON value. Please be sure to extract a function to handle the equivalence check.
It's too bad mesos doesn't use fixed point for scalar resources to avoid these issues. Looks like there's been recent momentum to make that change: https://issues.apache.org/jira/browse/MESOS-3997
They recently addressed an instance of this issue with this change:
https://github.com/apache/mesos/commit/5b5dd566aea71f654c51b2706a958ca1b5cb07a3
```
CHECK_NEAR(result.cpus().get(), cpus().get(), MIN_CPUS);
```
For reference, `MIN_CPUS` is 0.01.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114353
-----------------------------------------------------------
On Jan. 13, 2016, 5:39 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 5:39 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 14, 2016, 1:06 a.m., Dmitriy Shirchenko wrote:
> > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java, lines 216-217
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196146#file1196146line216>
> >
> > Is this intentionally commented out?
>
> Zhitao Li wrote:
> Yes this is intentially removed. Old behavior of changing this `config` here resulted in `resources` in static final object `OFFER` not sufficient to be allocated, and I don't see any point to use a different executor for this test case, so I've deleted this code.
>
> If any reviewer thinks there is a reason to test this with a different executor, I can reconstruct an `Offer` object with correct resource and add this back.
>
> Bill Farner wrote:
> This is slightly suspicious...are you sure resource accounting wasn't impacted in some way? I would expect old test cases to logically work with this feature.
I recalled why I commented this out in the first pass now (this was a bit tricky and I mistakenly thought it's logical bug while it's due to floating point precision).
In TaskExecutors.java, `SOME_OVERHEAD_EXECUTOR` uses `0.01` cpus:
```
public static final ExecutorSettings SOME_OVERHEAD_EXECUTOR =
TestExecutorSettings.thermosOnlyWithOverhead(
new ResourceSlot(0.01, Amount.of(256L, Data.MB), Amount.of(0L, Data.MB), 0));
```
Unfortunately, `5.01 - 5 - 0.01` **is not zero** in floating point math of Java, so the test keeps throwing `InsufficientResourcesException` even if I constructed an `Offer` whose resource is correctly equivalent to `ResourceSlot.from(TASK_CONFIG).add(TaskExecutors.SOME_OVERHEAD_EXECUTOR.getExecutorOverhead())`.
**My proposal is to change all comparison with zero in `AcceptedOffer` class (3 places in current patch) to compare with an `EPSILON` whose value is `1e-6` or something even smaller.** I don't think there is any reasonable usage of resource unit smaller than this granularity, and the floating point math in the class would be more robust. We can document this behavior somewhere if it sounds interesting.
The default resource usage for `THERMOS_EXECUTOR` is 0.25 so I don't encounter this elsewhere.
Some alternatives which I liked less:
1. Change `TaskExecutors.SOME_OVERHEAD_EXECUTOR`'s cpus to a value which could be precisely represented by floating point (0.125 or something similar);
2. Manually add a bit more cpu resource to the `Offer` object to trick the test to pass.
If the above proposed solution sounds fine, I can send an update to the patch.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114353
-----------------------------------------------------------
On Jan. 14, 2016, 1:39 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2016, 1:39 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Dmitriy Shirchenko <ca...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114353
-----------------------------------------------------------
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java (lines 216 - 217)
<https://reviews.apache.org/r/42126/#comment175191>
Is this intentionally commented out?
- Dmitriy Shirchenko
On Jan. 13, 2016, 11:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 11:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 14, 2016, 12:57 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java, lines 93-94
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196140#file1196140line93>
> >
> > This does not read right to me. If it's empty, a framework will be registered without any role at all.
>
> Bill Farner wrote:
> +1
Will fix to "`The Mesos role this framework will register as. If left empty, this framework will register without any role and only receive unreserved resources in offers.`"
> On Jan. 14, 2016, 12:57 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 82
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196137#file1196137line82>
> >
> > Suggest moving all static final fields to the top of this class.
Will do.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114348
-----------------------------------------------------------
On Jan. 13, 2016, 11:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 11:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 14, 2016, 12:57 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java, lines 93-94
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196140#file1196140line93>
> >
> > This does not read right to me. If it's empty, a framework will be registered without any role at all.
>
> Bill Farner wrote:
> +1
>
> Zhitao Li wrote:
> Will fix to "`The Mesos role this framework will register as. If left empty, this framework will register without any role and only receive unreserved resources in offers.`"
>
> Bill Farner wrote:
> To be clear, I believe Maxim and I are advocating a behavior change - the default should be to _not_ set a role (as opposed to role `*`).
I think this is more clear: "`The Mesos role this framework will register as. The default is to left this empty, and the framework will register without any role and only receive unreserved resources (role = "*") in offer.`"
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114348
-----------------------------------------------------------
On Jan. 13, 2016, 11:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 11:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 13, 2016, 4:57 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java, lines 93-94
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196140#file1196140line93>
> >
> > This does not read right to me. If it's empty, a framework will be registered without any role at all.
>
> Bill Farner wrote:
> +1
>
> Zhitao Li wrote:
> Will fix to "`The Mesos role this framework will register as. If left empty, this framework will register without any role and only receive unreserved resources in offers.`"
To be clear, I believe Maxim and I are advocating a behavior change - the default should be to _not_ set a role (as opposed to role `*`).
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114348
-----------------------------------------------------------
On Jan. 13, 2016, 3:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 3:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 13, 2016, 4:57 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java, lines 93-94
> > <https://reviews.apache.org/r/42126/diff/8/?file=1196140#file1196140line93>
> >
> > This does not read right to me. If it's empty, a framework will be registered without any role at all.
+1
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114348
-----------------------------------------------------------
On Jan. 13, 2016, 3:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 3:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114348
-----------------------------------------------------------
Only minor comments left.
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 82)
<https://reviews.apache.org/r/42126/#comment175188>
Suggest moving all static final fields to the top of this class.
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java (lines 93 - 94)
<https://reviews.apache.org/r/42126/#comment175186>
This does not read right to me. If it's empty, a framework will be registered without any role at all.
- Maxim Khutornenko
On Jan. 13, 2016, 11:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 11:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114346
-----------------------------------------------------------
Ship it!
Master (952ef6d) 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 Jan. 13, 2016, 11:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 11:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 11:50 p.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
Fix incorrect `RESERVED` predicate, and all comments from Maxim.
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs (updated)
-----
NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 13, 2016, 3:48 p.m., Zhitao Li wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 85
> > <https://reviews.apache.org/r/42126/diff/5/?file=1195816#file1195816line85>
> >
> > I noticed that this predicate is actually reversed (sorry for the bug).
> >
> > Fixing in next update.
Thanks, I was trying to comment on that but RB on a mobile web browser is clumsy at best :-)
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114326
-----------------------------------------------------------
On Jan. 13, 2016, 3:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 3:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114326
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 85)
<https://reviews.apache.org/r/42126/#comment175155>
I noticed that this predicate is actually reversed (sorry for the bug).
Fixing in next update.
- Zhitao Li
On Jan. 13, 2016, 10:32 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 10:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114323
-----------------------------------------------------------
Ship it!
Master (952ef6d) 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 Jan. 13, 2016, 10:32 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 10:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 13, 2016, 3:01 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 79
> > <https://reviews.apache.org/r/42126/diff/5/?file=1195816#file1195816line79>
> >
> > I'd expect this value populated from the command line arg supplied to driver setting module. Mind explaining the logic here? Why do we default to "*"?
>
> Bill Farner wrote:
> Is this not necessary to align with the special handling of `*` on the mesos side?
>
> Maxim Khutornenko wrote:
> I guess my question was about what this default even means. I'd expect this feature to be OFF by default by not registering with any role at all. It should be up to cluster operator to decide what role to register with (if at all).
>
> Bill Farner wrote:
> In this context, i believe the logic is appropriate. Role=`*` means any framework may use the resource. I don't think this is something you would want to configure or disable.
> https://mesos.apache.org/documentation/latest/roles/
>
> However i agree with you for the role the scheduler registers as (different part of this patch).
>
> Maxim Khutornenko wrote:
> What is the behavior if someone decides to register with a named role, say "aurora"? Since we are sorting by RESERVED (which is DEFAULT_ROLE_NAME) first, aren't we getting the reverse behavior here? E.g. we'd get anything which is NOT "aurora" first (mostly likely only *) and then "aurora" resources?
Just to clarify nomenclature, as Zhitao mentioned the naming is off in the current draft. Unreserved is `*`.
So the current behavior is to use the unreserved first.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114287
-----------------------------------------------------------
On Jan. 13, 2016, 3:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 3:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 13, 2016, 3:01 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 79
> > <https://reviews.apache.org/r/42126/diff/5/?file=1195816#file1195816line79>
> >
> > I'd expect this value populated from the command line arg supplied to driver setting module. Mind explaining the logic here? Why do we default to "*"?
Is this not necessary to align with the special handling of `*` on the mesos side?
> On Jan. 13, 2016, 3:01 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, lines 182-184
> > <https://reviews.apache.org/r/42126/diff/5/?file=1195816#file1195816line182>
> >
> > Does not Mesos expect resource role set when accepting an offer? Otherwise, how does it know what resource (from what role) is actually accepted?
`r.build()` accomplishes this by carrying the role value forward, right?
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114287
-----------------------------------------------------------
On Jan. 13, 2016, 2:32 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 13, 2016, 11:01 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 79
> > <https://reviews.apache.org/r/42126/diff/5/?file=1195816#file1195816line79>
> >
> > I'd expect this value populated from the command line arg supplied to driver setting module. Mind explaining the logic here? Why do we default to "*"?
>
> Bill Farner wrote:
> Is this not necessary to align with the special handling of `*` on the mesos side?
I guess my question was about what this default even means. I'd expect this feature to be OFF by default by not registering with any role at all. It should be up to cluster operator to decide what role to register with (if at all).
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114287
-----------------------------------------------------------
On Jan. 13, 2016, 10:32 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 10:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 13, 2016, 11:01 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 79
> > <https://reviews.apache.org/r/42126/diff/5/?file=1195816#file1195816line79>
> >
> > I'd expect this value populated from the command line arg supplied to driver setting module. Mind explaining the logic here? Why do we default to "*"?
>
> Bill Farner wrote:
> Is this not necessary to align with the special handling of `*` on the mesos side?
>
> Maxim Khutornenko wrote:
> I guess my question was about what this default even means. I'd expect this feature to be OFF by default by not registering with any role at all. It should be up to cluster operator to decide what role to register with (if at all).
>
> Bill Farner wrote:
> In this context, i believe the logic is appropriate. Role=`*` means any framework may use the resource. I don't think this is something you would want to configure or disable.
> https://mesos.apache.org/documentation/latest/roles/
>
> However i agree with you for the role the scheduler registers as (different part of this patch).
What is the behavior if someone decides to register with a named role, say "aurora"? Since we are sorting by RESERVED (which is DEFAULT_ROLE_NAME) first, aren't we getting the reverse behavior here? E.g. we'd get anything which is NOT "aurora" first (mostly likely only *) and then "aurora" resources?
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114287
-----------------------------------------------------------
On Jan. 13, 2016, 11:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 11:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 13, 2016, 11:01 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 79
> > <https://reviews.apache.org/r/42126/diff/5/?file=1195816#file1195816line79>
> >
> > I'd expect this value populated from the command line arg supplied to driver setting module. Mind explaining the logic here? Why do we default to "*"?
>
> Bill Farner wrote:
> Is this not necessary to align with the special handling of `*` on the mesos side?
>
> Maxim Khutornenko wrote:
> I guess my question was about what this default even means. I'd expect this feature to be OFF by default by not registering with any role at all. It should be up to cluster operator to decide what role to register with (if at all).
>
> Bill Farner wrote:
> In this context, i believe the logic is appropriate. Role=`*` means any framework may use the resource. I don't think this is something you would want to configure or disable.
> https://mesos.apache.org/documentation/latest/roles/
>
> However i agree with you for the role the scheduler registers as (different part of this patch).
>
> Maxim Khutornenko wrote:
> What is the behavior if someone decides to register with a named role, say "aurora"? Since we are sorting by RESERVED (which is DEFAULT_ROLE_NAME) first, aren't we getting the reverse behavior here? E.g. we'd get anything which is NOT "aurora" first (mostly likely only *) and then "aurora" resources?
>
> Bill Farner wrote:
> Just to clarify nomenclature, as Zhitao mentioned the naming is off in the current draft. Unreserved is `*`.
>
> So the current behavior is to use the unreserved first.
Yes that's the predicate bug I was referring to in my previous reply (still getting familiar with the collaboration and replying on review board).
This should be fixed in my last update, and a change in test actually picked the behavior up.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114287
-----------------------------------------------------------
On Jan. 13, 2016, 11:50 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 11:50 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 13, 2016, 11:01 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, lines 182-184
> > <https://reviews.apache.org/r/42126/diff/5/?file=1195816#file1195816line182>
> >
> > Does not Mesos expect resource role set when accepting an offer? Otherwise, how does it know what resource (from what role) is actually accepted?
>
> Bill Farner wrote:
> `r.build()` accomplishes this by carrying the role value forward, right?
You're right. Completely missed 'r' there.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114287
-----------------------------------------------------------
On Jan. 13, 2016, 10:32 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 10:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 13, 2016, 3:01 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 79
> > <https://reviews.apache.org/r/42126/diff/5/?file=1195816#file1195816line79>
> >
> > I'd expect this value populated from the command line arg supplied to driver setting module. Mind explaining the logic here? Why do we default to "*"?
>
> Bill Farner wrote:
> Is this not necessary to align with the special handling of `*` on the mesos side?
>
> Maxim Khutornenko wrote:
> I guess my question was about what this default even means. I'd expect this feature to be OFF by default by not registering with any role at all. It should be up to cluster operator to decide what role to register with (if at all).
In this context, i believe the logic is appropriate. Role=`*` means any framework may use the resource. I don't think this is something you would want to configure or disable.
https://mesos.apache.org/documentation/latest/roles/
However i agree with you for the role the scheduler registers as (different part of this patch).
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114287
-----------------------------------------------------------
On Jan. 13, 2016, 2:32 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114287
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 79)
<https://reviews.apache.org/r/42126/#comment175108>
I'd expect this value populated from the command line arg supplied to driver setting module. Mind explaining the logic here? Why do we default to "*"?
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 108)
<https://reviews.apache.org/r/42126/#comment175104>
I'd put private and final fields to the top as this better follows our usual arrangement.
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 113)
<https://reviews.apache.org/r/42126/#comment175103>
+newline after last arg here and in other places where args spill over.
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 120)
<https://reviews.apache.org/r/42126/#comment175105>
Pull 'throws' to the previous line and insert newline after it
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (lines 182 - 184)
<https://reviews.apache.org/r/42126/#comment175107>
Does not Mesos expect resource role set when accepting an offer? Otherwise, how does it know what resource (from what role) is actually accepted?
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java (line 94)
<https://reviews.apache.org/r/42126/#comment175130>
Does it have to be a default here? IMO, the less intrusive approach would be allow it to be empty and not populate framework role at all. This could be similar to FRAMEWORK_AUTHENTICATION_FILE approach and would result in a zero risk upgrade cycle.
- Maxim Khutornenko
On Jan. 13, 2016, 10:32 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 10:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 10:32 p.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
Rebase and merge changes in NEWS.
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs (updated)
-----
NEWS acaff9eb2ab184b0ef750f8b8a00c20131997f6b
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 10:02 p.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
Add an entry to the NEWS section for next release.
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs (updated)
-----
NEWS e2b26d9513c15451afb0766fa0aa6f534f6afe49
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 5:50 p.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
All review comments from this round, except the one about `makeMesosRangeResource`, which I replied separately.
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114137
-----------------------------------------------------------
Ship it!
Master (952ef6d) 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 Jan. 13, 2016, 2:08 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:08 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
> On Jan. 13, 2016, 2:57 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 129
> > <https://reviews.apache.org/r/42126/diff/3/?file=1195147#file1195147line129>
> >
> > This is a good general-purpose implementation of this behavior, but in practice it seems like overkill. Can you instead match the behavior currently on master and synthesize the `Resource` with `makeMesosRangeResource(PORTS, selectedPorts)`?
I think we still need this function, because ports can be reserved to a particular role, similar to cpus/mem/disk.
In practice, it seems possible to me that cluster operator decided to partition available port range among differnet frameworks. Certain Mesos framework (Cassandra) even requires fixed ports in resource offers to launch their workload, because their workload only works well with these fixed ports.
Given such possibility, it seems we still need to have correct role for the ports used when sending them back to Mesos, mostly for handling all possible configuration purpose.
Please let me know if you think otherwise.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114128
-----------------------------------------------------------
On Jan. 13, 2016, 2:08 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 2:08 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114128
-----------------------------------------------------------
Thanks for the changes so far, nearly there!
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 39)
<https://reviews.apache.org/r/42126/#comment174944>
I did a quick rearrangement of this method (after applying the comment below), i think the result is a bit easier to read, and uses fewer temp variables.
I also cheated and omitted the `requireNonNull` calls as nulls will immediately trigger NPE for all parameters.
```
public static AcceptedOffer create(
Offer offer,
ResourceSlot taskSlot,
ResourceSlot executorSlot,
Set<Integer> selectedPorts,
TierInfo tierInfo) throws Resources.InsufficientResourcesException {
List<Resource> reservedFirst = ImmutableList.<Resource>builder()
.addAll(Iterables.filter(offer.getResourcesList(), RESERVED))
.addAll(Iterables.filter(offer.getResourcesList(), NOT_RESERVED))
.build();
boolean revocable = tierInfo.isRevocable();
List<Resource.Builder> cpuResources = filterToBuilders(
reservedFirst,
ResourceType.CPUS.getName(),
revocable ? Resources.REVOCABLE : Resources.NON_REVOCABLE);
List<Resource.Builder> memResources = filterToBuilderNonRevocable(
reservedFirst, ResourceType.RAM_MB.getName());
List<Resource.Builder> diskResources = filterToBuilderNonRevocable(
reservedFirst, ResourceType.DISK_MB.getName());
List<Resource.Builder> portsResources = filterToBuilderNonRevocable(
reservedFirst, ResourceType.PORTS.getName());
List<Resource> taskResources = ImmutableList.<Resource>builder()
.addAll(allocateScalarType(cpuResources, taskSlot.getNumCpus(), revocable))
.addAll(allocateScalarType(memResources, taskSlot.getRam().as(Data.MB), false))
.addAll(allocateScalarType(diskResources, taskSlot.getDisk().as(Data.MB), false))
.addAll(allocateRangeType(portsResources, selectedPorts))
.build();
List<Resource> executorResources = ImmutableList.<Resource>builder()
.addAll(allocateScalarType(cpuResources, executorSlot.getNumCpus(), revocable))
.addAll(allocateScalarType(memResources, executorSlot.getRam().as(Data.MB), false))
.addAll(allocateScalarType(diskResources, executorSlot.getDisk().as(Data.MB), false))
.build();
return new AcceptedOffer(taskResources, executorResources);
}
```
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 119)
<https://reviews.apache.org/r/42126/#comment174949>
feel free to declare as `List` instead of `ImmutableList`, ditto in the constructor.
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 129)
<https://reviews.apache.org/r/42126/#comment174950>
This is a good general-purpose implementation of this behavior, but in practice it seems like overkill. Can you instead match the behavior currently on master and synthesize the `Resource` with `makeMesosRangeResource(PORTS, selectedPorts)`?
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 177)
<https://reviews.apache.org/r/42126/#comment174948>
Remove `final` here and throughout the method. We only use `final` when necessary, or to ensure immutability of class field references.
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 178)
<https://reviews.apache.org/r/42126/#comment174938>
This parameter is now effectively a return value, so instead make it a formal return type.
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 181)
<https://reviews.apache.org/r/42126/#comment174953>
`remaining` instead of `outstanding`
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 199)
<https://reviews.apache.org/r/42126/#comment174954>
Move this up to the line after the declaration of `used` to make for easier readability of the accounting logic.
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java (line 200)
<https://reviews.apache.org/r/42126/#comment174955>
`r.getScalarBuilder().setValue(available - used);`
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java (line 154)
<https://reviews.apache.org/r/42126/#comment174957>
`runMultipleRolesTest` to make it clear this is not intended to be a test case
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java (line 253)
<https://reviews.apache.org/r/42126/#comment174958>
You could simplify this a bit for callers by changing `Set<Integer>... values` to `Integer... values`. This way callers don't need the `ImmutableSet.of()` boilerplate. It also resolves the unchecked generic vararg compiler warning you have now.
On this end, you will remove the `for` loop and do:
```
resources.add(AcceptedOffer.makeMesosRangeResource(prototype.build(), ImmutableSet.copyOf(values));
```
- Bill Farner
On Jan. 12, 2016, 6:08 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2016, 6:08 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 2:08 a.m.)
Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
Changes
-------
Undo incorrect comment find/replace by Intellij.
Bugs: AURORA-1109
https://issues.apache.org/jira/browse/AURORA-1109
Repository: aurora
Description
-------
This review is a prototype for introducing multiple role support in Aurora.
This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
Current implementation prefers reserved resources over shared resources ('*' role) if both are present
Several caveats:
1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
Diff: https://reviews.apache.org/r/42126/diff/
Testing
-------
1. Unit tested with old and new tests;
2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
Thanks,
Zhitao Li
Re: Review Request 42126: New class to allocate resources of multiple
roles from offer.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42126/#review114130
-----------------------------------------------------------
Ship it!
Master (952ef6d) 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 Jan. 13, 2016, 1:34 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42126/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 1:34 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Dmitriy Shirchenko, and Bill Farner.
>
>
> Bugs: AURORA-1109
> https://issues.apache.org/jira/browse/AURORA-1109
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This review is a prototype for introducing multiple role support in Aurora.
> This creates a new class OfferAllocation, which allcoates resources to resources field in TaskInfo and ExecutorInfo from an offer.
>
> Current implementation prefers reserved resources over shared resources ('*' role) if both are present
>
> Several caveats:
> 1. This performs the allocate after scheduling decision in TaskAssigner.maybeAssign is done, which leaves possibility of inconsistency and late failure.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 7c3d681c216b78eeecebbe950186e5a79c6fe982
> src/main/java/org/apache/aurora/scheduler/Resources.java db422a959ee7b982c2a44323de41ad75d1a40754
> src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java 2255dd407cd1810c7df5baf17cfa85f79bfffeb8
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8fdadda67478bb3110aa442b7d78493cf9c3edb4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7e8e456e288986eb0ce92a123b294e1e25d8ed18
> src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java e4ae943303823ac4bfbe999ed22f5999484462d8
> src/test/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModuleTest.java 33149ab415292eff04f38b61f2b1d1eac79f347a
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java a5793bffabf4e5d6195b1b99f2363d241c0cecf9
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 3cbe9acd75def14ae2e0986914ba621fb164b3e4
>
> Diff: https://reviews.apache.org/r/42126/diff/
>
>
> Testing
> -------
>
> 1. Unit tested with old and new tests;
> 2. vagrant integration tests: I manually separate out the vagrant box's cpu and memory between 'aurora-test' role and '*' and verified that jobs can still be launched (I can post the vagrant change in another follow upon request).
>
>
> Thanks,
>
> Zhitao Li
>
>