Re: Review Request 38910: Added `-v` flag to `docker rm`.

[Jojy Varghese]

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



src/docker/docker.cpp (line 681)


wondering this behavior should be defaulted or not. We might be overloading 
stop with more than what it should be doing isnt it? Do we always want to force 
remove the volumes when docker stops?


- Jojy Varghese


On Sept. 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated Sept. 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38910: Added `-v` flag to `docker rm`.

[haosdent huang]

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



src/docker/docker.cpp (line 681)


The comment line should be move up before "const string cmd"?


- haosdent huang


On Sept. 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated Sept. 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38910: Added `-v` flag to `docker rm`.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Sept. 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated Sept. 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

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

(Updated Oct. 1, 2015, 3:32 a.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

Integrated with store; review comments;


Repository: mesos


Description (updated)
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38579: Refactored registry client

[Jojy Varghese]

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

(Updated Oct. 1, 2015, 3:29 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

broke up big methods; cleanup.


Summary (updated)
-

Refactored registry client


Repository: mesos


Description (updated)
---

- Moved ManifestResponse struct from RegistryClient to namespace.
- Cleanup


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
0a9663139b8a6d9e4fa458f7c893d33ca3a30e90 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38609: Refactored http::internal::request to use http::connect.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38595, 38596, 38597, 38598, 38600, 38599, 38601, 38603, 
38604, 38605, 38606, 38607, 38608, 38609]

All tests passed.

- Mesos ReviewBot


On Oct. 1, 2015, 12:19 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38609/
> ---
> 
> (Updated Oct. 1, 2015, 12:19 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gets rid of the duplicated code, in favor of leveraging http::Connection.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ba3f0bc7df33795e332c374fbad04106b9d56416 
>   3rdparty/libprocess/src/http.cpp d9925333f25491f92495bf11315237f54a0a2015 
> 
> Diff: https://reviews.apache.org/r/38609/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38910: Added `-v` flag to `docker rm`.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38910]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated Sept. 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Michael Park]

> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote:
> > One high level suggestion.
> > 
> > After looking at our http code, I realized that we use the same pattern 
> > again and again, for example:
> > ```
> > JSON::Array array;
> > array.values.reserve(status.network_infos().size()); // MESOS-2353.
> > foreach (const NetworkInfo& info, status.network_infos()) {
> >   array.values.push_back(model(info));
> > }
> > object.values["network_infos"] = std::move(array);
> > ```
> > We cannot use newly added `JSON::protobuf()` here, because a different way 
> > for rendering JSON from protobuf is used. Without digging deep inside, I 
> > know three ways how we create a `JSON` out of a proto in our codebase:
> > - wrap in `JSON::Protobuf()` for individual messages;
> > - wrap in one of the `model()` family functions;
> > - pass as it is for built-in types.
> > 
> > The proposed conversion function covers one of the possible ways. How about 
> > add one more convertion? Something like:
> > ```
> > template 
> > Array protobuf(const google::protobuf::RepeatedPtrField& repeated,
> > const lambda::function(const T&)>& converter)
> > {
> >   static_assert(std::is_convertible::value,
> > "T must be a google::protobuf::Message");
> >   JSON::Array array;
> >   array.values.reserve(repeated.size());
> >   foreach (const T& elem, repeated) {
> > array.values.push_back(converter(elem));
> >   }
> >   
> >   return array;
> > }
> > ```
> > 
> > Then the snippet above could be rewritten as:
> > ```
> > object.values["network_infos"] = 
> > std::move(JSON::protobuf(status.network_infos(), [](const NetworkInfo& 
> > info) { return model(info); });
> > ```
> > 
> > A further improvement would be to accept any iterable collection, not only 
> > `RepeatedPtrField<>`, for example `hashset`.
> > 
> > What do you think?
> 
> Klaus Ma wrote:
> Awesome! I've also try similar proposal, but failed when `function` 
> converting with `template`; your suggestion using lambda is great!
> For the `hashset`, I'd suggest to address it when we have such case in 
> our code :).
> 
> I'll also address your comments above.
> 
> Alexander Rukletsov wrote:
> We do have such cases in our codebase ; ). Here are a few as an example:
> - https://github.com/apache/mesos/blob/master/src/master/http.cpp#L217
> - https://github.com/apache/mesos/blob/master/src/master/http.cpp#L229

