> On Jan. 21, 2016, 7:17 a.m., Ben Mahler wrote: > > src/tests/hierarchical_allocator_tests.cpp, lines 544-560 > > <https://reviews.apache.org/r/42355/diff/2/?file=1202267#file1202267line544> > > > > In the bottom section of this test, I'm not sure folks without our > > context will understand what is being done and what is expected to occur. > > > > Maybe we just write something as simple as the following, for example: > > > > The allocator will ensure that offer filters are removed after at least > > one batch allocation has occurred. Therefore, we expect that after the > > timeout elapses, the filter remains active for the next allocation and the > > resources are sent to framework1. > > > > Then, I'm curious why you don't have a subsequent allocation to verify > > that the offer filter was really removed, is there a test for that?
Good suggestions. I'll reword comments and add one more allocation. > On Jan. 21, 2016, 7:17 a.m., Ben Mahler wrote: > > src/tests/hierarchical_allocator_tests.cpp, line 546 > > <https://reviews.apache.org/r/42355/diff/2/?file=1202267#file1202267line546> > > > > Can you use offerFilter.refuse_seconds() here? Or create a 'timeout' > > variable which is used to both set the offerFitlter.refuse_seconds and is > > passed to this advance() call? Yep, makes sense. Thanks! - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42355/#review115583 ----------------------------------------------------------- On Jan. 19, 2016, 11:32 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42355/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2016, 11:32 p.m.) > > > Review request for mesos, Ben Mahler and Joris Van Remoortere. > > > Bugs: MESOS-4302 > https://issues.apache.org/jira/browse/MESOS-4302 > > > Repository: mesos > > > Description > ------- > > Without the timeout, we rely on filter expiration only. This guarantees > that filter removal is scheduled after `allocate()` if the allocator is > backlogged given default parameters are used. Additionally we ensure the > filter timeout is at least as big as the allocation interval. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 48acde69b1a2f305b568a7e322a58708063dd30a > src/tests/hierarchical_allocator_tests.cpp > 9362dd306497ba01e0f387c3862456cdcac6f863 > > Diff: https://reviews.apache.org/r/42355/diff/ > > > Testing > ------- > > On Mac OS 10.10.4: > > `make check` > > `GTEST_FILTER="HierarchicalAllocatorTest.FilterTimeout" ./bin/mesos-tests.sh > --gtest_repeat=100 --gtest_break_on_failure` passes with the patch and fails > without. > > `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh > --gtest_repeat=100 --gtest_break_on_failure` > > > Thanks, > > Alexander Rukletsov > >
