mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 47258: Make sure 'activate' is a no-op if the client is already active.
Date Thu, 26 May 2016 17:03:37 GMT


> 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 :-)
> 
> Jiang Yan Xu wrote:
>     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.

Plus we can look at the [deactive](https://github.com/apache/mesos/blob/6ce476461f0fedfb4ed4e40c15f25bb79a39b0f3/src/master/allocator/sorter/drf/sorter.cpp#L95)
as an example: if it's already not "active", deactivate is a no-op.


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message