> On May 25, 2016, 1:24 p.m., Joris Van Remoortere wrote: > > src/master/allocator/sorter/drf/sorter.cpp, lines 90-91 > > <https://reviews.apache.org/r/47258/diff/1/?file=1379945#file1379945line90> > > > > Is this a guard as opposed to a `CHECK` because someone is using this > > API correctly and we are mitigating the performance impact of that? > > > > We could argue that it shouldn't be a `CHECK` because it's not an > > internal variant; however, I would like to know if there's a higher level > > bug we should be fixing :-)
I don't think it should be a CHECK here but it should be a CHECK in /r/47259/ as I commented on /r/47259/ for CHECK vs. if guard. The motivation for this patch is from debugging /r/43666/. > We could argue that it shouldn't be a CHECK because it's not an internal > variant; I think this is exactly right. The "high-level bug" is probably that due to the tight coupling between the allocator and the sorter we are forced to CHECK public method arguments as well. I think we should miminize this. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47258/#review134826 ----------------------------------------------------------- On May 19, 2016, 11:27 a.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47258/ > ----------------------------------------------------------- > > (Updated May 19, 2016, 11:27 a.m.) > > > Review request for mesos, Dario Rexin and Joris Van Remoortere. > > > Bugs: MESOS-5279 > https://issues.apache.org/jira/browse/MESOS-5279 > > > Repository: mesos > > > Description > ------- > > Make sure 'activate' is a no-op if the client is already active. > > > Diffs > ----- > > src/master/allocator/sorter/drf/sorter.cpp > 4306973277b9d32356eed31ceabac09fb2a03e6c > src/master/allocator/sorter/sorter.hpp > 9e04adf54f2d80541a95f0a9a49b329dc9e8f5e3 > > Diff: https://reviews.apache.org/r/47258/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jiang Yan Xu > >