-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34720/#review88124
-----------------------------------------------------------

Ship it!


LGTM. Some nits on logging.


src/slave/slave.cpp (line 4137)
<https://reviews.apache.org/r/34720/#comment140520>

    I think we already have
    ```
    using mesos::slave::QoSController;
    ```
    
    So you can remove the namespace prefix. It probably will fit in one line.



src/slave/slave.cpp (line 4161)
<https://reviews.apache.org/r/34720/#comment140522>

    Ditto on removing mesos::slave:: prefix. Please do a sweep.



src/slave/slave.cpp (line 4172)
<https://reviews.apache.org/r/34720/#comment140526>

    s/kill/KILL/
    
    Here and everywhere else. Also, to be consistent, let's use the same 
logging format:
    
    ```
    Ignoring QoS KILL correction: framework id not specified.
    ```



src/slave/slave.cpp (lines 4182 - 4183)
<https://reviews.apache.org/r/34720/#comment140533>

    ```
    Ignoring QoS KILL correction on framework ...: executor id not specified.
    ```



src/slave/slave.cpp (line 4191)
<https://reviews.apache.org/r/34720/#comment140527>

    ```Ignoring QoS KILL correction on framework ...: framework cannot be 
found```



src/slave/slave.cpp (line 4202)
<https://reviews.apache.org/r/34720/#comment140528>

    Ignoring QoS KILL correction on framework ...: framework is terminating.



src/slave/slave.cpp (line 4209)
<https://reviews.apache.org/r/34720/#comment140525>

    Shouldn't be 'executorId' here? Also, can you put single quotes for 
executorId since it's specifed by the user (i.e., might contain spaces).



src/slave/slave.cpp (lines 4209 - 4211)
<https://reviews.apache.org/r/34720/#comment140529>

    To be consistent, here should be:
    ```
    Ignoring QoS KILL correction on executor ... of framework ...: executor 
cannot be found
    ```



src/slave/slave.cpp (lines 4218 - 4220)
<https://reviews.apache.org/r/34720/#comment140530>

    HOw about
    ```
    Applying QoS KILL correction on executor ... of framework ...
    ```



src/slave/slave.cpp (line 4219)
<https://reviews.apache.org/r/34720/#comment140524>

    YOu forgot the single quote here.



src/slave/slave.cpp (lines 4236 - 4238)
<https://reviews.apache.org/r/34720/#comment140531>

    Ditto on consistency:
    ```
    Ignoring QoS KILL correction on executor ... of framework ...: executor is 
XXX
    ```



src/slave/slave.cpp (line 4247)
<https://reviews.apache.org/r/34720/#comment140532>

    QoS correction type ... is not supported.


- Jie Yu


On June 16, 2015, 8:42 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34720/
> -----------------------------------------------------------
> 
> (Updated June 16, 2015, 8:42 p.m.)
> 
> 
> Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-2653
>     https://issues.apache.org/jira/browse/MESOS-2653
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary
> 
> 
> Diffs
> -----
> 
>   src/slave/flags.hpp 6c24e56d15881b0e3aeec3d4824842cf57121fc6 
>   src/slave/flags.cpp 93690cfa9dbf2658ce642829299f4adf08bb1062 
>   src/slave/slave.hpp dbed46d76cc7fbdd1a8d3ebcc2a1ff08b75da10f 
>   src/slave/slave.cpp 361433063b5cb38d8326f8247cf4fc6f4a18e5c9 
>   src/tests/mesos.hpp ecdf9109d2e46e8730754eeeb4978863679d56e7 
>   src/tests/mesos.cpp 509f9f205fdb1fa094e313b6f0da53000ffecbb3 
> 
> Diff: https://reviews.apache.org/r/34720/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Reply via email to