> On June 25, 2015, 12:31 a.m., Ben Mahler wrote: > > Thanks Michael! Did you see jie's comment on > > [MESOS-1187](https://issues.apache.org/jira/browse/MESOS-1187)? Looks like > > that will make these kinds of checks problematic. > > > > Also, it's going to be possible for the slave to be over-allocated due to > > oversubscription (currently the total from addSlave doesn't include the > > oversubpription resources, and in the future we might want to express > > over-allocation, see: > > [MESOS-2930](https://issues.apache.org/jira/browse/MESOS-2930).
Hey Ben! I think the `CHECK` does need to be dropped, more so because of https://reviews.apache.org/r/35836. Question: should I just drop the `CHECK` in this patch and keep the test? or should I just drop this patch all together? > Did you see jie's comment on MESOS-1187? Looks like that will make these > kinds of checks problematic. Yeah, I spoke to Jie about [MESOS-1187](https://issues.apache.org/jira/browse/MESOS-1187) earlier today. That's really a separate concern though, since that bug breaks our expectations of how `contains` and `equality` works for `Resources` in general. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35816/#review89295 ----------------------------------------------------------- On June 24, 2015, 3:35 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35816/ > ----------------------------------------------------------- > > (Updated June 24, 2015, 3:35 p.m.) > > > Review request for mesos, Ben Mahler and Jie Yu. > > > Repository: mesos > > > Description > ------- > > Fixed the incorrect `CHECK_EQ` introduced in > [r35699](https://reviews.apache.org/r/35699/). > > From @bmahler: > > Whoops, Jie just noticed that this isn't correct because > > 'updatedAllocation' is for the framework only, whereas 'total' and > > 'available' are for all frameworks on the slave. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > ef18ff850addfb5ce3500ed28e8ffd801e2d24eb > src/tests/hierarchical_allocator_tests.cpp > 3258840135290cd008ca09235d18b7f093dafd2e > > Diff: https://reviews.apache.org/r/35816/diff/ > > > Testing > ------- > > (1) Added a test > `HierarchicalAllocatorTest.UpdateAllocationMultipleFrameworks` which breaks > the previous `CHECK_EQ` condition. > (2) `make check` > > > Thanks, > > Michael Park > >
