----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51027/#review150508 -----------------------------------------------------------
Looking to the test update. src/master/allocator/mesos/hierarchical.hpp (lines 223 - 224) <https://reviews.apache.org/r/51027/#comment218505> fit to one line src/master/allocator/mesos/hierarchical.hpp (lines 396 - 397) <https://reviews.apache.org/r/51027/#comment218520> Can you please update the comments a bit? I can see that the logic of add candidates for all events are actually centralized in `allocate(const hashset<SlaveID>& slaveIds);` via `allocationCandidates = allocationCandidates | slaveIds;`, I cannot see the logic of add candidates triggered in `addSlave` or others. src/master/allocator/mesos/hierarchical.cpp (lines 1810 - 1811) <https://reviews.apache.org/r/51027/#comment218522> I think it's not good to put it here, what about moving this to #1324? This can make the allocate() more clear: ``` _allocate(); _deallocate(); allocationCandidates.clear(); ``` - Guangya Liu On 九月 26, 2016, 4:27 p.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51027/ > ----------------------------------------------------------- > > (Updated 九月 26, 2016, 4:27 p.m.) > > > Review request for mesos, Benjamin Mahler, Guangya Liu, James Peach, Klaus > Ma, and Jiang Yan Xu. > > > Bugs: MESOS-3157 > https://issues.apache.org/jira/browse/MESOS-3157 > > > Repository: mesos > > > Description > ------- > > - Triggered allocations dispatch allocate() only > if there is no pending allocation in the queue. > - Allocation candidates are accumulated and only > cleared when enqueued allocations are processed. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > 2c31471ee0f5d6836393bf87ff9ecfd8df835013 > src/master/allocator/mesos/hierarchical.cpp > 3f51f4194c1ba7c1e4f08c3dd623281ca5754d39 > > Diff: https://reviews.apache.org/r/51027/diff/ > > > Testing > ------- > > make check > > note: check without filters depends on https://reviews.apache.org/r/51028 > > With new benchmark https://reviews.apache.org/r/49617: > Sample output without 51027: > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22 > Using 10000 agents and 3000 frameworks > Added 3000 frameworks in 57251us > Added 10000 agents in 3.21345353333333mins > allocator settled after 1.61236038333333mins > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22 > (290578 ms) > > Sample output with 51027: > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22 > Using 10000 agents and 3000 frameworks > Added 3000 frameworks in 39817us > Added 10000 agents in 3.22860541666667mins > allocator settled after 25.525654secs > [ OK ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22 > (220137 ms) > > > Thanks, > > Jacob Janco > >
