You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2016/02/22 06:19:24 UTC

Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43821/
-----------------------------------------------------------

Review request for mesos, Abhishek Dasgupta and Neil Conway.


Bugs: MESOS-4547
    https://issues.apache.org/jira/browse/MESOS-4547


Repository: mesos


Description
-------

See summary.


Diffs
-----

  docs/high-availability-framework-guide.md 0d9c483985d61b512339f50f395f9360de034e2d 

Diff: https://reviews.apache.org/r/43821/diff/


Testing
-------

N/A


Thanks,

Ben Mahler


Re: Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

Posted by Ben Mahler <be...@gmail.com>.

> On Feb. 22, 2016, 5:52 a.m., Klaus Ma wrote:
> > docs/high-availability-framework-guide.md, line 202
> > <https://reviews.apache.org/r/43821/diff/1/?file=1263875#file1263875line202>
> >
> >     I'd like to be "The `TASK_KILLING` state is optional; it is intended to indicate that the..."

Do you consider this an issue? I tend to suggest that folks use "issues" for things like bugs, necessary improvements, items where the reviewer wants to continue the discussion if the issue is dropped.

I dropped this one because I made it the same as the existing writing for TASK_STARTING:

```
The `TASK_STARTING` state is optional and intended primarily for use by
```


> On Feb. 22, 2016, 5:52 a.m., Klaus Ma wrote:
> > docs/high-availability-framework-guide.md, line 205
> > <https://reviews.apache.org/r/43821/diff/1/?file=1263875#file1263875line205>
> >
> >     Highlight the behaviour if executor generate `TASK_KILLING` without framework's capability: ignore, crash or undefined?

Well, the reason I mention the capability is that it's up to the framework to express whether it can process the status update (via the capability). We could say that mesos doesn't do any checking of this, we'll forward the update to the framework at which point the framework should log/expose errors or crash. I omitted it because it seems like this kind of detail belongs closer to the executor API?


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43821/#review120114
-----------------------------------------------------------


On Feb. 22, 2016, 5:19 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43821/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta and Neil Conway.
> 
> 
> Bugs: MESOS-4547
>     https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/high-availability-framework-guide.md 0d9c483985d61b512339f50f395f9360de034e2d 
> 
> Diff: https://reviews.apache.org/r/43821/diff/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43821/#review120114
-----------------------------------------------------------




docs/high-availability-framework-guide.md (line 202)
<https://reviews.apache.org/r/43821/#comment181490>

    I'd like to be "The `TASK_KILLING` state is optional; it is intended to indicate that the..."



docs/high-availability-framework-guide.md (line 205)
<https://reviews.apache.org/r/43821/#comment181491>

    Highlight the behaviour if executor generate `TASK_KILLING` without framework's capability: ignore, crash or undefined?


- Klaus Ma


On Feb. 22, 2016, 1:19 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43821/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 1:19 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta and Neil Conway.
> 
> 
> Bugs: MESOS-4547
>     https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/high-availability-framework-guide.md 0d9c483985d61b512339f50f395f9360de034e2d 
> 
> Diff: https://reviews.apache.org/r/43821/diff/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

Posted by Ben Mahler <be...@gmail.com>.

> On Feb. 22, 2016, 5:39 a.m., Neil Conway wrote:
> > docs/high-availability-framework-guide.md, line 204
> > <https://reviews.apache.org/r/43821/diff/1/?file=1263875#file1263875line204>
> >
> >     "has not yet been killed" ?

done


> On Feb. 22, 2016, 5:39 a.m., Neil Conway wrote:
> > docs/high-availability-framework-guide.md, line 206
> > <https://reviews.apache.org/r/43821/diff/1/?file=1263875#file1263875line206>
> >
> >     "`FrameworkInfo.capabilities`" ?

hm.. I couldn't phrase something clean with this, did the following:

    terminate gracefully. Executors must not generate this state unless the
    framework has the `TASK_KILLING_STATE` framework capability.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43821/#review120110
-----------------------------------------------------------


On Feb. 22, 2016, 5:19 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43821/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta and Neil Conway.
> 
> 
> Bugs: MESOS-4547
>     https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/high-availability-framework-guide.md 0d9c483985d61b512339f50f395f9360de034e2d 
> 
> Diff: https://reviews.apache.org/r/43821/diff/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43821/#review120110
-----------------------------------------------------------


Ship it!





docs/high-availability-framework-guide.md (line 204)
<https://reviews.apache.org/r/43821/#comment181486>

    "has not yet been killed" ?



docs/high-availability-framework-guide.md (line 206)
<https://reviews.apache.org/r/43821/#comment181485>

    "`FrameworkInfo.capabilities`" ?


Typo in commit summary.

- Neil Conway


On Feb. 22, 2016, 5:19 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43821/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta and Neil Conway.
> 
> 
> Bugs: MESOS-4547
>     https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/high-availability-framework-guide.md 0d9c483985d61b512339f50f395f9360de034e2d 
> 
> Diff: https://reviews.apache.org/r/43821/diff/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43821/#review120135
-----------------------------------------------------------



Patch looks great!

Reviews applied: [43821]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 22, 2016, 5:19 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43821/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta and Neil Conway.
> 
> 
> Bugs: MESOS-4547
>     https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/high-availability-framework-guide.md 0d9c483985d61b512339f50f395f9360de034e2d 
> 
> Diff: https://reviews.apache.org/r/43821/diff/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

Posted by Ben Mahler <be...@gmail.com>.

> On Feb. 22, 2016, 5:44 a.m., Abhishek Dasgupta wrote:
> > docs/high-availability-framework-guide.md, line 205
> > <https://reviews.apache.org/r/43821/diff/1/?file=1263875#file1263875line205>
> >
> >     How about we put an example here? Like the following:
> >     This is useful for tasks that require some time to terminate gracefully(e.g. stopping a docker container).
> >     As to stop a docker container, it always takes a little bit of time, so this might make a good example.

Does docker do something that inherently requires some time to terminate gracefully? How is it different from just a SIGTERM -> SIGKILL escalation?


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43821/#review120113
-----------------------------------------------------------


On Feb. 22, 2016, 5:19 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43821/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta and Neil Conway.
> 
> 
> Bugs: MESOS-4547
>     https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/high-availability-framework-guide.md 0d9c483985d61b512339f50f395f9360de034e2d 
> 
> Diff: https://reviews.apache.org/r/43821/diff/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 43821: Updated the HA framweork guide for TASK_KILLING.

Posted by Abhishek Dasgupta <a1...@linux.vnet.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43821/#review120113
-----------------------------------------------------------




docs/high-availability-framework-guide.md (line 205)
<https://reviews.apache.org/r/43821/#comment181489>

    How about we put an example here? Like the following:
    This is useful for tasks that require some time to terminate gracefully(e.g. stopping a docker container).
    As to stop a docker container, it always takes a little bit of time, so this might make a good example.


- Abhishek Dasgupta


On Feb. 22, 2016, 5:19 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43821/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2016, 5:19 a.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta and Neil Conway.
> 
> 
> Bugs: MESOS-4547
>     https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/high-availability-framework-guide.md 0d9c483985d61b512339f50f395f9360de034e2d 
> 
> Diff: https://reviews.apache.org/r/43821/diff/
> 
> 
> Testing
> -------
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>