CHECKs sound good to me.

On Fri, Sep 28, 2012 at 1:49 PM, Jie Yu <[email protected]> wrote:

> two possible solutions:
>
> 1) have a CHECK when cgroup initializing.
>
> 2) disable cgroup in configure if the kernel version is too old.
>
> I liked (1) because the runtime environment might be different from the
> compile environment.
>
> - Jie
>
> On Fri, Sep 28, 2012 at 4:38 PM, Benjamin Mahler <[email protected]>wrote:
>
>> I also like the tests being red if it's not available, like you said it
>> indicates that they shouldn't use cgroups.
>>
>> However, we should really have a way to _prevent_ them from using cgroups
>> if it's not available, have we thought about this yet?
>>
>>
>> On Fri, Sep 28, 2012 at 12:29 PM, Jie Yu <[email protected]> wrote:
>>
>>> That's the reason. Try to install a newer version of ubuntu (e.g. ubuntu
>>> 11.10, 12.04) so that you can test the oom_control functionality.
>>>
>>> Probably we should disable these tests if oom_control is not available.
>>> But seems that gtest does not provide a clean way to do this (dynamically
>>> disable a test).
>>>
>>> We can add another PREFIX to those test cases (like ROOT_CGROUP), but
>>> that seems to be tedious.
>>>
>>> Also, the break of these tests indicates a good thing: we do require
>>> oom_control capability in cgroups isolation module. If they cannot pass the
>>> unit test, they should not use the cgroups module.
>>>
>>> - Jie
>>>
>>>
>>> On Fri, Sep 28, 2012 at 3:23 PM, Benjamin Mahler <[email protected]>wrote:
>>>
>>>> $ uname -a
>>>> Linux ubuntu 2.6.32-38-generic #83-Ubuntu SMP Wed Jan 4 11:12:07 UTC
>>>> 2012 x86_64 GNU/Linux
>>>>
>>>> On Fri, Sep 28, 2012 at 12:22 PM, Jie Yu <[email protected]> wrote:
>>>>
>>>>>
>>>>>
>>>>> > 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)
>>>>>
>>>>> 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.
>>>>>
>>>>>
>>>>> - Jie
>>>>>
>>>>>
>>>>> -----------------------------------------------------------
>>>>>
>>>>> 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
>>>>> >
>>>>> >
>>>>>
>>>>>
>>>>
>>>
>>
>

Reply via email to