----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51027/#review148579 -----------------------------------------------------------
src/master/allocator/mesos/hierarchical.hpp (lines 227 - 228) <https://reviews.apache.org/r/51027/#comment216039> Hm.. I'm having a hard time understanding what a unique dispatch is, or if it's a useful concept. It seems to me that we could instead provide a generic operation batching abtraction that is provided as a library in libprocess (the registrar and the allocator could both use such an abstraction). This seems to be a clearer approach to me than providing some kind of alternate dispatch semantics (composition (build on top) vs inheritance (different flavors of dispatching)). src/master/allocator/mesos/hierarchical.cpp (lines 272 - 273) <https://reviews.apache.org/r/51027/#comment216038> It seems a bit odd that the caller has to both touch allocation candidates and then call ensureAllocation. A simpler way to think about this may be that `allocate` has changed from a synchronous function to an asynchronous one. I.e. the call-site here would be: ``` void allocate(...); // becomes: Future<Nothing> allocate(...); // Call-site: Future<Nothing> allocated = allocate(slave.keys()); // Actual allocation logic is a continuation now: Nothing _allocate(...); ``` (We probably do not need the Future just yet since callers don't need to set up continuations for now, however we might as well add it to express the asynchronous nature of the function.) This also avoids the need for the callers to touch the data structure correctly, which seems error-prone. They just call `allocate` and it deals with adding the SlaveIDs that need allocations performed. - Benjamin Mahler On Sept. 3, 2016, 6:05 a.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51027/ > ----------------------------------------------------------- > > (Updated Sept. 3, 2016, 6:05 a.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. > - Batched allocations are handled synchronously. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > dd07ed221d2c1755d2478369641ffdc46ecc4471 > src/master/allocator/mesos/hierarchical.cpp > 9e5db2196c6a541dc1208ba8b9f13ef9a518bcc4 > > 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 > >
