----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43824/#review121132 -----------------------------------------------------------
Thanks for following up with this. AlexR is on vacation now, so I'll review these changes. I only had a few suggestions. After reviewing the issues AlexR opened up on the previous patch, the only remaining question is whether to use a `std::set<FrameworkId>` instead of a `hashmap<FrameworkID, size_t> counts;`. src/tests/hierarchical_allocator_tests.cpp (lines 2647 - 2649) <https://reviews.apache.org/r/43824/#comment182778> Please fix the wrapping to 70 or 80 chars instead of 60. src/tests/hierarchical_allocator_tests.cpp (lines 2653 - 2656) <https://reviews.apache.org/r/43824/#comment182779> Can you say 100% and 0% instead of 1 and 0? And how is framework1's "share" '6'? Is that the number of agents/offers, rather than the framework's "share"? src/tests/hierarchical_allocator_tests.cpp (lines 2733 - 2736) <https://reviews.apache.org/r/43824/#comment182780> It shouldn't be necessary to advance the clock if updateWeights will notice the change and 'rebalance' immediately, right? src/tests/hierarchical_allocator_tests.cpp (lines 2793 - 2797) <https://reviews.apache.org/r/43824/#comment182781> Please comment why we don't manually advance the clock here. I guess `updateWeights` won't call `allocate()` since no framework exists in 'role3' yet, but `addFramework` will. - Adam B On Feb. 21, 2016, 11:23 p.m., Yongqiao Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43824/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2016, 11:23 p.m.) > > > Review request for mesos, Adam B and Alexander Rukletsov. > > > Bugs: MESOS-4200 > https://issues.apache.org/jira/browse/MESOS-4200 > > > Repository: mesos > > > Description > ------- > > Addressed comments of 41672. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 > > Diff: https://reviews.apache.org/r/43824/diff/ > > > Testing > ------- > > make && make check successfully. > > > Thanks, > > Yongqiao Wang > >
