Re: Auto usage in mesos

[Benjamin Hindman]

+1 to growing the list of use cases for 'auto' organically.

I agree with Cody that using 'auto' for return type deduction should also
be included, especially since we're already using it in libprocess.

On Mon, Sep 22, 2014 at 10:50 AM, Cody Maloney  wrote:

> There are some cases where using auto for function return type deduction is
> necessary to be able to state what you want generically in templates, and I
> think we should allow that use case. I can ping some friends for good
> examples if people would like. Definitely general use of return type
> deduction shouldn't happen, but inside templates a lot of the time the code
> is a lot simpler, more readable, and easier to maintain when using auto
> (Esp. c++14 return type deduction auto), than if a type has to be
> hand-deduced.
>
> Also note at times we just can't say what the type is, because it varies in
> the template instantiation. See my use in
> https://reviews.apache.org/r/25525/diff/,  slaveinfo_utils.cpp where
> writing explicit types for memberFunc is really hard if not impossible.
>
> On Mon, Sep 22, 2014 at 9:57 AM, Dominic Hamon 
> wrote:
>
> > For reference:
> > http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#auto
> >
> > We should be able to adopt that wholesale but please document anywhere
> you
> > think we would diverge from those examples.
> >
> > On Mon, Sep 22, 2014 at 7:28 AM, Alex Rukletsov 
> > wrote:
> >
> > > We now start using auto in the code (among several other C++11
> features).
> > > However we don't want to hamper reasoning about types. I would suggest
> we
> > > select several use cases where we all agree using auto is welcomed and
> > > several counterexamples. After the short conversation with BenH, we
> > agreed
> > > that iterators on one side and Try<>, Option<> on the other side are
> > first
> > > candidates for such list. I put the examples here
> > > .
> > >
> > > Any thoughts and ideas?
> > >
> >
> >
> >
> > --
> > Dominic Hamon | @mrdo | Twitter
> > *There are no bad ideas; only good ideas that go horribly wrong.*
> >
>

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

[Benjamin Hindman]

> On Sept. 22, 2014, 7:19 p.m., Ben Mahler wrote:
> > docs/mesos-c++-style-guide.md, lines 96-99
> > 
> >
> > Why would the iterator be called `containerizer`?
> > 
> > s/containerizer/iterator/ ?
> 
> Dominic Hamon wrote:
> -1
> 
> naming a variable after a type is never a good idea. in this case, you're 
> getting a containerizer (iterator) from the container of containerizers so 
> the name 'containerizer' makes sense.
> 
> Ben Mahler wrote:
> Sounds confusing.
> 
> Ben Mahler wrote:
> If 'auto' was not used here, would we call this 'containerizer'? In a 
> loop, this would typically be called `iterator`, no?
> 
> ```
> for (auto iterator = containerizers.begin(); iterator != 
> containerizers.end(); ++iterator) {
>   Containerizer* containerizer = *iterator;
> }
> ```
> 
> Why do something differently when auto is used?
> 
> If the iterator was being "de-referenced" then `containerizer` makes 
> sense:
> 
> ```
>   Containerizer* containerizer = *(containerizers.begin());
> ```
> 
> Alexander Rukletsov wrote:
> I agree with Dominic: it's more important what is stored in the container 
> and not how we access it (iterator, reference, etc.). Actually, the example 
> is taken from our code base, see `src/slave/containerizer/composing.cpp:394`
> 
> Ben Mahler wrote:
> Ok, since this example uses `containerizer` as a reference to the 
> `Containerizer*`, as opposed to an iterator, your points make sense. But in 
> general I don't think this is a pattern we'll want in our code because of the 
> masquerading types now hidden with 'auto'.
> 
> Iterators are not as simple as a pointer or reference. What I find 
> unfortunate is that we wouldn't apply the same naming scheme as soon as we 
> change the container type, which affects the iterator type:
> 
> ```
> map containerizers;
> auto containerizer = containerizers.begin(); // Wouldn't do this.
> ```
> 
> How about a different example here with .find() as opposed to .begin()? 
> Take a look at cache.hpp as an example:
> 
> ```
>   // Evict the least-recently used element from the cache.
>   void evict()
>   {
> const typename map::iterator& i = values.find(keys.front());
> CHECK(i != values.end());
> values.erase(i);
> keys.pop_front();
>   }
> ```
> 
> Here we definitely care about the iterator, as opposed to the value, and 
> would name 'i' accordingly.
> 
> Alexander Rukletsov wrote:
> I see. So you would like to be able to 1) distinguish between iterator 
> type and element type, and 2) be able to reason about the iterator type 
> somehow (possible from the variable name). I think, that makes sense.
> 
> Using `auto` hides the actual type, and this can be both good and bad. We 
> should use it in places where we don't care (or shouldn't care) about the 
> actual type. Do we care about the container and iterator types when 
> enumerating all elements? I tend to say no, but you are absolutely right that 
> iterators of different containers are used in different ways.
> 
> Regarding naming, we can choose something general, like "element" or 
> "item". It will be clear from the context, what this element is about and 
> implies neither value nor iterator and can be used for both.

To the best of my knowledge we've (or at least I've) used two different naming 
schemes for working with iterators in the code base:

(1) When doing a 'find' we've named the variable the "thing" we're trying to 
actually use, for example:

auto containerizer = containerizers.find("docker");

(2) When doing a 'begin' we've named the variable something that makes it clear 
it's an iterator, for example:

auto it = containerizers.begin();
auto iterator = containerizers.begin();

Why? Because for (1) it's highly likely that you'll want to do things like:

containerizer->launch(...);
containerizers.erase(containerizer);

But not:

++containerizer;

Which can be harder to read, especially when initially declared using 'auto'. 
Whereas for (2) it's highly likely that we _will_ want to do:

++iterator

Which when named 'iterator' is easy to understand what you're doing.

That's my suggestion for suggesting a naming scheme for this guide.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25622/#review54173
---


On Sept. 22, 2014, 1:10 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25622/
> ---
> 
> (Updated Sept. 22, 2014, 1:10 p.m.)
> 
> 

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

[Alexander Rukletsov]

> On Sept. 22, 2014, 7:19 p.m., Ben Mahler wrote:
> > docs/mesos-c++-style-guide.md, lines 96-99
> > 
> >
> > Why would the iterator be called `containerizer`?
> > 
> > s/containerizer/iterator/ ?
> 
> Dominic Hamon wrote:
> -1
> 
> naming a variable after a type is never a good idea. in this case, you're 
> getting a containerizer (iterator) from the container of containerizers so 
> the name 'containerizer' makes sense.
> 
> Ben Mahler wrote:
> Sounds confusing.
> 
> Ben Mahler wrote:
> If 'auto' was not used here, would we call this 'containerizer'? In a 
> loop, this would typically be called `iterator`, no?
> 
> ```
> for (auto iterator = containerizers.begin(); iterator != 
> containerizers.end(); ++iterator) {
>   Containerizer* containerizer = *iterator;
> }
> ```
> 
> Why do something differently when auto is used?
> 
> If the iterator was being "de-referenced" then `containerizer` makes 
> sense:
> 
> ```
>   Containerizer* containerizer = *(containerizers.begin());
> ```
> 
> Alexander Rukletsov wrote:
> I agree with Dominic: it's more important what is stored in the container 
> and not how we access it (iterator, reference, etc.). Actually, the example 
> is taken from our code base, see `src/slave/containerizer/composing.cpp:394`
> 
> Ben Mahler wrote:
> Ok, since this example uses `containerizer` as a reference to the 
> `Containerizer*`, as opposed to an iterator, your points make sense. But in 
> general I don't think this is a pattern we'll want in our code because of the 
> masquerading types now hidden with 'auto'.
> 
> Iterators are not as simple as a pointer or reference. What I find 
> unfortunate is that we wouldn't apply the same naming scheme as soon as we 
> change the container type, which affects the iterator type:
> 
> ```
> map containerizers;
> auto containerizer = containerizers.begin(); // Wouldn't do this.
> ```
> 
> How about a different example here with .find() as opposed to .begin()? 
> Take a look at cache.hpp as an example:
> 
> ```
>   // Evict the least-recently used element from the cache.
>   void evict()
>   {
> const typename map::iterator& i = values.find(keys.front());
> CHECK(i != values.end());
> values.erase(i);
> keys.pop_front();
>   }
> ```
> 
> Here we definitely care about the iterator, as opposed to the value, and 
> would name 'i' accordingly.
> 
> Alexander Rukletsov wrote:
> I see. So you would like to be able to 1) distinguish between iterator 
> type and element type, and 2) be able to reason about the iterator type 
> somehow (possible from the variable name). I think, that makes sense.
> 
> Using `auto` hides the actual type, and this can be both good and bad. We 
> should use it in places where we don't care (or shouldn't care) about the 
> actual type. Do we care about the container and iterator types when 
> enumerating all elements? I tend to say no, but you are absolutely right that 
> iterators of different containers are used in different ways.
> 
> Regarding naming, we can choose something general, like "element" or 
> "item". It will be clear from the context, what this element is about and 
> implies neither value nor iterator and can be used for both.
> 
> Benjamin Hindman wrote:
> To the best of my knowledge we've (or at least I've) used two different 
> naming schemes for working with iterators in the code base:
> 
> (1) When doing a 'find' we've named the variable the "thing" we're trying 
> to actually use, for example:
> 
> auto containerizer = containerizers.find("docker");
> 
> (2) When doing a 'begin' we've named the variable something that makes it 
> clear it's an iterator, for example:
> 
> auto it = containerizers.begin();
> auto iterator = containerizers.begin();
> 
> Why? Because for (1) it's highly likely that you'll want to do things 
> like:
> 
> containerizer->launch(...);
> containerizers.erase(containerizer);
> 
> But not:
> 
> ++containerizer;
> 
> Which can be harder to read, especially when initially declared using 
> 'auto'. Whereas for (2) it's highly likely that we _will_ want to do:
> 
> ++iterator
> 
> Which when named 'iterator' is easy to understand what you're doing.
> 
> That's my suggestion for suggesting a naming scheme for this guide.