I like the idea of taking a projection function as an argument, but let's do it 
as a separate ticket to keep the scope of this review narrow.


- Michael


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


On Sept. 27, 2015, 1:34 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Sept. 27, 2015, 1:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38608: Introduced an http::Connection for connection re-use and pipelining.

[Ben Mahler]

> On Sept. 30, 2015, 1:32 a.m., Vinod Kone wrote:
> >

Thanks! I've simplified things and also caught two issues that were missed 
originally.


> On Sept. 30, 2015, 1:32 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 1045-1059
> > 
> >
> > this seems like a repeated pattern in this file. can this be a factory 
> > method on Socket?

It's only repeated because I split out https://reviews.apache.org/r/38609/ . 
After applying the subsequent review it's not repeated. Agreed that we will 
want to see if this warrants being pulled out though if we end up with 
duplication.


> On Sept. 30, 2015, 1:32 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 1021-1042
> > 
> >
> > Seems generally useful. Would it make sense to make this a factory 
> > method on Address class?
> > 
> > Try create(const URL& url);

Yeah, although it seems a bit odd for Address to understand URL since we may 
have arbitrary things that can get converted to addresses.

One suggestion is to add this on URL:

```
class URL
{
  Try resolve(); // Or,
  Try address();
};
```

Added a TODO for follow-up.


> On Sept. 30, 2015, 1:32 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/http.cpp, line 976
> > 
> >
> > a comment on why you need 'false' here?

Thanks for the poke, this definitely warranted a comment!


> On Sept. 30, 2015, 1:32 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 969-971
> > 
> >
> > lets pull process below to spawn, like we do in most of our code base?

Doing a grep reveals that we do this both ways, but I'll change it since your 
suggestion seems a bit more readable.


> On Sept. 30, 2015, 1:32 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/http.cpp, line 791
> > 
> >
> > why the temporary?
> > 
> > can't you just do "return promise.future()"?

The promise is moved into the queue, so the return would have to be:

```
return std::get<1>(pipeline.back()).future();
```


> On Sept. 30, 2015, 1:32 a.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/http.cpp, line 832
> > 
> >
> > the whole disconnect(), closed() and fail() semantics are a bit hard to 
> > grok, esp around what calls what. can you think of a refactor that will 
> > make it easy? sorry, i don't have concrete suggestion yet.

Thanks for calling this out! This was definitely a difficulty when writing this.

I've now greatly simplified this by having just a single 'disconnected' method. 
I hope this reads a lot clearer now, have a look!


- Ben


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


