Re: Review Request 49549: Used the launch helper binary to launch user task in command executor.

2016-07-04 Thread Jie Yu


> On July 4, 2016, 8:03 a.m., Gilbert Song wrote:
> > This patch is great!
> > 
> > I am still thinking it may be a little risky for 1.0 using command task 
> > with container image, because this is what we are currently doing:
> > 
> > mesos containerizer fork a subprocess -> MesosContainerizerLaunch 
> > subcommand -> execvp mesos-executor binary -> executor fork a subprocess -> 
> > containerizer binary to call MesosContainerizerLaunch subcommand.

I don't think it's too risky. It also addressed some of the long standing 
TODO's in our code (for instance, to make command executor async signal safe 
after fork). I think this is the right way forward.


- Jie


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


On July 2, 2016, 1:02 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49549/
> ---
> 
> (Updated July 2, 2016, 1:02 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, command executor and 'mesos-containerizer launch' share a
> lot of the logic. Command executor should in fact, just use
> `mesos-containerizer launch` to launch the user task. Potentially,
> 'mesos-containerizer launch' can be also used by custom executor to
> launch user tasks.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
>   src/launcher/posix/executor.hpp e5f3c0a8e2d3687a5330adbf2f667db9106970c7 
>   src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49549/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 49549: Used the launch helper binary to launch user task in command executor.

2016-07-04 Thread Gilbert Song

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


Ship it!




This patch is great!

I am still thinking it may be a little risky for 1.0 using command task with 
container image, because this is what we are currently doing:

mesos containerizer fork a subprocess -> MesosContainerizerLaunch subcommand -> 
execvp mesos-executor binary -> executor fork a subprocess -> containerizer 
binary to call MesosContainerizerLaunch subcommand.

- Gilbert Song


On July 1, 2016, 6:02 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49549/
> ---
> 
> (Updated July 1, 2016, 6:02 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, command executor and 'mesos-containerizer launch' share a
> lot of the logic. Command executor should in fact, just use
> `mesos-containerizer launch` to launch the user task. Potentially,
> 'mesos-containerizer launch' can be also used by custom executor to
> launch user tasks.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
>   src/launcher/posix/executor.hpp e5f3c0a8e2d3687a5330adbf2f667db9106970c7 
>   src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49549/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 49549: Used the launch helper binary to launch user task in command executor.

2016-07-01 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [49549, 49548, 49542, 49541, 49540, 49523, 49524, 49479, 
49473, 49472, 49425, 49424, 49415]

Failed command: ./support/apply-review.sh -n -r 49541

Error:
2016-07-02 05:37:57 URL:https://reviews.apache.org/r/49541/diff/raw/ 
[1789/1789] -> "49541.patch" [1]
error: patch failed: src/launcher/executor.cpp:676
error: src/launcher/executor.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/14076/console

- Mesos ReviewBot


On July 2, 2016, 1:02 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49549/
> ---
> 
> (Updated July 2, 2016, 1:02 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, command executor and 'mesos-containerizer launch' share a
> lot of the logic. Command executor should in fact, just use
> `mesos-containerizer launch` to launch the user task. Potentially,
> 'mesos-containerizer launch' can be also used by custom executor to
> launch user tasks.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
>   src/launcher/posix/executor.hpp e5f3c0a8e2d3687a5330adbf2f667db9106970c7 
>   src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49549/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Review Request 49549: Used the launch helper binary to launch user task in command executor.

2016-07-01 Thread Jie Yu

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

Review request for mesos, Gilbert Song, Ian Downes, and Joris Van Remoortere.


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


Repository: mesos


Description
---

Currently, command executor and 'mesos-containerizer launch' share a
lot of the logic. Command executor should in fact, just use
`mesos-containerizer launch` to launch the user task. Potentially,
'mesos-containerizer launch' can be also used by custom executor to
launch user tasks.


Diffs
-

  src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
  src/launcher/posix/executor.hpp e5f3c0a8e2d3687a5330adbf2f667db9106970c7 
  src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
  src/slave/containerizer/mesos/launch.cpp 
83f4d7f28c066a605aa84862eca9fde900ec96c6 

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


Testing
---

make check


Thanks,

Jie Yu