You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/10/02 20:22:15 UTC
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/
-----------------------------------------------------------
(Updated Oct. 2, 2014, 11:22 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Changes
-------
Changed to chained futures, prepare with fail early on any, destroy will attempt all.
Repository: mesos-git
Description
-------
Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
Diff: https://reviews.apache.org/r/25861/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
Posted by Ian Downes <ia...@gmail.com>.
> On Oct. 7, 2014, 11:54 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 991-1002
> > <https://reviews.apache.org/r/25861/diff/2/?file=711202#file711202line991>
> >
> > Can you use f.onAny here so that you don't need to change the signature of `___destroy`?
Appears to be necessary for the compiler to correctly infer the types.
> On Oct. 7, 2014, 11:54 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 969
> > <https://reviews.apache.org/r/25861/diff/2/?file=711202#file711202line969>
> >
> > What's the purpose of that?
Appears to be necessary for the compiler to correctly infer the types.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/#review55674
-----------------------------------------------------------
On Oct. 14, 2014, 1:34 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25861/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2014, 1:34 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
>
> Diff: https://reviews.apache.org/r/25861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/#review55674
-----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment96039>
```
static list<Option<CommandInfo>> accumulate(
```
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment96038>
```
static Future<list<Option<CommandInfo>>> _prepare(
```
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment96041>
```
const list<Option<CommandInfo>> commands
````
Since `lambda::bind` will do the copy anyway.
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment96047>
static
s/_return/_cleanup/
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment96046>
static
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment96049>
What's the purpose of that?
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment96045>
See comments in the other patch.
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment96044>
Can you use f.onAny here so that you don't need to change the signature of `___destroy`?
- Jie Yu
On Oct. 3, 2014, 5:05 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25861/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2014, 5:05 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
>
> Diff: https://reviews.apache.org/r/25861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
Posted by Ian Downes <ia...@gmail.com>.
> On Oct. 14, 2014, 1:51 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 1022
> > <https://reviews.apache.org/r/25861/diff/3/?file=721061#file721061line1022>
> >
> > Not sure if I'm missing something, but if you chain all lambdas with .then, only the last one will be the failed one right?
The chaining is on the outer Future<list<Future>> which accumulates the individual Futures when they are [ready|failed|discarded], not the futures themselves. I want cleanup to be called on each isolator, regardless of the success of preceeding isolators. The list can contain any ordering of futures in any !pending state.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/#review56569
-----------------------------------------------------------
On Oct. 14, 2014, 1:34 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25861/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2014, 1:34 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
>
> Diff: https://reviews.apache.org/r/25861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/#review56569
-----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment96917>
Not sure if I'm missing something, but if you chain all lambdas with .then, only the last one will be the failed one right?
- Timothy Chen
On Oct. 14, 2014, 8:34 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25861/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2014, 8:34 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
>
> Diff: https://reviews.apache.org/r/25861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 10:39 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos-git
Description
-------
Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36
Diff: https://reviews.apache.org/r/25861/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/#review57407
-----------------------------------------------------------
Ship it!
LGTM. Please add a comment about why ```isolate``` calls are done in parallel.
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment98103>
Move this right above ```__destroy```.
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment98106>
Since it's not possible, could you just use a CHECK_READY here?
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25861/#comment98107>
No need?
- Jie Yu
On Oct. 20, 2014, 6:49 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25861/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 6:49 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
>
> Diff: https://reviews.apache.org/r/25861/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/
-----------------------------------------------------------
(Updated Oct. 20, 2014, 11:49 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos-git
Description
-------
Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
Diff: https://reviews.apache.org/r/25861/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/
-----------------------------------------------------------
(Updated Oct. 14, 2014, 1:34 p.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Changes
-------
Addressed comments.
Repository: mesos-git
Description
-------
Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
Diff: https://reviews.apache.org/r/25861/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 25861: Serialize isolator prepare and cleanup
(reversed).
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/
-----------------------------------------------------------
(Updated Oct. 3, 2014, 10:05 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos-git
Description
-------
Change from doing in parallel and collect()ing to serial according to the vector of isolators (reversed order for cleanup).
Diffs
-----
src/slave/containerizer/mesos/containerizer.hpp bf246ca649ca4a461cebf1aee6908a2d58eec362
src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
Diff: https://reviews.apache.org/r/25861/diff/
Testing
-------
make check
Thanks,
Ian Downes