On Sept. 22, 2015, 6:18 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38608/
> ---
> 
> (Updated Sept. 22, 2015, 6:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3332
> https://issues.apache.org/jira/browse/MESOS-3332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support connection re-use and pipelining of requests on a shared 
> connection, this introduces the notion of an http::Connection.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> fbd6cf7967173495875a8ea90ed28ade88b982a2 
>   3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
> 
> Diff: https://reviews.apache.org/r/38608/diff/
> 
> 
> Testing
> ---
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 38608: Added an http::Connection for connection re-use and pipelining.

[Ben Mahler]

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

(Updated Oct. 1, 2015, 12:19 a.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Some simplifications driven by vinod's feedback.


Summary (updated)
-

Added an http::Connection for connection re-use and pipelining.


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


Repository: mesos


Description
---

In order to support connection re-use and pipelining of requests on a shared 
connection, this introduces the notion of an http::Connection.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
ba3f0bc7df33795e332c374fbad04106b9d56416 
  3rdparty/libprocess/src/http.cpp d9925333f25491f92495bf11315237f54a0a2015 
  3rdparty/libprocess/src/tests/http_tests.cpp 
c380f356548cf9f5491044bccabcd9c66ad5f55a 

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


Testing
---

Added tests.


Thanks,

Ben Mahler

Re: Review Request 38609: Refactored http::internal::request to use http::connect.

[Ben Mahler]

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

(Updated Oct. 1, 2015, 12:19 a.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Rebase.


Repository: mesos


Description
---

This gets rid of the duplicated code, in favor of leveraging http::Connection.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
ba3f0bc7df33795e332c374fbad04106b9d56416 
  3rdparty/libprocess/src/http.cpp d9925333f25491f92495bf11315237f54a0a2015 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38910: Added `-v` flag to `docker rm`.

[Greg Mann]

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

Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.


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


Repository: mesos


Description
---

Added `-v` flag to `docker rm`.


Diffs
-

  src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 

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


Testing
---

`make check`

DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
VM, but this is a known issue: MESOS-3123


Thanks,

Greg Mann

Re: Review Request 38649: Add a benchmark to simulate frameworks declining offers.

[James Peach]

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

(Updated Sept. 30, 2015, 11:41 p.m.)


Review request for .


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


Repository: mesos


Description
---

This benchmark starts a number of slaves and frameworks, then cycles
the allocator. On each allocation pass, the frameworks decline all
the offers. This leads to increasing numbers of refusal filters in
the allocator, and the allocation slows down. After around 200
cycles the slowdown increases out of proportion.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 

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


Testing
---

make check && make bench


Thanks,

James Peach

Re: Review Request 38649: Add a benchmark to simulate frameworks declining offers.

[James Peach]

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

(Updated Sept. 30, 2015, 11:41 p.m.)


Review request for .


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


Repository: mesos


Description
---

This benchmark starts a number of slaves and frameworks, then cycles
the allocator. On each allocation pass, the frameworks decline all
the offers. This leads to increasing numbers of refusal filters in
the allocator, and the allocation slows down. After around 200
cycles the slowdown increases out of proportion.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 

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


Testing
---

make check && make bench


Thanks,

James Peach

Re: Review Request 38649: Add a benchmark to simulate frameworks declining offers.

[James Peach]

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

(Updated Sept. 30, 2015, 10:02 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

This benchmark starts a number of slaves and frameworks, then cycles
the allocator. On each allocation pass, the frameworks decline all
the offers. This leads to increasing numbers of refusal filters in
the allocator, and the allocation slows down. After around 200
cycles the slowdown increases out of proportion.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 

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


Testing
---

make check && make bench


Thanks,

James Peach

Re: Review Request 38649: Add a benchmark to simulate frameworks declining offers.

[James Peach]

> On Sept. 25, 2015, 9:55 p.m., Joris Van Remoortere wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 1397
> > 
> >
> > This causes a race that can corrupt the allocator:
> > - The calls to `allocator->recoverResources` are asynchronously invoked 
> > inside the actor (on a separate thread)
> > - This call to `hproc->allocate` is synchronous and runs in the 
> > execution context of the current thread
> > - The Allocator is an Actor, and all our actors are written to be 
> > driven by libprocess (i.e. only 1 thread invoking a function at any given 
> > point in time, only 1 execution context)
> > The way this code is currently set up the `recoverResources` functions 
> > may be executed simultaneously as the `allocate` function.
> > 
> > Let me know if this is not clear.
> > 
> > You want to do something along these lines:
> > ```
> > Future result = dispatch(hproc->self(), 
> > &HierarchicalDRFAllocatorProcess::allocate);
> > 
> > result.await();
> > ```
> > 
> > However; the allocate function would need to return something other 
> > than `void` such as `Nothing` for that to work.
> 
> James Peach wrote:
> Yeh, the place where I call ```allocate()``` directly is a total hack. To 
> do it right, I'd have to plumb an ```allocate()``` call all the way through 
> the ```Allocator``` interface. Is that the right approach?

I updated the test to force the allocation using ```Clock``` manipulation 
rather than explicitly calling the allocator interface. It still seems to work 
OK.


- James


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


On Sept. 30, 2015, 10 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38649/
> ---
> 
> (Updated Sept. 30, 2015, 10 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3493
> https://issues.apache.org/jira/browse/MESOS-3493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This benchmark starts a number of slaves and frameworks, then cycles
> the allocator. On each allocation pass, the frameworks decline all
> the offers. This leads to increasing numbers of refusal filters in
> the allocator, and the allocation slows down. After around 200
> cycles the slowdown increases out of proportion.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 
> 
> Diff: https://reviews.apache.org/r/38649/diff/
> 
> 
> Testing
> ---
> 
> make check && make bench
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 38649: Add a benchmark to simulate frameworks declining offers.

[James Peach]

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

(Updated Sept. 30, 2015, 10 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

This benchmark starts a number of slaves and frameworks, then cycles
the allocator. On each allocation pass, the frameworks decline all
the offers. This leads to increasing numbers of refusal filters in
the allocator, and the allocation slows down. After around 200
cycles the slowdown increases out of proportion.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 

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


Testing
---

make check && make bench


Thanks,

James Peach

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Ben Mahler]

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


Hm.. it is suprising that touching this header requires wide re-compilation. 
Could we audit why that is?

Looking through the includes, I can see tests/cluster.hpp pulls this in, but we 
should have a .cpp to avoid the dependency in the header. This should yield 
more dependency removals since we can cut other includes. The other culprit 
looks to be tests/mesos.hpp which exposes an allocator wrapper for mocking. 
We've been planning to split out the mocks into relevant mocks/*.hpp files, so 
this could be migrated to a mocks/allocator.hpp file that is only included when 
needed in a test. FWICT only 5 test files need this.

Doing the dependency cleanup should reduce the need for this refactor, and help 
speed up compilation more generally. I'm curious about the implications of this 
pattern for our other templated headers, so hoping to be able to avoid if 
possible :)

- Ben Mahler


On Sept. 30, 2015, 1:08 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38901]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 6:31 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Sept. 30, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Joris Van Remoortere]

> On Sept. 30, 2015, 5:10 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 19
> > 
> >
> > Shouldn't this go at the bottom of the includes?

I believe we are still trying to follow the guideline of including the header 
that we're implementing as the first file in the include list.
I recognize this is not consistent in the code-base. Joerg is trying to fix 
that.


- Joris


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


On Sept. 30, 2015, 1:08 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Joris Van Remoortere]

> On Sept. 30, 2015, 9:55 a.m., Alexander Rukletsov wrote:
> > I like when the code is pushed to `.cpp` files : ). Just to confirm that 
> > this is our intention: with the prvious design, `RoleSorter` and 
> > `FrameworkSorter` could be anything (well, anything that can be used as a 
> > sorter), now we restrict sorters to subclass `Sorter`. I think this is 
> > fine, but would like to confirm with you.
> > 
> > A high level question I have: why do you like to preserve template 
> > approach? I think we can require allocator writers to provide a factory 
> > with their own allocator and get rid of templates completely. What do you 
> > think?

