> 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
> 
>

Reply via email to