mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guangya Liu <gyliu...@gmail.com>
Subject Re: Review Request 45673: PoC: Docker Volume Isolator.
Date Tue, 19 Apr 2016 23:57:11 GMT


> On 四月 19, 2016, 11:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, lines 108-109
> > <https://reviews.apache.org/r/45673/diff/3/?file=1350676#file1350676line108>
> >
> >     To me, this is just an optimization, isn't it? We can still go through the 'infos'
list to see a volume is still begin used, right?
> >     
> >     Can we not introduce this optimizaiton yet?

Yes, as the reference will only be used in `cleanup`, I will move the logic to `cleanup`.


> On 四月 19, 2016, 11:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp, line 337
> > <https://reviews.apache.org/r/45673/diff/3/?file=1350677#file1350677line337>
> >
> >     We need to serialize calls to 'client' for a given driver and a given volume
name. You can use `Sequence` for each driver+name pair in DockerVolumeDriverClient to achieve
that.

Yes, that's why I was using `mountInfo` in `DockerVolumeClient` `mount` API, as I want to
keep the relationship between container path and mount point. ok, will try to see how `Sequence`
can help.


> On 四月 19, 2016, 11:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/state.hpp, line 34
> > <https://reviews.apache.org/r/45673/diff/3/?file=1350680#file1350680line34>
> >
> >     I suggest that we only put informations that we can recover in 'Info' struct.
For things like container_path, mount_point that we cannot recover, try not to put them in
'Info' struct. You can always create a parallel vector with those additional information,
and pass down using parameters in the lambda.

yes, I was putting it here is because I want to make this shared between `DockerVolumeClient`
and `Isolator`, but if I use `Sequence`, then there will be no need to keep this but just
move this to `isolator.hpp`


- Guangya


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


On 四月 19, 2016, 4:45 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45673/
> -----------------------------------------------------------
> 
> (Updated 四月 19, 2016, 4:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> PoC: Docker Volume Isolator.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
>   src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/slave/containerizer/mesos/containerizer.cpp 1e1a36903f4377497bb72b69e4ead63675d453c0

>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 786f917c0ec04b6839bbd524fa7c8de3729f9bdb

>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 0ad473dc3bd45e122fba55a670e1a893e61c977a

>   src/slave/containerizer/mesos/isolators/docker/volume/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp PRE-CREATION 
>   src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 
> 
> Diff: https://reviews.apache.org/r/45673/diff/
> 
> 
> Testing
> -------
> 
> state.proto
> 
> message DockerVolumeMount {
>   required string volume_driver = 1;
>   required string volume_name = 2;
> }
> 
> 
> message DockerVolumeMountList {
>   repeated DockerVolumeMount mounts = 1;
> }
> 
> 
> message DockerVolumeInfo {
>   // Docker volume mount info for volume driver and volume name.
>   required DockerVolumeMount dvm = 1;
> 
>   // Container path, this is optional as this is not needed by recover()
>   // but only for prepare() .
>   optional string container_path = 2;
> 
>   // Mount point for the container, this is optiona as this is only needed
>   // by prepare().
>   optional string mount_point = 3;
>   
>   // Volume driver mount options.
>   optional Parameters driver_options = 4;
> }
> 
> DockerVolumeMount will be checkpointed.
> 
> 
> //   /var/run/mesos/isolators/docker/volume
> //    |- <ID of Container1>/
> //    |  |-- DockerVolumeMountList
> //    |-- <ID of Container2>/
> //    |  |-- DockerVolumeMountList
> //    |-<ID of Container3>/
> //    |- ...
> 
> Info structure
> We assume that one container do not use multiple same volume driver and volume name pair.
> struct Info
> {
>   Info (const list<DockerVolumeInfo>& _volumeInfos)
>     : volumeInfos(_volumeInfos) {}
> 
>   list<DockerVolumeInfo> volumeInfos;
> };
> 
> Hashmap in isolator:
> hashmap<ContainerID, process::Owned<Info>> infos;
> hashmap<string, int> volumeReference; // string is volume_driver::volume_name,
e.g. convoy:dvd1
> 
> Cleanup()
> When cleanup, check the container’s mount point reference counter, if it is greater
than 1, do not remove the mount point; If it is 1, unmount the mount point.
> driver.cpp::mount
> We need to transfer the whole mount info to mount() as parameter to make sure that the
container path can get its own related mount point. If the mount() only return a string, then
we cannot get the relationship of container path and mount point.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


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