Indeed, the old implementation was just enforcing the same interface as 
`Sorter` but through templates.
I kept the template approach because it minimizes the size and impact of this 
patch. I think we can make a separate discussion around removing the template 
all together.
All the tests, and possibly code that people have written outside of our repo 
will continue to work using the current approach as the external interface does 
not change.
I realize that people shouldn't be relying on these internal files, but I think 
we can isolate this change to just a compile time optimization, leaving out the 
API change.
What are your thoughts?


> On Sept. 30, 2015, 9:55 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.hpp, lines 110-114
> > 
> >
> > Why have you decided to leave the c-tor here and not in the cpp file?

The diff generated by this is rather large, and I wanted to make it very 
obvious which parts of the code are changing (The top of the diff), and make it 
as easy as possible for someone to verify that nothing else changed (i.e. the 
rest of the file was just copy pasted, with template prototypes removed).


> On Sept. 30, 2015, 9:55 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 131-136
> > 
> >
> > How about pulling this code to the c-tor? this will allow us not to 
> > store a copy of `sorterFactory` in allocator.

We can. I didn't want to break any assumptions between constructor vs 
initializer ordering of these events.
Would it make sense to make this a separate patch as it changes the behavior?


- Joris


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


On Sept. 30, 2015, 1:08 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

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

(Updated Sept. 30, 2015, 8:11 p.m.)


Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.


Changes
---

more tests; dangling pointer;


Repository: mesos


Description
---

