Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Alexander Rukletsov]

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

(Updated Nov. 24, 2017, 6:21 p.m.)


Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.


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


Repository: mesos


Description
---

`ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
to a driver-based executor. Since these messages are not retried,
the executor will never start a task and will remain idle, ignoring
kill task request. This patch shutdown driver-based executors if kill
task arrives before the task has been started.


Diffs
-

  src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
  src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 


Diff: https://reviews.apache.org/r/64033/diff/1/


Testing (updated)
---

make check on MacOS 10.11.6

Manual testing using modified "exec.cpp" that drops executor registration 
confirmation.


Thanks,

Alexander Rukletsov

Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Alexander Rukletsov]

> On Nov. 22, 2017, 5:44 p.m., Vinod Kone wrote:
> > can you write tests please?

Writing a test here is not trivial. We don't have infrastructure in place to 
"hook" to our executors. All built-in executors are implemented in ".cpp" 
files, which does not allow to create an instance of executor process in a test 
and, for example, drop specific calls.


- Alexander


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


On Nov. 22, 2017, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64033/
> ---
> 
> (Updated Nov. 22, 2017, 5:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.
> 
> 
> Bugs: MESOS-8247
> https://issues.apache.org/jira/browse/MESOS-8247
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
> to a driver-based executor. Since these messages are not retried,
> the executor will never start a task and will remain idle, ignoring
> kill task request. This patch shutdown driver-based executors if kill
> task arrives before the task has been started.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
>   src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 
> 
> 
> Diff: https://reviews.apache.org/r/64033/diff/1/
> 
> 
> Testing
> ---
> 
> make check on MacOS 10.11.6
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Alexander Rukletsov]

> On Nov. 24, 2017, 1:07 p.m., Andrei Budnik wrote:
> > src/docker/executor.cpp
> > Lines 386 (patched)
> > 
> >
> > Consider moving this check to [the 
> > driver](https://github.com/apache/mesos/blob/708902d6f207b6f93c94b3589a31d54c812a7657/src/exec/exec.cpp#L343).

This is a very good suggestion.

First off, this way we guarantee that all driver-based executors behave 
properly in this case.

Second, behaviour of the command executor in the HTTP mode is not affected, 
which is a good thing.

And third, if the `killTask` message is not delivered to the executor (for 
example, due to V0-V1 executor adapter holding off messages until `registered` 
is received), the executor will eventually shutdown.

I will update the patch in a while.


- Alexander


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


On Nov. 22, 2017, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64033/
> ---
> 
> (Updated Nov. 22, 2017, 5:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.
> 
> 
> Bugs: MESOS-8247
> https://issues.apache.org/jira/browse/MESOS-8247
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
> to a driver-based executor. Since these messages are not retried,
> the executor will never start a task and will remain idle, ignoring
> kill task request. This patch shutdown driver-based executors if kill
> task arrives before the task has been started.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
>   src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 
> 
> 
> Diff: https://reviews.apache.org/r/64033/diff/1/
> 
> 
> Testing
> ---
> 
> make check on MacOS 10.11.6
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Andrei Budnik]

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




src/docker/executor.cpp
Lines 386 (patched)


Consider moving this check to [the 
driver](https://github.com/apache/mesos/blob/708902d6f207b6f93c94b3589a31d54c812a7657/src/exec/exec.cpp#L343).


- Andrei Budnik


On Nov. 22, 2017, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64033/
> ---
> 
> (Updated Nov. 22, 2017, 5:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.
> 
> 
> Bugs: MESOS-8247
> https://issues.apache.org/jira/browse/MESOS-8247
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
> to a driver-based executor. Since these messages are not retried,
> the executor will never start a task and will remain idle, ignoring
> kill task request. This patch shutdown driver-based executors if kill
> task arrives before the task has been started.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
>   src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 
> 
> 
> Diff: https://reviews.apache.org/r/64033/diff/1/
> 
> 
> Testing
> ---
> 
> make check on MacOS 10.11.6
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Vinod Kone]

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



can you write tests please?


src/docker/executor.cpp
Lines 390 (patched)


`return` here for clarity?

Also, please log a message because this is a rare case.



src/launcher/executor.cpp
Lines 782 (patched)


`return` here?

Also, please log a message because this is a rare case.


- Vinod Kone


On Nov. 22, 2017, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64033/
> ---
> 
> (Updated Nov. 22, 2017, 5:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.
> 
> 
> Bugs: MESOS-8247
> https://issues.apache.org/jira/browse/MESOS-8247
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
> to a driver-based executor. Since these messages are not retried,
> the executor will never start a task and will remain idle, ignoring
> kill task request. This patch shutdown driver-based executors if kill
> task arrives before the task has been started.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
>   src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 
> 
> 
> Diff: https://reviews.apache.org/r/64033/diff/1/
> 
> 
> Testing
> ---
> 
> make check on MacOS 10.11.6
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 64033: Terminated executors if kill task received before launch task.

[Alexander Rukletsov]

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

(Updated Nov. 22, 2017, 5:05 p.m.)


Review request for mesos, Andrei Budnik, Armand Grillet, and Vinod Kone.


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


Repository: mesos


Description
---

`ExecutorRegisteredMessage` or `RunTaskMessage` may not be delivered
to a driver-based executor. Since these messages are not retried,
the executor will never start a task and will remain idle, ignoring
kill task request. This patch shutdown driver-based executors if kill
task arrives before the task has been started.


Diffs
-

  src/docker/executor.cpp 3974f20052e3c12eb154a5146d19d4dc1759859f 
  src/launcher/executor.cpp c688c04e598ac140421fd5e47359b0e48d30bcc5 


Diff: https://reviews.apache.org/r/64033/diff/1/


Testing (updated)
---

make check on MacOS 10.11.6


Thanks,

Alexander Rukletsov