Makes sense, especially `++containerizer`. To make the example less confusing, 
I change it to `.find()`.


- Alexander


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25622/#review54173
---


On Sept. 22, 2014, 1:

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

[Alexander Rukletsov]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25622/
---

(Updated Sept. 25, 2014, 11:47 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Till 
Toenshoff.


Changes
---

Update auto example.


Bugs: MESOS-1793
https://issues.apache.org/jira/browse/MESOS-1793


Repository: mesos-git


Description
---

Explicitly prohibit the use of namespace aliases. The discussion about using 
namespace aliases took place in [the other 
review](https://reviews.apache.org/r/25434/#comment91754). The majority agreed 
not to introduce them in code.

Give examples of preferable way of using auto.


Diffs (updated)
-

  docs/mesos-c++-style-guide.md 59a39df 

Diff: https://reviews.apache.org/r/25622/diff/


Testing
---

Documentation change, no `make check` needed.


Thanks,

Alexander Rukletsov

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25622/#review54532
---


Patch looks great!

Reviews applied: [25622]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2014, 11:47 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25622/
> ---
> 
> (Updated Sept. 25, 2014, 11:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-1793
> https://issues.apache.org/jira/browse/MESOS-1793
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Explicitly prohibit the use of namespace aliases. The discussion about using 
> namespace aliases took place in [the other 
> review](https://reviews.apache.org/r/25434/#comment91754). The majority 
> agreed not to introduce them in code.
> 
> Give examples of preferable way of using auto.
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md 59a39df 
> 
> Diff: https://reviews.apache.org/r/25622/diff/
> 
> 
> Testing
> ---
> 
> Documentation change, no `make check` needed.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 25789: Variadic strings join

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25789/
---

(Updated Sept. 25, 2014, 5:10 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

fix style issues.


Repository: mesos-git


Description
---

Add Variadic strings join.
There is a second version of the variadic join which takes a reference to a 
stringstream as a parameter. This is handy when strings::join is just a part of 
a larger string manipulation.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 

Diff: https://reviews.apache.org/r/25789/diff/


Testing
---

Ran make check for stout. Added test cases for join as these were missing.


Thanks,

Joris Van Remoortere

Re: Review Request 25789: Variadic strings join

[Benjamin Hindman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25789/#review54549
---

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp


Very useful comment, thanks Joris!



3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp


Silly style thing, please move 'stream' to newline.



3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp


Just a style thing, we tend to declare/define as close to where they're 
used as possible (i.e., not C style).



3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp


Silly style thing, but in this case both arguments should be indented the 
same.


- Benjamin Hindman


On Sept. 25, 2014, 5:10 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25789/
> ---
> 
> (Updated Sept. 25, 2014, 5:10 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add Variadic strings join.
> There is a second version of the variadic join which takes a reference to a 
> stringstream as a parameter. This is handy when strings::join is just a part 
> of a larger string manipulation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 
>   3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 
> 
> Diff: https://reviews.apache.org/r/25789/diff/
> 
> 
> Testing
> ---
> 
> Ran make check for stout. Added test cases for join as these were missing.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25864: Add 'Future cgroups::empty()'.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25864/#review54553
---

Ship it!



src/linux/cgroups.cpp


Instead of CHECK, can you just fail the promise since interval is specified 
by the user.



src/linux/cgroups.cpp


Instead of CHECK, can you just fail the promise since interval is specified 
by the user.



src/linux/cgroups.cpp


4 space indent please.



src/linux/cgroups.cpp


I am a little against reusing the timedout function as it requires jumping 
in the source code while reading it. But it's up to you.


- Jie Yu


On Sept. 23, 2014, 11:39 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25864/
> ---
> 
> (Updated Sept. 23, 2014, 11:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Polls cgroups.procs until no processes in the cgroup. Poll interval and 
> timeout can be specified.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp abf31df1b4dbf6f715f93256b83c9996a45099cf 
>   src/linux/cgroups.cpp 62df4b7645c6ab061a47634058d79ca849caa6b9 
> 
> Diff: https://reviews.apache.org/r/25864/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

[Ben Mahler]

> On Sept. 22, 2014, 7:19 p.m., Ben Mahler wrote:
> > docs/mesos-c++-style-guide.md, lines 96-99
> > 
> >
> > Why would the iterator be called `containerizer`?
> > 
> > s/containerizer/iterator/ ?
> 
> Dominic Hamon wrote:
> -1
> 
> naming a variable after a type is never a good idea. in this case, you're 
> getting a containerizer (iterator) from the container of containerizers so 
> the name 'containerizer' makes sense.
> 
> Ben Mahler wrote:
> Sounds confusing.
> 
> Ben Mahler wrote:
> If 'auto' was not used here, would we call this 'containerizer'? In a 
> loop, this would typically be called `iterator`, no?
> 
> ```
> for (auto iterator = containerizers.begin(); iterator != 
> containerizers.end(); ++iterator) {
>   Containerizer* containerizer = *iterator;
> }
> ```
> 
> Why do something differently when auto is used?
> 
> If the iterator was being "de-referenced" then `containerizer` makes 
> sense:
> 
> ```
>   Containerizer* containerizer = *(containerizers.begin());
> ```
> 
> Alexander Rukletsov wrote:
> I agree with Dominic: it's more important what is stored in the container 
> and not how we access it (iterator, reference, etc.). Actually, the example 
> is taken from our code base, see `src/slave/containerizer/composing.cpp:394`
> 
> Ben Mahler wrote:
> Ok, since this example uses `containerizer` as a reference to the 
> `Containerizer*`, as opposed to an iterator, your points make sense. But in 
> general I don't think this is a pattern we'll want in our code because of the 
> masquerading types now hidden with 'auto'.
> 
> Iterators are not as simple as a pointer or reference. What I find 
> unfortunate is that we wouldn't apply the same naming scheme as soon as we 
> change the container type, which affects the iterator type:
> 
> ```
> map containerizers;
> auto containerizer = containerizers.begin(); // Wouldn't do this.
> ```
> 
> How about a different example here with .find() as opposed to .begin()? 
> Take a look at cache.hpp as an example:
> 
> ```
>   // Evict the least-recently used element from the cache.
>   void evict()
>   {
> const typename map::iterator& i = values.find(keys.front());
> CHECK(i != values.end());
> values.erase(i);
> keys.pop_front();
>   }
> ```
> 
> Here we definitely care about the iterator, as opposed to the value, and 
> would name 'i' accordingly.
> 
> Alexander Rukletsov wrote:
> I see. So you would like to be able to 1) distinguish between iterator 
> type and element type, and 2) be able to reason about the iterator type 
> somehow (possible from the variable name). I think, that makes sense.
> 
> Using `auto` hides the actual type, and this can be both good and bad. We 
> should use it in places where we don't care (or shouldn't care) about the 
> actual type. Do we care about the container and iterator types when 
> enumerating all elements? I tend to say no, but you are absolutely right that 
> iterators of different containers are used in different ways.
> 
> Regarding naming, we can choose something general, like "element" or 
> "item". It will be clear from the context, what this element is about and 
> implies neither value nor iterator and can be used for both.
> 
> Benjamin Hindman wrote:
> To the best of my knowledge we've (or at least I've) used two different 
> naming schemes for working with iterators in the code base:
> 
> (1) When doing a 'find' we've named the variable the "thing" we're trying 
> to actually use, for example:
> 
> auto containerizer = containerizers.find("docker");
> 
> (2) When doing a 'begin' we've named the variable something that makes it 
> clear it's an iterator, for example:
> 
> auto it = containerizers.begin();
> auto iterator = containerizers.begin();
> 
> Why? Because for (1) it's highly likely that you'll want to do things 
> like:
> 
> containerizer->launch(...);
> containerizers.erase(containerizer);
> 
> But not:
> 
> ++containerizer;
> 
> Which can be harder to read, especially when initially declared using 
> 'auto'. Whereas for (2) it's highly likely that we _will_ want to do:
> 
> ++iterator
> 
> Which when named 'iterator' is easy to understand what you're doing.
> 
> That's my suggestion for suggesting a naming scheme for this guide.
> 
> Alexander Rukletsov wrote:
> Makes sense, especially `++containerizer`. To make the example less 
> confusing, I change it to `.find()`.

Great, Alexander and I chatted off thread as well to go with an example of 
.find().

Ben: If your example (1) is calling map::find (vector::find does not exist), 
then you're actually getting an iterator to map::value_type. Th

Re: Review Request 25789: Variadic strings join

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25789/
---

(Updated Sept. 25, 2014, 6:25 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

fix style issues.


Repository: mesos-git


Description
---

Add Variadic strings join.
There is a second version of the variadic join which takes a reference to a 
stringstream as a parameter. This is handy when strings::join is just a part of 
a larger string manipulation.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 

Diff: https://reviews.apache.org/r/25789/diff/


Testing
---

Ran make check for stout. Added test cases for join as these were missing.


Thanks,

Joris Van Remoortere

Re: Review Request 25191: Switch [stout] to using compiler intrinsics for unreachable, exit, and abort

[Cody Maloney]

> On Sept. 5, 2014, 9 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp, line 21
> > 
> >
> > can we use __builtin_unreachable instead?
> > 
> > we should probably check for it (__has_builtin(__builtin_unreachable)).
> 
> Cody Maloney wrote:
> The compiler intrinsic just acts as a hint to the compiler allowing it to 
> suppress some erroneous warnings as well as better optimize the code, so it 
> doesn't really do the same thing. I think the message is useful for us for 
> debugging / dev / recovering from error. In the case of 
> `__builtin_unreachable` the behavior just becomes undefined / the compiler is 
> free to make the code do absolutely anything it wants (Including if the point 
> is reached execute arbitrary other code from the executable).
> 
> Could you commit this group of three patches (25191, 25192, 25192)?
> 
> Dominic Hamon wrote:
> I'm waiting for benjaminhindman to take a look.
> 
> Cody Maloney wrote:
> ping

This has been sitting here quite a while (I tried pining Ben H directly ~ a 
week ago). Could you commit them for me? Or would you like me to ping another 
Mesos commiter to take a quick look?


- Cody


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review52506
---


On Aug. 29, 2014, 5:53 p.m., Patrick Reilly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> ---
> 
> (Updated Aug. 29, 2014, 5:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Dominic Hamon.
> 
> 
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea 
>   3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896 
>   3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886 
> 
> Diff: https://reviews.apache.org/r/25191/diff/
> 
> 
> Testing
> ---
> 
> Make check runs.
> 
> 
> Thanks,
> 
> Patrick Reilly
> 
>

Re: Review Request 25945: Pass Promise to DispatchEvent to correctly discard on rejection.

[Ben Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25945/#review54571
---


This seems like a good approach, I'm curious if there's any room for cleanup 
now that we've split the types of Dispatch events. Can you add benh to the 
reviewers for this patch?

It would be great if you could split this patch so that the cleanups you did 
(shared_ptr -> Owned, assertions -> CHECK, etc) are in a separate patch from 
the change in semantics, thanks!


3rdparty/libprocess/include/process/dispatch.hpp


Could the left hand side store a DispatchEvent*?



3rdparty/libprocess/include/process/event.hpp


Do you need this forward declaration? Or, can we remove the future.hpp 
include?



3rdparty/libprocess/include/process/event.hpp


Do we want to add an explicit virtual destructor here per the other Events 
above? Ditto in PromiseDispatchEvent.



3rdparty/libprocess/include/process/event.hpp


Could you add a comment here that this is a DispatchEvent with a result?

s/R/T/



3rdparty/libprocess/include/process/event.hpp


Can you be more explicit with .get()? I'm looking at shared.hpp and seeing 
that we avoided the implicit bool operator.



src/tests/master_tests.cpp


Instead of writing an integration test within Mesos, what about writing 
some simple libprocess tests that demonstrate the added semantics of dispatch:

(1) Process terminated: discarded.
(2) Non-existent UPID: discarded.


- Ben Mahler


On Sept. 23, 2014, 5:24 p.m., Dominic Hamon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25945/
> ---
> 
> (Updated Sept. 23, 2014, 5:24 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1456 and MESOS-1751
> https://issues.apache.org/jira/browse/MESOS-1456
> https://issues.apache.org/jira/browse/MESOS-1751
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Create a PromiseDispatchEvent to allow discarding of promises if the 
> DispatchEvent can't be enqueued.
> 
> Also converted some shared_ptr to Owned to correctly track ownership.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/c++11/dispatch.hpp 
> 76da2828cf36b6143d627dac66f3e0cc4416bae4 
>   3rdparty/libprocess/include/process/defer.hpp 
> ebe6f2db47cab2a3306288d8ebabb720e7cd8976 
>   3rdparty/libprocess/include/process/delay.hpp 
> 487f652c9e9b7c8c3aa8b4edc9e59789cffd8da9 
>   3rdparty/libprocess/include/process/dispatch.hpp 
> 88570f7c078faa7d79b2c187aa6a15e4e939878c 
>   3rdparty/libprocess/include/process/event.hpp 
> bf689d7270df2c8f1f5c9165d2bbcfdca06e15a8 
>   3rdparty/libprocess/src/process.cpp 
> 8adc8093ab08a51c437b18dc37a7e0ae3f56870b 
>   src/tests/master_tests.cpp 8e4ec1d85c4530b5421387de55036f7d40ee3180 
> 
> Diff: https://reviews.apache.org/r/25945/diff/
> 
> 
> Testing
> ---
> 
> Added test that fails without the patch.
> 
> Ran all tests (make check) with g++-4.4 and clang-3.5.
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Re: Review Request 25191: Switch [stout] to using compiler intrinsics for unreachable, exit, and abort

[Patrick Reilly]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/
---

(Updated Sept. 25, 2014, 12:34 p.m.)


Review request for mesos, Ben Mahler and Dominic Hamon.


Changes
---

s/benjaminhindman/bmahler

Ben, could you take a look for sanity check?


Bugs: MESOS-1744
https://issues.apache.org/jira/browse/MESOS-1744


Repository: mesos-git


Description
---

Use compiler intrinsics for unreachable, exit, and abort
Makes the functions not need to pretend to return something while
still silencing the compiler warnings.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea 
  3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896 
  3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886 

Diff: https://reviews.apache.org/r/25191/diff/


Testing
---

Make check runs.


Thanks,

Patrick Reilly

Review Request 26052: Add docker container prefix to docker docs

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26052/
---

Review request for mesos and Niklas Nielsen.


Repository: mesos-git


Description
---

Add docker container prefix to docker docs


Diffs
-

  docs/docker-containerizer.md f312ca825aafa23053a994eeaae045f6d714979c 

Diff: https://reviews.apache.org/r/26052/diff/


Testing
---


Thanks,

Timothy Chen

Re: Review Request 26052: Add docker container prefix to docker docs

[Niklas Nielsen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26052/#review54584
---



docs/docker-containerizer.md


LGTM - On one line maybe?


- Niklas Nielsen


On Sept. 25, 2014, 12:36 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26052/
> ---
> 
> (Updated Sept. 25, 2014, 12:36 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add docker container prefix to docker docs
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md f312ca825aafa23053a994eeaae045f6d714979c 
> 
> Diff: https://reviews.apache.org/r/26052/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 26052: Add docker container prefix to docker docs

[Timothy Chen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26052/
---

(Updated Sept. 25, 2014, 7:37 p.m.)


Review request for mesos and Niklas Nielsen.


Repository: mesos-git


Description (updated)
---

Review: https://reviews.apache.org/r/26052


Diffs (updated)
-

  docs/docker-containerizer.md f312ca825aafa23053a994eeaae045f6d714979c 

Diff: https://reviews.apache.org/r/26052/diff/


Testing
---


Thanks,

Timothy Chen

Re: Review Request 26052: Add docker container prefix to docker docs

[Niklas Nielsen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26052/#review54585
---

Ship it!


Ship It!

- Niklas Nielsen


On Sept. 25, 2014, 12:37 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26052/
> ---
> 
> (Updated Sept. 25, 2014, 12:37 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/26052
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md f312ca825aafa23053a994eeaae045f6d714979c 
> 
> Diff: https://reviews.apache.org/r/26052/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 25868: Refactor Libprocess: class ProcessReference

[Niklas Nielsen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25868/#review54588
---

Ship it!


Ship It!

- Niklas Nielsen


On Sept. 22, 2014, 6:18 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25868/
> ---
> 
> (Updated Sept. 22, 2014, 6:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Move class ProcessReference out of process.cpp and into its own header.
> Part of refactoring process.cpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am edbe54b 
>   3rdparty/libprocess/src/process.cpp 8adc809 
>   3rdparty/libprocess/src/process_reference.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25868/diff/
> 
> 
> Testing
> ---
> 
> make check in 3rdparty/libprocess
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Jenkins build is back to normal : Mesos-Ubuntu-distcheck #356

[Apache Jenkins Server]

See 

Re: Review Request 25866: Updated semantics of disconnected/deactivated slaves/frameworks in master.

[Vinod Kone]

> On Sept. 24, 2014, 11:39 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 743-744
> > 
> >
> > Why do we check slave->connected before calling disconnect below, but 
> > we don't check framework->connected here?
> > 
> > Probably warrants a comment.

this was done as part of MESOS-675 (by you :)). Added a comment.


> On Sept. 24, 2014, 11:39 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1653-1666
> > 
> >
> > Should this be below deactivateFramework, to keep disconnect and 
> > deactivate adjacent (like in the header file)?

done.


> On Sept. 24, 2014, 11:39 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1655
> > 
> >
> > Re: question above
> > 
> > Should this be idempotent? i.e. do nothing when already disconnected.
> > 
> > Or do you want the caller to never call this when already disconnected?

yup, the latter, esp when calling methods that take pointers.


- Vinod


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25866/#review54384
---


On Sept. 22, 2014, 11:50 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25866/
> ---
> 
> (Updated Sept. 22, 2014, 11:50 p.m.)
> 
> 
> Review request for mesos, Adam B and Ben Mahler.
> 
> 
> Bugs: MESOS-1081 and MESOS-1811
> https://issues.apache.org/jira/browse/MESOS-1081
> https://issues.apache.org/jira/browse/MESOS-1811
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Made consistent what connected and active frameworks/slaves means.
> 
> Fixed MESOS-1811 along the way.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 3f5a01dfddca9cea73563100d88e0c03f600d6b1 
>   src/master/master.hpp f5d74aef185fad861139186be1cab089f8005a94 
>   src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 
>   src/tests/fault_tolerance_tests.cpp 
> 154386044d0247b39d84719d7ff14250682a0695 
>   src/tests/master_tests.cpp 8e4ec1d85c4530b5421387de55036f7d40ee3180 
> 
> Diff: https://reviews.apache.org/r/25866/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 25866: Updated semantics of disconnected/deactivated slaves/frameworks in master.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25866/
---

(Updated Sept. 25, 2014, 8:37 p.m.)


Review request for mesos, Adam B and Ben Mahler.


Changes
---

Addressed comments. NNFR.


Bugs: MESOS-1081 and MESOS-1811
https://issues.apache.org/jira/browse/MESOS-1081
https://issues.apache.org/jira/browse/MESOS-1811


Repository: mesos-git


Description
---

Made consistent what connected and active frameworks/slaves means.

Fixed MESOS-1811 along the way.


Diffs (updated)
-

  src/master/http.cpp 3f5a01dfddca9cea73563100d88e0c03f600d6b1 
  src/master/master.hpp f5d74aef185fad861139186be1cab089f8005a94 
  src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 
  src/tests/fault_tolerance_tests.cpp 154386044d0247b39d84719d7ff14250682a0695 
  src/tests/master_tests.cpp 8e4ec1d85c4530b5421387de55036f7d40ee3180 

Diff: https://reviews.apache.org/r/25866/diff/


Testing
---

make check


Thanks,

Vinod Kone

Re: Review Request 25867: Updated ping message to embed the slave registered status.

[Vinod Kone]

> On Sept. 25, 2014, 12:30 a.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1746
> > 
> >
> > You don't need the `->self()` here and below, dispatch can take a 
> > pointer to do it for you.

thanks!


> On Sept. 25, 2014, 12:30 a.m., Ben Mahler wrote:
> > src/master/master.cpp, line 179
> > 
> >
> > Generally I get a bit worried about these versioned TODO's unless we 
> > have tickets with the correct Target/Fix Version to make sure we follow up. 
> > Will you create tickets when this lands?
> > 
> > Otherwise we'll most definitely forget!

Definitely. I'll add them shortly.


> On Sept. 25, 2014, 12:30 a.m., Ben Mahler wrote:
> > src/tests/partition_tests.cpp, lines 107-109
> > 
> >
> > Couldn't you just expect the dispatch on SlaveObserver::disconnect, 
> > instead of the allocator dispatch + clock::settle?

SlaveObserver is private. Didn't want to make it public.


- Vinod


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25867/#review54480
---


On Sept. 22, 2014, 11:51 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25867/
> ---
> 
> (Updated Sept. 22, 2014, 11:51 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1668
> https://issues.apache.org/jira/browse/MESOS-1668
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Embeded slave registration status in ping message to solicit slave 
> re-registration during one way master --> slave partition.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
>   src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 
>   src/messages/messages.proto 7cb3ce651997c04ef1ef95539098ed2a99270b11 
>   src/slave/slave.hpp 4f3df5c49a8cf72fc7153158c9eb045196b6cf13 
>   src/slave/slave.cpp 28eb02852ddcc10efe589a8069dba9c895bc160e 
>   src/tests/partition_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25867/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 25867: Updated ping message to embed the slave registered status.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25867/
---

(Updated Sept. 25, 2014, 8:41 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

BenM's. NNFR. 


Bugs: MESOS-1668
https://issues.apache.org/jira/browse/MESOS-1668


Repository: mesos-git


Description
---

Embeded slave registration status in ping message to solicit slave 
re-registration during one way master --> slave partition.


Diffs (updated)
-

  src/Makefile.am b821a3bd6c3bcc3cca9bd7f72f1d7b9fb9c4ff18 
  src/master/master.cpp e5d30e9c7ba1ec0cdd640c81610790f3397f3062 
  src/messages/messages.proto 7cb3ce651997c04ef1ef95539098ed2a99270b11 
  src/slave/slave.hpp 4f3df5c49a8cf72fc7153158c9eb045196b6cf13 
  src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 
  src/tests/partition_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/25867/diff/


Testing
---

make check


Thanks,

Vinod Kone

Re: Review Request 26052: Add docker container prefix to docker docs

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26052/#review54592
---


Patch looks great!

Reviews applied: [26052]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2014, 7:37 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26052/
> ---
> 
> (Updated Sept. 25, 2014, 7:37 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/26052
> 
> 
> Diffs
> -
> 
>   docs/docker-containerizer.md f312ca825aafa23053a994eeaae045f6d714979c 
> 
> Diff: https://reviews.apache.org/r/26052/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 25191: Switch [stout] to using compiler intrinsics for unreachable, exit, and abort

[Ben Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review54593
---

Ship it!


The whole chain looks good to me, can you reach out to Dominic to get the chain 
committed for you?


3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp


Once you commit this, no one will know about the multi message version 
you're referring to ;)

I would avoid the TODO altogether and we'll extend _Abort when the need 
arises.


- Ben Mahler


On Sept. 25, 2014, 7:34 p.m., Patrick Reilly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> ---
> 
> (Updated Sept. 25, 2014, 7:34 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Dominic Hamon.
> 
> 
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea 
>   3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896 
>   3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886 
> 
> Diff: https://reviews.apache.org/r/25191/diff/
> 
> 
> Testing
> ---
> 
> Make check runs.
> 
> 
> Thanks,
> 
> Patrick Reilly
> 
>

Review Request 26056: Prevent multiple definitions of synchronized header.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26056/
---

Review request for mesos and Niklas Nielsen.


Repository: mesos-git


Description
---

Defend against multiple definitions of synchronized. Add conditional include.


Diffs
-

  3rdparty/libprocess/src/synchronized.hpp 577a5fc 

Diff: https://reviews.apache.org/r/26056/diff/


Testing
---

make check in 3rdparty/libprocess
support/mesos-style.py


Thanks,

Joris Van Remoortere

Re: Review Request 26056: Prevent multiple definitions of synchronized header.

[Niklas Nielsen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26056/#review54596
---



3rdparty/libprocess/src/synchronized.hpp


I know it is pretty inconsistent in libprocess, but let's surround the 
include guard with '__' and do a scan of the other headers too


- Niklas Nielsen


On Sept. 25, 2014, 2 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26056/
> ---
> 
> (Updated Sept. 25, 2014, 2 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Defend against multiple definitions of synchronized. Add conditional include.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/synchronized.hpp 577a5fc 
> 
> Diff: https://reviews.apache.org/r/26056/diff/
> 
> 
> Testing
> ---
> 
> make check in 3rdparty/libprocess
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25986: Added reconcileTasks to python scheduler.

[Ben Mahler]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25986/#review54597
---

Ship it!


Great, didn't realize this was all that's needed for MESOS-1461, I'll assign 
that to you.

Can you link the ticket into this review?

- Ben Mahler


On Sept. 24, 2014, 6:02 a.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25986/
> ---
> 
> (Updated Sept. 24, 2014, 6:02 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> The last step of wiring up reconcileTasks in the python bindings.
> 
> 
> Diffs
> -
> 
>   src/python/interface/src/mesos/interface/__init__.py 818f41b 
> 
> Diff: https://reviews.apache.org/r/25986/diff/
> 
> 
> Testing
> ---
> 
> Functional testing
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 25986: Added reconcileTasks to python scheduler.

[Niklas Nielsen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25986/
---

(Updated Sept. 25, 2014, 9:09 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Linked in MESOS-1461.

--bmahler


Bugs: MESOS-1461
https://issues.apache.org/jira/browse/MESOS-1461


Repository: mesos-git


Description
---

The last step of wiring up reconcileTasks in the python bindings.


Diffs
-

  src/python/interface/src/mesos/interface/__init__.py 818f41b 

Diff: https://reviews.apache.org/r/25986/diff/


Testing
---

Functional testing


Thanks,

Niklas Nielsen

Re: Review Request 25986: Added reconcileTasks to python scheduler.

[Niklas Nielsen]

> On Sept. 25, 2014, 2:08 p.m., Ben Mahler wrote:
> > Great, didn't realize this was all that's needed for MESOS-1461, I'll 
> > assign that to you.
> > 
> > Can you link the ticket into this review?

Yeah - wired everything up but the last step :-P
We should wire task reconciliation up in the test frameworks to exercise the 
api.


- Niklas


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25986/#review54597
---


On Sept. 25, 2014, 2:09 p.m., Niklas Nielsen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25986/
> ---
> 
> (Updated Sept. 25, 2014, 2:09 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1461
> https://issues.apache.org/jira/browse/MESOS-1461
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> The last step of wiring up reconcileTasks in the python bindings.
> 
> 
> Diffs
> -
> 
>   src/python/interface/src/mesos/interface/__init__.py 818f41b 
> 
> Diff: https://reviews.apache.org/r/25986/diff/
> 
> 
> Testing
> ---
> 
> Functional testing
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>

Re: Review Request 25191: Switch [stout] to using compiler intrinsics for unreachable, exit, and abort

[Dominic Hamon]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25191/#review54600
---



3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp


make check fails here for me (on clang) because this is not marked inline.

same for the other methods in this diff.


- Dominic Hamon


On Sept. 25, 2014, 12:34 p.m., Patrick Reilly wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25191/
> ---
> 
> (Updated Sept. 25, 2014, 12:34 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Dominic Hamon.
> 
> 
> Bugs: MESOS-1744
> https://issues.apache.org/jira/browse/MESOS-1744
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Use compiler intrinsics for unreachable, exit, and abort
> Makes the functions not need to pretend to return something while
> still silencing the compiler warnings.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp f20feea 
>   3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp aaccbb4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5bbf829 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 5607896 
>   3rdparty/libprocess/3rdparty/stout/include/stout/unreachable.hpp 3568886 
> 
> Diff: https://reviews.apache.org/r/25191/diff/
> 
> 
> Testing
> ---
> 
> Make check runs.
> 
> 
> Thanks,
> 
> Patrick Reilly
> 
>

Re: Review Request 25945: Pass Promise to DispatchEvent to correctly discard on rejection.

[Dominic Hamon]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25945/
---

(Updated Sept. 25, 2014, 2:42 p.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
---

added benh


Bugs: MESOS-1456 and MESOS-1751
https://issues.apache.org/jira/browse/MESOS-1456
https://issues.apache.org/jira/browse/MESOS-1751


Repository: mesos-git


Description
---

Create a PromiseDispatchEvent to allow discarding of promises if the 
DispatchEvent can't be enqueued.

Also converted some shared_ptr to Owned to correctly track ownership.


Diffs
-

  3rdparty/libprocess/include/process/c++11/dispatch.hpp 
76da2828cf36b6143d627dac66f3e0cc4416bae4 
  3rdparty/libprocess/include/process/defer.hpp 
ebe6f2db47cab2a3306288d8ebabb720e7cd8976 
  3rdparty/libprocess/include/process/delay.hpp 
487f652c9e9b7c8c3aa8b4edc9e59789cffd8da9 
  3rdparty/libprocess/include/process/dispatch.hpp 
88570f7c078faa7d79b2c187aa6a15e4e939878c 
  3rdparty/libprocess/include/process/event.hpp 
bf689d7270df2c8f1f5c9165d2bbcfdca06e15a8 
  3rdparty/libprocess/src/process.cpp 8adc8093ab08a51c437b18dc37a7e0ae3f56870b 
  src/tests/master_tests.cpp 8e4ec1d85c4530b5421387de55036f7d40ee3180 

Diff: https://reviews.apache.org/r/25945/diff/


Testing
---

Added test that fails without the patch.

Ran all tests (make check) with g++-4.4 and clang-3.5.


Thanks,

Dominic Hamon

Re: Review Request 26056: Prevent multiple definitions of synchronized header.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26056/#review54606
---


Patch looks great!

Reviews applied: [26056]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2014, 9 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26056/
> ---
> 
> (Updated Sept. 25, 2014, 9 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Defend against multiple definitions of synchronized. Add conditional include.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/synchronized.hpp 577a5fc 
> 
> Diff: https://reviews.apache.org/r/26056/diff/
> 
> 
> Testing
> ---
> 
> make check in 3rdparty/libprocess
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Review Request 26060: Refactor Libprocess: class HttpProxy

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26060/
---

Review request for mesos and Niklas Nielsen.


Repository: mesos-git


Description
---

Move http_proxy out of process.cpp, which forces the bare scaffolding for the 
event_manager abstraction dependency injection.
For now: SocketManager -> EventManager -> internal::EventManager.
In this step we only virtualized (raised) the mandatory send() functions.
internal::EventManager exists to inject only the functions that http_proxy 
depends on.


Diffs
-

  3rdparty/libprocess/Makefile.am 616618e 
  3rdparty/libprocess/src/event_manager.hpp PRE-CREATION 
  3rdparty/libprocess/src/event_manager_base.hpp PRE-CREATION 
  3rdparty/libprocess/src/http_proxy.hpp PRE-CREATION 
  3rdparty/libprocess/src/http_proxy.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp e46e6b1 

Diff: https://reviews.apache.org/r/26060/diff/


Testing
---

make check
support/mesos-style.py


Thanks,

Joris Van Remoortere

Re: Review Request 26060: Refactor Libprocess: class HttpProxy

[Dominic Hamon]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26060/#review54612
---



3rdparty/libprocess/src/event_manager.hpp


i'm missing some subtlety here about needing both the 
internal::EventManager and EventManager. If http_proxy requires 
EventManager::send then it should be in EventManager, and we don't need the 
base class.

What am I missing?



3rdparty/libprocess/src/process.cpp


override may not be supported in g++-4.4. We don't use it anywhere else.


- Dominic Hamon


On Sept. 25, 2014, 3:01 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26060/
> ---
> 
> (Updated Sept. 25, 2014, 3:01 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Move http_proxy out of process.cpp, which forces the bare scaffolding for the 
> event_manager abstraction dependency injection.
> For now: SocketManager -> EventManager -> internal::EventManager.
> In this step we only virtualized (raised) the mandatory send() functions.
> internal::EventManager exists to inject only the functions that http_proxy 
> depends on.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 616618e 
>   3rdparty/libprocess/src/event_manager.hpp PRE-CREATION 
>   3rdparty/libprocess/src/event_manager_base.hpp PRE-CREATION 
>   3rdparty/libprocess/src/http_proxy.hpp PRE-CREATION 
>   3rdparty/libprocess/src/http_proxy.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp e46e6b1 
> 
> Diff: https://reviews.apache.org/r/26060/diff/
> 
> 
> Testing
> ---
> 
> make check
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Jenkins build is back to normal : Mesos-Trunk-Ubuntu-Build-In-Src-Set-JAVA_HOME #2125

[Apache Jenkins Server]

See 

Re: Review Request 26060: Refactor Libprocess: class HttpProxy

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26060/#review54616
---


Bad patch!

Reviews applied: [26060]

Failed command: make -j3 distcheck

Error:
 make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
if test -d "mesos-0.21.0"; then find "mesos-0.21.0" -type d ! -perm -200 -exec 
chmod u+w {} ';' && rm -rf "mesos-0.21.0" || { sleep 5 && rm -rf 
"mesos-0.21.0"; }; else :; fi
test -d "mesos-0.21.0" || mkdir "mesos-0.21.0"
 (cd 3rdparty && make  top_distdir=../mesos-0.21.0 
distdir=../mesos-0.21.0/3rdparty \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
 (cd libprocess && make  top_distdir=../../mesos-0.21.0 
distdir=../../mesos-0.21.0/3rdparty/libprocess \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
:
test -d "../../mesos-0.21.0/3rdparty/libprocess" || mkdir 
"../../mesos-0.21.0/3rdparty/libprocess"
 (cd 3rdparty && make  top_distdir=../../../mesos-0.21.0 
distdir=../../../mesos-0.21.0/3rdparty/libprocess/3rdparty \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
 (cd stout && make  top_distdir=../../../../mesos-0.21.0 
distdir=../../../../mesos-0.21.0/3rdparty/libprocess/3rdparty/stout \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[5]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
:
test -d "../../../../mesos-0.21.0/3rdparty/libprocess/3rdparty/stout" || mkdir 
"../../../../mesos-0.21.0/3rdparty/libprocess/3rdparty/stout"
 (cd include && make  top_distdir=../../../../../mesos-0.21.0 
distdir=../../../../../mesos-0.21.0/3rdparty/libprocess/3rdparty/stout/include \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[6]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
make[6]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
test -n ":" \
|| find "../../../../mesos-0.21.0/3rdparty/libprocess/3rdparty/stout" 
-type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -444 -exec /bin/bash 
/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/install-sh
 -c -m a+r {} {} \; \
|| chmod -R a+r 
"../../../../mesos-0.21.0/3rdparty/libprocess/3rdparty/stout"
make[5]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
make[4]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
 (cd include && make  top_distdir=../../../mesos-0.21.0 
distdir=../../../mesos-0.21.0/3rdparty/libprocess/include \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
make[4]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
test -n ":" \
|| find "../../mesos-0.21.0/3rdparty/libprocess" -type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -444 -exec /bin/bash 
/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/install-sh
 -c -m a+r {} {} \; \
|| chmod -R a+r "../../mesos-0.21.0/3rdparty/libprocess"
make[3]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[2]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
 (cd src && make  top_distdir=../mesos-0.21.0 distdir=../mesos-0.21.0/src \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
test ".." = ".." || \
(/bin/mkdir -p python/src/mesos && cp -pf 
./python/src/mesos/__init__.py python/src/mesos/__init__.py)
test ".." = ".." || \
(/bin/mkdir -p python/interface/src/mes

Build failed in Jenkins: mesos-reviewbot #1723

[Apache Jenkins Server]

See 

Changes:

[niklas] Added reconcileTasks to python scheduler.

--
[...truncated 7953 lines...]
warning: failed to remove mesos-0.21.0/m4/lt~obsolete.m4
warning: failed to remove mesos-0.21.0/m4/ax_python_devel.m4
warning: failed to remove mesos-0.21.0/config.guess
warning: failed to remove mesos-0.21.0/depcomp
warning: failed to remove mesos-0.21.0/config.sub
warning: failed to remove mesos-0.21.0/_build
warning: failed to remove mesos-0.21.0/aclocal.m4
warning: failed to remove mesos-0.21.0/configure
warning: failed to remove mesos-0.21.0/mpi/README
warning: failed to remove mesos-0.21.0/mpi/mpiexec-mesos.in
warning: failed to remove mesos-0.21.0/mpi/mpiexec-mesos.py
warning: failed to remove mesos-0.21.0/ar-lib
warning: failed to remove mesos-0.21.0/Makefile.am
warning: failed to remove mesos-0.21.0/configure.ac
warning: failed to remove mesos-0.21.0/NOTICE
warning: failed to remove mesos-0.21.0/missing
warning: failed to remove mesos-0.21.0/Makefile.in
warning: failed to remove mesos-0.21.0/support/atexit.sh
warning: failed to remove mesos-0.21.0/support/colors.sh
warning: failed to remove mesos-0.21.0/bin/mesos-tests-flags.sh.in
warning: failed to remove mesos-0.21.0/bin/mesos-master.sh.in
warning: failed to remove mesos-0.21.0/bin/lldb-mesos-master.sh.in
warning: failed to remove mesos-0.21.0/bin/gdb-mesos-tests.sh.in
warning: failed to remove mesos-0.21.0/bin/mesos-slave.sh.in
warning: failed to remove mesos-0.21.0/bin/mesos-local-flags.sh.in
warning: failed to remove mesos-0.21.0/bin/gdb-mesos-master.sh.in
warning: failed to remove mesos-0.21.0/bin/valgrind-mesos-tests.sh.in
warning: failed to remove mesos-0.21.0/bin/mesos-slave-flags.sh.in
warning: failed to remove mesos-0.21.0/bin/mesos-tests.sh.in
warning: failed to remove mesos-0.21.0/bin/valgrind-mesos-slave.sh.in
warning: failed to remove mesos-0.21.0/bin/valgrind-mesos-master.sh.in
warning: failed to remove mesos-0.21.0/bin/gdb-mesos-slave.sh.in
warning: failed to remove mesos-0.21.0/bin/mesos-local.sh.in
warning: failed to remove mesos-0.21.0/bin/gdb-mesos-local.sh.in
warning: failed to remove mesos-0.21.0/bin/lldb-mesos-tests.sh.in
warning: failed to remove mesos-0.21.0/bin/lldb-mesos-slave.sh.in
warning: failed to remove mesos-0.21.0/bin/mesos.sh.in
warning: failed to remove mesos-0.21.0/bin/mesos-master-flags.sh.in
warning: failed to remove mesos-0.21.0/bin/lldb-mesos-local.sh.in
warning: failed to remove mesos-0.21.0/bin/valgrind-mesos-local.sh.in
warning: failed to remove mesos-0.21.0/ec2/mesos-ec2
warning: failed to remove mesos-0.21.0/ec2/mesos_ec2.py
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/persistent-hdfs/conf/mapred-site.xml
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/persistent-hdfs/conf/core-site.xml
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/persistent-hdfs/conf/slaves
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/persistent-hdfs/conf/masters
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/persistent-hdfs/conf/hadoop-env.sh
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/persistent-hdfs/conf/hdfs-site.xml
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/start-mesos
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/hypertable/Capfile
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/hypertable/hypertable.cfg
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/create-swap
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/redeploy-mesos
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/mesos-daemon
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/setup-slave
warning: failed to remove mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/zoo
warning: failed to remove mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/slaves
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/stop-mesos
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/hadoop-framework-conf/mapred-site.xml
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/hadoop-framework-conf/core-site.xml
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/hadoop-framework-conf/hadoop-env.sh
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/setup-torque
warning: failed to remove mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/setup
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/copy-dir
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/haproxy+apache/haproxy.config.template
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec2/masters
warning: failed to remove 
mesos-0.21.0/ec2/deploy.amazon64/root/mesos-ec

Re: Review Request 24177: Pass executor directory to Isolator::prepare().

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/#review54618
---

Ship it!



src/tests/isolator_tests.cpp


probably not part of this review, but we avoid this pattern because it 
could lead to leaked temp directories if the test fails.

since you are already inside a sandbox (because you inherit from 
TemporaryDirectoryTest), you should use relative paths to create temporary 
directories.

s/os::mkdtemp()/os::mkdtemp("./XX")/

mind adding a TODO?


- Vinod Kone


On Sept. 23, 2014, 11:42 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24177/
> ---
> 
> (Updated Sept. 23, 2014, 11:42 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Pass executor directory to Isolator::prepare().
> 
> Will be used for FilesystemIsolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolator.hpp 
> e52e8b15c740c62ef64b49897d3d6ae5179d4719 
>   src/slave/containerizer/isolator.cpp 
> 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0 
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 
> 2187c296ea9b1a7de9ae3f09fdf1983f98a3d01b 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 
> 7164ecc0f068d4a72248521e3cbd345958efa880 
>   src/slave/containerizer/isolators/cgroups/mem.hpp 
> b1b4f5a2bd9e01b03fdfa74f187f7dee8119b812 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> b3d4a5daa90a842e501bc6be2f0cf20fe22906ac 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> f7283d830cd6af7b3c9006c098de0a6ad48b7c82 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 4ced508e600e13f3e5ae9d12ea199de743def652 
>   src/slave/containerizer/isolators/posix.hpp 
> f120aafef96343d84f93c5636484509dc972a0a8 
>   src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af 
>   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
> 
> Diff: https://reviews.apache.org/r/24177/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 26056: Normalize & enforce 3rdparty/libprocess conditional include style.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26056/
---

(Updated Sept. 25, 2014, 11:04 p.m.)


Review request for mesos and Niklas Nielsen.


Changes
---

Changing from synchronized.hpp specific to normalizing all of 
3rdparty/libprocess conditional includes.


Summary (updated)
-

Normalize & enforce 3rdparty/libprocess conditional include style.


Repository: mesos-git


Description (updated)
---

Normalize 3rdparty/libprocess conditional include style.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 0a97f12 
  3rdparty/libprocess/include/process/tuples/details.hpp 80b0539 
  3rdparty/libprocess/src/config.hpp cbaf41d 
  3rdparty/libprocess/src/fatal.hpp 38646f3 
  3rdparty/libprocess/src/net.hpp 1ba3dbb 
  3rdparty/libprocess/src/process_reference.hpp 4964107 
  3rdparty/libprocess/src/synchronized.hpp 577a5fc 

Diff: https://reviews.apache.org/r/26056/diff/


Testing
---

make check in 3rdparty/libprocess
support/mesos-style.py


Thanks,

Joris Van Remoortere

Re: Review Request 25861: Serialize isolator prepare and cleanup (reversed).

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25861/#review54619
---


lgtm, modulo comments. i'll let jie give the final shipit.


src/slave/containerizer/mesos/containerizer.cpp


s/isolators/isolator/


- Vinod Kone


On Sept. 22, 2014, 6:45 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25861/
> ---
> 
> (Updated Sept. 22, 2014, 6:45 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 26056: Normalize & enforce 3rdparty/libprocess conditional include style.

[Niklas Nielsen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26056/#review54625
---

Ship it!


Ship It!

- Niklas Nielsen


On Sept. 25, 2014, 4:04 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26056/
> ---
> 
> (Updated Sept. 25, 2014, 4:04 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Normalize 3rdparty/libprocess conditional include style.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 0a97f12 
>   3rdparty/libprocess/include/process/tuples/details.hpp 80b0539 
>   3rdparty/libprocess/src/config.hpp cbaf41d 
>   3rdparty/libprocess/src/fatal.hpp 38646f3 
>   3rdparty/libprocess/src/net.hpp 1ba3dbb 
>   3rdparty/libprocess/src/process_reference.hpp 4964107 
>   3rdparty/libprocess/src/synchronized.hpp 577a5fc 
> 
> Diff: https://reviews.apache.org/r/26056/diff/
> 
> 
> Testing
> ---
> 
> make check in 3rdparty/libprocess
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 26056: Normalize & enforce 3rdparty/libprocess conditional include style.

[Niklas Nielsen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26056/#review54626
---



3rdparty/libprocess/src/net.hpp


Missed this one s/HH/HPP/


- Niklas Nielsen


On Sept. 25, 2014, 4:04 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26056/
> ---
> 
> (Updated Sept. 25, 2014, 4:04 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Normalize 3rdparty/libprocess conditional include style.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 0a97f12 
>   3rdparty/libprocess/include/process/tuples/details.hpp 80b0539 
>   3rdparty/libprocess/src/config.hpp cbaf41d 
>   3rdparty/libprocess/src/fatal.hpp 38646f3 
>   3rdparty/libprocess/src/net.hpp 1ba3dbb 
>   3rdparty/libprocess/src/process_reference.hpp 4964107 
>   3rdparty/libprocess/src/synchronized.hpp 577a5fc 
> 
> Diff: https://reviews.apache.org/r/26056/diff/
> 
> 
> Testing
> ---
> 
> make check in 3rdparty/libprocess
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 26056: Normalize & enforce 3rdparty/libprocess conditional include style.

[Niklas Nielsen]

> On Sept. 25, 2014, 4:35 p.m., Niklas Nielsen wrote:
> > 3rdparty/libprocess/src/net.hpp, line 232
> > 
> >
> > Missed this one s/HH/HPP/

Fixed it when committing - thanks!


- Niklas


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26056/#review54626
---


On Sept. 25, 2014, 4:04 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26056/
> ---
> 
> (Updated Sept. 25, 2014, 4:04 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Normalize 3rdparty/libprocess conditional include style.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 0a97f12 
>   3rdparty/libprocess/include/process/tuples/details.hpp 80b0539 
>   3rdparty/libprocess/src/config.hpp cbaf41d 
>   3rdparty/libprocess/src/fatal.hpp 38646f3 
>   3rdparty/libprocess/src/net.hpp 1ba3dbb 
>   3rdparty/libprocess/src/process_reference.hpp 4964107 
>   3rdparty/libprocess/src/synchronized.hpp 577a5fc 
> 
> Diff: https://reviews.apache.org/r/26056/diff/
> 
> 
> Testing
> ---
> 
> make check in 3rdparty/libprocess
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25622/#review54629
---


Could you do a drive-by fix here to make the 
[web](http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/) 
version display the C++11 whitelist correctly? It seems to display correctly on 
[github](https://github.com/apache/mesos/blob/master/docs/mesos-c%2B%2B-style-guide.md).

On the web version I'm seeing:

## Empty Lines

* 1 blank line at the end of the file.
* Elements outside classes (classes, structs, global functions, etc.) should be 
spaced apart by 2 blank lines.
* Elements inside classes (member variables and functions) should not be spaced 
apart by more than 1 blank line.

## C++11
We still support older compilers. The whitelist of supported C++11 features is: 
* Static assertions. * Multiple right angle brackets. * Type inference (`auto` 
and `decltype`). * Rvalue references. * Variadic templates.

- Michael Park


On Sept. 25, 2014, 11:47 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25622/
> ---
> 
> (Updated Sept. 25, 2014, 11:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-1793
> https://issues.apache.org/jira/browse/MESOS-1793
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Explicitly prohibit the use of namespace aliases. The discussion about using 
> namespace aliases took place in [the other 
> review](https://reviews.apache.org/r/25434/#comment91754). The majority 
> agreed not to introduce them in code.
> 
> Give examples of preferable way of using auto.
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md 59a39df 
> 
> Diff: https://reviews.apache.org/r/25622/diff/
> 
> 
> Testing
> ---
> 
> Documentation change, no `make check` needed.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 25549: Basic filesystem isolator for Linux.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25549/#review54627
---



src/slave/containerizer/isolators/filesystem/shared.cpp


why are the remount commands different for proc and sys? also, why do we 
need to remount /sys and /proc in the fs isolator? it's not clear to me from 
the "reflect namespace changes for the container processes". maybe expand on 
the comment?



src/slave/containerizer/isolators/network/port_mapping.cpp


Hmm. Instead of returning an error here, how about just adding 
"filesystem/shared" isolator to the list of isolators in mesos containerizer, 
if either of "filesystem/shared" or "network/portmapping" is specified in flags?


- Vinod Kone


On Sept. 24, 2014, 6:09 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25549/
> ---
> 
> (Updated Sept. 24, 2014, 6:09 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1586
> https://issues.apache.org/jira/browse/MESOS-1586
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Does not report usage or enforce quota but can create 'private' directories 
> for each container which mask parts of the shared host filesystem.
> 
> This review replaces https://reviews.apache.org/r/24178/ because of some file 
> renaming. I addressed all comments from earlier reviews.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto be45494b2c2f5c1295409889b70004462c6eba49 
>   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
>   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
>   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
>   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
>   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 2766a00ff81dc550c21387f920666f81705db4f0 
>   src/slave/containerizer/linux_launcher.cpp 
> f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
>   src/slave/flags.hpp 32e51d214b0dbbb2f106236c6fa42ddec9774585 
>   src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 
>   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
>   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
> 
> Diff: https://reviews.apache.org/r/25549/diff/
> 
> 
> Testing
> ---
> 
> make check # added a test
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 25863: Rename stout/os/setns.hpp to namespaces.hpp.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25863/#review54630
---

Ship it!


Ship It!

- Vinod Kone


On Sept. 23, 2014, 11:39 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25863/
> ---
> 
> (Updated Sept. 23, 2014, 11:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Also, added os::getns(pid, ns) to get the namespace inode for comparisons 
> between pids' namespaces.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 2ee5a0bcc8bef0a5769dafc8ae54aea284993d6e 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> d4a8ad4e776bcfe1f008e561b5a92340f4d84bd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp 
> 5278996f201a4a3d69282c1bd7b0d230d0f6cd39 
>   3rdparty/libprocess/3rdparty/stout/tests/os/namespaces_tests.cpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp 
> ad8e37aa2f5a29f8b421dde6b7cd5dfe241eabb5 
> 
> Diff: https://reviews.apache.org/r/25863/diff/
> 
> 
> Testing
> ---
> 
> Added test to check a clone(NEW_PIDNS) results in a new pid namespace.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 25965: Update libprocess Makefile for setns namechange.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25965/#review54632
---

Ship it!


Ship It!

- Vinod Kone


On Sept. 24, 2014, 6:13 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25965/
> ---
> 
> (Updated Sept. 24, 2014, 6:13 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Update libprocess Makefile for setns namechange.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 2766a00ff81dc550c21387f920666f81705db4f0 
> 
> Diff: https://reviews.apache.org/r/25965/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 25964: Update libprocess Makefile for setns namechange.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25964/#review54631
---

Ship it!


Ship It!

- Vinod Kone


On Sept. 24, 2014, 6:14 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25964/
> ---
> 
> (Updated Sept. 24, 2014, 6:14 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Update libprocess Makefile for setns namechange.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> bd1dc8df0259a318a9171a9c045a223800e64f47 
> 
> Diff: https://reviews.apache.org/r/25964/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Jenkins build is back to normal : mesos-reviewbot #1724

[Apache Jenkins Server]

See 

Re: Review Request 25848: Introducing mesos modules.

[Kapil Arya]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25848/
---

(Updated Sept. 25, 2014, 8:45 p.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and 
Timothy St. Clair.


Changes
---

Added a few negative tests.


Bugs: MESOS-1384
https://issues.apache.org/jira/browse/MESOS-1384


Repository: mesos-git


Description
---

Adding a first class primitive, abstraction and process for dynamic library 
writing and loading can make it easier to extend inner workings of Mesos. 
Making it possible to have dynamic loadable resource allocators, isolators, 
containerizes, authenticators and much more.


Diffs (updated)
-

  configure.ac c4b43911f5f8f651ddf8f2e12c263849e07e8089 
  include/mesos/module.hpp.in PRE-CREATION 
  src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
  src/examples/test_module.hpp PRE-CREATION 
  src/examples/test_module_impl.cpp PRE-CREATION 
  src/examples/test_module_impl2.cpp PRE-CREATION 
  src/module/manager.hpp PRE-CREATION 
  src/module/manager.cpp PRE-CREATION 
  src/tests/module_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/25848/diff/


Testing
---

Ran make check with added tests for verifying library/module loading and simple 
version check.


Thanks,

Kapil Arya

Re: Review Request 25848: Introducing mesos modules.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25848/#review54638
---


Bad patch!

Reviews applied: [25848]

Failed command: git apply --index 25848.patch

Error:
 error: patch failed: src/Makefile.am:1104
error: src/Makefile.am: patch does not apply

- Mesos ReviewBot


On Sept. 26, 2014, 12:45 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25848/
> ---
> 
> (Updated Sept. 26, 2014, 12:45 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, 
> and Timothy St. Clair.
> 
> 
> Bugs: MESOS-1384
> https://issues.apache.org/jira/browse/MESOS-1384
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Adding a first class primitive, abstraction and process for dynamic library 
> writing and loading can make it easier to extend inner workings of Mesos. 
> Making it possible to have dynamic loadable resource allocators, isolators, 
> containerizes, authenticators and much more.
> 
> 
> Diffs
> -
> 
>   configure.ac c4b43911f5f8f651ddf8f2e12c263849e07e8089 
>   include/mesos/module.hpp.in PRE-CREATION 
>   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
>   src/examples/test_module.hpp PRE-CREATION 
>   src/examples/test_module_impl.cpp PRE-CREATION 
>   src/examples/test_module_impl2.cpp PRE-CREATION 
>   src/module/manager.hpp PRE-CREATION 
>   src/module/manager.cpp PRE-CREATION 
>   src/tests/module_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25848/diff/
> 
> 
> Testing
> ---
> 
> Ran make check with added tests for verifying library/module loading and 
> simple version check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 25865: Pid namespace isolator for the MesosContainerizer.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/#review54637
---



src/slave/containerizer/isolators/namespaces/pid.hpp


s/NamespacesPid/PidNamespace/ ?



src/slave/containerizer/isolators/namespaces/pid.hpp


kill new line.



src/slave/containerizer/isolators/namespaces/pid.cpp


Comment?



src/slave/containerizer/isolators/namespaces/pid.cpp


As mentioned in the previous review, instead of requiring users/operators 
to know this dependency, we should just automatically use fileystem/shared 
isoator when using pid or network isolation.



src/slave/containerizer/isolators/namespaces/pid.cpp


Who is calling this method?



src/slave/containerizer/isolators/namespaces/pid.cpp


// Cleanup orphans. ?



src/slave/containerizer/isolators/namespaces/pid.cpp


if you use hashset, you can just do !containerers.contain().



src/slave/containerizer/isolators/namespaces/pid.cpp


Why not just call cleanup() here?



src/slave/containerizer/isolators/namespaces/pid.cpp


Can you also say why in the comment? Presumably because you dont want 
containers to see other containers runninng in the system?



src/slave/containerizer/isolators/namespaces/pid.cpp


Add a comment that you are doing this for the ability to cleanup orphans 
during recovery?

Also, what is the need for manual cleanup or orphans?



src/slave/containerizer/linux_launcher.cpp


why is this pulled out?



src/tests/isolator_tests.cpp


s/NamespacesPidIsolatorTest/PidNamespaceIsolatorTest/



src/tests/isolator_tests.cpp


you are writing to files, not stdout and stderr right?


- Vinod Kone


On Sept. 23, 2014, 11:39 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25865/
> ---
> 
> (Updated Sept. 23, 2014, 11:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add namespaces/pid to --isolation slave flag. Places executor into a pid 
> namespace so it and all descendants will be contained in the namespace. 
> Requires the filesystem/shared isolator so /proc and /sys are remounted to 
> reflect the different namespace.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
>   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
>   src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION 
>   src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.cpp 
> f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
>   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
> 
> Diff: https://reviews.apache.org/r/25865/diff/
> 
> 
> Testing
> ---
> 
> Added test that command in pid namespaced container is in a different 
> namespace and that the command is 'init' (verifies remount of /proc).
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 25865: Pid namespace isolator for the MesosContainerizer.

[Vinod Kone]

> On Sept. 26, 2014, 1:17 a.m., Vinod Kone wrote:
> >

Can you attach the bug. Also, need documentation.


- Vinod


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/#review54637
---


On Sept. 23, 2014, 11:39 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25865/
> ---
> 
> (Updated Sept. 23, 2014, 11:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add namespaces/pid to --isolation slave flag. Places executor into a pid 
> namespace so it and all descendants will be contained in the namespace. 
> Requires the filesystem/shared isolator so /proc and /sys are remounted to 
> reflect the different namespace.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
>   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
>   src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION 
>   src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.cpp 
> f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
>   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
> 
> Diff: https://reviews.apache.org/r/25865/diff/
> 
> 
> Testing
> ---
> 
> Added test that command in pid namespaced container is in a different 
> namespace and that the command is 'init' (verifies remount of /proc).
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 25848: Introducing mesos modules.

[Kapil Arya]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25848/
---

(Updated Sept. 25, 2014, 9:27 p.m.)


Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, and 
Timothy St. Clair.


Changes
---

Rebased to mesos:master.


Bugs: MESOS-1384
https://issues.apache.org/jira/browse/MESOS-1384


Repository: mesos-git


Description
---

Adding a first class primitive, abstraction and process for dynamic library 
writing and loading can make it easier to extend inner workings of Mesos. 
Making it possible to have dynamic loadable resource allocators, isolators, 
containerizes, authenticators and much more.


Diffs (updated)
-

  configure.ac 86d448c3ad00ad01d3d069c1039dc7ad524af567 
  include/mesos/module.hpp.in PRE-CREATION 
  src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b 
  src/examples/test_module.hpp PRE-CREATION 
  src/examples/test_module_impl.cpp PRE-CREATION 
  src/examples/test_module_impl2.cpp PRE-CREATION 
  src/module/manager.hpp PRE-CREATION 
  src/module/manager.cpp PRE-CREATION 
  src/tests/module_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/25848/diff/


Testing
---

Ran make check with added tests for verifying library/module loading and simple 
version check.


Thanks,

Kapil Arya

Re: Review Request 25966: Use pid namespace in LinuxLauncher::destroy().

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25966/#review54641
---



src/slave/containerizer/linux_launcher.cpp


do we need to call destroy?

how about improving cgroups::remove() to do recursive removal and just 
calling that here?



src/slave/containerizer/linux_launcher.cpp


Tests?


- Vinod Kone


On Sept. 23, 2014, 11:41 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25966/
> ---
> 
> (Updated Sept. 23, 2014, 11:41 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Check if a container is running in a pid namespace and thus all processes can 
> be killed by the kernel, rather than using the freezer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
> 
> Diff: https://reviews.apache.org/r/25966/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 25864: Add 'Future cgroups::empty()'.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25864/#review54643
---

Ship it!


lgtm modulo comments.

- Vinod Kone


On Sept. 23, 2014, 11:39 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25864/
> ---
> 
> (Updated Sept. 23, 2014, 11:39 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Polls cgroups.procs until no processes in the cgroup. Poll interval and 
> timeout can be specified.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp abf31df1b4dbf6f715f93256b83c9996a45099cf 
>   src/linux/cgroups.cpp 62df4b7645c6ab061a47634058d79ca849caa6b9 
> 
> Diff: https://reviews.apache.org/r/25864/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 25549: Basic filesystem isolator for Linux.

[Vinod Kone]

> On Sept. 25, 2014, 11:56 p.m., Vinod Kone wrote:
> >

Mind adding documentation for this?


- Vinod


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25549/#review54627
---


On Sept. 24, 2014, 6:09 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25549/
> ---
> 
> (Updated Sept. 24, 2014, 6:09 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1586
> https://issues.apache.org/jira/browse/MESOS-1586
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Does not report usage or enforce quota but can create 'private' directories 
> for each container which mask parts of the shared host filesystem.
> 
> This review replaces https://reviews.apache.org/r/24178/ because of some file 
> renaming. I addressed all comments from earlier reviews.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto be45494b2c2f5c1295409889b70004462c6eba49 
>   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
>   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
>   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
>   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
>   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 2766a00ff81dc550c21387f920666f81705db4f0 
>   src/slave/containerizer/linux_launcher.cpp 
> f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
>   src/slave/flags.hpp 32e51d214b0dbbb2f106236c6fa42ddec9774585 
>   src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 
>   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
>   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
> 
> Diff: https://reviews.apache.org/r/25549/diff/
> 
> 
> Testing
> ---
> 
> make check # added a test
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 25848: Introducing mesos modules.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25848/#review54646
---


Patch looks great!

Reviews applied: [25848]

All tests passed.

- Mesos ReviewBot


On Sept. 26, 2014, 1:27 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25848/
> ---
> 
> (Updated Sept. 26, 2014, 1:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Niklas Nielsen, 
> and Timothy St. Clair.
> 
> 
> Bugs: MESOS-1384
> https://issues.apache.org/jira/browse/MESOS-1384
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Adding a first class primitive, abstraction and process for dynamic library 
> writing and loading can make it easier to extend inner workings of Mesos. 
> Making it possible to have dynamic loadable resource allocators, isolators, 
> containerizes, authenticators and much more.
> 
> 
> Diffs
> -
> 
>   configure.ac 86d448c3ad00ad01d3d069c1039dc7ad524af567 
>   include/mesos/module.hpp.in PRE-CREATION 
>   src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b 
>   src/examples/test_module.hpp PRE-CREATION 
>   src/examples/test_module_impl.cpp PRE-CREATION 
>   src/examples/test_module_impl2.cpp PRE-CREATION 
>   src/module/manager.hpp PRE-CREATION 
>   src/module/manager.cpp PRE-CREATION 
>   src/tests/module_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25848/diff/
> 
> 
> Testing
> ---
> 
> Ran make check with added tests for verifying library/module loading and 
> simple version check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Review Request 26069: Introduce ClangFormat to Mesos.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26069/
---

Review request for mesos, Benjamin Hindman, Cody Maloney, Dominic Hamon, and 
Timothy Chen.


Bugs: MESOS-1291
https://issues.apache.org/jira/browse/MESOS-1291


Repository: mesos-git


Description
---

We spend way too much our time formatting, not to mention the amount of time 
wasted during the review cycle to address style issues. Let’s get ClangFormat 
to help us!

If you don't know what ClangFormat is or how it works, take a look at the 
Chandler Carruth's [live 
demo](http://www.youtube.com/watch?v=uvddFPavYZQ#t=23m28s) from 23m 28s to 33m 
27s.

Further details and analysis are available 
[here](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing).


Diffs
-

  .clang-format PRE-CREATION 
  .gitignore-template 3e2b11ca8a9f9e77f58718ae3425a983a24d5865 
  support/clang-format PRE-CREATION 

Diff: https://reviews.apache.org/r/26069/diff/


Testing
---

Refer to the __Example Diffs__ section in the [Google 
Doc](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing)


Thanks,

Michael Park

Review Request 26071: Sample isolator module.

[Kapil Arya]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26071/
---

Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Bugs: MESOS-1384
https://issues.apache.org/jira/browse/MESOS-1384


Repository: mesos-git


Description
---

Sample isolator module copied from cgroups/cpu.


Diffs
-

  src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b 
  src/examples/test_isolator_module.cpp PRE-CREATION 
  src/examples/test_module.hpp PRE-CREATION 
  src/tests/module_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/26071/diff/


Testing
---

make check with an  Isolator module test.


Thanks,

Kapil Arya

Re: Review Request 26069: Introduce ClangFormat to Mesos.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26069/
---

(Updated Sept. 26, 2014, 5:56 a.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, Dominic Hamon, and 
Timothy Chen.


Bugs: MESOS-1291
https://issues.apache.org/jira/browse/MESOS-1291


Repository: mesos-git


Description (updated)
---

We spend way too much of our time formatting, not to mention the amount of time 
wasted during the review cycle to address style issues. Let’s get ClangFormat 
to help us!

If you don't know what ClangFormat is or how it works, take a look at the 
Chandler Carruth's [live 
demo](http://www.youtube.com/watch?v=uvddFPavYZQ#t=23m28s) from 23m 28s to 33m 
27s.

Further details and analysis are available 
[here](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing).


Diffs
-

  .clang-format PRE-CREATION 
  .gitignore-template 3e2b11ca8a9f9e77f58718ae3425a983a24d5865 
  support/clang-format PRE-CREATION 

Diff: https://reviews.apache.org/r/26069/diff/


Testing
---

Refer to the __Example Diffs__ section in the [Google 
Doc](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing)


Thanks,

Michael Park

Re: Review Request 26069: Introduce ClangFormat to Mesos.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26069/
---

(Updated Sept. 26, 2014, 6:09 a.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, Dominic Hamon, and 
Timothy Chen.


Bugs: MESOS-1291
https://issues.apache.org/jira/browse/MESOS-1291


Repository: mesos-git


Description
---

We spend way too much of our time formatting, not to mention the amount of time 
wasted during the review cycle to address style issues. Let’s get ClangFormat 
to help us!

If you don't know what ClangFormat is or how it works, take a look at the 
Chandler Carruth's [live 
demo](http://www.youtube.com/watch?v=uvddFPavYZQ#t=23m28s) from 23m 28s to 33m 
27s.

Further details and analysis are available 
[here](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing).


Diffs
-

  .clang-format PRE-CREATION 
  .gitignore-template 3e2b11ca8a9f9e77f58718ae3425a983a24d5865 
  support/clang-format PRE-CREATION 

Diff: https://reviews.apache.org/r/26069/diff/


Testing (updated)
---

Refer to the __Sample Diff__ section in the [Google 
Doc](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing)


Thanks,

Michael Park