You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2014/05/07 02:29:40 UTC
Review Request 21141: Allowed waiting for a link to be removed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21141/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
Bugs: MESOS-1228
https://issues.apache.org/jira/browse/MESOS-1228
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/linux/routing/link/link.hpp PRE-CREATION
src/linux/routing/link/link.cpp PRE-CREATION
src/tests/routing_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/21141/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 21141: Allowed waiting for a link to be removed.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21141/#review42380
-----------------------------------------------------------
Patch looks great!
Reviews applied: [20292, 20781, 20295, 20296, 20297, 20347, 21141]
All tests passed.
- Mesos ReviewBot
On May 7, 2014, 12:29 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21141/
> -----------------------------------------------------------
>
> (Updated May 7, 2014, 12:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Bugs: MESOS-1228
> https://issues.apache.org/jira/browse/MESOS-1228
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/linux/routing/link/link.hpp PRE-CREATION
> src/linux/routing/link/link.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21141/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 21141: Allowed waiting for a link to be removed.
Posted by Jie Yu <yu...@gmail.com>.
> On May 13, 2014, 1:31 a.m., Vinod Kone wrote:
> > src/linux/routing/link/link.hpp, lines 63-66
> > <https://reviews.apache.org/r/21141/diff/1/?file=575834#file575834line63>
> >
> > Why not have remove() return a Future that gets satisfied when the link is removed. Are there other cases where 'wait' will be used?
> >
> > Also, if we do we need wait, I would recommend
> > s/wait/removed/ to be more explicit.
Yeah, in the network isolator, the link will be removed automatically by the kernel. We need to wait for it to be removed explicitly.
Changed it to 'removed'.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21141/#review42802
-----------------------------------------------------------
On May 7, 2014, 12:29 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21141/
> -----------------------------------------------------------
>
> (Updated May 7, 2014, 12:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Bugs: MESOS-1228
> https://issues.apache.org/jira/browse/MESOS-1228
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/linux/routing/link/link.hpp PRE-CREATION
> src/linux/routing/link/link.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21141/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 21141: Allowed waiting for a link to be removed.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21141/#review42802
-----------------------------------------------------------
src/linux/routing/link/link.hpp
<https://reviews.apache.org/r/21141/#comment76725>
Why not have remove() return a Future that gets satisfied when the link is removed. Are there other cases where 'wait' will be used?
Also, if we do we need wait, I would recommend
s/wait/removed/ to be more explicit.
- Vinod Kone
On May 7, 2014, 12:29 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21141/
> -----------------------------------------------------------
>
> (Updated May 7, 2014, 12:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Bugs: MESOS-1228
> https://issues.apache.org/jira/browse/MESOS-1228
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/linux/routing/link/link.hpp PRE-CREATION
> src/linux/routing/link/link.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21141/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 21141: Allowed waiting for a link to be removed.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21141/#review43074
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On May 15, 2014, 12:47 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21141/
> -----------------------------------------------------------
>
> (Updated May 15, 2014, 12:47 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Bugs: MESOS-1228
> https://issues.apache.org/jira/browse/MESOS-1228
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/linux/routing/link/link.hpp PRE-CREATION
> src/linux/routing/link/link.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21141/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 21141: Allowed waiting for a link to be removed.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21141/
-----------------------------------------------------------
(Updated May 15, 2014, 12:47 a.m.)
Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
Changes
-------
Review comments. Rebased.
Bugs: MESOS-1228
https://issues.apache.org/jira/browse/MESOS-1228
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/linux/routing/link/link.hpp PRE-CREATION
src/linux/routing/link/link.cpp PRE-CREATION
src/tests/routing_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/21141/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 21141: Allowed waiting for a link to be removed.
Posted by Jie Yu <yu...@gmail.com>.
> On May 8, 2014, 4:22 p.m., Ian Downes wrote:
> > src/linux/routing/link/link.cpp, line 140
> > <https://reviews.apache.org/r/21141/diff/1/?file=575835#file575835line140>
> >
> > It's checking the negative, i.e., that a link does *not* exist. Can you modify the comment and perhaps class to make this clearer?
>
> Jie Yu wrote:
> Modified the comments.
In fact, I removed the timeout as well per our discussion:)
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21141/#review42510
-----------------------------------------------------------
On May 7, 2014, 12:29 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21141/
> -----------------------------------------------------------
>
> (Updated May 7, 2014, 12:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Bugs: MESOS-1228
> https://issues.apache.org/jira/browse/MESOS-1228
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/linux/routing/link/link.hpp PRE-CREATION
> src/linux/routing/link/link.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21141/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 21141: Allowed waiting for a link to be removed.
Posted by Jie Yu <yu...@gmail.com>.
> On May 8, 2014, 4:22 p.m., Ian Downes wrote:
> > src/linux/routing/link/link.cpp, line 140
> > <https://reviews.apache.org/r/21141/diff/1/?file=575835#file575835line140>
> >
> > It's checking the negative, i.e., that a link does *not* exist. Can you modify the comment and perhaps class to make this clearer?
Modified the comments.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21141/#review42510
-----------------------------------------------------------
On May 7, 2014, 12:29 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21141/
> -----------------------------------------------------------
>
> (Updated May 7, 2014, 12:29 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Bugs: MESOS-1228
> https://issues.apache.org/jira/browse/MESOS-1228
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/linux/routing/link/link.hpp PRE-CREATION
> src/linux/routing/link/link.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21141/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 21141: Allowed waiting for a link to be removed.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21141/#review42510
-----------------------------------------------------------
Ship it!
src/linux/routing/link/link.cpp
<https://reviews.apache.org/r/21141/#comment76304>
It's checking the negative, i.e., that a link does *not* exist. Can you modify the comment and perhaps class to make this clearer?
- Ian Downes
On May 6, 2014, 5:29 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21141/
> -----------------------------------------------------------
>
> (Updated May 6, 2014, 5:29 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Chi Zhang, Ian Downes, Vinod Kone, and Cong Wang.
>
>
> Bugs: MESOS-1228
> https://issues.apache.org/jira/browse/MESOS-1228
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/linux/routing/link/link.hpp PRE-CREATION
> src/linux/routing/link/link.cpp PRE-CREATION
> src/tests/routing_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/21141/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>