You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alexander Rukletsov <al...@mesosphere.io> on 2014/10/09 18:28:38 UTC

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. 9, 2014, 4:28 p.m.)


Review request for mesos, Benjamin Hindman and Till Toenshoff.


Changes
-------

Rebase.


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
-------

make check (OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 25250: Mark running tasks killed during framework shutdown.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
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.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> 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.

Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
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.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
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.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

> 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.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
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.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

> 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.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25250/#review56042
-----------------------------------------------------------



src/tests/master_tests.cpp
<https://reviews.apache.org/r/25250/#comment96397>

    Not yours, but can you add a comment describing the test?



src/tests/master_tests.cpp
<https://reviews.apache.org/r/25250/#comment96396>

    Any reason for these changes? Do you have any reference on if '>>' disambiguation is supported by our graced compilers?
    
    Here and below.



src/tests/master_tests.cpp
<https://reviews.apache.org/r/25250/#comment96399>

    Mind expanding a bit on the expected state you are inspecting (and reason for the newly added code)?



src/tests/master_tests.cpp
<https://reviews.apache.org/r/25250/#comment96398>

    Can you do this more reliably? For example setting an expectation for the message to be sent? If not - mind commenting on why? :-)


- Niklas Nielsen


On Oct. 9, 2014, 10:05 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25250/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2014, 10:05 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)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 25250: Mark running tasks killed during framework shutdown.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
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.


Changes
-------

adding @bmahler to reviewers  -- @vinodkone


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