You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anand Mazumdar <an...@apache.org> on 2017/02/22 18:16:45 UTC

Re: Review Request 56830: Introduced `TaskInfo::TerminationPolicy` protobuf.

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

(Updated Feb. 22, 2017, 6:16 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


Changes
-------

Review comments + Modified `PROCEED` action to `DO_NOTHING` since it better signifies the intent based on offline discussions.

+ Adding Neil as a reviewer too.


Summary (updated)
-----------------

Introduced `TaskInfo::TerminationPolicy` protobuf.


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


Repository: mesos


Description (updated)
-------

Describes the termination policy associated with a task and is
applied upon a task termination.


Diffs (updated)
-----

  include/mesos/mesos.proto 030e79c003f6560e9c0627db12fb1baba411151d 
  include/mesos/v1/mesos.proto 7f6f858a7d9387d202510730d400e490298e6574 

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


Testing
-------

make check (tests later in the chain)


Thanks,

Anand Mazumdar


Re: Review Request 56830: Introduced `TaskInfo::TerminationPolicy` protobuf.

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




include/mesos/mesos.proto (line 1603)
<https://reviews.apache.org/r/56830/#comment238313>

    It isn't clear that "task termination" does not include "when the task is explicitly killed by the scheduler." I'd say something like:
    
    "Describes the policy that will be used when a task terminates. This policy is only used when a task terminates (i.e., finishes successfully or exits with an error); it is not used for tasks that are explicitly killed by the scheduler."
    
    You should also explain whether the policy is used when tasks are killed by the agent (e.g., due to oversubscription or because of a containerizer error).



include/mesos/mesos.proto (line 1611)
<https://reviews.apache.org/r/56830/#comment238314>

    "it is"



include/mesos/mesos.proto (line 1614)
<https://reviews.apache.org/r/56830/#comment238315>

    `IGNORE` would be an alternative to `DO_NOTHING`; I don't have a strong view on which term is better.



include/mesos/mesos.proto (line 1623)
<https://reviews.apache.org/r/56830/#comment238316>

    "The task group will always be killed, regardless of whether the task finished successfully or failed -- i.e., transitioned to the TASK_FINISHED or TASK_FAILED states, respectively."
    
    Last sentence doesn't make sense to me -- i.e., "zero exit code or failed" covers all cases, right?



include/mesos/mesos.proto (line 1629)
<https://reviews.apache.org/r/56830/#comment238317>

    "The task group will only be killed when the task fails -- i.e., transitions to the TASK_FAILED state. For the default executor, this happens when the task exits with a non-zero exit code."
    
    Note that OOM killer (i.e., SIGKILL) does _not_ result in the exit code being "unknown" -- see http://unix.stackexchange.com/a/99134


- Neil Conway


On Feb. 22, 2017, 6:16 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56830/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2017, 6:16 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7067
>     https://issues.apache.org/jira/browse/MESOS-7067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Describes the termination policy associated with a task and is
> applied upon a task termination.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 030e79c003f6560e9c0627db12fb1baba411151d 
>   include/mesos/v1/mesos.proto 7f6f858a7d9387d202510730d400e490298e6574 
> 
> Diff: https://reviews.apache.org/r/56830/diff/
> 
> 
> Testing
> -------
> 
> make check (tests later in the chain)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 56830: [WIP] Introduced `TaskInfo::TerminationPolicy` protobuf.

Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56830/
-----------------------------------------------------------

(Updated Feb. 23, 2017, 12:09 a.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


Changes
-------

Review comments from Neil. Changing the summary to be WIP as there were a few offline discussions between me and Vinod that might result in further changes
to the way the protobuf is structured.


Summary (updated)
-----------------

[WIP] Introduced `TaskInfo::TerminationPolicy` protobuf.


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


Repository: mesos


Description
-------

Describes the termination policy associated with a task and is
applied upon a task termination.


Diffs (updated)
-----

  include/mesos/mesos.proto 030e79c003f6560e9c0627db12fb1baba411151d 
  include/mesos/v1/mesos.proto 7f6f858a7d9387d202510730d400e490298e6574 

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


Testing
-------

make check (tests later in the chain)


Thanks,

Anand Mazumdar