> On Oct. 23, 2014, 12:54 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, lines 1270-1273 > > <https://reviews.apache.org/r/23912/diff/13/?file=724332#file724332line1270> > > > > Hey Bernd and Vinod, coverity just reported this, looks like it's now > > dead code! > > > > Can you follow up? > > Bernd Mathiske wrote: > Coverty is correct. I believe we can remove this code and the subsequent > line as well. According to how I understand it (given Vinod's reviews), the > framework must still exist as long as any pending task exists. It follows > that this code must have already been de facto dead before, because the patch > for MESOS-947 does not change when a framework gets instantiated. It just > reclaims it more eagerly.
Filed MESOS-238 and proposed patch https://reviews.apache.org/r/27567/ - Bernd ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review58074 ----------------------------------------------------------- On Oct. 17, 2014, 9:25 a.m., Bernd Mathiske wrote: > > ----------------------------------------------------------- > 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. > > > Bugs: MESOS-947 > https://issues.apache.org/jira/browse/MESOS-947 > > > Repository: mesos-git > > > Description > ------- > > Fixes MESOS-947 "Slave should properly handle a killTask() that arrives > between runTask() and _runTask()". > > Slave::killTask() did not check for task in question combination to be > "pending" (i.e. Slave::runTask had happened, but Slave::_runTask had not yet) > and then erroneously assumed that Slave::runTask() had not been executed. The > task was then marked "LOST" instead of "KILLED". But Slave::runTask had > already scheduled Slave::_runTask to follow. Now the entry for being > "pending" is removed, and the task is marked "KILLED", and _runTask gets > informed about this. It checks whether the task in question is currently > "pending" and if it is not, then it infers that the task has been killed and > does not erroneously try to complete launching it. > > > Diffs > ----- > > src/slave/slave.hpp 342b09fc084c20d98d096bb129830440179c092c > src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 > src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 > src/tests/mesos.cpp 3dcb2acd5ad4ab5e3a7b4fe524ee077558112773 > src/tests/slave_tests.cpp f585bdd20ae1af466f2c1b4d85331ac67451552f > > Diff: https://reviews.apache.org/r/23912/diff/ > > > Testing > ------- > > Wrote a unit test that reliably created the situation described in the > ticket. Observed that TASK_LOST and the listed log output occurred. This > pointed directly to the lines in killTask() where the problem is rooted. Ran > the test after fixing, it succeeded. Checked the log. It looks like a "clean > kill" now :-) > > > Thanks, > > Bernd Mathiske > >
