Re: Review Request 46321: Renamed a variable in command executors for clarity.
--- 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 > >
Re: Review Request 46321: Renamed a variable in command executors for clarity.
--- 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 (updated) - 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
Re: Review Request 46321: Renamed a variable in command executors for clarity.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46321/ --- (Updated April 22, 2016, 2:35 p.m.) Review request for mesos and Ben Mahler. Changes --- + Docker executor; introduced `terminated` state flag in command executors. 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 (updated) - src/docker/executor.cpp 579905f6c2ece7db9c853abf481541fdbc3116b7 src/launcher/executor.cpp 9f1d2168bc4ddbce1bcd25ff38dc1c34714eb28b src/launcher/http_command_executor.cpp 2ce296740abac096aa85ad11dd4d191e42c1aa06 Diff: https://reviews.apache.org/r/46321/diff/ Testing --- The whole chain is tested in https://reviews.apache.org/r/46325/ Thanks, Alexander Rukletsov
Re: Review Request 46321: Renamed a variable in command executors for clarity.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46321/ --- (Updated April 21, 2016, 2:27 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 (updated) - src/launcher/executor.cpp bec9cba091df2d1b340e68e67966ec322558c315 src/launcher/http_command_executor.cpp 9dfe48108cababeb2d4a6af74434880d79245c21 Diff: https://reviews.apache.org/r/46321/diff/ Testing --- The whole chain is tested in https://reviews.apache.org/r/46325/ Thanks, Alexander Rukletsov
Re: Review Request 46321: Renamed a variable in command executors for clarity.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46321/ --- (Updated April 21, 2016, 2:01 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 (updated) - src/launcher/executor.cpp bec9cba091df2d1b340e68e67966ec322558c315 src/launcher/http_command_executor.cpp ad484e0e6f5067b6c166111c91b1ff1e8c05d9ac Diff: https://reviews.apache.org/r/46321/diff/ Testing --- The whole chain is tested in https://reviews.apache.org/r/46325/ Thanks, Alexander Rukletsov
Review Request 46321: Renamed a variable in command executors for clarity.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46321/ --- 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/launcher/executor.cpp bec9cba091df2d1b340e68e67966ec322558c315 src/launcher/http_command_executor.cpp ad484e0e6f5067b6c166111c91b1ff1e8c05d9ac Diff: https://reviews.apache.org/r/46321/diff/ Testing --- The whole chain is tested in https://reviews.apache.org/r/46325/ Thanks, Alexander Rukletsov
