mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Timothy Chen" <tnac...@apache.org>
Subject Re: Review Request 23771: Added a Docker containerizer.
Date Tue, 29 Jul 2014 06:30:33 GMT


> On July 28, 2014, 9:33 p.m., Jie Yu wrote:
> > src/slave/containerizer/docker.cpp, lines 839-840
> > <https://reviews.apache.org/r/23771/diff/3/?file=643413#file643413line839>
> >
> >     Hum, this looks problematic to me. These two static variables will be initialized
even before 'main' is called, at which time, the mesos containerizer (potentially) may not
have initialized the cgroups mounts yet.
> 
> Timothy Chen wrote:
>     I don't think we're relying on the Mesos Containerizer to initialize the cgroups
mount though, as we call this when _update is called it should already been mounted.
> 
> Jie Yu wrote:
>     I am referring to the cases where --containerizers=docker,mesos
>     
>     MesosContainerizer is gonna call cgroups::prepare for cpu/memory which will mount
cpu/memory subsystems if they are not yet mounted.
>     
>     But seems that according to C++ standard, local static variables will be initialized
the first time its translation unit is entered. At which time, we should already mount the
cgroups. So your code is fine, but I would rather either store it as a field member, or call
cgroups::hierarchy each time 'update' is called.

But if it becomes a field member it will get evaluated before cgroups::prepare is called then
right?
Ben intentionally put it as a static variable to avoid re-doing the hierarchy lookup since
it doesn't change, this is pretty standard C++ and don't really see what are the good reasons
to call this each time.


- Timothy


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


On July 29, 2014, 1:04 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23771/
> -----------------------------------------------------------
> 
> (Updated July 29, 2014, 1:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Docker implementation.
> This is all the docker code Ben, I and Yifan worked on excluding the composing containerizer
patches.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 45afcd1 
>   src/common/status_utils.hpp 1980551 
>   src/docker/docker.hpp PRE-CREATION 
>   src/docker/docker.cpp PRE-CREATION 
>   src/examples/docker_no_executor_framework.cpp PRE-CREATION 
>   src/health-check/main.cpp 707810a 
>   src/launcher/executor.cpp 9c80848 
>   src/linux/cgroups.hpp decad9d 
>   src/linux/cgroups.cpp 6a73dd7 
>   src/master/master.cpp 251b699 
>   src/slave/containerizer/containerizer.cpp 1b71f33 
>   src/slave/containerizer/docker.hpp PRE-CREATION 
>   src/slave/containerizer/docker.cpp PRE-CREATION 
>   src/slave/containerizer/external_containerizer.cpp 3f28d85 
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 780037b 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 3265a80 
>   src/slave/containerizer/isolators/cgroups/mem.hpp 8c476c7 
>   src/slave/containerizer/isolators/cgroups/mem.cpp e8d1e35 
>   src/slave/containerizer/isolators/posix.hpp 17bbd10 
>   src/slave/flags.hpp 1fe7b7d 
>   src/slave/slave.cpp f42ab60 
>   src/tests/cgroups_tests.cpp 01cf498 
>   src/tests/docker_containerizer_tests.cpp PRE-CREATION 
>   src/tests/docker_tests.cpp PRE-CREATION 
>   src/tests/environment.cpp 434b3f7 
>   src/tests/flags.hpp a003e7f 
>   src/tests/script.cpp 15a6542 
>   src/usage/usage.hpp 5a76746 
>   src/usage/usage.cpp 29014d1 
> 
> Diff: https://reviews.apache.org/r/23771/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message