You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Thomas Marshall <tw...@gmail.com> on 2013/07/08 23:46:55 UTC
Review Request 12340: Reservations 1 - Add roles to Resources
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12340/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Bugs: MESOS-505
https://issues.apache.org/jira/browse/MESOS-505
Repository: mesos
Description
-------
As part of the addition of reserved resources to the allocator, we decided to mark resources with roles. This makes it easy for the allocator to track what reserved pool different resources came from. This patch updates the current resource operators to consider roles, so for example only resources with the same role will be added together.
Diffs
-----
include/mesos/mesos.proto 233e3c3
src/common/resources.hpp 42dfb6a
src/common/resources.cpp 8bc1057
src/tests/resources_tests.cpp 67a11b3
Diff: https://reviews.apache.org/r/12340/diff/
Testing
-------
make check
Thanks,
Thomas Marshall
Re: Review Request 12340: Reservations 1 - Add roles to Resources
Posted by Thomas Marshall <tw...@gmail.com>.
> On July 12, 2013, 2:12 a.m., Benjamin Hindman wrote:
> > src/common/resources.hpp, line 385
> > <https://reviews.apache.org/r/12340/diff/1/?file=318919#file318919line385>
> >
> > Please add a TODO: Replace with 'allocatable'. ;)
I know that we talked about this before, but the semantics are different now - before, I was using isZero in the <= operator where a non-allocatable resource should be considered <= an allocatable resource, since being non-allocatable means either being zero or negative. In this context, though, I'm using isZero to prevent the minus operator from returning zero valued resources. If we used allocatable here instead of isZero, then the minus operator would also not return negative valued resources, which wouldn't be correct.
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12340/#review23050
-----------------------------------------------------------
On July 8, 2013, 9:46 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12340/
> -----------------------------------------------------------
>
> (Updated July 8, 2013, 9:46 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-505
> https://issues.apache.org/jira/browse/MESOS-505
>
>
> Repository: mesos
>
>
> Description
> -------
>
> As part of the addition of reserved resources to the allocator, we decided to mark resources with roles. This makes it easy for the allocator to track what reserved pool different resources came from. This patch updates the current resource operators to consider roles, so for example only resources with the same role will be added together.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 233e3c3
> src/common/resources.hpp 42dfb6a
> src/common/resources.cpp 8bc1057
> src/tests/resources_tests.cpp 67a11b3
>
> Diff: https://reviews.apache.org/r/12340/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request 12340: Reservations 1 - Add roles to Resources
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12340/#review23050
-----------------------------------------------------------
Ship it!
src/common/resources.hpp
<https://reviews.apache.org/r/12340/#comment46866>
Can you move this either before or after the operators?
src/common/resources.hpp
<https://reviews.apache.org/r/12340/#comment46867>
What about calling this 'flatten'? Seems like we need a "primitive" to refer to this both in discussions and in code. Likewise, what about taking a 'role' parameter as the role name to flatten all these resources into (rather than always the default '*').
src/common/resources.hpp
<https://reviews.apache.org/r/12340/#comment46868>
Please add a TODO: Replace with 'allocatable'. ;)
src/common/resources.cpp
<https://reviews.apache.org/r/12340/#comment46869>
s/matches (/matches(/
src/common/resources.cpp
<https://reviews.apache.org/r/12340/#comment46870>
How about a 'break' here and remove the '&& !found' above?
src/tests/resources_tests.cpp
<https://reviews.apache.org/r/12340/#comment46871>
You should be able to fit the entire string on the next line:
string output =
"cpus=45.55 (*); ports=[10000-20000, 30000-50000] (*); disks={sda1} (*)";
src/tests/resources_tests.cpp
<https://reviews.apache.org/r/12340/#comment46872>
Woah, that's a bug!!!!!! Thanks for the fix!
- Benjamin Hindman
On July 8, 2013, 9:46 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12340/
> -----------------------------------------------------------
>
> (Updated July 8, 2013, 9:46 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-505
> https://issues.apache.org/jira/browse/MESOS-505
>
>
> Repository: mesos
>
>
> Description
> -------
>
> As part of the addition of reserved resources to the allocator, we decided to mark resources with roles. This makes it easy for the allocator to track what reserved pool different resources came from. This patch updates the current resource operators to consider roles, so for example only resources with the same role will be added together.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 233e3c3
> src/common/resources.hpp 42dfb6a
> src/common/resources.cpp 8bc1057
> src/tests/resources_tests.cpp 67a11b3
>
> Diff: https://reviews.apache.org/r/12340/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request 12340: Reservations 1 - Add roles to Resources
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12340/#review23436
-----------------------------------------------------------
Can you update this one with the format I mentioned in the other review:
"cpus(*):4; cpus(jenkins):8; memory(*):10GB; memory(jenkins):20GB"
- Ben Mahler
On July 12, 2013, 2:49 a.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12340/
> -----------------------------------------------------------
>
> (Updated July 12, 2013, 2:49 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-505
> https://issues.apache.org/jira/browse/MESOS-505
>
>
> Repository: mesos
>
>
> Description
> -------
>
> As part of the addition of reserved resources to the allocator, we decided to mark resources with roles. This makes it easy for the allocator to track what reserved pool different resources came from. This patch updates the current resource operators to consider roles, so for example only resources with the same role will be added together.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 233e3c3
> src/common/resources.hpp 42dfb6a
> src/common/resources.cpp 8bc1057
> src/tests/resources_tests.cpp 67a11b3
>
> Diff: https://reviews.apache.org/r/12340/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request 12340: Reservations 1 - Add roles to Resources
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12340/#review23586
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On July 18, 2013, 9:11 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12340/
> -----------------------------------------------------------
>
> (Updated July 18, 2013, 9:11 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-505
> https://issues.apache.org/jira/browse/MESOS-505
>
>
> Repository: mesos
>
>
> Description
> -------
>
> As part of the addition of reserved resources to the allocator, we decided to mark resources with roles. This makes it easy for the allocator to track what reserved pool different resources came from. This patch updates the current resource operators to consider roles, so for example only resources with the same role will be added together.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 233e3c3
> src/common/resources.hpp 42dfb6a
> src/common/resources.cpp 8bc1057
> src/tests/resources_tests.cpp 67a11b3
>
> Diff: https://reviews.apache.org/r/12340/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request 12340: Reservations 1 - Add roles to Resources
Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12340/
-----------------------------------------------------------
(Updated July 18, 2013, 9:11 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Updated Resources output format to be:
"name(role):value;name(role):value;..."
Bugs: MESOS-505
https://issues.apache.org/jira/browse/MESOS-505
Repository: mesos
Description
-------
As part of the addition of reserved resources to the allocator, we decided to mark resources with roles. This makes it easy for the allocator to track what reserved pool different resources came from. This patch updates the current resource operators to consider roles, so for example only resources with the same role will be added together.
Diffs (updated)
-----
include/mesos/mesos.proto 233e3c3
src/common/resources.hpp 42dfb6a
src/common/resources.cpp 8bc1057
src/tests/resources_tests.cpp 67a11b3
Diff: https://reviews.apache.org/r/12340/diff/
Testing
-------
make check
Thanks,
Thomas Marshall
Re: Review Request 12340: Reservations 1 - Add roles to Resources
Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12340/
-----------------------------------------------------------
(Updated July 12, 2013, 2:49 a.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Ben's review.
Bugs: MESOS-505
https://issues.apache.org/jira/browse/MESOS-505
Repository: mesos
Description
-------
As part of the addition of reserved resources to the allocator, we decided to mark resources with roles. This makes it easy for the allocator to track what reserved pool different resources came from. This patch updates the current resource operators to consider roles, so for example only resources with the same role will be added together.
Diffs (updated)
-----
include/mesos/mesos.proto 233e3c3
src/common/resources.hpp 42dfb6a
src/common/resources.cpp 8bc1057
src/tests/resources_tests.cpp 67a11b3
Diff: https://reviews.apache.org/r/12340/diff/
Testing
-------
make check
Thanks,
Thomas Marshall