You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Zhitao Li <zh...@gmail.com> on 2018/03/26 21:48:52 UTC
Review Request 66291: Added support to max_duration in default
executor.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66291/
-----------------------------------------------------------
Review request for mesos, Jason Lai and James Peach.
Bugs: MESOS-8725
https://issues.apache.org/jira/browse/MESOS-8725
Repository: mesos
Description
-------
Added support to max_duration in default executor.
Diffs
-----
src/launcher/default_executor.cpp 906836f3b8e0af79d7c61f90fd8a95f193b26e84
Diff: https://reviews.apache.org/r/66291/diff/1/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 66291: Added support to max_duration in default
executor.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66291/#review200119
-----------------------------------------------------------
src/launcher/default_executor.cpp
Lines 1098-1102 (patched)
<https://reviews.apache.org/r/66291/#comment280775>
I think I also need to handle the case if a task run to success *before* reaching max duration. This is not a big issue for other built-in executors since they only run one task anyway, but we need to make sureJ:
- other tasks in the group are not affected;
- timer should be cancelled properly;
- timer triggered after this task completed should not bring executor down accidentally.
- Zhitao Li
On March 26, 2018, 2:48 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66291/
> -----------------------------------------------------------
>
> (Updated March 26, 2018, 2:48 p.m.)
>
>
> Review request for mesos, Jason Lai and James Peach.
>
>
> Bugs: MESOS-8725
> https://issues.apache.org/jira/browse/MESOS-8725
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added support to max_duration in default executor.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp 906836f3b8e0af79d7c61f90fd8a95f193b26e84
>
>
> Diff: https://reviews.apache.org/r/66291/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66291: Added support to `max_completion_time` in
default executor.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66291/
-----------------------------------------------------------
(Updated May 1, 2018, 9:42 a.m.)
Review request for mesos, Jason Lai and James Peach.
Changes
-------
Review comments.
Bugs: MESOS-8725
https://issues.apache.org/jira/browse/MESOS-8725
Repository: mesos
Description
-------
When a task group has multiple tasks:
- each task can have its own `max_completion_time`, or not have one;
- if a task succeeds before its `max_completion_time`, all other tasks
will keep running;
- if a task fails, all other tasks in the same group will fail (as
before);
- if a task does not succeed before its `max_completion_time`, it will
fail with `TASK_FAILED` and reason `REASON_MAX_COMPLETION_TIME_REACHED`,
while other tasks will be killed without above reason.
Diffs (updated)
-----
src/launcher/default_executor.cpp ea0d425bb60e970f209f411632e1d486c279259b
Diff: https://reviews.apache.org/r/66291/diff/4/
Changes: https://reviews.apache.org/r/66291/diff/3-4/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 66291: Added support to `max_completion_time` in
default executor.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66291/#review202156
-----------------------------------------------------------
Fix it, then Ship it!
src/launcher/default_executor.cpp
Lines 1291 (patched)
<https://reviews.apache.org/r/66291/#comment283796>
Use a log error message:
```
LOG(INFO) << "Killing task " << taskId
<< " which exceeded its maximum completion time of " << duration;
```
- James Peach
On April 23, 2018, 11:56 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66291/
> -----------------------------------------------------------
>
> (Updated April 23, 2018, 11:56 p.m.)
>
>
> Review request for mesos, Jason Lai and James Peach.
>
>
> Bugs: MESOS-8725
> https://issues.apache.org/jira/browse/MESOS-8725
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When a task group has multiple tasks:
> - each task can have its own `max_completion_time`, or not have one;
> - if a task succeeds before its `max_completion_time`, all other tasks
> will keep running;
> - if a task fails, all other tasks in the same group will fail (as
> before);
> - if a task does not succeed before its `max_completion_time`, it will
> fail with `TASK_FAILED` and reason `REASON_MAX_COMPLETION_TIME_REACHED`,
> while other tasks will be killed without above reason.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp ea0d425bb60e970f209f411632e1d486c279259b
>
>
> Diff: https://reviews.apache.org/r/66291/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66291: Added support to `max_completion_time` in
default executor.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66291/
-----------------------------------------------------------
(Updated April 23, 2018, 4:56 p.m.)
Review request for mesos, Jason Lai and James Peach.
Changes
-------
Use `killedByCompletionTimeout` to indicate kill reason, and refactor `kill()` to reuse a grace period.
Bugs: MESOS-8725
https://issues.apache.org/jira/browse/MESOS-8725
Repository: mesos
Description
-------
When a task group has multiple tasks:
- each task can have its own `max_completion_time`, or not have one;
- if a task succeeds before its `max_completion_time`, all other tasks
will keep running;
- if a task fails, all other tasks in the same group will fail (as
before);
- if a task does not succeed before its `max_completion_time`, it will
fail with `TASK_FAILED` and reason `REASON_MAX_COMPLETION_TIME_REACHED`,
while other tasks will be killed without above reason.
Diffs (updated)
-----
src/launcher/default_executor.cpp ea0d425bb60e970f209f411632e1d486c279259b
Diff: https://reviews.apache.org/r/66291/diff/3/
Changes: https://reviews.apache.org/r/66291/diff/2-3/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 66291: Added support to `max_completion_time` in
default executor.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66291/
-----------------------------------------------------------
(Updated April 7, 2018, 5:53 p.m.)
Review request for mesos, Jason Lai and James Peach.
Changes
-------
Rename and proper state.
Summary (updated)
-----------------
Added support to `max_completion_time` in default executor.
Bugs: MESOS-8725
https://issues.apache.org/jira/browse/MESOS-8725
Repository: mesos
Description (updated)
-------
When a task group has multiple tasks:
- each task can have its own `max_completion_time`, or not have one;
- if a task succeeds before its `max_completion_time`, all other tasks
will keep running;
- if a task fails, all other tasks in the same group will fail (as
before);
- if a task does not succeed before its `max_completion_time`, it will
fail with `TASK_FAILED` and reason `REASON_MAX_COMPLETION_TIME_REACHED`,
while other tasks will be killed without above reason.
Diffs (updated)
-----
src/launcher/default_executor.cpp ea0d425bb60e970f209f411632e1d486c279259b
Diff: https://reviews.apache.org/r/66291/diff/2/
Changes: https://reviews.apache.org/r/66291/diff/1-2/
Testing
-------
Thanks,
Zhitao Li