----------------------------------------------------------- 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/apply-review.sh -n -r 23912 Error: --2014-10-17 16:51:19-- https://reviews.apache.org/r/23912/diff/raw/ Resolving reviews.apache.org (reviews.apache.org)... 140.211.11.74 Connecting to reviews.apache.org (reviews.apache.org)|140.211.11.74|:443... connected. HTTP request sent, awaiting response... 200 OK Length: 13584 (13K) [text/x-patch] Saving to: '23912.patch' 0K .......... ... 100% 123M=0s 2014-10-17 16:51:19 (123 MB/s) - '23912.patch' saved [13584/13584] Successfully applied: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask() 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. Review: https://reviews.apache.org/r/23912 Checking 5 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo src/slave/slave.cpp:1197: Weird number of spaces at line-start. Are you using a 2-space indent? [whitespace/indent] [3] Total errors found: 1 Failed to commit patch - Mesos ReviewBot On Oct. 17, 2014, 4:25 p.m., Bernd Mathiske wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23912/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2014, 4:25 p.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 > >
