----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68134/#review206913 -----------------------------------------------------------
Fix it, then Ship it! Much appreciated Greg! Looks like there was no test coverage of the unsuppression? Is there test coverage of the filter clearing? W.r.t. to clearing filters: (1) If we look at filters as filtering resources (explicitly) and attributes (implicitly), then we could make an argument around clearing filters when the attributes change. Although to be pendantic, if we were to match the resource semantics, we would keep the filter but do attribute subset checking (I don't think this makes sense for attributes). (2) If we consider the use cases for attribute based filtering, my sense is that we need a mechanism than the offer filter. E.g. let the framework specify some global attribute constraints. Not sure if we'll go down this route since it's at conflict with shared state scheduling and it adds complexity. I could see an argument with (1) for clearing filters when the agent's attributes change. src/master/allocator/mesos/hierarchical.cpp Lines 759-768 (original), 757-759 (patched) <https://reviews.apache.org/r/68134/#comment290045> Probably an explanation to the reader is warranted on why we do this? - Benjamin Mahler On July 31, 2018, 6:57 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68134/ > ----------------------------------------------------------- > > (Updated July 31, 2018, 6:57 p.m.) > > > Review request for mesos, Benno Evers and Benjamin Mahler. > > > Bugs: MESOS-9124 > https://issues.apache.org/jira/browse/MESOS-9124 > > > Repository: mesos > > > Description > ------- > > When agent reconfiguration was enabled in Mesos, the allocator was > also updated to remove all offer filters associated with an agent when > that agent's attributes change. In addition, whenever filters for an > agent are removed, the framework is revived for all roles that it is > registered in. > > While this ensures that schedulers will have an opportunity to use > resources on an agent after reconfiguration, modifying the scheduler's > suppression may put the scheduler in an inconsistent state, where it > believes it is suppressed in a particular role when it is not. > > This patch eliminates the suppression modification code, while > keeping the code which removes a reconfigured agent's offer filters. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 35992474eacb8b14ae57e1dc23307e1542f63cb5 > > > Diff: https://reviews.apache.org/r/68134/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Greg Mann > >
