You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chi Zhang <ch...@gmail.com> on 2015/09/09 23:33:13 UTC

Review Request 38234: Check if swap is enabled before running memory pressure related tests.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38234/
-----------------------------------------------------------

Review request for mesos and Jie Yu.


Bugs: mesos-2918
    https://issues.apache.org/jira/browse/mesos-2918


Repository: mesos


Description
-------

Check if swap is enabled before running memory pressure related tests.


Diffs
-----

  src/tests/containerizer/cgroups_tests.cpp 75a3bc0009c037dc18ce319db2eb44630f083e8c 

Diff: https://reviews.apache.org/r/38234/diff/


Testing
-------


Thanks,

Chi Zhang


Re: Review Request 38234: Check if swap is enabled before running memory pressure related tests.

Posted by Vinod Kone <vi...@gmail.com>.

> On Nov. 17, 2015, 7:27 p.m., Vinod Kone wrote:
> > src/tests/containerizer/cgroups_tests.cpp, lines 560-568
> > <https://reviews.apache.org/r/38234/diff/1/?file=1066658#file1066658line560>
> >
> >     instead of asserting, it would be better if we can disable the test automatically if we detect that swap is enabled.
> >     
> >     for example, we can change the test name to ROOT_CGROUPS_OOM_Listen
> >     
> >     and in environment.cpp, disable the test if 'OOM' token is present in the test name and swap is enabled.
> >     
> >     
> >     also, does the swap issue not affect the balloon framewok test?
> 
> Chi Zhang wrote:
>     Yeah, I wanted to do that to disable other tests dymaically before and final result was just to use this style, so I did the same to be consistent.
>     
>     re-read the code again for balloon frame tests: slave has 96MB memory, executor uses 64MB, 32 left to the task, balloon() allocates 64MB at a time, which basically means first allocation request will cause OOM (64 > 32).
>     
>     If the step was set to say do 3 times 16MBs, (3x16 > 32), the extra 16MB will be eaten by swap and not cause a OOM if swap is enabled.

not sure why dynamica disabling didn't work before, but i'll add a TODO for now and commit this.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38234/#review106919
-----------------------------------------------------------


On Sept. 9, 2015, 9:33 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38234/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: mesos-2918
>     https://issues.apache.org/jira/browse/mesos-2918
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Check if swap is enabled before running memory pressure related tests.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/cgroups_tests.cpp 75a3bc0009c037dc18ce319db2eb44630f083e8c 
> 
> Diff: https://reviews.apache.org/r/38234/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>


Re: Review Request 38234: Check if swap is enabled before running memory pressure related tests.

Posted by Chi Zhang <ch...@gmail.com>.

> On Nov. 17, 2015, 7:27 p.m., Vinod Kone wrote:
> > src/tests/containerizer/cgroups_tests.cpp, lines 560-568
> > <https://reviews.apache.org/r/38234/diff/1/?file=1066658#file1066658line560>
> >
> >     instead of asserting, it would be better if we can disable the test automatically if we detect that swap is enabled.
> >     
> >     for example, we can change the test name to ROOT_CGROUPS_OOM_Listen
> >     
> >     and in environment.cpp, disable the test if 'OOM' token is present in the test name and swap is enabled.
> >     
> >     
> >     also, does the swap issue not affect the balloon framewok test?

Yeah, I wanted to do that to disable other tests dymaically before and final result was just to use this style, so I did the same to be consistent.

re-read the code again for balloon frame tests: slave has 96MB memory, executor uses 64MB, 32 left to the task, balloon() allocates 64MB at a time, which basically means first allocation request will cause OOM (64 > 32).

If the step was set to say do 3 times 16MBs, (3x16 > 32), the extra 16MB will be eaten by swap and not cause a OOM if swap is enabled.


- Chi


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38234/#review106919
-----------------------------------------------------------


On Sept. 9, 2015, 9:33 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38234/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: mesos-2918
>     https://issues.apache.org/jira/browse/mesos-2918
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Check if swap is enabled before running memory pressure related tests.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/cgroups_tests.cpp 75a3bc0009c037dc18ce319db2eb44630f083e8c 
> 
> Diff: https://reviews.apache.org/r/38234/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>


Re: Review Request 38234: Check if swap is enabled before running memory pressure related tests.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38234/#review106919
-----------------------------------------------------------



src/tests/containerizer/cgroups_tests.cpp (lines 560 - 568)
<https://reviews.apache.org/r/38234/#comment165721>

    instead of asserting, it would be better if we can disable the test automatically if we detect that swap is enabled.
    
    for example, we can change the test name to ROOT_CGROUPS_OOM_Listen
    
    and in environment.cpp, disable the test if 'OOM' token is present in the test name and swap is enabled.
    
    also, does the swap issue not affect the balloon framewok test?


- Vinod Kone


On Sept. 9, 2015, 9:33 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38234/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: mesos-2918
>     https://issues.apache.org/jira/browse/mesos-2918
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Check if swap is enabled before running memory pressure related tests.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/cgroups_tests.cpp 75a3bc0009c037dc18ce319db2eb44630f083e8c 
> 
> Diff: https://reviews.apache.org/r/38234/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>


Re: Review Request 38234: Check if swap is enabled before running memory pressure related tests.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38234/#review98330
-----------------------------------------------------------


Patch looks great!

Reviews applied: [38233, 38234]

All tests passed.

- Mesos ReviewBot


On Sept. 9, 2015, 9:33 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38234/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: mesos-2918
>     https://issues.apache.org/jira/browse/mesos-2918
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Check if swap is enabled before running memory pressure related tests.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/cgroups_tests.cpp 75a3bc0009c037dc18ce319db2eb44630f083e8c 
> 
> Diff: https://reviews.apache.org/r/38234/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>