----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52534/#review163633 -----------------------------------------------------------
Fix it, then Ship it! src/master/allocator/mesos/hierarchical.hpp (lines 229 - 230) <https://reviews.apache.org/r/52534/#comment235107> This comment here seems a bit odd, we just need to describe what it does and not how it's being called. The original comment LGTM and it can cover both `expire` and `_expire` as they are of a group. src/master/allocator/mesos/hierarchical.hpp (line 243) <https://reviews.apache.org/r/52534/#comment235108> No need to change this. src/master/allocator/mesos/hierarchical.cpp (line 932) <https://reviews.apache.org/r/52534/#comment235109> No need to change this. src/master/allocator/mesos/hierarchical.cpp (lines 1061 - 1064) <https://reviews.apache.org/r/52534/#comment235111> The ojective is already described in the paragraph above so we don't have to reiterate it. We just need to explain the mechanism: ``` // Because `batch()` performs the allocation through a dispatch, we expire the filter also through a dispatched `_expire()` to achieve above. ``` src/master/allocator/mesos/hierarchical.cpp (lines 1806 - 1816) <https://reviews.apache.org/r/52534/#comment235112> Actually no need to disambiguate now that we only have one `_expire`. - Jiang Yan Xu On Jan. 30, 2017, 5:47 p.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52534/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2017, 5:47 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 > 2cda3e311ce339d82065d68de83e86439fa192ff > src/master/allocator/mesos/hierarchical.cpp > f471b6848bebae601a7a0509e9c6ad5eab4fa4a2 > > 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 > >
