You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Michael Park <mc...@gmail.com> on 2015/03/03 08:53:16 UTC
Review Request 31667: Piped hashmap from allocator through to sorter.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/
-----------------------------------------------------------
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Bugs: MESOS-2373
https://issues.apache.org/jira/browse/MESOS-2373
Repository: mesos
Description
-------
`Sorter` changes:
- Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
- `allocation` returns `hashmap<SlaveID, Resources>`.
`DRFSorter` changes:
- `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
- `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
Diffs
-----
src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
Diff: https://reviews.apache.org/r/31667/diff/
Testing
-------
make check
Thanks,
Michael Park
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review75145
-----------------------------------------------------------
Patch looks great!
Reviews applied: [31183, 31664, 31665, 31666, 31667]
All tests passed.
- Mesos ReviewBot
On March 3, 2015, 8:29 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 8:29 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Michael Park <mc...@gmail.com>.
> On March 6, 2015, 11:09 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/sorter/drf/sorter.hpp, line 89
> > <https://reviews.apache.org/r/31667/diff/3/?file=883431#file883431line89>
> >
> > `SlaveID` as a function parameter?
Are you suggesting adding `SlaveID` to this function? or are you suggesting adding a `virtual Resources allocation(const std::string& name, const SlaveID& slaveId);`?
If former, we actually use this form of `allocation` in `removeFramework`.
If latter, we would have to justify the performance benefit or have a good reason why it should be in the `Sorter` API.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review75561
-----------------------------------------------------------
On March 3, 2015, 8:29 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 8:29 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Michael Park <mc...@gmail.com>.
> On March 6, 2015, 11:09 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.hpp, lines 313-317
> > <https://reviews.apache.org/r/31667/diff/3/?file=883430#file883430line313>
> >
> > See my comment to https://reviews.apache.org/r/31666/.
See my comment in [r31666](https://reviews.apache.org/r/31666/).
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review75561
-----------------------------------------------------------
On March 3, 2015, 8:29 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 8:29 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review75561
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.hpp
<https://reviews.apache.org/r/31667/#comment122746>
See my comment to https://reviews.apache.org/r/31666/.
src/master/allocator/sorter/drf/sorter.hpp
<https://reviews.apache.org/r/31667/#comment122748>
`SlaveID` as a function parameter?
- Alexander Rukletsov
On March 3, 2015, 8:29 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 8:29 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review75630
-----------------------------------------------------------
Patch looks great!
Reviews applied: [31183, 31664, 31665, 31666, 31667]
All tests passed.
- Mesos ReviewBot
On March 7, 2015, 10:03 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated March 7, 2015, 10:03 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review80429
-----------------------------------------------------------
Patch looks great!
Reviews applied: [31183, 31664, 31665, 31666, 31667]
All tests passed.
- Mesos ReviewBot
On April 17, 2015, 4:57 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 4:57 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 9f9a631ee52a3ab194e678f9be139e8dabc7f3db
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review83171
-----------------------------------------------------------
Patch looks great!
Reviews applied: [31183, 31664, 31665, 31666, 31667]
All tests passed.
- Mesos ReviewBot
On May 10, 2015, 12:21 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated May 10, 2015, 12:21 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 09adced9d8712b3eeda885d598443791186890db
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review83669
-----------------------------------------------------------
Ship it!
Looks good, just some minor notes below. Went over these with Michael so I'll make the updates for him.
src/master/allocator/sorter/drf/sorter.cpp
<https://reviews.apache.org/r/31667/#comment134685>
Hm.. since we're guaranteed to just re-insert on the next line, let's not bother with this?
src/master/allocator/sorter/drf/sorter.cpp
<https://reviews.apache.org/r/31667/#comment134686>
Maybe we should make it more clear about why we're even talking about safety in the first place? Might not be clear to the reader that Resources::sum is known to be problematic for non-scalars:
```
// NOTE: Summation is incorrect for non-scalars, but since we
// only care about scalar resources, this is safe.
```
src/master/allocator/sorter/sorter.hpp
<https://reviews.apache.org/r/31667/#comment134680>
Thanks!
src/master/allocator/sorter/sorter.hpp
<https://reviews.apache.org/r/31667/#comment134682>
#include <stout/hashmap.hpp> for this?
src/tests/sorter_tests.cpp
<https://reviews.apache.org/r/31667/#comment134687>
Let's break this line consistently with the one below, just to make it more readable :)
src/tests/sorter_tests.cpp
<https://reviews.apache.org/r/31667/#comment134688>
Would be nice to also validate that there is only 1 entry in the map :)
src/tests/sorter_tests.cpp
<https://reviews.apache.org/r/31667/#comment134689>
How about MultipleSlaves and UpdateMultipleSlaves? Since this is these are the only tests now with multiple slaves :)
src/tests/sorter_tests.cpp
<https://reviews.apache.org/r/31667/#comment134690>
Let's check the size too?
src/tests/sorter_tests.cpp
<https://reviews.apache.org/r/31667/#comment134700>
EXPECT_EQ wants the expected value as the first argument (to print expected vs actual), so let's flip the arguments here
src/tests/sorter_tests.cpp
<https://reviews.apache.org/r/31667/#comment134692>
This says construct an offer operation, but you just flatten? Seems like we should go through the apply() operation code here.
src/tests/sorter_tests.cpp
<https://reviews.apache.org/r/31667/#comment134691>
Let's check the size too?
src/tests/sorter_tests.cpp
<https://reviews.apache.org/r/31667/#comment134701>
Ditto here, let's flip the arguments.
- Ben Mahler
On May 10, 2015, 12:21 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated May 10, 2015, 12:21 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 09adced9d8712b3eeda885d598443791186890db
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/
-----------------------------------------------------------
(Updated May 10, 2015, 12:21 a.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Erase the `SlaveID` keys once the resources get to empty.
Bugs: MESOS-2373
https://issues.apache.org/jira/browse/MESOS-2373
Repository: mesos
Description
-------
`Sorter` changes:
- Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
- `allocation` returns `hashmap<SlaveID, Resources>`.
`DRFSorter` changes:
- `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
- `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp 09adced9d8712b3eeda885d598443791186890db
src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
Diff: https://reviews.apache.org/r/31667/diff/
Testing
-------
make check
Thanks,
Michael Park
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/
-----------------------------------------------------------
(Updated April 17, 2015, 4:57 a.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Addressed AlexR's comments.
Bugs: MESOS-2373
https://issues.apache.org/jira/browse/MESOS-2373
Repository: mesos
Description
-------
`Sorter` changes:
- Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
- `allocation` returns `hashmap<SlaveID, Resources>`.
`DRFSorter` changes:
- `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
- `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp 9f9a631ee52a3ab194e678f9be139e8dabc7f3db
src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
Diff: https://reviews.apache.org/r/31667/diff/
Testing
-------
make check
Thanks,
Michael Park
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Michael Park <mc...@gmail.com>.
> On April 8, 2015, 2:09 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 349
> > <https://reviews.apache.org/r/31667/diff/4/?file=888331#file888331line349>
> >
> > Does it need to be inside the loop?
Absolutely not! Good catch. Fixed.
> On April 8, 2015, 2:09 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/sorter/drf/sorter.cpp, line 254
> > <https://reviews.apache.org/r/31667/diff/4/?file=888333#file888333line254>
> >
> > Is it a TODO or a NOTE?
Should have been a `NOTE`. Fixed.
> On April 8, 2015, 2:09 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/sorter/drf/sorter.cpp, line 258
> > <https://reviews.apache.org/r/31667/diff/4/?file=888333#file888333line258>
> >
> > `_allocations` is per-client allocation, let's reflect this in the name and get rid of leading underscore. How about `clientAllocations`?
Fixed to `s/_allocations/clientAllocation/`. Dropped the 's' because it's a single allocation for a client out of `allocations`. The semantics are the same as the `allocation` function.
Also `s/_resources/totalResources/`.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review79345
-----------------------------------------------------------
On April 17, 2015, 4:57 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 4:57 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 9f9a631ee52a3ab194e678f9be139e8dabc7f3db
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review79345
-----------------------------------------------------------
Ship it!
src/master/allocator/mesos/hierarchical.hpp
<https://reviews.apache.org/r/31667/#comment128609>
Does it need to be inside the loop?
src/master/allocator/sorter/drf/sorter.cpp
<https://reviews.apache.org/r/31667/#comment128611>
Is it a TODO or a NOTE?
src/master/allocator/sorter/drf/sorter.cpp
<https://reviews.apache.org/r/31667/#comment128612>
`_allocations` is per-client allocation, let's reflect this in the name and get rid of leading underscore. How about `clientAllocations`?
- Alexander Rukletsov
On March 7, 2015, 10:03 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated March 7, 2015, 10:03 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/
-----------------------------------------------------------
(Updated March 7, 2015, 10:03 a.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Rebased.
Bugs: MESOS-2373
https://issues.apache.org/jira/browse/MESOS-2373
Repository: mesos
Description
-------
`Sorter` changes:
- Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
- `allocation` returns `hashmap<SlaveID, Resources>`.
`DRFSorter` changes:
- `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
- `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
Diff: https://reviews.apache.org/r/31667/diff/
Testing
-------
make check
Thanks,
Michael Park
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/
-----------------------------------------------------------
(Updated March 3, 2015, 8:29 p.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Moved `sum` into `Resources`.
Bugs: MESOS-2373
https://issues.apache.org/jira/browse/MESOS-2373
Repository: mesos
Description
-------
`Sorter` changes:
- Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
- `allocation` returns `hashmap<SlaveID, Resources>`.
`DRFSorter` changes:
- `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
- `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
Diff: https://reviews.apache.org/r/31667/diff/
Testing
-------
make check
Thanks,
Michael Park
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/#review74924
-----------------------------------------------------------
Patch looks great!
Reviews applied: [31183, 31664, 31665, 31666, 31667]
All tests passed.
- Mesos ReviewBot
On March 3, 2015, 8:17 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31667/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 8:17 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> `Sorter` changes:
>
> - Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
> - `allocation` returns `hashmap<SlaveID, Resources>`.
>
> `DRFSorter` changes:
>
> - `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
> - `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
> src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31667/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 31667: Piped hashmap from allocator through to sorter.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31667/
-----------------------------------------------------------
(Updated March 3, 2015, 8:17 a.m.)
Review request for mesos, Alexander Rukletsov and Ben Mahler.
Changes
-------
Simplified `DRFSorter::calculateShare` by using the `sum(resources.values())` pattern.
Bugs: MESOS-2373
https://issues.apache.org/jira/browse/MESOS-2373
Repository: mesos
Description
-------
`Sorter` changes:
- Augmented `add`, `remove`, `allocated`, `unallocated`, `update` with `SlaveID`.
- `allocation` returns `hashmap<SlaveID, Resources>`.
`DRFSorter` changes:
- `allocations` is updated from `hashmap<std::string, Resources>` to `hashmap<std::string, hashmap<SlaveID, Resources>>`.
- `resources` is updated from `Resources` to `hashmap<SlaveID, Resources>`.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp c0b1da75565d9dc7728c5566f01815234163fc47
src/master/allocator/sorter/drf/sorter.hpp 4366710d6530b784aa5094813328d0e338239ba0
src/master/allocator/sorter/drf/sorter.cpp 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
src/master/allocator/sorter/sorter.hpp e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
Diff: https://reviews.apache.org/r/31667/diff/
Testing
-------
make check
Thanks,
Michael Park