Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review105139 --- Ship it! Ship It! - Kapil Arya On Sept. 23, 2015, 9:55 p.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 23, 2015, 9:55 p.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > This test is to isolate one single process with one thread in cgroup, and > checks the number of processes and threads in the cgroup before/after > isolation. The previous test case uses "sh -c "while true; do sleep 1; > done;"" as the process, however, it periodically forks a child process "sleep > 1" in cgroup causing the test failure (expected number of process/thread > should be 1, but here is 2). This patch is to issue a "cat" command, so there > will be only one process in cgroup. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > --gtest_repeat=1000 --gtest_break_on_failure > > > Thanks, > > Jian Qiu > >
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review100354 --- Patch looks great! Reviews applied: [38454] All tests passed. - Mesos ReviewBot On Sept. 24, 2015, 1:55 a.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 24, 2015, 1:55 a.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > This test is to isolate one single process with one thread in cgroup, and > checks the number of processes and threads in the cgroup before/after > isolation. The previous test case uses "sh -c "while true; do sleep 1; > done;"" as the process, however, it periodically forks a child process "sleep > 1" in cgroup causing the test failure (expected number of process/thread > should be 1, but here is 2). This patch is to issue a "cat" command, so there > will be only one process in cgroup. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > --gtest_repeat=1000 --gtest_break_on_failure > > > Thanks, > > Jian Qiu > >
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/ --- (Updated Sept. 24, 2015, 1:55 a.m.) Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. Bugs: MESOS-3293 https://issues.apache.org/jira/browse/MESOS-3293 Repository: mesos Description --- This test is to isolate one single process with one thread in cgroup, and checks the number of processes and threads in the cgroup before/after isolation. The previous test case uses "sh -c "while true; do sleep 1; done;"" as the process, however, it periodically forks a child process "sleep 1" in cgroup causing the test failure (expected number of process/thread should be 1, but here is 2). This patch is to issue a "cat" command, so there will be only one process in cgroup. Diffs (updated) - src/tests/containerizer/isolator_tests.cpp a25ae97a519feb8ead6177da160df8a276ca15bf Diff: https://reviews.apache.org/r/38454/diff/ Testing --- ./mesos-tests.sh --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" --gtest_repeat=1000 --gtest_break_on_failure Thanks, Jian Qiu
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/ --- (Updated Sept. 24, 2015, 1:52 a.m.) Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. Bugs: MESOS-3293 https://issues.apache.org/jira/browse/MESOS-3293 Repository: mesos Description (updated) --- This test is to isolate one single process with one thread in cgroup, and checks the number of processes and threads in the cgroup before/after isolation. The previous test case uses "sh -c "while true; do sleep 1; done;"" as the process, however, it periodically forks a child process "sleep 1" in cgroup causing the test failure (expected number of process/thread should be 1, but here is 2). This patch is to issue a "cat" command, so there will be only one process in cgroup. Diffs - src/tests/containerizer/isolator_tests.cpp a25ae97a519feb8ead6177da160df8a276ca15bf Diff: https://reviews.apache.org/r/38454/diff/ Testing --- ./mesos-tests.sh --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" --gtest_repeat=1000 --gtest_break_on_failure Thanks, Jian Qiu
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> On Sept. 17, 2015, 4:39 p.m., Marco Massenzio wrote: > > Thanks for fixing this, but I'm wondering how this fixes the issue. > > (in fact, not even what is it testing). > > > > If you invoke `cat` without input, it will just hang there waiting on > > STDIN, how does this make the test work? > > (sorry for all the questions, but unfortunately I'm not familiar w/ this > > particular test) > > > > Can you please add more information in the Description field? > > It's pretty obvious (or it should be :) ) that a patch fixes the bug it's > > related to, but what folks need to understand is: (a) what the problem was; > > (b) how it affected the test and (c) what you did to fix it. > > > > Also, how does this preserve the spirit of the test? I can fix any test by > > just having ASSERT(true) :) > > > > BTW - it's fantastic this fixed the issue and you did the repeat-cycle test! > > haosdent huang wrote: > Hi @marco. "cat" is equals to "sleep forever" command(we conld not > execute sleep forver only could sleep xxx). I think this patch should fix the > problem according I repeat test this patch tonight. And the cause of this > issue has beed documented in jira by @qiujian. The old command sometimes have > two processes when running so this test case is flaky. > > haosdent huang wrote: > Ref: Bash: infinite sleep (infinite blocking) > http://stackoverflow.com/questions/2935183/bash-infinite-sleep-infinite-blocking Thanks for comments, I have udpated the description accordingly. - Jian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review99400 --- On Sept. 18, 2015, 2:51 a.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 18, 2015, 2:51 a.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > This test is to isolate one single process with on thread in cgroup, and > checks the number of processes and threads in the cgroup before/after > isolation. The previous test case uses "sh -c "while true; do sleep 1; > done;"" as the process, however, it periodically forks a child process "sleep > 1" in cgroup causing the test failure (expected number of process/thread > should be 1, but here is 2). This patch is to issue a "cat" command, so there > will be only one process in cgroup. > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > --gtest_repeat=1000 --gtest_break_on_failure > > > Thanks, > > Jian Qiu > >
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/ --- (Updated Sept. 18, 2015, 2:51 a.m.) Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. Bugs: MESOS-3293 https://issues.apache.org/jira/browse/MESOS-3293 Repository: mesos Description (updated) --- This test is to isolate one single process with on thread in cgroup, and checks the number of processes and threads in the cgroup before/after isolation. The previous test case uses "sh -c "while true; do sleep 1; done;"" as the process, however, it periodically forks a child process "sleep 1" in cgroup causing the test failure (expected number of process/thread should be 1, but here is 2). This patch is to issue a "cat" command, so there will be only one process in cgroup. Diffs - src/tests/containerizer/isolator_tests.cpp a25ae97a519feb8ead6177da160df8a276ca15bf Diff: https://reviews.apache.org/r/38454/diff/ Testing --- ./mesos-tests.sh --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" --gtest_repeat=1000 --gtest_break_on_failure Thanks, Jian Qiu
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> On Sept. 17, 2015, 4:39 p.m., Marco Massenzio wrote: > > Thanks for fixing this, but I'm wondering how this fixes the issue. > > (in fact, not even what is it testing). > > > > If you invoke `cat` without input, it will just hang there waiting on > > STDIN, how does this make the test work? > > (sorry for all the questions, but unfortunately I'm not familiar w/ this > > particular test) > > > > Can you please add more information in the Description field? > > It's pretty obvious (or it should be :) ) that a patch fixes the bug it's > > related to, but what folks need to understand is: (a) what the problem was; > > (b) how it affected the test and (c) what you did to fix it. > > > > Also, how does this preserve the spirit of the test? I can fix any test by > > just having ASSERT(true) :) > > > > BTW - it's fantastic this fixed the issue and you did the repeat-cycle test! > > haosdent huang wrote: > Hi @marco. "cat" is equals to "sleep forever" command(we conld not > execute sleep forver only could sleep xxx). I think this patch should fix the > problem according I repeat test this patch tonight. And the cause of this > issue has beed documented in jira by @qiujian. The old command sometimes have > two processes when running so this test case is flaky. Ref: Bash: infinite sleep (infinite blocking) http://stackoverflow.com/questions/2935183/bash-infinite-sleep-infinite-blocking - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review99400 --- On Sept. 17, 2015, 9:55 a.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 17, 2015, 9:55 a.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > --gtest_repeat=1000 --gtest_break_on_failure > > > Thanks, > > Jian Qiu > >
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> On Sept. 17, 2015, 4:39 p.m., Marco Massenzio wrote: > > Thanks for fixing this, but I'm wondering how this fixes the issue. > > (in fact, not even what is it testing). > > > > If you invoke `cat` without input, it will just hang there waiting on > > STDIN, how does this make the test work? > > (sorry for all the questions, but unfortunately I'm not familiar w/ this > > particular test) > > > > Can you please add more information in the Description field? > > It's pretty obvious (or it should be :) ) that a patch fixes the bug it's > > related to, but what folks need to understand is: (a) what the problem was; > > (b) how it affected the test and (c) what you did to fix it. > > > > Also, how does this preserve the spirit of the test? I can fix any test by > > just having ASSERT(true) :) > > > > BTW - it's fantastic this fixed the issue and you did the repeat-cycle test! Hi @marco. "cat" is equals to "sleep forever" command(we conld not execute sleep forver only could sleep xxx). I think this patch should fix the problem according I repeat test this patch tonight. And the cause of this issue has beed documented in jira by @qiujian. The old command sometimes have two processes when running so this test case is flaky. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review99400 --- On Sept. 17, 2015, 9:55 a.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 17, 2015, 9:55 a.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > --gtest_repeat=1000 --gtest_break_on_failure > > > Thanks, > > Jian Qiu > >
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review99400 --- Thanks for fixing this, but I'm wondering how this fixes the issue. (in fact, not even what is it testing). If you invoke `cat` without input, it will just hang there waiting on STDIN, how does this make the test work? (sorry for all the questions, but unfortunately I'm not familiar w/ this particular test) Can you please add more information in the Description field? It's pretty obvious (or it should be :) ) that a patch fixes the bug it's related to, but what folks need to understand is: (a) what the problem was; (b) how it affected the test and (c) what you did to fix it. Also, how does this preserve the spirit of the test? I can fix any test by just having ASSERT(true) :) BTW - it's fantastic this fixed the issue and you did the repeat-cycle test! - Marco Massenzio On Sept. 17, 2015, 9:55 a.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 17, 2015, 9:55 a.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > --gtest_repeat=1000 --gtest_break_on_failure > > > Thanks, > > Jian Qiu > >
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review99369 --- Patch looks great! Reviews applied: [38454] All tests passed. - Mesos ReviewBot On Sept. 17, 2015, 9:55 a.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 17, 2015, 9:55 a.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > --gtest_repeat=1000 --gtest_break_on_failure > > > Thanks, > > Jian Qiu > >
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/ --- (Updated Sept. 17, 2015, 9:55 a.m.) Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. Bugs: MESOS-3293 https://issues.apache.org/jira/browse/MESOS-3293 Repository: mesos Description --- Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids Diffs - src/tests/containerizer/isolator_tests.cpp a25ae97a519feb8ead6177da160df8a276ca15bf Diff: https://reviews.apache.org/r/38454/diff/ Testing (updated) --- ./mesos-tests.sh --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" --gtest_repeat=1000 --gtest_break_on_failure Thanks, Jian Qiu
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> On Sept. 17, 2015, 8:48 a.m., haosdent huang wrote: > > Could you use > > > > ``` > > sudo ./mesos-tests.sh > > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > > --gtest_repeat=1000 --gtest_break_on_failure > > ``` > > > > to test it? Done, thanks for reminding! - Jian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review99361 --- On Sept. 17, 2015, 8:28 a.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 17, 2015, 8:28 a.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > > > Thanks, > > Jian Qiu > >
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review99365 --- Ship it! Ship It! - Guangya Liu On Sept. 17, 2015, 8:28 a.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 17, 2015, 8:28 a.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > > > Thanks, > > Jian Qiu > >
Re: Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/#review99361 --- Ship it! Could you use ``` sudo ./mesos-tests.sh --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" --gtest_repeat=1000 --gtest_break_on_failure ``` to test it? - haosdent huang On Sept. 17, 2015, 8:28 a.m., Jian Qiu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38454/ > --- > > (Updated Sept. 17, 2015, 8:28 a.m.) > > > Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. > > > Bugs: MESOS-3293 > https://issues.apache.org/jira/browse/MESOS-3293 > > > Repository: mesos > > > Description > --- > > Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids > > > Diffs > - > > src/tests/containerizer/isolator_tests.cpp > a25ae97a519feb8ead6177da160df8a276ca15bf > > Diff: https://reviews.apache.org/r/38454/diff/ > > > Testing > --- > > ./mesos-tests.sh > --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" > > > Thanks, > > Jian Qiu > >
Review Request 38454: Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38454/ --- Review request for mesos, haosdent huang, Marco Massenzio, and Vinod Kone. Bugs: MESOS-3293 https://issues.apache.org/jira/browse/MESOS-3293 Repository: mesos Description --- Fix failed test LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids Diffs - src/tests/containerizer/isolator_tests.cpp a25ae97a519feb8ead6177da160df8a276ca15bf Diff: https://reviews.apache.org/r/38454/diff/ Testing --- ./mesos-tests.sh --gtest_filter="LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids" Thanks, Jian Qiu
