You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jian Qiu <qi...@cn.ibm.com> on 2015/12/23 07:29:21 UTC
Review Request 41600: Speed up SlaveTest.CommandExecutorWithOverride
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/
-----------------------------------------------------------
Review request for mesos and Alexander Rukletsov.
Bugs: MESOS-4161
https://issues.apache.org/jira/browse/MESOS-4161
Repository: mesos
Description
-------
To speed up SlaveTest.CommandExecutorWithOverride, we need to explicitly kill the executor process and advance reap interval.
Diffs
-----
src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7
Diff: https://reviews.apache.org/r/41600/diff/
Testing
-------
make check -j4 GTEST_FILTER=SlaveTest.CommandExecutorWithOverride takes 415ms
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from SlaveTest
[ RUN ] SlaveTest.CommandExecutorWithOverride
[ OK ] SlaveTest.CommandExecutorWithOverride (405 ms)
[----------] 1 test from SlaveTest (405 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (415 ms total)
[ PASSED ] 1 test.
Thanks,
Jian Qiu
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/#review112280
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41600]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 30, 2015, 2:02 a.m., Jian Qiu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> -----------------------------------------------------------
>
> (Updated Dec. 30, 2015, 2:02 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since the reason for the test case being slow is MESOS-4111, we will add comment in the test case, and remove it when MESOS-4111 is fixed.
>
>
> Diffs
> -----
>
> src/tests/slave_tests.cpp 68d75176ce7cccafdf870fff48ac6a11089311ef
>
> Diff: https://reviews.apache.org/r/41600/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jian Qiu
>
>
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/#review113228
-----------------------------------------------------------
Ship it!
src/tests/slave_tests.cpp (line 320)
<https://reviews.apache.org/r/41600/#comment173728>
Let's make the comment self-contained. How about:
"`CommmandExecutor::reaped()` sleeps 1 second to avoid races, hence this test takes more than 1 second to finish. The root cause is tracked via MESOS-4111."
- Alexander Rukletsov
On Dec. 30, 2015, 2:02 a.m., Jian Qiu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> -----------------------------------------------------------
>
> (Updated Dec. 30, 2015, 2:02 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since the reason for the test case being slow is MESOS-4111, we will add comment in the test case, and remove it when MESOS-4111 is fixed.
>
>
> Diffs
> -----
>
> src/tests/slave_tests.cpp 68d75176ce7cccafdf870fff48ac6a11089311ef
>
> Diff: https://reviews.apache.org/r/41600/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jian Qiu
>
>
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/#review113253
-----------------------------------------------------------
Bad patch!
Reviews applied: [41600]
Failed command: ./support/apply-review.sh -n -r 41600
Error:
2016-01-07 14:01:53 URL:https://reviews.apache.org/r/41600/diff/raw/ [691/691] -> "41600.patch" [1]
Total errors found: 0
Checking 1 files
Error: Commit message summary (the first line) must end in a period.
- Mesos ReviewBot
On Jan. 7, 2016, 1:41 p.m., Jian Qiu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 1:41 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since the reason for the test case being slow is MESOS-4111, we will add comment in the test case, and remove it when MESOS-4111 is fixed.
>
>
> Diffs
> -----
>
> src/tests/slave_tests.cpp 328c853a338145256dadfc9364b9c2a0e6c356cb
>
> Diff: https://reviews.apache.org/r/41600/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jian Qiu
>
>
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/#review113427
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41600]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 8, 2016, 2:13 a.m., Jian Qiu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2016, 2:13 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since the reason for the test case being slow is MESOS-4111, we will add comment in the test case, and remove it when MESOS-4111 is fixed.
>
>
> Diffs
> -----
>
> src/tests/slave_tests.cpp 328c853a338145256dadfc9364b9c2a0e6c356cb
>
> Diff: https://reviews.apache.org/r/41600/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jian Qiu
>
>
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride.
Posted by Jian Qiu <qi...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/
-----------------------------------------------------------
(Updated 一月 8, 2016, 2:13 a.m.)
Review request for mesos and Alexander Rukletsov.
Summary (updated)
-----------------
Speed up SlaveTest.CommandExecutorWithOverride.
Bugs: MESOS-4161
https://issues.apache.org/jira/browse/MESOS-4161
Repository: mesos
Description
-------
Since the reason for the test case being slow is MESOS-4111, we will add comment in the test case, and remove it when MESOS-4111 is fixed.
Diffs (updated)
-----
src/tests/slave_tests.cpp 328c853a338145256dadfc9364b9c2a0e6c356cb
Diff: https://reviews.apache.org/r/41600/diff/
Testing
-------
Thanks,
Jian Qiu
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Jian Qiu <qi...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/
-----------------------------------------------------------
(Updated 一月 7, 2016, 1:41 p.m.)
Review request for mesos and Alexander Rukletsov.
Bugs: MESOS-4161
https://issues.apache.org/jira/browse/MESOS-4161
Repository: mesos
Description
-------
Since the reason for the test case being slow is MESOS-4111, we will add comment in the test case, and remove it when MESOS-4111 is fixed.
Diffs (updated)
-----
src/tests/slave_tests.cpp 328c853a338145256dadfc9364b9c2a0e6c356cb
Diff: https://reviews.apache.org/r/41600/diff/
Testing
-------
Thanks,
Jian Qiu
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Jian Qiu <qi...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/
-----------------------------------------------------------
(Updated 十二月 30, 2015, 2:02 a.m.)
Review request for mesos and Alexander Rukletsov.
Changes
-------
rebase
Bugs: MESOS-4161
https://issues.apache.org/jira/browse/MESOS-4161
Repository: mesos
Description
-------
Since the reason for the test case being slow is MESOS-4111, we will add comment in the test case, and remove it when MESOS-4111 is fixed.
Diffs (updated)
-----
src/tests/slave_tests.cpp 68d75176ce7cccafdf870fff48ac6a11089311ef
Diff: https://reviews.apache.org/r/41600/diff/
Testing
-------
Thanks,
Jian Qiu
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Jian Qiu <qi...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/
-----------------------------------------------------------
(Updated 十二月 30, 2015, 2 a.m.)
Review request for mesos and Alexander Rukletsov.
Bugs: MESOS-4161
https://issues.apache.org/jira/browse/MESOS-4161
Repository: mesos
Description (updated)
-------
Since the reason for the test case being slow is MESOS-4111, we will add comment in the test case, and remove it when MESOS-4111 is fixed.
Diffs (updated)
-----
docs/containerizer-internals.md 4386bbda21686b616ccf85db408899fece3680df
docs/external-containerizer.md 8a16e5a75e374fc2848e57cb108dc5b4df7e5be1
docs/home.md d929838206817a6c49cc2343b4de82fa085da682
docs/powered-by-mesos.md 3f9f2dc7e004a3c949e4e268d494637e3dee100a
src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7
support/apply-reviews.py ea5e43a2b0d88f366efae56c6290c6929f4a4d5b
Diff: https://reviews.apache.org/r/41600/diff/
Testing (updated)
-------
Thanks,
Jian Qiu
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Jian Qiu <qi...@cn.ibm.com>.
> On 十二月 28, 2015, 11:15 a.m., Alexander Rukletsov wrote:
> > src/tests/slave_tests.cpp, line 435
> > <https://reviews.apache.org/r/41600/diff/1/?file=1175197#file1175197line435>
> >
> > Mind explaining why do you think explicitly killing the executor is the right solution here? I would even propose to do it in MESOS-4161 directly for posterity.
> >
> > To clarify some approaches here for posterity (I will be glad to continue the conversation in the JIRA ticket once you post your explanation there). There are two separate techniques used in this review request: explicit process kill and `Clock::advance()`. Though the latter may indeed slightly speed up the test, it's not longer necessary since `process::reap()` has been accelerated (see MESOS-1199). I would let @Jian Qiu comment on the former approach.
Let's add a comment instead of explicitly killing the executor
- Jian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/#review111967
-----------------------------------------------------------
On 十二月 30, 2015, 2:02 a.m., Jian Qiu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> -----------------------------------------------------------
>
> (Updated 十二月 30, 2015, 2:02 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Since the reason for the test case being slow is MESOS-4111, we will add comment in the test case, and remove it when MESOS-4111 is fixed.
>
>
> Diffs
> -----
>
> src/tests/slave_tests.cpp 68d75176ce7cccafdf870fff48ac6a11089311ef
>
> Diff: https://reviews.apache.org/r/41600/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jian Qiu
>
>
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/#review111967
-----------------------------------------------------------
src/tests/slave_tests.cpp (line 435)
<https://reviews.apache.org/r/41600/#comment172262>
Mind explaining why do you think explicitly killing the executor is the right solution here? I would even propose to do it in MESOS-4161 directly for posterity.
To clarify some approaches here for posterity (I will be glad to continue the conversation in the JIRA ticket once you post your explanation there). There are two separate techniques used in this review request: explicit process kill and `Clock::advance()`. Though the latter may indeed slightly speed up the test, it's not longer necessary since `process::reap()` has been accelerated (see MESOS-1199). I would let @Jian Qiu comment on the former approach.
- Alexander Rukletsov
On Dec. 23, 2015, 6:43 a.m., Jian Qiu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> -----------------------------------------------------------
>
> (Updated Dec. 23, 2015, 6:43 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To speed up SlaveTest.CommandExecutorWithOverride, we need to explicitly kill the executor process and advance reap interval.
>
>
> Diffs
> -----
>
> src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7
>
> Diff: https://reviews.apache.org/r/41600/diff/
>
>
> Testing
> -------
>
> make check -j4 GTEST_FILTER=SlaveTest.CommandExecutorWithOverride takes 415ms
>
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from SlaveTest
> [ RUN ] SlaveTest.CommandExecutorWithOverride
> [ OK ] SlaveTest.CommandExecutorWithOverride (405 ms)
> [----------] 1 test from SlaveTest (405 ms total)
>
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (415 ms total)
> [ PASSED ] 1 test.
>
>
> Thanks,
>
> Jian Qiu
>
>
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/#review111743
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41600]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 23, 2015, 6:43 a.m., Jian Qiu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41600/
> -----------------------------------------------------------
>
> (Updated Dec. 23, 2015, 6:43 a.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-4161
> https://issues.apache.org/jira/browse/MESOS-4161
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To speed up SlaveTest.CommandExecutorWithOverride, we need to explicitly kill the executor process and advance reap interval.
>
>
> Diffs
> -----
>
> src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7
>
> Diff: https://reviews.apache.org/r/41600/diff/
>
>
> Testing
> -------
>
> make check -j4 GTEST_FILTER=SlaveTest.CommandExecutorWithOverride takes 415ms
>
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from SlaveTest
> [ RUN ] SlaveTest.CommandExecutorWithOverride
> [ OK ] SlaveTest.CommandExecutorWithOverride (405 ms)
> [----------] 1 test from SlaveTest (405 ms total)
>
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (415 ms total)
> [ PASSED ] 1 test.
>
>
> Thanks,
>
> Jian Qiu
>
>
Re: Review Request 41600: Speed up
SlaveTest.CommandExecutorWithOverride
Posted by Jian Qiu <qi...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41600/
-----------------------------------------------------------
(Updated 十二月 23, 2015, 6:43 a.m.)
Review request for mesos and Alexander Rukletsov.
Bugs: MESOS-4161
https://issues.apache.org/jira/browse/MESOS-4161
Repository: mesos
Description
-------
To speed up SlaveTest.CommandExecutorWithOverride, we need to explicitly kill the executor process and advance reap interval.
Diffs
-----
src/tests/slave_tests.cpp 90d56b987c60b99d9ca3e4ffef9cb71815bfc9b7
Diff: https://reviews.apache.org/r/41600/diff/
Testing
-------
make check -j4 GTEST_FILTER=SlaveTest.CommandExecutorWithOverride takes 415ms
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from SlaveTest
[ RUN ] SlaveTest.CommandExecutorWithOverride
[ OK ] SlaveTest.CommandExecutorWithOverride (405 ms)
[----------] 1 test from SlaveTest (405 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (415 ms total)
[ PASSED ] 1 test.
Thanks,
Jian Qiu