----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52534/#review162235 -----------------------------------------------------------
Fix it, then Ship it! This adjusts the code to fix the problem, so looks good to me. Longer term it would be great to have something that's less brittle for dealing with the timer expirations after an allocation on the agent has occurred. The current approach relies on non-local delay and dispatch knowledge, vs. an approach that was explicitly marking filters or waiting for a future (e.g. `process::collect(delay, allocationHasOccurred)`). src/master/allocator/mesos/hierarchical.cpp (lines 1078 - 1087) <https://reviews.apache.org/r/52534/#comment233548> The naming here seems backwards? `_expire()` then `expire()`? src/master/allocator/mesos/hierarchical.cpp (lines 1078 - 1081) <https://reviews.apache.org/r/52534/#comment233549> Hm.. seems like this should be incorporated to the comment above that is explaining when to expire the filter. Also, maybe since the explanation is here we can just use a lambda instead of the additional top level function? - Benjamin Mahler On Jan. 12, 2017, 6:56 p.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52534/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2017, 6:56 p.m.) > > > Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Jiang Yan > Xu. > > > Bugs: MESOS-6904 > https://issues.apache.org/jira/browse/MESOS-6904 > > > Repository: mesos > > > Description > ------- > > - With an asynchronous `batch()` allocation, > this ensures that filters do not expire > before the next allocation. > - This patch should be reverted when allocation > occurs on resource recovery. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > a6424d624864155e1c87a28a63b784512c5c8722 > src/master/allocator/mesos/hierarchical.cpp > 91b1ec43940a788459f045ca4a4b82d4e8373bca > > Diff: https://reviews.apache.org/r/52534/diff/ > > > Testing > ------- > > With https://reviews.apache.org/r/51027/: > > GTEST_FILTER="-*SmallOfferFilter*" make check -j8 > > > Thanks, > > Jacob Janco > >
