Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 23, 2014, 12:54 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, lines 1270-1273 > > > > > > Hey Bernd and Vinod, coverity just reported this, looks like it's now > > dead code! > > > > Can you fo

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 23, 2014, 12:54 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, lines 1270-1273 > > > > > > Hey Bernd and Vinod, coverity just reported this, looks like it's now > > dead code! > > > > Can you fo

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review58074 --- src/slave/slave.cpp

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 17, 2014, 5:39 p.m., Vinod Kone wrote: > > Ship It! I committed this after some minor fixes. Thanks Bernd! - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review57172

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review57172 --- Ship it! Ship It! - Vinod Kone On Oct. 17, 2014, 4:25 p.m., Bern

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review57159 --- Bad patch! Reviews applied: [23912] Failed command: ./support/appl

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 17, 2014, 9:25 a.m.) Review request for mesos and Vinod Kone. C

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 10, 2014, 11:24 a.m., Vinod Kone wrote: > > src/slave/slave.cpp, line 1413 > > > > > > It is weird to me that you remove the task here but (potentially) > > remove the executor up in _runTask(). It's not ob

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review56679 --- Patch looks great! Reviews applied: [23912] All tests passed. - M

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 14, 2014, 3:33 p.m., Vinod Kone wrote: > > src/tests/mesos.cpp, lines 343-345 > > > > > > Sorry, missed this earlier. We align constructor arguments as follows: > > > > MockSlave::MockSlave(const sl

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 15, 2014, 1:06 a.m.) Review request for mesos and Vinod Kone. C

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 15, 2014, 1:03 a.m.) Review request for mesos and Vinod Kone. C

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 10, 2014, 11:24 a.m., Vinod Kone wrote: > > src/slave/slave.cpp, line 1413 > > > > > > It is weird to me that you remove the task here but (potentially) > > remove the executor up in _runTask(). It's not ob

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 10, 2014, 6:24 p.m., Vinod Kone wrote: > > src/slave/slave.cpp, line 1413 > > > > > > It is weird to me that you remove the task here but (potentially) > > remove the executor up in _runTask(). It's not obv

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review56584 --- src/tests/mesos.cpp

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 14, 2014, 9:54 p.m.) Review request for mesos and Vinod Kone. C

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review56412 --- Patch looks great! Reviews applied: [23912] All tests passed. - M

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 13, 2014, 8:53 a.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 13, 2014, 3:57 a.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 10, 2014, 11:24 a.m., Vinod Kone wrote: > > src/slave/slave.cpp, line 1201 > > > > > > why the change here? > > > > is it possible for pending[executorId] to be empty() if you didn't > > erase th

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 10, 2014, 11:24 a.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp, line 1027 > > > > > > why this change? More readable. But I'll revert it. > On Oct. 10, 2014, 11:24 a.m., Vinod Kone wrote: > > src

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review56016 --- src/slave/slave.cpp

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55993 --- Patch looks great! Reviews applied: [23912] All tests passed. - M

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 9, 2014, 7:10 a.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Aug. 5, 2014, 2:29 p.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp, lines 958-962 > > > > > > Kill these expectations since an executor won't be launched in this > > test. > > Vinod Kone wrote: > doesn

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 8, 2014, 10:42 a.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp, lines 1071-1075 > > > > > > This has not been moved !? I have been searching through the previous review comments, but have not been

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55834 --- src/slave/slave.cpp

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55787 --- Patch looks great! Reviews applied: [23912] All tests passed. - M

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 8, 2014, 3:57 a.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 7, 2014, 11:22 a.m., Michael Park wrote: > > src/slave/slave.cpp, lines 1354-1356 > > > > > > It looks like `taskMap.erase(taskId)` needs to modify the > > `framework->pending` hashmap. I think we want `for

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Oct. 7, 2014, 10:38 a.m., Vinod Kone wrote: > > I don't see any changes in the diff? Bad diff? Sorry, git pilot error. Will re-post. - Bernd --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55671 --- src/slave/slave.cpp

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55665 --- I don't see any changes in the diff? Bad diff? - Vinod Kone On Oc

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55659 --- Patch looks great! Reviews applied: [23912] All tests passed. - M

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 7, 2014, 8:18 a.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Aug. 7, 2014, 5:57 p.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp, lines 989-998 > > > > > > Do you have to send killTask() before runTask() is called? AFAICT, you > > just need to make sure to call it bef

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55092 --- src/slave/slave.cpp

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review53790 --- Patch looks great! Reviews applied: [23912] All tests passed. - M

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Sept. 17, 2014, 6:47 p.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review50121 --- Bad patch! Reviews applied: [23912] Failed command: git apply --in

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Aug. 5, 2014, 9:29 p.m., Vinod Kone wrote: > > src/slave/slave.cpp, line 1337 > > > > > > s/executor is running/it was launched/ doesn't look like this was addressed. > On Aug. 5, 2014, 9:29 p.m., Vinod Kone w

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review49980 --- src/tests/slave_tests.cpp

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Aug. 6, 2014, 4:51 p.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Aug. 5, 2014, 2:29 p.m., Vinod Kone wrote: > > src/slave/slave.cpp, lines 1330-1341 > > > > > > I would recommend pulling this logic outside 'if (executor == NULL)' to > > #1326 to make it easy to reason about.

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Aug. 5, 2014, 9:29 p.m., Vinod Kone wrote: > > src/slave/slave.cpp, lines 1330-1341 > > > > > > I would recommend pulling this logic outside 'if (executor == NULL)' to > > #1326 to make it easy to reason about.

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

> On Aug. 5, 2014, 9:29 p.m., Vinod Kone wrote: > > src/tests/mesos.cpp, lines 330-335 > > > > > > were you unable to directly call Slave::* methods here instead of > > having the redirection through unmocked_* method

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review49658 --- File Attachment: stout patch - MESOS-947-stout.patch2

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Aug. 5, 2014, 3:45 p.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Aug. 5, 2014, 3:44 p.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review49552 --- src/slave/slave.hpp

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated July 28, 2014, 1:24 p.m.) Review request for mesos. Changes ---

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated July 24, 2014, 6:01 p.m.) Review request for mesos. Changes ---

Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- Review request for mesos. Bugs: MESOS-947 https://issues.apache.org/jira/br