You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Paul Brett <pa...@twopensource.com> on 2015/03/31 01:28:24 UTC
Review Request 32653: Replace busy look on ready file with a more
relaxed loop
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/
-----------------------------------------------------------
Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
Bugs: mesos-2332
https://issues.apache.org/jira/browse/mesos-2332
Repository: mesos
Description
-------
Replace busy look on ready file with a more relaxed loop
Diffs
-----
src/tests/port_mapping_tests.cpp 8192deac8d9b7ea1896bb62a8b5961ef90326fa4
Diff: https://reviews.apache.org/r/32653/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/#review78287
-----------------------------------------------------------
Patch looks great!
Reviews applied: [32653]
All tests passed.
- Mesos ReviewBot
On March 30, 2015, 11:28 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32653/
> -----------------------------------------------------------
>
> (Updated March 30, 2015, 11:28 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: mesos-2332
> https://issues.apache.org/jira/browse/mesos-2332
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace busy look on ready file with a more relaxed loop
>
>
> Diffs
> -----
>
> src/tests/port_mapping_tests.cpp 8192deac8d9b7ea1896bb62a8b5961ef90326fa4
>
> Diff: https://reviews.apache.org/r/32653/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Paul Brett <pa...@twopensource.com>.
> On March 31, 2015, 10:42 p.m., Chi Zhang wrote:
> > src/tests/port_mapping_tests.cpp, line 396
> > <https://reviews.apache.org/r/32653/diff/2/?file=910398#file910398line396>
> >
> > how about at least have this function return a future and change call sites to use AWAIT_READY?
> >
> > When this goes into the library, the tests don't have to be changed anymore.
> >
> > More useful, AWAIT_READY supports timeout and has a default timeout time of 10s so you can save the timeout logic here.
> >
> > It'd be great see this in the library code!
Good idea but not required for this change. I will make this a TODO so that we can quickly fix the current issue where a failure can cause the test to hang forever.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/#review78421
-----------------------------------------------------------
On April 1, 2015, 3:43 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32653/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 3:43 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: mesos-2332
> https://issues.apache.org/jira/browse/mesos-2332
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace busy look on ready file with a more relaxed loop
>
>
> Diffs
> -----
>
> src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
>
> Diff: https://reviews.apache.org/r/32653/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Chi Zhang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/#review78421
-----------------------------------------------------------
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/32653/#comment127084>
how about at least have this function return a future and change call sites to use AWAIT_READY?
When this goes into the library, the tests don't have to be changed anymore.
More useful, AWAIT_READY supports timeout and has a default timeout time of 10s so you can save the timeout logic here.
It'd be great see this in the library code!
- Chi Zhang
On March 31, 2015, 3:33 a.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32653/
> -----------------------------------------------------------
>
> (Updated March 31, 2015, 3:33 a.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: mesos-2332
> https://issues.apache.org/jira/browse/mesos-2332
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace busy look on ready file with a more relaxed loop
>
>
> Diffs
> -----
>
> src/tests/port_mapping_tests.cpp f4124c3
>
> Diff: https://reviews.apache.org/r/32653/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 32653: Replace busy loop on ready file with a more
relaxed loop
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/#review78637
-----------------------------------------------------------
Patch looks great!
Reviews applied: [32653]
All tests passed.
- Mesos ReviewBot
On April 1, 2015, 11 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32653/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 11 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: mesos-2332
> https://issues.apache.org/jira/browse/mesos-2332
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace busy loop on ready file with a more relaxed loop
>
>
> Diffs
> -----
>
> src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
>
> Diff: https://reviews.apache.org/r/32653/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 32653: Replace busy loop on ready file with a more
relaxed loop
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/#review78666
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On April 1, 2015, 11 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32653/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 11 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: mesos-2332
> https://issues.apache.org/jira/browse/mesos-2332
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace busy loop on ready file with a more relaxed loop
>
>
> Diffs
> -----
>
> src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
>
> Diff: https://reviews.apache.org/r/32653/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 32653: Replace busy loop on ready file with a more
relaxed loop
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/
-----------------------------------------------------------
(Updated April 1, 2015, 11 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
Summary (updated)
-----------------
Replace busy loop on ready file with a more relaxed loop
Bugs: mesos-2332
https://issues.apache.org/jira/browse/mesos-2332
Repository: mesos
Description
-------
Replace busy loop on ready file with a more relaxed loop
Diffs
-----
src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
Diff: https://reviews.apache.org/r/32653/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/
-----------------------------------------------------------
(Updated April 1, 2015, 11 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
Bugs: mesos-2332
https://issues.apache.org/jira/browse/mesos-2332
Repository: mesos
Description (updated)
-------
Replace busy loop on ready file with a more relaxed loop
Diffs (updated)
-----
src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
Diff: https://reviews.apache.org/r/32653/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Paul Brett <pa...@twopensource.com>.
> On April 1, 2015, 10:22 p.m., Jie Yu wrote:
> > src/tests/port_mapping_tests.cpp, line 403
> > <https://reviews.apache.org/r/32653/diff/3/?file=911841#file911841line403>
> >
> > 60seconds might be too long. Probably change it to 15 seconds so that it's consistent with AWAIT_READY default.
60 seconds is appropriate for some of these networking tests since when we have high packet losses, the tests can take some time to complete (I have often seen times in the 20-30 second range).
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/#review78605
-----------------------------------------------------------
On April 1, 2015, 8:54 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32653/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 8:54 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: mesos-2332
> https://issues.apache.org/jira/browse/mesos-2332
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace busy look on ready file with a more relaxed loop
>
>
> Diffs
> -----
>
> src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
>
> Diff: https://reviews.apache.org/r/32653/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 32653: Replace busy loop on ready file with a more
relaxed loop
Posted by Jie Yu <yu...@gmail.com>.
> On April 1, 2015, 10:22 p.m., Jie Yu wrote:
> > src/tests/port_mapping_tests.cpp, lines 405-406
> > <https://reviews.apache.org/r/32653/diff/3/?file=911841#file911841line405>
> >
> > Please include <stout/stopwatch.hpp>
Has this been resolved?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/#review78605
-----------------------------------------------------------
On April 1, 2015, 11 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32653/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 11 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: mesos-2332
> https://issues.apache.org/jira/browse/mesos-2332
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace busy loop on ready file with a more relaxed loop
>
>
> Diffs
> -----
>
> src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
>
> Diff: https://reviews.apache.org/r/32653/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/#review78605
-----------------------------------------------------------
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/32653/#comment127506>
Kill this extra line.
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/32653/#comment127519>
TODO(pbrett): Consider generalizing this function and moving it to a common header.
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/32653/#comment127505>
static bool waitForFileCreation
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/32653/#comment127511>
s/timeout/duration/
since we have process::Timeout (Clock aware), it's better to avoid confusion here.
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/32653/#comment127507>
60seconds might be too long. Probably change it to 15 seconds so that it's consistent with AWAIT_READY default.
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/32653/#comment127520>
Please include <stout/stopwatch.hpp>
src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/32653/#comment127512>
s/Seconds(0)/Duration::zero()/
- Jie Yu
On April 1, 2015, 8:54 p.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32653/
> -----------------------------------------------------------
>
> (Updated April 1, 2015, 8:54 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: mesos-2332
> https://issues.apache.org/jira/browse/mesos-2332
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace busy look on ready file with a more relaxed loop
>
>
> Diffs
> -----
>
> src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
>
> Diff: https://reviews.apache.org/r/32653/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/
-----------------------------------------------------------
(Updated April 1, 2015, 8:54 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
Bugs: mesos-2332
https://issues.apache.org/jira/browse/mesos-2332
Repository: mesos
Description
-------
Replace busy look on ready file with a more relaxed loop
Diffs
-----
src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
Diff: https://reviews.apache.org/r/32653/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/
-----------------------------------------------------------
(Updated April 1, 2015, 3:43 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
Bugs: mesos-2332
https://issues.apache.org/jira/browse/mesos-2332
Repository: mesos
Description
-------
Replace busy look on ready file with a more relaxed loop
Diffs
-----
src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
Diff: https://reviews.apache.org/r/32653/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/
-----------------------------------------------------------
(Updated April 1, 2015, 12:40 a.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
Bugs: mesos-2332
https://issues.apache.org/jira/browse/mesos-2332
Repository: mesos
Description
-------
Replace busy look on ready file with a more relaxed loop
Diffs (updated)
-----
src/tests/port_mapping_tests.cpp f4124c3e880e043729579a829e1057727741d131
Diff: https://reviews.apache.org/r/32653/diff/
Testing
-------
make check
Thanks,
Paul Brett
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/#review78329
-----------------------------------------------------------
Patch looks great!
Reviews applied: [32653]
All tests passed.
- Mesos ReviewBot
On March 31, 2015, 3:33 a.m., Paul Brett wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32653/
> -----------------------------------------------------------
>
> (Updated March 31, 2015, 3:33 a.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
>
>
> Bugs: mesos-2332
> https://issues.apache.org/jira/browse/mesos-2332
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Replace busy look on ready file with a more relaxed loop
>
>
> Diffs
> -----
>
> src/tests/port_mapping_tests.cpp f4124c3
>
> Diff: https://reviews.apache.org/r/32653/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Paul Brett
>
>
Re: Review Request 32653: Replace busy look on ready file with a more
relaxed loop
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32653/
-----------------------------------------------------------
(Updated March 31, 2015, 3:33 a.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Cong Wang.
Changes
-------
Fix misapplied patch
Bugs: mesos-2332
https://issues.apache.org/jira/browse/mesos-2332
Repository: mesos
Description
-------
Replace busy look on ready file with a more relaxed loop
Diffs (updated)
-----
src/tests/port_mapping_tests.cpp f4124c3
Diff: https://reviews.apache.org/r/32653/diff/
Testing
-------
make check
Thanks,
Paul Brett