Added SHA256, SHA512 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38883: Removed calls to apply-review.sh script. Added support for amending commit messages.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38705, 38883]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 6:21 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38883/
> ---
> 
> (Updated Sept. 30, 2015, 6:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38883/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38900: Update command executor to support rootfs.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38900]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 6:09 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38900/
> ---
> 
> (Updated Sept. 30, 2015, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update command executor to support rootfs.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 4c1419290645ad4c44360a81618a6cea7ad190df 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38900/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

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

(Updated Sept. 30, 2015, 6:50 p.m.)


Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.


Changes
---

added more tests; fixed possible dangling pointer.


Repository: mesos


Description
---

Added SHA256, SHA512 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

[Alex Clemmer]

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



src/Makefile.am (line 537)


Could we please add this file to the CMakeLists file in src/ also?


- Alex Clemmer


On Sept. 30, 2015, 6:31 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Sept. 30, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 38901: Serialize Docker Image Spec as Protobuf

[Gilbert Song]

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

Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Serialize Docker Image Spec as Protobuf


Diffs
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (ubuntu 14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 38883: Removed calls to apply-review.sh script. Added support for amending commit messages.

[Artem Harutyunyan]

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

(Updated Sept. 30, 2015, 11:21 a.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38884: Added log message when suppress offers

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 30, 2015, 6:57 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38884/
> ---
> 
> (Updated Sept. 30, 2015, 6:57 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added log message when suppress offers
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
> 
> Diff: https://reviews.apache.org/r/38884/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38899: Added handler for Executor->Framework message for the Executor HTTP API

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38899]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 5:02 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38899/
> ---
> 
> (Updated Sept. 30, 2015, 5:02 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez and Vinod Kone.
> 
> 
> Bugs: MESOS-2296
> https://issues.apache.org/jira/browse/MESOS-2296
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Trivial change to call the `executorMessage(...)` function that already 
> existed in `src/slave/slave.cpp` to handle ExecutorToFrameworkMessage.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
> 
> Diff: https://reviews.apache.org/r/38899/diff/
> 
> 
> Testing
> ---
> 
> make check. Would add tests later after the MESOS-3480 review chain gets 
> committed.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 38900: Update command executor to support rootfs.

[Timothy Chen]

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

Review request for mesos, Jie Yu and Jiang Yan Xu.


Repository: mesos


Description
---

Update command executor to support rootfs.


Diffs
-

  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/slave/containerizer/mesos/containerizer.hpp 
4c1419290645ad4c44360a81618a6cea7ad190df 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38883: Removed calls to apply-review.sh script. Added support for amending commit messages.

[Joseph Wu]

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


Preliminary review.


support/apply-reviews.py (line 27)


Propagated typo: Retutns



support/apply-reviews.py (line 32)


Propagated typo: Retutns



support/apply-reviews.py (line 105)


To stay consistent: s/reviewboard/Review Board/



support/apply-reviews.py (line 132)


Spaces around the `=`.


- Joseph Wu


On Sept. 29, 2015, 11:18 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38883/
> ---
> 
> (Updated Sept. 29, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38883/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37714, 37996, 37997, 37998, 37999, 38000, 38094]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 8:21 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Sept. 30, 2015, 8:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fb9bd04832779ac43151f2feb3dfbf58cb996434 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> c380f356548cf9f5491044bccabcd9c66ad5f55a 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

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

Ship it!


LGTM!

Note: The long-form cmd-line arguments (i.e. `--dry-run`) are not really 
necessary, but might be nice.


support/apply-reviews.py (line 12)


Typo: Retutns


- Joseph Wu


On Sept. 29, 2015, 11:09 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 29, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Joseph Wu]

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



src/master/allocator/mesos/hierarchical.cpp (line 19)


Shouldn't this go at the bottom of the includes?


- Joseph Wu


On Sept. 29, 2015, 6:08 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 29, 2015, 6:08 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Review Request 38899: Added handler for Executor->Framework message for the Executor HTTP API

[Anand Mazumdar]

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

Review request for mesos, Isabel Jimenez and Vinod Kone.


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


Repository: mesos


Description
---

Trivial change to call the `executorMessage(...)` function that already existed 
in `src/slave/slave.cpp` to handle ExecutorToFrameworkMessage.


Diffs
-

  src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 

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


Testing
---

make check. Would add tests later after the MESOS-3480 review chain gets 
committed.


Thanks,

Anand Mazumdar

Re: Review Request 38884: Added log message when suppress offers

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38884]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 6:57 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38884/
> ---
> 
> (Updated Sept. 30, 2015, 6:57 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added log message when suppress offers
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
> 
> Diff: https://reviews.apache.org/r/38884/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38161: Replaced a hard-coded number for registration backoff with a proper constant and extended comments.

[Alexander Rukletsov]

> On Sept. 29, 2015, 7:12 p.m., Ben Mahler wrote:
> > Thanks Alex, didn't realize I was on the reviewers here, so apologies for 
> > the delay! Why don't we just name the constant with DEFAULT to avoid the 
> > need for this and to make the meaning of the constant more clear?

Sure, I'll rename it.


> On Sept. 29, 2015, 7:12 p.m., Ben Mahler wrote:
> > src/tests/fault_tolerance_tests.cpp, lines 620-621
> > 
> >
> > Don't think you need these comments, we should just 
> > s/REGISTRATION_BACKOFF_FACTOR/DEFAULT_REGISTRATION_BACKOFF_FACTOR/ to be 
> > more explicit about it. We've started doing this for some of our constants 
> > already, but need to continue! :)

Yep, makes sense.


> On Sept. 29, 2015, 7:12 p.m., Ben Mahler wrote:
> > src/tests/fault_tolerance_tests.cpp, lines 1547-1551
> > 
> >
> > Ditto here, it seems like if we use DEFAULT in the name, we don't have 
> > to bother setting it here, thoughts?

Default `CreateSlaveFlags()` may tinker with this flag (and it actually does), 
therefore I would like to restore the default value anyway to ensure we advance 
the clock for the correct value.


- Alexander


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


On Sept. 28, 2015, 5:08 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38161/
> ---
> 
> (Updated Sept. 28, 2015, 5:08 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> c97bc4691f9bac4a8677e6d2247be96ee9674b57 
> 
> Diff: https://reviews.apache.org/r/38161/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Alexander Rukletsov]

> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote:
> > One high level suggestion.
> > 
> > After looking at our http code, I realized that we use the same pattern 
> > again and again, for example:
> > ```
> > JSON::Array array;
> > array.values.reserve(status.network_infos().size()); // MESOS-2353.
> > foreach (const NetworkInfo& info, status.network_infos()) {
> >   array.values.push_back(model(info));
> > }
> > object.values["network_infos"] = std::move(array);
> > ```
> > We cannot use newly added `JSON::protobuf()` here, because a different way 
> > for rendering JSON from protobuf is used. Without digging deep inside, I 
> > know three ways how we create a `JSON` out of a proto in our codebase:
> > - wrap in `JSON::Protobuf()` for individual messages;
> > - wrap in one of the `model()` family functions;
> > - pass as it is for built-in types.
> > 
> > The proposed conversion function covers one of the possible ways. How about 
> > add one more convertion? Something like:
> > ```
> > template 
> > Array protobuf(const google::protobuf::RepeatedPtrField& repeated,
> > const lambda::function(const T&)>& converter)
> > {
> >   static_assert(std::is_convertible::value,
> > "T must be a google::protobuf::Message");
> >   JSON::Array array;
> >   array.values.reserve(repeated.size());
> >   foreach (const T& elem, repeated) {
> > array.values.push_back(converter(elem));
> >   }
> >   
> >   return array;
> > }
> > ```
> > 
> > Then the snippet above could be rewritten as:
> > ```
> > object.values["network_infos"] = 
> > std::move(JSON::protobuf(status.network_infos(), [](const NetworkInfo& 
> > info) { return model(info); });
> > ```
> > 
> > A further improvement would be to accept any iterable collection, not only 
> > `RepeatedPtrField<>`, for example `hashset`.
> > 
> > What do you think?
> 
> Klaus Ma wrote:
> Awesome! I've also try similar proposal, but failed when `function` 
> converting with `template`; your suggestion using lambda is great!
> For the `hashset`, I'd suggest to address it when we have such case in 
> our code :).
> 
> I'll also address your comments above.

We do have such cases in our codebase ; ). Here are a few as an example:
- https://github.com/apache/mesos/blob/master/src/master/http.cpp#L217
- https://github.com/apache/mesos/blob/master/src/master/http.cpp#L229


- Alexander


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


On Sept. 27, 2015, 1:34 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Sept. 27, 2015, 1:34 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Alexander Rukletsov]

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


