You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2016/11/28 17:07:49 UTC

Re: Review Request 53688: Implement a namespace/ipc isolator.

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

(Updated Nov. 28, 2016, 5:07 p.m.)


Review request for mesos and Jie Yu.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

Implement a namespace/ipc isolator.


Diffs (updated)
-----

  src/Makefile.am 85eda538caf39f81f052896e744b7b0c724f81bb 
  src/slave/containerizer/mesos/containerizer.cpp 9b33495d9babc3ee489a8712fe1977746c41043f 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 9766aaf144722b18d88f694ff37ffd53974cb60d 

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


Testing
-------

Make check on Fedora 24.


Thanks,

James Peach


Re: Review Request 53688: Implement a namespace/ipc isolator.

Posted by Avinash sridharan <av...@mesosphere.io>.

> On Dec. 2, 2016, 6:20 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp, line 28
> > <https://reviews.apache.org/r/53688/diff/6/?file=1573534#file1573534line28>
> >
> >     Any reason why we are not inhertiting it from a `MesosIsolatorProcess` ? I see that given its current implementation it doesn't need to be an actor. But might make sense to follow the idiom set by other isolators to keep it Future proof?
> 
> James Peach wrote:
>     Since it doesn't need an actor, it seemed better to avoid the memory, performance and compilation time costs. If it does need one later that's preetty easy to change.

Not sure the performance and memory consumption is that great (don't have any numbers) given that the whole point of having a libprocess actor is its low overhead, so would prefer maintaining conformance over performance :). But if you feel strongly about this, don't have have a strong argument against it either, so go ahead and drop the comment in that case.


- Avinash


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


On Dec. 2, 2016, 5:43 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53688/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2016, 5:43 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-6557
>     https://issues.apache.org/jira/browse/MESOS-6557
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implement a namespace/ipc isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt ea6e399c40d3b2cda195091dc7e74230ff3f68fd 
>   src/Makefile.am 7750ed756d60aa61225667c129df35c6ec70f239 
>   src/slave/containerizer/mesos/containerizer.cpp 7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp da4627846730abd3a817c3d538ff5676c3c1ef45 
> 
> Diff: https://reviews.apache.org/r/53688/diff/
> 
> 
> Testing
> -------
> 
> Make check on Fedora 24.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 53688: Implement a namespace/ipc isolator.

Posted by James Peach <jp...@apache.org>.

> On Dec. 2, 2016, 6:20 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp, line 28
> > <https://reviews.apache.org/r/53688/diff/6/?file=1573534#file1573534line28>
> >
> >     Any reason why we are not inhertiting it from a `MesosIsolatorProcess` ? I see that given its current implementation it doesn't need to be an actor. But might make sense to follow the idiom set by other isolators to keep it Future proof?

Since it doesn't need an actor, it seemed better to avoid the memory, performance and compilation time costs. If it does need one later that's preetty easy to change.


- James


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


On Dec. 1, 2016, 12:24 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53688/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Avinash Gautam and Jie Yu.
> 
> 
> Bugs: MESOS-6557
>     https://issues.apache.org/jira/browse/MESOS-6557
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implement a namespace/ipc isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt ea6e399c40d3b2cda195091dc7e74230ff3f68fd 
>   src/Makefile.am 7750ed756d60aa61225667c129df35c6ec70f239 
>   src/slave/containerizer/mesos/containerizer.cpp 7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp da4627846730abd3a817c3d538ff5676c3c1ef45 
> 
> Diff: https://reviews.apache.org/r/53688/diff/
> 
> 
> Testing
> -------
> 
> Make check on Fedora 24.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 53688: Implement a namespace/ipc isolator.

Posted by Avinash sridharan <av...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53688/#review157703
-----------------------------------------------------------




src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp (line 28)
<https://reviews.apache.org/r/53688/#comment228325>

    Any reason why we are not inhertiting it from a `MesosIsolatorProcess` ? I see that given its current implementation it doesn't need to be an actor. But might make sense to follow the idiom set by other isolators to keep it Future proof?


- Avinash sridharan


On Dec. 1, 2016, 12:24 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53688/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Avinash Gautam and Jie Yu.
> 
> 
> Bugs: MESOS-6557
>     https://issues.apache.org/jira/browse/MESOS-6557
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implement a namespace/ipc isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt ea6e399c40d3b2cda195091dc7e74230ff3f68fd 
>   src/Makefile.am 7750ed756d60aa61225667c129df35c6ec70f239 
>   src/slave/containerizer/mesos/containerizer.cpp 7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp da4627846730abd3a817c3d538ff5676c3c1ef45 
> 
> Diff: https://reviews.apache.org/r/53688/diff/
> 
> 
> Testing
> -------
> 
> Make check on Fedora 24.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 53688: Implement a namespace/ipc isolator.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53688/
-----------------------------------------------------------

(Updated Dec. 1, 2016, 12:24 a.m.)


Review request for mesos, Avinash Gautam and Jie Yu.


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


Repository: mesos


Description
-------

Implement a namespace/ipc isolator.


Diffs (updated)
-----

  src/CMakeLists.txt ea6e399c40d3b2cda195091dc7e74230ff3f68fd 
  src/Makefile.am 7750ed756d60aa61225667c129df35c6ec70f239 
  src/slave/containerizer/mesos/containerizer.cpp 7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp da4627846730abd3a817c3d538ff5676c3c1ef45 

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


Testing
-------

Make check on Fedora 24.


Thanks,

James Peach


Re: Review Request 53688: Implement a namespace/ipc isolator.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53688/
-----------------------------------------------------------

(Updated Nov. 29, 2016, 9:19 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
-------

Implement a namespace/ipc isolator.


Diffs (updated)
-----

  src/Makefile.am 85eda538caf39f81f052896e744b7b0c724f81bb 
  src/slave/containerizer/mesos/containerizer.cpp 9b33495d9babc3ee489a8712fe1977746c41043f 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 9766aaf144722b18d88f694ff37ffd53974cb60d 

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


Testing
-------

Make check on Fedora 24.


Thanks,

James Peach