Re: Review Request 34720: Added kill executor correction to slave.

2015-06-16 Thread Jie Yu
--- 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

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-16 Thread Jie Yu
On June 16, 2015, 9:22 p.m., Jie Yu wrote: LGTM. Some nits on logging. IN fact, can you consistently use `QoS correction KILL` (instead of QoS KILL correction) in the logging? - Jie --- This is an automatically generated e-mail. To

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-16 Thread Niklas Nielsen
On June 12, 2015, 3:49 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 4203-4210 https://reviews.apache.org/r/34720/diff/5/?file=983931#file983931line4203 Can you add a TODO here saying that we may want to check if containerId matches or not due to race (i.e., qos controller

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-16 Thread Niklas Nielsen
On June 16, 2015, 2:22 p.m., Jie Yu wrote: src/slave/slave.cpp, line 4210 https://reviews.apache.org/r/34720/diff/6/?file=985948#file985948line4210 Shouldn't be 'executorId' here? Also, can you put single quotes for executorId since it's specifed by the user (i.e., might contain

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-12 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34720/ --- (Updated June 12, 2015, 2:48 p.m.) Review request for mesos, Bartek Plotka,

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34720/#review87767 --- src/slave/slave.hpp

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-12 Thread Jie Yu
On June 12, 2015, 6:43 p.m., Jie Yu wrote: src/slave/slave.cpp, lines 4383-4400 https://reviews.apache.org/r/34720/diff/4/?file=983235#file983235line4383 First of all, we don't need to check if 'executor' is NULL or not here. If you looked at the callers, it's guaranteed that

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-11 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34720/ --- (Updated June 11, 2015, 3:58 p.m.) Review request for mesos, Bartek Plotka,

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-04 Thread Jie Yu
On June 4, 2015, 5:58 p.m., Vinod Kone wrote: src/slave/slave.cpp, lines 4400-4413 https://reviews.apache.org/r/34720/diff/3/?file=979257#file979257line4400 This is getting a little hairy. As the TODO says we really ought bubble this up via the Termination protobuf. Have you

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-04 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34720/#review86669 --- src/slave/slave.cpp

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-04 Thread Jie Yu
On June 4, 2015, 5:58 p.m., Vinod Kone wrote: src/slave/slave.cpp, lines 4400-4413 https://reviews.apache.org/r/34720/diff/3/?file=979257#file979257line4400 This is getting a little hairy. As the TODO says we really ought bubble this up via the Termination protobuf. Have you

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-04 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34720/ --- (Updated June 4, 2015, 10:43 a.m.) Review request for mesos, Bartek Plotka,

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-04 Thread Niklas Nielsen
On June 4, 2015, 10:58 a.m., Vinod Kone wrote: src/slave/slave.cpp, lines 4400-4413 https://reviews.apache.org/r/34720/diff/3/?file=979257#file979257line4400 This is getting a little hairy. As the TODO says we really ought bubble this up via the Termination protobuf. Have you

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-04 Thread Jie Yu
On June 4, 2015, 5:58 p.m., Vinod Kone wrote: src/slave/slave.cpp, lines 4400-4413 https://reviews.apache.org/r/34720/diff/3/?file=979257#file979257line4400 This is getting a little hairy. As the TODO says we really ought bubble this up via the Termination protobuf. Have you

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-04 Thread Niklas Nielsen
On June 4, 2015, 10:58 a.m., Vinod Kone wrote: src/slave/slave.cpp, lines 4400-4413 https://reviews.apache.org/r/34720/diff/3/?file=979257#file979257line4400 This is getting a little hairy. As the TODO says we really ought bubble this up via the Termination protobuf. Have you

Re: Review Request 34720: Added kill executor correction to slave.

2015-06-04 Thread Niklas Nielsen
On June 4, 2015, 10:58 a.m., Vinod Kone wrote: src/slave/slave.cpp, lines 4400-4413 https://reviews.apache.org/r/34720/diff/3/?file=979257#file979257line4400 This is getting a little hairy. As the TODO says we really ought bubble this up via the Termination protobuf. Have you