----------------------------------------------------------- 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 > >