You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2016/04/29 03:08:04 UTC
Review Request 46810: Generalizing port resource management.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46810/
-----------------------------------------------------------
Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
Repository: aurora
Description
-------
This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A few implementation notes:
- `ResourceType` gets an optional `ResourceMapper` property applicable to any resources types that require mapping and assignment of matched values
- All port specific matching/assigning logic is now encapsulated within `PortMapper` implementing `ResourceMapper` interface
- `TaskAssigner` now passes a callback into the `StateManager` to map and assign port values
- The newly introduced `ResourceManager` will eventually replace both `Resources` and `ResourceSlot` classes.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef
src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 625e6d5e1db1c7327154823f5a604efd3a4ff690
src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 1df2c11a84db8d46158c87e7c3709fc08016a107
src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 6e4d694cc6449a211297919d7834996382b21b21
src/main/java/org/apache/aurora/scheduler/resources/Resources.java 894c6a66545ac336d594d83c595ee83b7c91e21d
src/main/java/org/apache/aurora/scheduler/state/StateManager.java 5d34fe38492cae3f50ea9ed0baca11472295af60
src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java e5b2f41f55aec161840c3fb17f2ff73161b77482
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7d43d4ad2de60f76183c91b860b00e7022d79bcb
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 731327950099f2567430f3af75633854910443ff
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6370a126340dd4312ae0f2da52fee6ccdbced595
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java bf18d5d53f7eda62120299146a956fa0a0985f71
src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java d5f2172131ce4b765a1f85d63aeddf1daaa0b606
src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 04a82382e8535e1a233d48370bd0650f5abcd787
src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java ceb81e42e558fadb372cf4de862c6d3bcd293099
src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 498da78169be42fd5edeb9963d8262af02895f0e
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java a2df3114a299218e7fb04a21cdedc8d911029bb0
Diff: https://reviews.apache.org/r/46810/diff/
Testing
-------
unit and e2e tests
Thanks,
Maxim Khutornenko
Re: Review Request 46810: Generalizing port resource management.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On April 29, 2016, 3:50 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java, lines 353-358
> > <https://reviews.apache.org/r/46810/diff/1/?file=1365268#file1365268line353>
> >
> > These two cases are now mutually exclusive, use `else if` for the second?
Done.
> On April 29, 2016, 3:50 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java, line 45
> > <https://reviews.apache.org/r/46810/diff/1/?file=1365270#file1365270line45>
> >
> > nit: use a more appropriate variable name than `e` here to give some context (`r` or `resource`?).
Changed to `r`.
> On April 29, 2016, 3:50 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java, line 28
> > <https://reviews.apache.org/r/46810/diff/1/?file=1365271#file1365271line28>
> >
> > nit: just import `Protos.Offer`?
Done.
> On April 29, 2016, 3:50 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/state/StateManager.java, line 19
> > <https://reviews.apache.org/r/46810/diff/1/?file=1365275#file1365275line19>
> >
> > Use `java.util.Function`?
Sure.
> On April 29, 2016, 3:50 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java, line 114
> > <https://reviews.apache.org/r/46810/diff/1/?file=1365277#file1365277line114>
> >
> > this should be passing `assigned` to `mapAndAssign` not `task`, right? In case there are multiple resource types with mappers we don't want to discard any changes to the task from previous mappers.
Good catch! This was to appease static checker complaining about direct assignment of the argument variable.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46810/#review131044
-----------------------------------------------------------
On April 29, 2016, 1:08 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46810/
> -----------------------------------------------------------
>
> (Updated April 29, 2016, 1:08 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A few implementation notes:
> - `ResourceType` gets an optional `ResourceMapper` property applicable to any resources types that require mapping and assignment of matched values
> - All port specific matching/assigning logic is now encapsulated within `PortMapper` implementing `ResourceMapper` interface
> - `TaskAssigner` now passes a callback into the `StateManager` to map and assign port values
> - The newly introduced `ResourceManager` will eventually replace both `Resources` and `ResourceSlot` classes.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 625e6d5e1db1c7327154823f5a604efd3a4ff690
> src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 1df2c11a84db8d46158c87e7c3709fc08016a107
> src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 6e4d694cc6449a211297919d7834996382b21b21
> src/main/java/org/apache/aurora/scheduler/resources/Resources.java 894c6a66545ac336d594d83c595ee83b7c91e21d
> src/main/java/org/apache/aurora/scheduler/state/StateManager.java 5d34fe38492cae3f50ea9ed0baca11472295af60
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java e5b2f41f55aec161840c3fb17f2ff73161b77482
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7d43d4ad2de60f76183c91b860b00e7022d79bcb
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 731327950099f2567430f3af75633854910443ff
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6370a126340dd4312ae0f2da52fee6ccdbced595
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java bf18d5d53f7eda62120299146a956fa0a0985f71
> src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java d5f2172131ce4b765a1f85d63aeddf1daaa0b606
> src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 04a82382e8535e1a233d48370bd0650f5abcd787
> src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java ceb81e42e558fadb372cf4de862c6d3bcd293099
> src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 498da78169be42fd5edeb9963d8262af02895f0e
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java a2df3114a299218e7fb04a21cdedc8d911029bb0
>
> Diff: https://reviews.apache.org/r/46810/diff/
>
>
> Testing
> -------
>
> unit and e2e tests
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 46810: Generalizing port resource management.
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46810/#review131044
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java (lines 351 - 356)
<https://reviews.apache.org/r/46810/#comment194993>
These two cases are now mutually exclusive, use `else if` for the second?
src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java (line 45)
<https://reviews.apache.org/r/46810/#comment194995>
nit: use a more appropriate variable name than `e` here to give some context (`r` or `resource`?).
src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java (line 67)
<https://reviews.apache.org/r/46810/#comment194996>
Same here and below?
(I notice this is fairly common, presumably `e` is short for "element" in these cases, but I find that giving it a more contextual name helps grok what's going on in the absence of explicit types, feel free to punt on this though if it's a major pain ;)).
src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java (line 28)
<https://reviews.apache.org/r/46810/#comment194997>
nit: just import `Protos.Offer`?
src/main/java/org/apache/aurora/scheduler/state/StateManager.java (line 18)
<https://reviews.apache.org/r/46810/#comment194998>
Use `java.util.Function`?
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java (line 111)
<https://reviews.apache.org/r/46810/#comment194999>
this should be passing `assigned` to `mapAndAssign` not `task`, right? In case there are multiple resource types with mappers we don't want to discard any changes to the task from previous mappers.
- Joshua Cohen
On April 29, 2016, 1:08 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46810/
> -----------------------------------------------------------
>
> (Updated April 29, 2016, 1:08 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A few implementation notes:
> - `ResourceType` gets an optional `ResourceMapper` property applicable to any resources types that require mapping and assignment of matched values
> - All port specific matching/assigning logic is now encapsulated within `PortMapper` implementing `ResourceMapper` interface
> - `TaskAssigner` now passes a callback into the `StateManager` to map and assign port values
> - The newly introduced `ResourceManager` will eventually replace both `Resources` and `ResourceSlot` classes.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 625e6d5e1db1c7327154823f5a604efd3a4ff690
> src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 1df2c11a84db8d46158c87e7c3709fc08016a107
> src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 6e4d694cc6449a211297919d7834996382b21b21
> src/main/java/org/apache/aurora/scheduler/resources/Resources.java 894c6a66545ac336d594d83c595ee83b7c91e21d
> src/main/java/org/apache/aurora/scheduler/state/StateManager.java 5d34fe38492cae3f50ea9ed0baca11472295af60
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java e5b2f41f55aec161840c3fb17f2ff73161b77482
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7d43d4ad2de60f76183c91b860b00e7022d79bcb
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 731327950099f2567430f3af75633854910443ff
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6370a126340dd4312ae0f2da52fee6ccdbced595
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java bf18d5d53f7eda62120299146a956fa0a0985f71
> src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java d5f2172131ce4b765a1f85d63aeddf1daaa0b606
> src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 04a82382e8535e1a233d48370bd0650f5abcd787
> src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java ceb81e42e558fadb372cf4de862c6d3bcd293099
> src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 498da78169be42fd5edeb9963d8262af02895f0e
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java a2df3114a299218e7fb04a21cdedc8d911029bb0
>
> Diff: https://reviews.apache.org/r/46810/diff/
>
>
> Testing
> -------
>
> unit and e2e tests
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 46810: Generalizing port resource management.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46810/#review131145
-----------------------------------------------------------
Ship it!
Master (450d881) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On April 29, 2016, 4:43 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46810/
> -----------------------------------------------------------
>
> (Updated April 29, 2016, 4:43 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A few implementation notes:
> - `ResourceType` gets an optional `ResourceMapper` property applicable to any resources types that require mapping and assignment of matched values
> - All port specific matching/assigning logic is now encapsulated within `PortMapper` implementing `ResourceMapper` interface
> - `TaskAssigner` now passes a callback into the `StateManager` to map and assign port values
> - The newly introduced `ResourceManager` will eventually replace both `Resources` and `ResourceSlot` classes.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 625e6d5e1db1c7327154823f5a604efd3a4ff690
> src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 1df2c11a84db8d46158c87e7c3709fc08016a107
> src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 6e4d694cc6449a211297919d7834996382b21b21
> src/main/java/org/apache/aurora/scheduler/resources/Resources.java 894c6a66545ac336d594d83c595ee83b7c91e21d
> src/main/java/org/apache/aurora/scheduler/state/StateManager.java 5d34fe38492cae3f50ea9ed0baca11472295af60
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java e5b2f41f55aec161840c3fb17f2ff73161b77482
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7d43d4ad2de60f76183c91b860b00e7022d79bcb
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 731327950099f2567430f3af75633854910443ff
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6370a126340dd4312ae0f2da52fee6ccdbced595
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java bf18d5d53f7eda62120299146a956fa0a0985f71
> src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java d5f2172131ce4b765a1f85d63aeddf1daaa0b606
> src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 04a82382e8535e1a233d48370bd0650f5abcd787
> src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java ceb81e42e558fadb372cf4de862c6d3bcd293099
> src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 498da78169be42fd5edeb9963d8262af02895f0e
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java a2df3114a299218e7fb04a21cdedc8d911029bb0
>
> Diff: https://reviews.apache.org/r/46810/diff/
>
>
> Testing
> -------
>
> unit and e2e tests
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 46810: Generalizing port resource management.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46810/#review131155
-----------------------------------------------------------
Ship it!
Ship It!
- Zameer Manji
On April 29, 2016, 9:43 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46810/
> -----------------------------------------------------------
>
> (Updated April 29, 2016, 9:43 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A few implementation notes:
> - `ResourceType` gets an optional `ResourceMapper` property applicable to any resources types that require mapping and assignment of matched values
> - All port specific matching/assigning logic is now encapsulated within `PortMapper` implementing `ResourceMapper` interface
> - `TaskAssigner` now passes a callback into the `StateManager` to map and assign port values
> - The newly introduced `ResourceManager` will eventually replace both `Resources` and `ResourceSlot` classes.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 625e6d5e1db1c7327154823f5a604efd3a4ff690
> src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 1df2c11a84db8d46158c87e7c3709fc08016a107
> src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 6e4d694cc6449a211297919d7834996382b21b21
> src/main/java/org/apache/aurora/scheduler/resources/Resources.java 894c6a66545ac336d594d83c595ee83b7c91e21d
> src/main/java/org/apache/aurora/scheduler/state/StateManager.java 5d34fe38492cae3f50ea9ed0baca11472295af60
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java e5b2f41f55aec161840c3fb17f2ff73161b77482
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7d43d4ad2de60f76183c91b860b00e7022d79bcb
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 731327950099f2567430f3af75633854910443ff
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6370a126340dd4312ae0f2da52fee6ccdbced595
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java bf18d5d53f7eda62120299146a956fa0a0985f71
> src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java d5f2172131ce4b765a1f85d63aeddf1daaa0b606
> src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 04a82382e8535e1a233d48370bd0650f5abcd787
> src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java ceb81e42e558fadb372cf4de862c6d3bcd293099
> src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 498da78169be42fd5edeb9963d8262af02895f0e
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java a2df3114a299218e7fb04a21cdedc8d911029bb0
>
> Diff: https://reviews.apache.org/r/46810/diff/
>
>
> Testing
> -------
>
> unit and e2e tests
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 46810: Generalizing port resource management.
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46810/#review131143
-----------------------------------------------------------
Ship it!
Ship It!
- Joshua Cohen
On April 29, 2016, 4:43 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46810/
> -----------------------------------------------------------
>
> (Updated April 29, 2016, 4:43 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A few implementation notes:
> - `ResourceType` gets an optional `ResourceMapper` property applicable to any resources types that require mapping and assignment of matched values
> - All port specific matching/assigning logic is now encapsulated within `PortMapper` implementing `ResourceMapper` interface
> - `TaskAssigner` now passes a callback into the `StateManager` to map and assign port values
> - The newly introduced `ResourceManager` will eventually replace both `Resources` and `ResourceSlot` classes.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 625e6d5e1db1c7327154823f5a604efd3a4ff690
> src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 1df2c11a84db8d46158c87e7c3709fc08016a107
> src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 6e4d694cc6449a211297919d7834996382b21b21
> src/main/java/org/apache/aurora/scheduler/resources/Resources.java 894c6a66545ac336d594d83c595ee83b7c91e21d
> src/main/java/org/apache/aurora/scheduler/state/StateManager.java 5d34fe38492cae3f50ea9ed0baca11472295af60
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java e5b2f41f55aec161840c3fb17f2ff73161b77482
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7d43d4ad2de60f76183c91b860b00e7022d79bcb
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 731327950099f2567430f3af75633854910443ff
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6370a126340dd4312ae0f2da52fee6ccdbced595
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java bf18d5d53f7eda62120299146a956fa0a0985f71
> src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java d5f2172131ce4b765a1f85d63aeddf1daaa0b606
> src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 04a82382e8535e1a233d48370bd0650f5abcd787
> src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java ceb81e42e558fadb372cf4de862c6d3bcd293099
> src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 498da78169be42fd5edeb9963d8262af02895f0e
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java a2df3114a299218e7fb04a21cdedc8d911029bb0
>
> Diff: https://reviews.apache.org/r/46810/diff/
>
>
> Testing
> -------
>
> unit and e2e tests
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 46810: Generalizing port resource management.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46810/
-----------------------------------------------------------
(Updated April 29, 2016, 4:43 p.m.)
Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
Changes
-------
Joshua's comments.
Repository: aurora
Description
-------
This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A few implementation notes:
- `ResourceType` gets an optional `ResourceMapper` property applicable to any resources types that require mapping and assignment of matched values
- All port specific matching/assigning logic is now encapsulated within `PortMapper` implementing `ResourceMapper` interface
- `TaskAssigner` now passes a callback into the `StateManager` to map and assign port values
- The newly introduced `ResourceManager` will eventually replace both `Resources` and `ResourceSlot` classes.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef
src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 625e6d5e1db1c7327154823f5a604efd3a4ff690
src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 1df2c11a84db8d46158c87e7c3709fc08016a107
src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 6e4d694cc6449a211297919d7834996382b21b21
src/main/java/org/apache/aurora/scheduler/resources/Resources.java 894c6a66545ac336d594d83c595ee83b7c91e21d
src/main/java/org/apache/aurora/scheduler/state/StateManager.java 5d34fe38492cae3f50ea9ed0baca11472295af60
src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java e5b2f41f55aec161840c3fb17f2ff73161b77482
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7d43d4ad2de60f76183c91b860b00e7022d79bcb
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 731327950099f2567430f3af75633854910443ff
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6370a126340dd4312ae0f2da52fee6ccdbced595
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java bf18d5d53f7eda62120299146a956fa0a0985f71
src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java d5f2172131ce4b765a1f85d63aeddf1daaa0b606
src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 04a82382e8535e1a233d48370bd0650f5abcd787
src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java ceb81e42e558fadb372cf4de862c6d3bcd293099
src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 498da78169be42fd5edeb9963d8262af02895f0e
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java a2df3114a299218e7fb04a21cdedc8d911029bb0
Diff: https://reviews.apache.org/r/46810/diff/
Testing
-------
unit and e2e tests
Thanks,
Maxim Khutornenko
Re: Review Request 46810: Generalizing port resource management.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46810/#review131029
-----------------------------------------------------------
Ship it!
Master (450d881) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On April 29, 2016, 1:08 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46810/
> -----------------------------------------------------------
>
> (Updated April 29, 2016, 1:08 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Bill Farner, and Zameer Manji.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This patch effectively gets rid of `ITaskConfig.getRequestedPorts()` usage. A few implementation notes:
> - `ResourceType` gets an optional `ResourceMapper` property applicable to any resources types that require mapping and assignment of matched values
> - All port specific matching/assigning logic is now encapsulated within `PortMapper` implementing `ResourceMapper` interface
> - `TaskAssigner` now passes a callback into the `StateManager` to map and assign port values
> - The newly introduced `ResourceManager` will eventually replace both `Resources` and `ResourceSlot` classes.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 9a15a4b5c1cdc3efbf0588e8bd81636b66e938ef
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 625e6d5e1db1c7327154823f5a604efd3a4ff690
> src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceMapper.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java 1df2c11a84db8d46158c87e7c3709fc08016a107
> src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 6e4d694cc6449a211297919d7834996382b21b21
> src/main/java/org/apache/aurora/scheduler/resources/Resources.java 894c6a66545ac336d594d83c595ee83b7c91e21d
> src/main/java/org/apache/aurora/scheduler/state/StateManager.java 5d34fe38492cae3f50ea9ed0baca11472295af60
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java e5b2f41f55aec161840c3fb17f2ff73161b77482
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 7d43d4ad2de60f76183c91b860b00e7022d79bcb
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 731327950099f2567430f3af75633854910443ff
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6370a126340dd4312ae0f2da52fee6ccdbced595
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java bf18d5d53f7eda62120299146a956fa0a0985f71
> src/test/java/org/apache/aurora/scheduler/resources/AcceptedOfferTest.java d5f2172131ce4b765a1f85d63aeddf1daaa0b606
> src/test/java/org/apache/aurora/scheduler/resources/PortMapperTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 04a82382e8535e1a233d48370bd0650f5abcd787
> src/test/java/org/apache/aurora/scheduler/resources/ResourcesTest.java ceb81e42e558fadb372cf4de862c6d3bcd293099
> src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 498da78169be42fd5edeb9963d8262af02895f0e
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java a2df3114a299218e7fb04a21cdedc8d911029bb0
>
> Diff: https://reviews.apache.org/r/46810/diff/
>
>
> Testing
> -------
>
> unit and e2e tests
>
>
> Thanks,
>
> Maxim Khutornenko
>
>