> On Feb. 23, 2015, 7:36 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/sorter/sorter.hpp, lines 64-66
> > <https://reviews.apache.org/r/31183/diff/2/?file=869986#file869986line64>
> >
> > Maybe we can provide overrides here to avoid duplicating `foreach`s in
> > the client code in `HierarchicalAllocator`?
I had considered this and pulled back on it because the call-sites currently
look like:
```cpp
foreachpair (const SlaveID& slaveId, const Resources& allocated, used) {
roleSorter->allocated(role, slaveId, allocated.unreserved());
frameworkSorters[role]->add(slaveId, allocated);
frameworkSorters[role]->allocated(frameworkId.value(), slaveId, allocated);
}
```
With the overload, it doesn't get all that much better because of the
`allocated.unreserved()` call.
```cpp
foreachpair (const SlaveID& slaveId, const Resources& allocated, used) {
roleSorter->allocated(role, slaveId, allocated.unreserved());
}
frameworkSorters[role]->add(used);
frameworkSorters[role]->allocated(frameworkId.value(), used);
```
I think with 2 such loops we can punt on it for now?
> On Feb. 23, 2015, 7:36 p.m., Alexander Rukletsov wrote:
> > src/master/master.hpp, lines 1194-1205
> > <https://reviews.apache.org/r/31183/diff/2/?file=869988#file869988line1194>
> >
> > Looks like this code does the same as one in `master/http.cpp`. Doesn't
> > it make sense to introduce a resource union method? Something like:
> > ```
> > hashmap<SlaveID, Resources> union(
> > hashmap<SlaveID, Resources> arg1,
> > const hashmap<SlaveID, Resources>& arg2)
> > { ...
> > return arg1;
> > }
> > ```
Introduced `operator+=` and `operator+` for `hashmap<SlaveID, Resources>`.
> On Feb. 23, 2015, 7:36 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/sorter/sorter.hpp, lines 72-75
> > <https://reviews.apache.org/r/31183/diff/2/?file=869986#file869986line72>
> >
> > Ditto here and below for `add()` and `remove()`.
See my comment above for `add` and `remove`. For `update`, I think we actually
want to take (`slaveId`, `oldAllocation`, `newAllocation`), rather than
(`oldSlaveId`, `oldAllocation`, `newSlaveId`, `newAllocation`).
> On Feb. 23, 2015, 7:36 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 90-94
> > <https://reviews.apache.org/r/31183/diff/2/?file=869987#file869987line90>
> >
> > Shouldn't it go before Ben Mahler's comment?
Fixed.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31183/#review73586
-----------------------------------------------------------
On Feb. 24, 2015, 11:27 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31183/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2015, 11:27 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Benjamin Hindman, Ben Mahler,
> and Jie Yu.
>
>
> Bugs: MESOS-2373
> https://issues.apache.org/jira/browse/MESOS-2373
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am
> ac2bbed6fe86623fb51cac3613d79d7b1372df9d
> 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp
> 3293086a009a8f7cf7bd343eb7d3e85623636550
> 3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp
> fb98317a68986cb1228c584a8cd83b07737895a8
> configure.ac 9b2d7f15f535aaaf85faf9b4f7af750f1dbdf472
> docs/modules.md a8b471541cdfa584eeb89fbe96643f93c712cfd4
> docs/slave-recovery.md 57eb786f94b2b1dee7bb35017618af90b4dc4a31
> include/mesos/resources.hpp c242bcc29c490841354d6fdc8d0de16eeea602ed
> src/common/resources.cpp a45bbaf696a6cc61a06daaa52a84f0014e7fe8cb
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/allocator/mesos/allocator.hpp
> fb898f1175b61b442204e6e38c69ccc2838a646f
> src/master/allocator/mesos/hierarchical.hpp
> 9b7ded34ad7546f36dd41f64fe2e3cf41c1cf702
> src/master/allocator/sorter/drf/sorter.hpp
> 4366710d6530b784aa5094813328d0e338239ba0
> src/master/allocator/sorter/drf/sorter.cpp
> 2f69f384b95ff20d3ee429a4570a8cffa74d8e8b
> src/master/allocator/sorter/sorter.hpp
> e2efb27b11dbea42dd73f81e5db0d6d2b0a6034b
> src/master/http.cpp 117c0ee720a60a1d8a25359028bad803f1fc2b07
> src/master/master.hpp 8c44d6ed57ad1b94a17bef8142a5e6a15889a810
> src/master/master.cpp 713307e1be596651283cc2cc95f114c42ad34a5e
> src/slave/containerizer/isolators/network/port_mapping.cpp
> 5227987cdb7b904c2f4bb2bdf5c5d705a435010d
> src/tests/hierarchical_allocator_tests.cpp
> 93753d1c04159a04a733927a487eb69505438e32
> src/tests/master_slave_reconciliation_tests.cpp
> 54cae62fda6be56ce438371c0e37bdf8cc8ace15
> src/tests/mesos.hpp e17afe74796ffe148274e278b5fa574e5c1bd34c
> src/tests/port_mapping_tests.cpp e2c8ba12b5574b06a6ba60c3c3a30b63cea1d23c
> src/tests/slave_tests.cpp 7ea012ab0883cc030b3e62f59879613866dab0fa
> src/tests/sorter_tests.cpp 42442353afe7bd3d1a5b43992f8ae191ac19bdcd
>
> Diff: https://reviews.apache.org/r/31183/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
