> On July 6, 2016, 6:24 p.m., Jiang Yan Xu wrote: > > src/tests/hierarchical_allocator_tests.cpp, lines 3711-3725 > > <https://reviews.apache.org/r/49617/diff/1/?file=1436562#file1436562line3711> > > > > 1. We should recover resources after the whole batch is finished and > > not after each `activateFramework` is called, as this is often the case in > > realistic scenarios. > > 2. If we don't `settle()` then vector is actually not thread-safe so I > > think we need to settle here. > > > > How about this for the entire loop? > > > > ``` > > for (unsigned batchStart = 0; > > batchStart < frameworkCount; > > batchStart += batchSize) { > > unsigned batchEnd = min(batchStart + batchSize, frameworkCount); > > > > // Activate all frameworks in this batch to trigger allocations. > > for (unsigned i = batchStart; i < batchEnd; i++) { > > allocator->activateFramework(frameworks[i].id()); > > } > > > > // Suppress offers in this batch after all activations. This is > > // requires another message round trip between schedulers and the > > // master following the activation. > > for (unsigned i = batchStart; i < batchEnd; i++) { > > allocator->suppressOffers(frameworks[k].id()); > > } > > > > // Make sure all resources are offered. This avoids data races in > > // the offers vector. > > Clock::settle(); > > > > foreach (auto offer, offers) { > > allocator->recoverResources( > > offer.frameworkId, > > offer.slaveId, > > offer.resources, > > None()); > > } > > > > offers.clear(); > > } > > ``` > > Jacob Janco wrote: > added a termination condition for (unsigned i = batchStart; i < batchEnd > && i < frameworkCount; i++) { > > to account for all frameworkCount cases. > > Also, I recover offers there because only framework 1 of the batch will > get an offer, so we will get 1005 offers for the whole of a test run with > 1000 agents the 5 being the 5 batches of activate/suppress. What do you think? > > Jacob Janco wrote: > Actually strike that wasn't thinking, I'll report offers.size() at the > end! agree with this completely!
This seems irrelevant to the test, I'll remove that metric entirely. - Jacob ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49617/#review140887 ----------------------------------------------------------- On July 8, 2016, 1:22 a.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49617/ > ----------------------------------------------------------- > > (Updated July 8, 2016, 1:22 a.m.) > > > Review request for mesos, Joris Van Remoortere and Jiang Yan Xu. > > > Bugs: MESOS-5780 > https://issues.apache.org/jira/browse/MESOS-5780 > > > Repository: mesos > > > Description > ------- > > - This benchmark measures latency to stability of > the allocator following disconnection and > reconnection of all frameworks. > - In this scenario, frameworks are offered resources > and suppressed in batches. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > 0498cd5e54b0e4b87a767585a77699653aa52179 > > Diff: https://reviews.apache.org/r/49617/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jacob Janco > >