----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49616/#review143901 -----------------------------------------------------------
Fix it, then Ship it! src/tests/hierarchical_allocator_tests.cpp (lines 3808 - 3809) <https://reviews.apache.org/r/49616/#comment209861> Move this to right above `agentTotal`. src/tests/hierarchical_allocator_tests.cpp (line 3813) <https://reviews.apache.org/r/49616/#comment209859> The asymmetry between the string typed `agentTotal` and the `Resources` typed `allocation` looks jarring to me, how about just: ``` SlaveInfo agent = createSlaveInfo("cpus:24;mem:4096;disk:4096;ports:[31000-32000]"); // Then later, agents.push_back(agent); ``` src/tests/hierarchical_allocator_tests.cpp (line 3847) <https://reviews.apache.org/r/49616/#comment209891> The TODO would be to parameterize the `allocationsCount` right? src/tests/hierarchical_allocator_tests.cpp (line 3855) <https://reviews.apache.org/r/49616/#comment209862> const Allocation& allocation src/tests/hierarchical_allocator_tests.cpp (lines 3868 - 3869) <https://reviews.apache.org/r/49616/#comment209893> The implication is that there can be `frameworkCount % allocationsCount` frameworks not suppressed, which is fine in our test. Let's add a comment on this. Note that we could always choose `allocationsCount`s so `frameworkCount % allocationsCount == 0` even when we parameterize it but still we are fine even if it's not the case. src/tests/hierarchical_allocator_tests.cpp (line 3874) <https://reviews.apache.org/r/49616/#comment209892> We need a settle right before `watch.start()` so we are truely only measuring the allocation time. - Jiang Yan Xu On July 26, 2016, 12:28 a.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49616/ > ----------------------------------------------------------- > > (Updated July 26, 2016, 12:28 a.m.) > > > Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu. > > > Bugs: MESOS-5781 > https://issues.apache.org/jira/browse/MESOS-5781 > > > Repository: mesos > > > Description > ------- > > - Useful for high framework clusters which carry > many suppressed frameworks that are considered > during allocation. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > bb6947fcecb5b78047e98d10fc1278c612a69548 > > Diff: https://reviews.apache.org/r/49616/diff/ > > > Testing > ------- > > MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check > > > Thanks, > > Jacob Janco > >
