----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52534/#review158858 -----------------------------------------------------------
Fix it, then Ship it! I feel it's hard to explain this patch without /r/51027/. It's probably better to have it go after /r/51027/ in the chain. It's fine if /r/51027/ requires this patch to pass the tests. src/master/allocator/mesos/hierarchical.hpp (line 230) <https://reviews.apache.org/r/52534/#comment229684> Can we conslidate the comments in one place so we can provide better context without too much redundancy? Here we can say something like: ``` Helper for dispatching `expire()` after a delay. See comments in `recoverResources()`. ``` src/master/allocator/mesos/hierarchical.cpp (lines 1072 - 1073) <https://reviews.apache.org/r/52534/#comment229688> How about the following comment? I feel it's clearer. ``` Because the batch `allocate()` dispatches `run()`, we use `_expire()` to dispatch `expire()` after at least the same delay to make sure the offer filter remains active for at least a single allocation run. ``` - Jiang Yan Xu On Oct. 4, 2016, 4:28 p.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52534/ > ----------------------------------------------------------- > > (Updated Oct. 4, 2016, 4:28 p.m.) > > > Review request for mesos. > > > 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 > 2c31471ee0f5d6836393bf87ff9ecfd8df835013 > src/master/allocator/mesos/hierarchical.cpp > c8f9492ee1b69e125a1e841116d22a578a9b524e > > 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 > >