I like when the code is pushed to `.cpp` files : ). Just to confirm that this 
is our intention: with the prvious design, `RoleSorter` and `FrameworkSorter` 
could be anything (well, anything that can be used as a sorter), now we 
restrict sorters to subclass `Sorter`. I think this is fine, but would like to 
confirm with you.

A high level question I have: why do you like to preserve template approach? I 
think we can require allocator writers to provide a factory with their own 
allocator and get rid of templates completely. What do you think?


src/master/allocator/mesos/hierarchical.hpp 


We should leave at least
```
#include 
#include 
```
here.



src/master/allocator/mesos/hierarchical.hpp (line 22)


I think `` should be here. Alternatively, 
`HierarchicalAllocatorProcess`'s c-tor can be moved into `.cpp`.

Also, though `` pulls ``, I think it 
should be included explicitly.



src/master/allocator/mesos/hierarchical.hpp 


`` should stay here.



src/master/allocator/mesos/hierarchical.hpp (line 28)


I think `` should be included here.



src/master/allocator/mesos/hierarchical.hpp (line 62)


virtual d-tor?



src/master/allocator/mesos/hierarchical.hpp (lines 91 - 95)


Why have you decided to leave the c-tor here and not in the cpp file?



src/master/allocator/mesos/hierarchical.cpp (line 33)


