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



Current comments while going through the code. Will do a more detailed pass 
later.


src/slave/containerizer/mesos/isolators/disk/xfs.hpp (lines 47 - 48)
<https://reviews.apache.org/r/44342/#comment183810>

    indentation for function parameter should be 4



src/slave/containerizer/mesos/isolators/disk/xfs.hpp (line 79)
<https://reviews.apache.org/r/44342/#comment183809>

    { in the next line.



src/slave/containerizer/mesos/isolators/disk/xfs.hpp (line 84)
<https://reviews.apache.org/r/44342/#comment183817>

    This should be optional because some directories are not isolated.



src/slave/containerizer/mesos/isolators/disk/xfs.hpp (line 87)
<https://reviews.apache.org/r/44342/#comment183811>

    We usually do not use typedef, we prefer to be explicit



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 550)
<https://reviews.apache.org/r/44342/#comment183818>

    So we're not creating a Info for those directories that we don't track, 
right? If that's the case, please move this down.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 610 - 611)
<https://reviews.apache.org/r/44342/#comment183813>

    I don't see 'quota' being used?



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 615)
<https://reviews.apache.org/r/44342/#comment183814>

    Should we call update here to enforce the quota before container starts? 
Otherwise, there'll be a short window that the container is not subject to 
quota?



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 714 - 716)
<https://reviews.apache.org/r/44342/#comment183816>

    Print warnings here if something fails.



src/slave/flags.hpp (line 133)
<https://reviews.apache.org/r/44342/#comment183807>

    wrap with #ifdef WITH_XFS_ISOLATOR



src/slave/flags.cpp (line 694)
<https://reviews.apache.org/r/44342/#comment183808>

    Ditto. Wrap with WITH_XFS_ISOLATOR


- Jie Yu


On March 3, 2016, 6:06 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44342/
> -----------------------------------------------------------
> 
> (Updated March 3, 2016, 6:06 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
>     https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -----
> 
>   configure.ac b045d3c68a2d440bed4d1b3e6ab21a1bbe063517 
>   src/Makefile.am b30cc25f61856d6417437547baaa0bb338a30d63 
>   src/slave/containerizer/mesos/containerizer.cpp 
> af3ff5750649497d8852b4761c78d4cae5455a02 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
> 
> Diff: https://reviews.apache.org/r/44342/diff/
> 
> 
> Testing
> -------
> 
> Manual testing on Fedora 23 w/ XFS. Make check on Fedora and OS X.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Reply via email to