Re: Review Request 25250: Mark running tasks killed during framework shutdown.
On Oct. 22, 2014, 9:19 a.m., Adam B wrote: I took care of these for Alex since he was blocked on my JSON::Object.find updates. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review57771 --- On Oct. 10, 2014, 2:15 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 10, 2014, 2:15 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review57771 --- src/master/master.cpp https://reviews.apache.org/r/25250/#comment98651 s/descrptive/descriptive/ src/tests/master_tests.cpp https://reviews.apache.org/r/25250/#comment98652 Why 'auto' instead of 'Offer'? Only saves 1 character of typing. src/tests/master_tests.cpp https://reviews.apache.org/r/25250/#comment98653 Wrap after the '=' instead of between values.find, if possible src/tests/master_tests.cpp https://reviews.apache.org/r/25250/#comment98654 Wrap after '=' - Adam B On Oct. 10, 2014, 7:15 a.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 10, 2014, 7:15 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
On Oct. 9, 2014, 7:55 p.m., Timothy Chen wrote: src/tests/master_tests.cpp, line 265 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line265 Perhaps you should do EXPECT so you can cleanly shutdown in the end. If the response has not been parsed successfully, the test will fail right after. Same holds for assertions below. I mark this as dropped, but feel free to reopen if I vioalte the guidline or we have an agreed way to act here. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review56041 --- On Oct. 9, 2014, 5:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 9, 2014, 5:05 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
On Oct. 9, 2014, 8:01 p.m., Niklas Nielsen wrote: src/tests/master_tests.cpp, line 168 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line168 Any reason for these changes? Do you have any reference on if '' disambiguation is supported by our graced compilers? Here and below. To the best of my knowledge all supported compilers handle `` correctly and we agreed to make use of it in new patches. So I decided to clean-up while modifying the test. But can rollback it if you prefer the conservative way. On Oct. 9, 2014, 8:01 p.m., Niklas Nielsen wrote: src/tests/master_tests.cpp, line 165 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line165 Not yours, but can you add a comment describing the test? Sure. On Oct. 9, 2014, 8:01 p.m., Niklas Nielsen wrote: src/tests/master_tests.cpp, line 247 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line247 Mind expanding a bit on the expected state you are inspecting (and reason for the newly added code)? Expanded a comment above and one below. On Oct. 9, 2014, 8:01 p.m., Niklas Nielsen wrote: src/tests/master_tests.cpp, lines 253-255 https://reviews.apache.org/r/25250/diff/6/?file=716710#file716710line253 Can you do this more reliably? For example setting an expectation for the message to be sent? If not - mind commenting on why? :-) My understanding is that this sequence guarantees all non-delayed messages in the mailbox are processed. I want to be sure that the `UnregisterFrameworkMessage` is processed by the master before I ask for the `state.json`. I've expanded a comment a bit, please let me know if there is a better way to achieve this. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review56042 --- On Oct. 9, 2014, 5:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 9, 2014, 5:05 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 10, 2014, 2:15 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Changes --- Fix the test (by using the new way to update the task status) and expand comments in the test. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs (updated) - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing (updated) --- make check (OS X, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review56155 --- Patch looks great! Reviews applied: [25250] All tests passed. - Mesos ReviewBot On Oct. 10, 2014, 2:15 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 10, 2014, 2:15 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X, Ubuntu 14.04) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review56041 --- src/tests/master_tests.cpp https://reviews.apache.org/r/25250/#comment96393 Perhaps you should do EXPECT so you can cleanly shutdown in the end. - Timothy Chen On Oct. 9, 2014, 5:05 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 9, 2014, 5:05 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp cb46cec src/tests/master_tests.cpp d9dc40c Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Sept. 22, 2014, 4:25 p.m.) Review request for mesos, Benjamin Hindman and Till Toenshoff. Changes --- Explain why we prefer TASK_KILLED and not sending the update. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs (updated) - src/master/master.cpp e5d30e9 src/tests/master_tests.cpp 8e4ec1d Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
On Sept. 15, 2014, 4:38 p.m., Benjamin Hindman wrote: src/master/master.cpp, line 4010 https://reviews.apache.org/r/25250/diff/4/?file=682254#file682254line4010 I suggest we use TASK_LOST here instead. We definitely want a terminal state like TASK_KILLED, but we've reserved TASK_KILLED for when a framework has actually intiated the kill itself, and thus I'd prefer not to overload the semantics. This might be a good candidate for a new task state, e.g., TASK_REMOVED, which has been discussed in the past, but I can't recall if there is a JIRA for that or not. If not, it would be great to have you create one Alex so we can have a discussion about how to introduce new task states (and maybe even a way to introduce sub-states that framework writers themselves could customize). Alexander Rukletsov wrote: I used to have `TASK_LOST` here, but my understanding is that `TASK_LOST` is used for abnormal situations, i.e. when the task is not finished not because of scheduler's direct command, but because of some external reasons. I agree, that a new task state is a very good solution. We have [this ticket](https://issues.apache.org/jira/browse/MESOS-343), one solution for which would be to introduce something like `TaskStatusExplained` or a protobuf message for every task. But maybe for this situation something like `TASK_ABANDONED` would be rather descriptive. Keeping TASK_KILLED, but added comments for clarity. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review53350 --- On Sept. 22, 2014, 4:25 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Sept. 22, 2014, 4:25 p.m.) Review request for mesos, Benjamin Hindman and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp e5d30e9 src/tests/master_tests.cpp 8e4ec1d Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
On Sept. 15, 2014, 4:38 p.m., Benjamin Hindman wrote: src/master/master.cpp, line 4010 https://reviews.apache.org/r/25250/diff/4/?file=682254#file682254line4010 I suggest we use TASK_LOST here instead. We definitely want a terminal state like TASK_KILLED, but we've reserved TASK_KILLED for when a framework has actually intiated the kill itself, and thus I'd prefer not to overload the semantics. This might be a good candidate for a new task state, e.g., TASK_REMOVED, which has been discussed in the past, but I can't recall if there is a JIRA for that or not. If not, it would be great to have you create one Alex so we can have a discussion about how to introduce new task states (and maybe even a way to introduce sub-states that framework writers themselves could customize). I used to have `TASK_LOST` here, but my understanding is that `TASK_LOST` is used for abnormal situations, i.e. when the task is not finished not because of scheduler's direct command, but because of some external reasons. I agree, that a new task state is a very good solution. We have [this ticket](https://issues.apache.org/jira/browse/MESOS-343), one solution for which would be to introduce something like `TaskStatusExplained` or a protobuf message for every task. But maybe for this situation something like `TASK_ABANDONED` would be rather descriptive. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review53350 --- On Sept. 7, 2014, 6:35 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Sept. 7, 2014, 6:35 p.m.) Review request for mesos, Benjamin Hindman and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp c6393b2 src/tests/master_tests.cpp 3d080b2 Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Sept. 7, 2014, 6:35 p.m.) Review request for mesos, Benjamin Hindman and Till Toenshoff. Changes --- Reuse existing test (ShutdownFrameworkWhileTaskRunning). Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs (updated) - src/master/master.cpp c6393b2 src/tests/master_tests.cpp 3d080b2 Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review52560 --- Patch looks great! Reviews applied: [25250] All tests passed. - Mesos ReviewBot On Sept. 7, 2014, 6:35 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Sept. 7, 2014, 6:35 p.m.) Review request for mesos, Benjamin Hindman and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs - src/master/master.cpp c6393b2 src/tests/master_tests.cpp 3d080b2 Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check (OS X) Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review52351 --- Patch looks great! Reviews applied: [25250] All tests passed. - Mesos ReviewBot On Sept. 3, 2014, 9:56 p.m., Alexander Rukletsov wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Sept. 3, 2014, 9:56 p.m.) Review request for mesos, Benjamin Hindman and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. WIP: bad patch, not reviewable. Diffs - src/master/master.cpp 2508b38e86b8399886bffcbaca8ec11c731363d8 src/tests/master_tests.cpp eaa1675aa3c598ee9b59334852eafcce65687f18 Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Sept. 3, 2014, 9:56 p.m.) Review request for mesos, Benjamin Hindman and Till Toenshoff. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. WIP: bad patch, not reviewable. Diffs (updated) - src/master/master.cpp 2508b38e86b8399886bffcbaca8ec11c731363d8 src/tests/master_tests.cpp eaa1675aa3c598ee9b59334852eafcce65687f18 Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check Thanks, Alexander Rukletsov
Re: Review Request 25250: Mark running tasks killed during framework shutdown.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Sept. 2, 2014, 5:44 p.m.) Review request for mesos, Benjamin Hindman and Till Toenshoff. Changes --- Address Till's suggestions. Bugs: MESOS-1736 https://issues.apache.org/jira/browse/MESOS-1736 Repository: mesos-git Description --- When a framework is shut down e.g. by calling driver.stop() from the scheduler, running tasks are marked KILLED before migrating them to completed. Diffs (updated) - src/master/master.cpp 2508b38e86b8399886bffcbaca8ec11c731363d8 src/tests/master_tests.cpp eaa1675aa3c598ee9b59334852eafcce65687f18 Diff: https://reviews.apache.org/r/25250/diff/ Testing --- make check Thanks, Alexander Rukletsov
