You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Akash Gupta <ak...@hotmail.com> on 2018/05/04 17:19:38 UTC

Review Request 66959: Windows: Fixed pipe inheritance in Mesos containerizer.

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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.


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


Repository: mesos


Description
-------

The Mesos containerizer needs an inheritable pipe that is not used for
stdio of the child process. It is used for signaling when the child
process should start running, so the pipe needs to be inheritable and
not overlapped.


Diffs
-----

  src/slave/containerizer/mesos/containerizer.cpp 01386ac3d36ec7a401b8d1be7834bc1f3fce55ef 


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


Testing
-------


Thanks,

Akash Gupta


Re: Review Request 66959: Windows: Fixed pipe inheritance in Mesos containerizer.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66959/#review203695
-----------------------------------------------------------




src/slave/containerizer/mesos/containerizer.cpp
Lines 1874-1878 (patched)
<https://reviews.apache.org/r/66959/#comment285966>

    NOTE: This will likely be changed soon to go back to non-inheritable, as Radhika's chain is plumbing through the work to pass this pipe explicitly (through args, so we can whitelist it) instead of implicitly (through environment variables). But it is as it should be for now.


- Andrew Schwartzmeyer


On May 4, 2018, 10:19 a.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66959/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 10:19 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8674
>     https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Mesos containerizer needs an inheritable pipe that is not used for
> stdio of the child process. It is used for signaling when the child
> process should start running, so the pipe needs to be inheritable and
> not overlapped.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
> 
> 
> Diff: https://reviews.apache.org/r/66959/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>


Re: Review Request 66959: Windows: Fixed pipe inheritance in Mesos containerizer.

Posted by Radhika Jandhyala via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66959/#review202495
-----------------------------------------------------------


Ship it!




Ship It!


src/slave/containerizer/mesos/containerizer.cpp
Lines 1864 (patched)
<https://reviews.apache.org/r/66959/#comment284308>

    Would this mean that on windows process launched by the containerizer would not be able to perform async io?


- Radhika Jandhyala


On May 4, 2018, 5:19 p.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66959/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 5:19 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8674
>     https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Mesos containerizer needs an inheritable pipe that is not used for
> stdio of the child process. It is used for signaling when the child
> process should start running, so the pipe needs to be inheritable and
> not overlapped.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp 01386ac3d36ec7a401b8d1be7834bc1f3fce55ef 
> 
> 
> Diff: https://reviews.apache.org/r/66959/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>


Re: Review Request 66959: Windows: Fixed pipe inheritance in Mesos containerizer.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66959/#review203693
-----------------------------------------------------------


Ship it!




Ship It!

- Andrew Schwartzmeyer


On May 4, 2018, 10:19 a.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66959/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 10:19 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8674
>     https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Mesos containerizer needs an inheritable pipe that is not used for
> stdio of the child process. It is used for signaling when the child
> process should start running, so the pipe needs to be inheritable and
> not overlapped.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
> 
> 
> Diff: https://reviews.apache.org/r/66959/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>


Re: Review Request 66959: Windows: Fixed pipe inheritance in Mesos containerizer.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66959/#review202586
-----------------------------------------------------------




src/slave/containerizer/mesos/containerizer.cpp
Lines 1866 (patched)
<https://reviews.apache.org/r/66959/#comment284453>

    Perhaps the default should be `false, false` for non-overlapped semantics in `os::pipe`.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1869-1879 (patched)
<https://reviews.apache.org/r/66959/#comment284455>

    Nit: but you chould just do this in a foreach loop over the two pipes, save some duplicated LOC.


- Andrew Schwartzmeyer


On May 4, 2018, 10:19 a.m., Akash Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66959/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 10:19 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8674
>     https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The Mesos containerizer needs an inheritable pipe that is not used for
> stdio of the child process. It is used for signaling when the child
> process should start running, so the pipe needs to be inheritable and
> not overlapped.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp 01386ac3d36ec7a401b8d1be7834bc1f3fce55ef 
> 
> 
> Diff: https://reviews.apache.org/r/66959/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>