----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29688/#review67519 -----------------------------------------------------------
src/slave/containerizer/isolators/disk_quota.hpp <https://reviews.apache.org/r/29688/#comment111543> s/the future/future/ src/slave/containerizer/isolators/disk_quota.hpp <https://reviews.apache.org/r/29688/#comment111544> const? src/slave/containerizer/isolators/disk_quota.hpp <https://reviews.apache.org/r/29688/#comment111547> Can you add a comment here on what these hashmaps represent? src/slave/containerizer/isolators/disk_quota.cpp <https://reviews.apache.org/r/29688/#comment111556> CHECK(..) << "Executor work directory " << state.directory << " doesn't exist"; src/slave/containerizer/isolators/disk_quota.cpp <https://reviews.apache.org/r/29688/#comment111558> I'm confused. What does this hashmap contain for each container? IIUC, a container can have only one working directory that need to be checked for quota. src/slave/containerizer/isolators/disk_quota.cpp <https://reviews.apache.org/r/29688/#comment111560> CHECK(..) << "Unknown path " << path; src/slave/containerizer/isolators/disk_quota.cpp <https://reviews.apache.org/r/29688/#comment111680> const? src/slave/containerizer/isolators/disk_quota.cpp <https://reviews.apache.org/r/29688/#comment111682> s/do/does/ src/slave/containerizer/isolators/disk_quota.cpp <https://reviews.apache.org/r/29688/#comment111755> include 'entry->path' in the error message. src/slave/containerizer/isolators/disk_quota.cpp <https://reviews.apache.org/r/29688/#comment111759> Seems a bit weird to put the error case first and normal case latter. Can you swap them? if (status.get().get() == 0) { // 'du' succeeded. io::read(..) .onAny(...,&Self::__schedule,...); } else { // 'du' failed. io::read(..) .onAny(...,&Self::___schedule,...); } src/slave/containerizer/isolators/disk_quota.cpp <https://reviews.apache.org/r/29688/#comment111760> based on the above comment, s/__schedule/___schedule/ and put this below the current ___schedule src/slave/containerizer/isolators/disk_quota.cpp <https://reviews.apache.org/r/29688/#comment111761> s/___schedule/__schedule/ and pull this above. - Vinod Kone On Jan. 12, 2015, 6:46 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29688/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2015, 6:46 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Ian Downes. > > > Bugs: MESOS-1588 > https://issues.apache.org/jira/browse/MESOS-1588 > > > Repository: mesos-git > > > Description > ------- > > Added DiskQuotaIsolator to enforce disk quota. I created a DiskUsageChecker > to check disk usage by calling 'du'. The DiskUsageChecker is throttled (see > comments). The isolator uses DiskUsageChecker to enforce disk quota. > > > Diffs > ----- > > src/Makefile.am fc0c3227466ccf364353a739fec8d9532ea3c683 > src/slave/containerizer/isolators/disk_quota.hpp PRE-CREATION > src/slave/containerizer/isolators/disk_quota.cpp PRE-CREATION > src/slave/containerizer/mesos/containerizer.cpp > 5c014ebe360b9527b3edd505d47e57a4d5ce5c52 > src/slave/flags.hpp f1b8dfbb7391167b67a9498561742aa9ab9089a6 > > Diff: https://reviews.apache.org/r/29688/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
