mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 47511: Added documentation for `docker/volume` isolator.
Date Sun, 12 Jun 2016 18:57:02 GMT

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




docs/docker-volume-isolator.md (line 2)
<https://reviews.apache.org/r/47511/#comment202333>

    I would rename the filename to be `docker-volume.md`.
    
    I would rename the title to be `Docker Volume Support in Mesos Containerizer`



docs/docker-volume-isolator.md (line 6)
<https://reviews.apache.org/r/47511/#comment202334>

    Please adjust the title here as well.



docs/docker-volume-isolator.md (lines 8 - 10)
<https://reviews.apache.org/r/47511/#comment202339>

    ```
    Mesos 1.0 adds the docker volume support for MesosContainerizer (a.k.a., the unified containerizer)
by introducing a new isolator, called `docker/volume` isolator.
    ```



docs/docker-volume-isolator.md (line 12)
<https://reviews.apache.org/r/47511/#comment202340>

    s/of docker volume support//



docs/docker-volume-isolator.md (line 13)
<https://reviews.apache.org/r/47511/#comment202342>

    s/of all the involved components//



docs/docker-volume-isolator.md (line 34)
<https://reviews.apache.org/r/47511/#comment202343>

    have a link to the persistent volume doc.
    
    ALso, i would consistently use `Mesos` (not mesos) in this doc. Can you do a sweep to
fix all of them?



docs/docker-volume-isolator.md (line 36)
<https://reviews.apache.org/r/47511/#comment202344>

    I would say:
    ```
    The external storage is needed for applications that need much larger storage (e.g., databases).
    ```



docs/docker-volume-isolator.md (line 39)
<https://reviews.apache.org/r/47511/#comment202345>

    can be integrated with



docs/docker-volume-isolator.md (lines 39 - 40)
<https://reviews.apache.org/r/47511/#comment202349>

    This is the 'motivation' section. I would focus on why we need docker volume support.
You mentioned local disk size limit, which is very good.
    
    I think you should also mention that the other motivation is that we want Mesos to be
able to integrate with external storage providers like EBS, GCE volume, etc. so that the data
persisted is beyond the lifetime of a single host.
    
    And then, you should have a paragraph exlaining why we choose Docker volume plugin API.



docs/docker-volume-isolator.md (lines 42 - 45)
<https://reviews.apache.org/r/47511/#comment202347>

    I would move this to the following section (i.e., how does it work).



docs/docker-volume-isolator.md (line 49)
<https://reviews.apache.org/r/47511/#comment202346>

    s/Engine//



docs/docker-volume-isolator.md (line 67)
<https://reviews.apache.org/r/47511/#comment202350>

    Have a link to below (i.e., framework API)



docs/docker-volume-isolator.md (line 95)
<https://reviews.apache.org/r/47511/#comment202363>

    Please use `Docker` (not docker) consistently in this doc.



docs/docker-volume-isolator.md (line 105)
<https://reviews.apache.org/r/47511/#comment202352>

    by frameworks to specify the docker volumes



docs/docker-volume-isolator.md (lines 125 - 127)
<https://reviews.apache.org/r/47511/#comment202355>

    two space indentation. I would put each flag in its own line.



docs/docker-volume-isolator.md (line 151)
<https://reviews.apache.org/r/47511/#comment202356>

    can you kill extra lines here to make it compact



docs/docker-volume-isolator.md (line 181)
<https://reviews.apache.org/r/47511/#comment202357>

    WHy operator?



docs/docker-volume-isolator.md (line 184)
<https://reviews.apache.org/r/47511/#comment202358>

    WHy operator?



docs/docker-volume-isolator.md (line 225)
<https://reviews.apache.org/r/47511/#comment202359>

    that want to
    
    need to set



docs/docker-volume-isolator.md (lines 233 - 251)
<https://reviews.apache.org/r/47511/#comment202361>

    Please make it a JSON (adding quotes for keys) format.



docs/docker-volume-isolator.md (lines 305 - 306)
<https://reviews.apache.org/r/47511/#comment202362>

    It is not recommended to use the same Docker volume from both DockerContainerizer and
MesosContainerizer...


- Jie Yu


On May 31, 2016, 12:46 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47511/
> -----------------------------------------------------------
> 
> (Updated May 31, 2016, 12:46 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5216
>     https://issues.apache.org/jira/browse/MESOS-5216
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added documentation for `docker/volume` isolator.
> 
> 
> Diffs
> -----
> 
>   docs/docker-volume-isolator.md PRE-CREATION 
>   docs/images/docker-volume-isolator.png PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47511/diff/
> 
> 
> Testing
> -------
> 
> You can review the document here: https://github.com/jay-lau/mesos/blob/master/docs/docker-volume-isolator.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


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