remove this newline.



src/master/allocator/mesos/hierarchical.cpp (lines 131 - 136)


How about pulling this code to the c-tor? this will allow us not to store a 
copy of `sorterFactory` in allocator.


- Alexander Rukletsov


On Sept. 30, 2015, 1:08 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38000: Added an API for libprocess users to interact with http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Sept. 30, 2015, 10:21 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Adds functions which allows libprocess users to add handlers which require 
authentication as well as functions to install and remove authenticators.

Includes tests.


Diffs (updated)
-

  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 
8b086f296c80a43be2edaf496a04dadf0c64251a 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/tests/http_tests.cpp 
c380f356548cf9f5491044bccabcd9c66ad5f55a 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

[Alexander Rojas]

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

(Updated Sept. 30, 2015, 10:21 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
c380f356548cf9f5491044bccabcd9c66ad5f55a 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37999: Implemented http::AuthenticatorManager

[Alexander Rojas]

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

(Updated Sept. 30, 2015, 10:20 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Introduces the authenticator manager, which is a class which handles the actual 
authentication procedure during the execution of `ProcessManager::handle()` and 
it also takes care of the life cycle of instances of http::Authenticator.

No tests are added at this point since no public API is generated, the goal of 
this patch is to implement the manager and verify nothing breaks afterwards. 
Authenticator manager tests proper come in a latter patch.


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/include/process/http.hpp 
ba3f0bc7df33795e332c374fbad04106b9d56416 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37998: Made ProcessManager::handle() a void returning method.

[Alexander Rojas]

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

(Updated Sept. 30, 2015, 10:18 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

`ProcessManager::handle()` is a method used to process HTTP requests, its 
signature returns a boolean which is not used anywhere and it doesn't allow for 
asynchronous operations within handle.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37997: Running InheritanceTree tests from libprocess Makefile

[Alexander Rojas]

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

(Updated Sept. 30, 2015, 10:18 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Change in PathInheritedProperties -> InheritanceTree.


Summary (updated)
-

Running InheritanceTree tests from libprocess Makefile


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


Repository: mesos


Description
---

Stout tests are actually ran from libprocess makefile, which requires new tests 
to be split in two commits.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
e64e3d9e958fab3c7c25fad17dcc2b279d255500 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 37996: Added property manager

[Alexander Rojas]

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

(Updated Sept. 30, 2015, 10:16 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Changes the name of the property manager to InheritanceTree. Rebases.


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


Repository: mesos


Description (updated)
---

Introduces the `InheritanceTree` class which allows to create a tree where 
nodes can be tag with some property. This property is then inherited by 
children nodes.

Two behaviors are implemented, overriding, i.e. Adding a property to the child 
node of another node with a property already will result in the ancestor 
property being lost. The second behavior, accumulating, takes a function and 
accumulates
properties of all ancestors.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
76e1674e08bbe65a4fdf86731823a61f231d6d12 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 
  3rdparty/libprocess/3rdparty/stout/tests/properties_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38883: Removed calls to apply-review.sh script. Added support for amending commit messages.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38705, 38883]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 6:18 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38883/
> ---
> 
> (Updated Sept. 30, 2015, 6:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38883/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>