> On Nov. 15, 2012, 10:42 p.m., Vinod Kone wrote: > > src/linux/cgroups.cpp, line 1252 > > <https://reviews.apache.org/r/8058/diff/1/?file=189977#file189977line1252> > > > > i think 'freeze cgroup' is more clear. why did you remove?
Yes, I like it better too. Thanks. > On Nov. 15, 2012, 10:42 p.m., Vinod Kone wrote: > > src/linux/cgroups.cpp, lines 1410-1411 > > <https://reviews.apache.org/r/8058/diff/1/?file=189977#file189977line1410> > > > > sounds like this should be a CHECK? This is actually a passed into the function as a Duration, so a programmer could have passed a negative duration. As with a lot of our abstractions, we want to treat the cgroups code as much like a library as possible and we don't want a library crashing the program because someone else got it wrong. (I'd also be willing to have a conversation about making Duration always be positive?) > On Nov. 15, 2012, 10:42 p.m., Vinod Kone wrote: > > src/slave/cgroups_isolation_module.cpp, line 99 > > <https://reviews.apache.org/r/8058/diff/1/?file=189978#file189978line99> > > > > s/on/in/ ? Done. > On Nov. 15, 2012, 10:42 p.m., Vinod Kone wrote: > > src/slave/cgroups_isolation_module.cpp, line 219 > > <https://reviews.apache.org/r/8058/diff/1/?file=189978#file189978line219> > > > > i feel like checking for nest cgroup support should be part of the > > cgroup library? Are you suggesting when we attempt to create a nested cgroup failing with an error then? I'd prefer to fail earlier before things start running. Or are you suggesting another primitive/function to add that would check this? I'd be happy with the latter, but I"ll punt it for the future. > On Nov. 15, 2012, 10:42 p.m., Vinod Kone wrote: > > src/slave/cgroups_isolation_module.cpp, line 268 > > <https://reviews.apache.org/r/8058/diff/1/?file=189978#file189978line268> > > > > Wait, so were actually disabling the kernel OOM killer? I thought we > > couldn't escape the first OOM kill by the kernel? Yes, we can make the kernel pause/stop a process if it causes an OOM! - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8058/#review13491 ----------------------------------------------------------- On Nov. 14, 2012, 11:57 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8058/ > ----------------------------------------------------------- > > (Updated Nov. 14, 2012, 11:57 p.m.) > > > Review request for mesos, Vinod Kone and Ben Mahler. > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/linux/cgroups.hpp 1351134dc8b40fb7d7d692c4dd24f0efa24eadb0 > src/linux/cgroups.cpp 039e2b4121edc419e9f1a48a7c0b99ca77aa44ff > src/slave/cgroups_isolation_module.cpp > 8211618d7729350654e2d17946c5b912ed9dda6a > src/tests/assert.hpp 62fde12fa7ddddbbc9e8812f26314af5307a02df > src/tests/cgroups_tests.cpp 85e81854a6767bad5c19f6411130d154c2870d99 > > Diff: https://reviews.apache.org/r/8058/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
