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



Let's punt on this rename in favor of introducing explicit task lifecycle 
states later.

When I see "killing" (since it is present tense) it suggests that it will be 
false once the task is terminated (or "killed"). However, that's not what the 
code is doing (it remains true even after the task terminates). There's also a 
second way to interpret "killed": the "kill" has been issued. If I were to see 
"killed" and "terminated" then I could intuit that these represent the kill 
being issued to the process and the process having terminated.

Also, could push the introduction of "terminated" to the patch that reads the 
value? It's only written to in this patch AFAICT.

- Ben Mahler


On May 5, 2016, 3:37 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46321/
> -----------------------------------------------------------
> 
> (Updated May 5, 2016, 3:37 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When we kill (with SIGTERM) the underlying task, it does not
> necessarily mean it complies immediately, hence the name `killed`
> is misleading.
> 
> 
> Diffs
> -----
> 
>   src/docker/executor.cpp 579905f6c2ece7db9c853abf481541fdbc3116b7 
>   src/launcher/executor.cpp 4d5e2a9b876cd65be1386bf170dba2f71af86750 
>   src/launcher/http_command_executor.cpp 
> d2f15b0447d91f3a4cd92f07114cb366647cc7d3 
> 
> Diff: https://reviews.apache.org/r/46321/diff/
> 
> 
> Testing
> -------
> 
> The whole chain is tested in https://reviews.apache.org/r/46325/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Reply via email to