> On Sept. 28, 2012, 7:16 p.m., Ben Mahler wrote: > > I've patched this in and I'm seeing a a failing test: > > > > [ RUN ] CgroupsTest.ROOT_CGROUPS_ListenEvent > > ../../src/tests/cgroups_tests.cpp:365: Failure > > Value of: cgroups::writeControl(hierarchy, "/prof", "memory.oom_control", > > "1").isSome() > > Actual: false > > Expected: true > > [ FAILED ] CgroupsTest.ROOT_CGROUPS_ListenEvent (290 ms) > > Jie Yu wrote: > What's your kernel version? Please do a 'uname -a'. > > We saw this before, the reason is you are using an old kernel which does > not have oom_control capability. > > Ben Mahler wrote: > Ok, they're all green on 12.04 for me too. > > Benjamin Hindman wrote: > Can we detect this via a CHECK at runtime? > > Jie Yu wrote: > Sorry, I don't get it. A CHECK will abort the entire test run I presume? > > I think probably we need a print saying that oom_control is not available.
Either we: (a) Don't run the tests if we know that they'll fail (because we've checked the kernel version). (b) Fail the tests with a useful error message that explains why they are failing. Right now when the tests fail someone will send an email to mesos-dev and say, why are the tests failing? And then you, or me, or someone else, will have to respond and say: "What's your kernel version? Please do a 'uname -a'. We saw this before, the reason is you are using an old kernel which does not have oom_control capability." I would obviously prefer (a) but I'm okay with a savvy user getting the error message from (b) and then explicitly doing '--gtest_filter=-Cgroups*' (which one could imagine the error message suggests). When it comes to running one of the binaries (e.g., mesos-slave, mesos-local) it's imperative that we keep a user from launching a slave with '--isolation=cgroups' if we don't have all the functionality. Otherwise, we're probably going to be introducing semantics we haven't thought about and debugging cases that are very esoteric (which could waste a lot of time). - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7338/#review12031 ----------------------------------------------------------- On Sept. 28, 2012, 5:33 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7338/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2012, 5:33 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Description > ------- > > The recent refactor changes break the assumptions in the cgroups code. > > > Diffs > ----- > > src/linux/cgroups.cpp cdafe6e > third_party/libprocess/include/stout/os.hpp 13dbc71 > > Diff: https://reviews.apache.org/r/7338/diff/ > > > Testing > ------- > > make check. > > Tested on my vm (latest ubuntu 12.04) > > > Thanks, > > Jie Yu > >
