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