> On Feb. 6, 2019, 6:19 a.m., Meng Zhu wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 1818 (patched) > > <https://reviews.apache.org/r/69902/diff/1/?file=2123939#file2123939line1818> > > > > why not put `.allocatableTo(role)` like below? > > ditto blew in the second stage > > Benjamin Mahler wrote: > This condition is for **break**ing the role loop, whereas > allocatableTo(role) would be a continue. > > I initially had an **additional** continue-based allocatableTo(role) > check after this breaking check. But, it didn't seem worth the additional > code given that condition is already inside the framework loop as a break. It > would only save the framework sorter sort() call, which didn't seem like a > big win. > > We could certainly benchmark an additional continue check to skip the > sort() call, but in this patch I'm only aiming for fixing the incorrect > breaking without regressing in performance, and wanted to keep things simple.
I guess by my own logic here, we could further simplify by eliminating the first continue check, since it only avoids the role sort call and the same check is used as a break inside the role loop. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69902/#review212601 ----------------------------------------------------------- On Feb. 5, 2019, 10:30 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69902/ > ----------------------------------------------------------- > > (Updated Feb. 5, 2019, 10:30 p.m.) > > > Review request for mesos, Benjamin Bannier and Meng Zhu. > > > Bugs: MESOS-9554 > https://issues.apache.org/jira/browse/MESOS-9554 > > > Repository: mesos > > > Description > ------- > > The allocation loop was breaking out of the framework loop based on > resources filtered by the framework's capabilities. This can lead > to incorrect breaking in the case that a framework is incapable of > receiving resources whereas others are capable of receiving them. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > bb9a9c95979f36c0564af5b3babb1c43077a363b > > > Diff: https://reviews.apache.org/r/69902/diff/1/ > > > Testing > ------- > > make check > > Ran benchmarks: > > Before: > ``` > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/22 > Using 10000 agents and 3000 frameworks > Added 3000 frameworks in 161.434141ms > Added 10000 agents in 3.334691038secs > round 0 allocate() took 2.770901571secs to make 10000 offers after filtering > 10000 offers > round 1 allocate() took 2.470565355secs to make 10000 offers after filtering > 20000 offers > round 2 allocate() took 2.594315228secs to make 10000 offers after filtering > 30000 offers > round 3 allocate() took 2.389658909secs to make 10000 offers after filtering > 40000 offers > round 4 allocate() took 2.580871077secs to make 10000 offers after filtering > 50000 offers > round 5 allocate() took 2.522765768secs to make 10000 offers after filtering > 60000 offers > round 6 allocate() took 2.513849225secs to make 10000 offers after filtering > 70000 offers > round 7 allocate() took 2.500960884secs to make 10000 offers after filtering > 80000 offers > round 8 allocate() took 2.52565301secs to make 10000 offers after filtering > 90000 offers > round 9 allocate() took 2.445142852secs to make 10000 offers after filtering > 100000 offers > round 10 allocate() took 2.450911751secs to make 10000 offers after filtering > 110000 offers > ``` > > After: > ``` > [ RUN ] > SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/22 > Using 10000 agents and 3000 frameworks > Added 3000 frameworks in 168.484316ms > Added 10000 agents in 3.371731069secs > round 0 allocate() took 2.836207029secs to make 10000 offers after filtering > 10000 offers > round 1 allocate() took 2.383960243secs to make 10000 offers after filtering > 20000 offers > round 2 allocate() took 2.340208926secs to make 10000 offers after filtering > 30000 offers > round 3 allocate() took 2.396341093secs to make 10000 offers after filtering > 40000 offers > round 4 allocate() took 2.39952541secs to make 10000 offers after filtering > 50000 offers > round 5 allocate() took 2.401885613secs to make 10000 offers after filtering > 60000 offers > round 6 allocate() took 2.338538188secs to make 10000 offers after filtering > 70000 offers > round 7 allocate() took 2.301651652secs to make 10000 offers after filtering > 80000 offers > round 8 allocate() took 2.416522086secs to make 10000 offers after filtering > 90000 offers > round 9 allocate() took 2.68406696secs to make 10000 offers after filtering > 100000 offers > round 10 allocate() took 2.764043651secs to make 10000 offers after filtering > 110000 offers > ``` > > > Thanks, > > Benjamin Mahler > >
