Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38577: Added synchronous validation for Call in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 8 a.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Added validation for Call protobuf message in Agent /api/v1/executor endpoint.


Diffs (updated)
-

  src/Makefile.am 8aa4566 
  src/slave/http.cpp cddc8ad 
  src/slave/validation.hpp PRE-CREATION 
  src/slave/validation.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

[Adam B]

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

Ship it!


This looks great! Thanks for the clarifications. I have a few suggestions, and 
if you're willing to integrate them, I'll go ahead and get this committed.

Also, I notice your "Testing Done" section is blank. ;) Did you try rendering 
this patch to see if the lists/indents still render properly? I strongly 
recommend using https://github.com/mesosphere/mesos-website-container


docs/submitting-a-patch.md (line 18)


Let's add a link to a query filtered to only Accepted issues.

https://issues.apache.org/jira/browse/MESOS-1?jql=project%20%3D%20MESOS%20AND%20status%20%3D%20Accepted
And mention the 'newbie' label, in case they want a good starter task.

https://issues.apache.org/jira/browse/MESOS-1?jql=project%20%3D%20MESOS%20AND%20status%20%3D%20Accepted%20AND%20labels%20%3D%20newbie



docs/submitting-a-patch.md (line 45)


Note that running mesos-tests does not recompile the tests, so you may want 
to first run `GTEST_FILTER="" make check` to recompile the tests without 
running any tests.



docs/submitting-a-patch.md (line 64)


While we're at it, let's add a recommendation for "Testing Done":
11. Under "Testing Done", explain what new tests you have created, what 
tests were modified, and what procedures you went through to test your changes.


- Adam B


On Sept. 24, 2015, 2:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38468/
> ---
> 
> (Updated Sept. 24, 2015, 2:29 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added suggestion that new contributors seek to find a shepherd for their work
> before their begin writing code. Made a few other improvements.
> 
> 
> Diffs
> -
> 
>   docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
>   docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 
> 
> Diff: https://reviews.apache.org/r/38468/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 8:08 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

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

[Alexander Rukletsov]

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



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 621)


Not yours, but while you're editing here, can you `.reserve()` please?



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 638)


ditto



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (lines 760 - 761)


I've seen your discussion with @Jan above, here is what I think regarding 
`ABORT` vs. `Try<>`. 

First off, some background. We tend to use `ABORT()` and `CHECK_*` macros 
when so-called "internal invariant" is violated, for example an object is being 
used without proper initialization, or a change is being made to an instance, 
that does not exists any more. On the other side `Try<>` and `Error<>` family 
is used when our expectation about the outer world does not hold, or, in other 
words, when an action cannot be completed, but no internal invariant is broken. 
User input, I/O, network operations are good examples of the latter case.

Let's try to figure out what we have here. I would say, it's an internal 
invariant, and here is why. JSON is less strict as Protobuf, therefore 
conversion Protobuf->JSON always exists (note that this is not symmetrical, 
JSON->Protobuf is not always possible, hence we use `Try<>` in e.g. 
`protobuf::parse<>()`). The only reason it may fail (remember we do not handle 
OOM exceptions) is because we convert a protobuf message of a yet unknown 
format (most probably future proto versions), which is a violation of an 
internal invariant!

Therefore I would suggest we keep `ABORT()` but document in the preambular 
comment why we use `ABORT()` and not `Try<>`, explaining what internal 
invariant is in this case.


- Alexander Rukletsov


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 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Alexander Rukletsov]

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



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (line 776)


How about `.emplace_back()` here?


- Alexander Rukletsov


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 38335: Add JSON::protobuf for google::protobuf::RepeatedPtrField

[Alexander Rukletsov]

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



src/master/http.cpp (lines 261 - 262)


Fits in one line.



src/tests/mesos.hpp (line 1844)


We tend not to use `typedef`s in the codebase. However, this looks like a 
good idea, but I would place it in `Resources`, e.g. `Resources::ProtoType`.

However, I'll leave the decision whether to use `typedef` or not to you and 
your shepherd.



src/tests/reservation_endpoints_tests.cpp (line 105)


How about `static_cast`? It's more explicit. Here and everywhere else.


- Alexander Rukletsov


On Sept. 24, 2015, 2:43 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38335/
> ---
> 
> (Updated Sept. 24, 2015, 2:43 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> 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
> -
> 
>   src/common/http.cpp 99b843a 
>   src/docker/executor.cpp 1e49013 
>   src/examples/persistent_volume_framework.cpp 426d338 
>   src/launcher/executor.cpp 50b3c6e 
>   src/master/contender.cpp 67562f1 
>   src/master/http.cpp cd37c91 
>   src/master/maintenance.cpp 5fe9358 
>   src/master/registrar.cpp d81560a 
>   src/slave/containerizer/fetcher.cpp 2b2298c 
>   src/slave/containerizer/mesos/containerizer.cpp b904b2d 
>   src/slave/monitor.cpp 8d8b422 
>   src/tests/containerizer/launch_tests.cpp de655ec 
>   src/tests/fault_tolerance_tests.cpp c97bc46 
>   src/tests/http_api_tests.cpp 7938bbb 
>   src/tests/master_maintenance_tests.cpp 89ad138 
>   src/tests/master_tests.cpp f26344d 
>   src/tests/mesos.hpp 3e58b45 
>   src/tests/mesos.cpp ab2d85b 
>   src/tests/monitor_tests.cpp 583e711 
>   src/tests/reservation_endpoints_tests.cpp 398a2e1 
>   src/tests/resources_tests.cpp 6584fc6 
>   src/tests/script.cpp bcc1fab 
>   src/tests/slave_tests.cpp dccdbb0 
>   src/usage/main.cpp 86fd796 
> 
> Diff: https://reviews.apache.org/r/38335/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

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

[Alexander Rukletsov]

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


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?

- Alexander Rukletsov


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 38577: Added synchronous validation for Call in Agent

[Alex Clemmer]

> On Sept. 25, 2015, 6:52 p.m., Vinod Kone wrote:
> > src/Makefile.am, line 501
> > 
> >
> > I think you'll make Alex happy if you also update the corresponding 
> > CMake file(s).
> 
> Isabel Jimenez wrote:
> We don't have cmake support for the agent yet.
> 
> Alex Clemmer wrote:
> I'm pleased to report that we do, actually! Could we please put this line 
> in the `CMakeLists.txt` found in `src/`.

(I should mention that the reason I didn't appear earlier to remind you is 
because this was only checked in a day or so ago.)


- Alex


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


On Sept. 29, 2015, 8 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38577/
> ---
> 
> (Updated Sept. 29, 2015, 8 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added validation for Call protobuf message in Agent /api/v1/executor endpoint.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa4566 
>   src/slave/http.cpp cddc8ad 
>   src/slave/validation.hpp PRE-CREATION 
>   src/slave/validation.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38577/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38837: CMake: Disable agent build on Windows.

[Alex Clemmer]

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

(Updated Sept. 29, 2015, 4:49 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Summary (updated)
-

CMake: Disable agent build on Windows.


Repository: mesos


Description
---

CMake: Disable slave build on Windows.


Diffs
-

  CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 38837: CMake: Disable agent build on Windows.

[Joseph Wu]

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


Can you explain why you're disabling this part?  (Is it simply not implemented 
yet, so you're temporarily skipping it?)

- Joseph Wu


On Sept. 29, 2015, 9:49 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38837/
> ---
> 
> (Updated Sept. 29, 2015, 9:49 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Disable slave build on Windows.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 
> 
> Diff: https://reviews.apache.org/r/38837/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38828: docker: containerizer: fixed double-closing of STDIN in error handling.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Sept. 29, 2015, 12:50 a.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38828/
> ---
> 
> (Updated Sept. 29, 2015, 12:50 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The slave will destroy executors that are not launched successfully, which 
> will
> close all open file descriptors, thus the extra close is not needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp efa37266368ee12fea9134b35ebc5047a2820f94 
> 
> Diff: https://reviews.apache.org/r/38828/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38827: CMake: Expanded support for compiling much of the master.

[Joseph Wu]

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

Ship it!


Tested on OSX + Clang 7.0.

Confirmed that some of the new files were compiled.  i.e. (many lines omitted):
```
Building CXX object src/CMakeFiles/mesos-0.26.0.dir/hook/manager.cpp.o
Building CXX object src/CMakeFiles/mesos-0.26.0.dir/master/maintenance.cpp.o
Building CXX object src/CMakeFiles/mesos-0.26.0.dir/scheduler/scheduler.cpp.o
...
```

- Joseph Wu


On Sept. 28, 2015, 3:56 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38827/
> ---
> 
> (Updated Sept. 28, 2015, 3:56 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Expanded support for compiling much of the master.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 98a08ee99299c873577b0b2a93a153f82ef75b29 
>   src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
> 
> Diff: https://reviews.apache.org/r/38827/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38577: Added synchronous validation for Call in Agent

[Alex Clemmer]

> On Sept. 25, 2015, 6:52 p.m., Vinod Kone wrote:
> > src/Makefile.am, line 501
> > 
> >
> > I think you'll make Alex happy if you also update the corresponding 
> > CMake file(s).
> 
> Isabel Jimenez wrote:
> We don't have cmake support for the agent yet.

I'm pleased to report that we do, actually! Could we please put this line in 
the `CMakeLists.txt` found in `src/`.


- Alex


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


On Sept. 29, 2015, 8 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38577/
> ---
> 
> (Updated Sept. 29, 2015, 8 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added validation for Call protobuf message in Agent /api/v1/executor endpoint.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa4566 
>   src/slave/http.cpp cddc8ad 
>   src/slave/validation.hpp PRE-CREATION 
>   src/slave/validation.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38577/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38826: CMake: Added support for libevent for Unix builds.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 28, 2015, 3:58 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38826/
> ---
> 
> (Updated Sept. 28, 2015, 3:58 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3543
> https://issues.apache.org/jira/browse/MESOS-3543
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added support for libevent for Unix builds.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 
>   src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
>   src/slave/cmake/SlaveConfigure.cmake 
> 230e574f3ddc6aa12cc74105de7201a33dd7cdab 
> 
> Diff: https://reviews.apache.org/r/38826/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

[Jie Yu]

> On Sept. 29, 2015, 8:46 p.m., Ian Downes wrote:
> > src/tests/containerizer/fs_tests.cpp, line 203
> > 
> >
> > So the mount won't get cleaned up if the test asserts early?

We have a universal cleanup code here:
https://github.com/apache/mesos/blob/master/src/tests/environment.cpp#L458

So the mount will be cleaned up properly even if test asserts early.


- Jie


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


On Sept. 29, 2015, 7:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38452: Updated C++ style guide for namespace usage

[Guangya Liu]

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

(Updated 九月 29, 2015, 11:04 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Add "Scoping" section according to Ben's comments


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


Repository: mesos


Description
---

Updated C++ style guide for namespace usage


Diffs (updated)
-

  docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

[Ian Downes]

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

Ship it!



src/tests/containerizer/fs_tests.cpp (line 198)


Can you get the mount id before you do the mount and then verify the shared 
sibling id is correct? This might be easiest to do with a second self bind 
mount so you can find the sibling.



src/tests/containerizer/fs_tests.cpp (line 200)


s/Cleanup/Clean up/



src/tests/containerizer/fs_tests.cpp (line 201)


So the mount won't get cleaned up if the test asserts early?



src/tests/containerizer/fs_tests.cpp (line 241)


Like above, can you verify the master id is correct?



src/tests/containerizer/fs_tests.cpp (line 243)


s/Cleanup/Clean up/


- Ian Downes


On Sept. 29, 2015, 12:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 12:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

[Jie Yu]

> On Sept. 29, 2015, 9:20 p.m., Jiang Yan Xu wrote:
> > src/linux/fs.hpp, line 174
> > 
> >
> > s/resides/resides in/

'in which' (in is already there).


- Jie


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


On Sept. 29, 2015, 7:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38858: Ensured that slave's work_dir is a shared mount in its own peer group when LinuxFilesystemIsolator is used.

[Jiang Yan Xu]

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

Ship it!


Modulo Ian's comments.


src/slave/containerizer/isolators/filesystem/linux.cpp (line 144)


s/group/groups/


- Jiang Yan Xu


On Sept. 29, 2015, 12:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38858/
> ---
> 
> (Updated Sept. 29, 2015, 12:48 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that slave's work_dir is a shared mount in its own peer group when 
> LinuxFilesystemIsolator is used.
> 
> This addressed the TODO in the current code.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8fa929f12838044762be82287e1b714406d9 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 41d098c7d66eba0befc3708c6660a6a3e5f0d2de 
> 
> Diff: https://reviews.apache.org/r/38858/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38860: Linked more doc pages from home.md.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38860]

All tests passed.

- Mesos ReviewBot


On Sept. 29, 2015, 8:37 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38860/
> ---
> 
> (Updated Sept. 29, 2015, 8:37 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also made various cleanups and fixes throughout the docs.
> 
> 
> Diffs
> -
> 
>   docs/allocation-module.md 4ba5ba8c27cccebf0e5fc5064fdc1d6af05d38f0 
>   docs/app-framework-development-guide.md 
> bada7fea8af545f18876cae584f67c9422f45eaf 
>   docs/attributes-resources.md 1ea5dde7cf32af5a0ab93510642863cc952f9780 
>   docs/deploy-scripts.md 58b3a69042746ccdc6515ad652eac50b9c67e8eb 
>   docs/fetcher-cache-internals.md d17b41e7113ce72679734fc0bed16614011b6917 
>   docs/fetcher.md 638f5305a24fe7dbf459c937872a295e8edd5d44 
>   docs/home.md f8c2b62c486dfbce310705de51038247701cf54a 
>   docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 
>   docs/oversubscription.md b1a6c9979025ce33190792a07b8e9618c518430e 
>   docs/persistent-volume.md ae5b0e53997c9cb2a8bab22e0109e6768124f82b 
>   docs/reconciliation.md b50d692d415cf677d23051fdd8a2fddbd4c4d134 
>   docs/scheduler_http_api.md cfac5105109519dcb275e50671906382960ff39b 
>   docs/tools.md 09619f2c162b1765e7718fc314e77b0f77148b97 
> 
> Diff: https://reviews.apache.org/r/38860/diff/
> 
> 
> Testing
> ---
> 
> Previewed via https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38452: Updated C++ style guide for namespace usage

[Ben Mahler]

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

Ship it!


Thanks! I'll get this committed shortly.


docs/mesos-c++-style-guide.md (line 11)


s/##/###/ here


- Ben Mahler


On Sept. 29, 2015, 11:43 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38452/
> ---
> 
> (Updated Sept. 29, 2015, 11:43 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3450
> https://issues.apache.org/jira/browse/MESOS-3450
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated C++ style guide for namespace usage
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 
> 
> Diff: https://reviews.apache.org/r/38452/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38858: Ensured that slave's work_dir is a shared mount in its own peer group when LinuxFilesystemIsolator is used.

[Ian Downes]

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



src/slave/containerizer/isolators/filesystem/linux.cpp (lines 88 - 94)


This has appeared in multiple places, perhaps refactor this into a helper?



src/slave/containerizer/isolators/filesystem/linux.cpp (line 121)


Suggestion, refactor to avoid the nesting?

if (workDirMount.isNone() {
  // mount it and get the fs::MountInfoTable::Entry
}

// Check it's a slave mount.

// Check it's a shared mount.



src/slave/containerizer/isolators/filesystem/linux.cpp (line 126)


If it's already mounted (so it's in /etc/mtab) there's no reason to shell 
out; you can use fs::mount().



src/slave/containerizer/isolators/filesystem/linux.cpp (line 146)


If it's already mounted (so it's in /etc/mtab) there's no reason to shell 
out; you can use fs::mount().



src/tests/containerizer/filesystem_isolator_tests.cpp (lines 926 - 930)


This doesn't verify the work_dir *is* bind mounted, just that *if* it is 
then it's a shared mount...



src/tests/containerizer/filesystem_isolator_tests.cpp (lines 949 - 953)


Ditto.


- Ian Downes


On Sept. 29, 2015, 12:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38858/
> ---
> 
> (Updated Sept. 29, 2015, 12:48 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that slave's work_dir is a shared mount in its own peer group when 
> LinuxFilesystemIsolator is used.
> 
> This addressed the TODO in the current code.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8fa929f12838044762be82287e1b714406d9 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 41d098c7d66eba0befc3708c6660a6a3e5f0d2de 
> 
> Diff: https://reviews.apache.org/r/38858/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38577: Added synchronous validation for Call in Agent

[Isabel Jimenez]

> On Sept. 29, 2015, 8:29 p.m., Anand Mazumdar wrote:
> > src/slave/validation.cpp, line 57
> > 
> >
> > Sorry for the ping pong iterations on this but noticed another missing 
> > validation check for `Call::UPDATE` while working on MESOS-3480.
> > 
> > Can we also check if `TaskState` is not `TASK_STAGING` and return an 
> > validation error if so ?
> > 
> > https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L2838

This introduces a change of behavior, I'll reflect this on the design Doc too.


- Isabel


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


On Sept. 29, 2015, 8:15 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38577/
> ---
> 
> (Updated Sept. 29, 2015, 8:15 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added validation for Call protobuf message in Agent /api/v1/executor endpoint.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 891f951 
>   src/Makefile.am 8aa4566 
>   src/slave/http.cpp f9cf7bb 
>   src/slave/validation.hpp PRE-CREATION 
>   src/slave/validation.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38577/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Joris Van Remoortere]

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



3rdparty/libprocess/src/socket.cpp (lines 79 - 81)


We're using `s.get()` above here, so `owned` should never be `true`?



3rdparty/libprocess/src/socket.cpp (lines 91 - 93)


We're using `s.get()` above here, so `owned` should never be `true`?


- Joris Van Remoortere


On Sept. 29, 2015, 8:30 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38809/
> ---
> 
> (Updated Sept. 29, 2015, 8:30 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3519
> https://issues.apache.org/jira/browse/MESOS-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> socket: refactor to use Option and fix file descriptor leaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 817cb37 
>   3rdparty/libprocess/src/socket.cpp 5879423 
> 
> Diff: https://reviews.apache.org/r/38809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Review Request 38867: Added drop message method in Agent

[Isabel Jimenez]

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

Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.


Repository: mesos


Description
---

We need a drop message method in Agent, similar to the one existing in Master.


Diffs
-

  src/slave/slave.hpp 18be4f8 
  src/slave/slave.cpp d1c9977 

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


Testing
---

make


Thanks,

Isabel Jimenez

Re: Review Request 37540: Add perf event API

[Cong Wang]

> On Sept. 25, 2015, 9:53 p.m., Vinod Kone wrote:
> > src/linux/perf.cpp, line 792
> > 
> >
> > so the semantics of this read() are a bit confusing.
> > 
> > if the caller calls the read() back to back, they will get the same 
> > event if the first io::poll() is still running? but they will get a 
> > different event if io::poll() has finished before the 2nd read() is called?

That is correct. I want to ensure two different threads can get the same result 
when they read from the same perf event handler.


- Cong


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


On Sept. 18, 2015, 10:11 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37540/
> ---
> 
> (Updated Sept. 18, 2015, 10:11 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Abstract Linux kernel perf event API and provide API to collect schedule 
> events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
>   src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
>   src/tests/containerizer/perf_tests.cpp 
> ed5212ee31b8aa47339b8b8fab184bbdf85be82a 
> 
> Diff: https://reviews.apache.org/r/37540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

[Jie Yu]

> On Sept. 29, 2015, 9:09 p.m., Timothy Chen wrote:
> > src/linux/fs.cpp, line 156
> > 
> >
> > Do you know in what situations will the conversion fail? Also for 
> > master: as well.
> > I wonder if we should return Result instead of ignoring the error.

It shouldn't unless the kernel has a bug. Maybe a CHECK is more appropriate?


- Jie


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


On Sept. 29, 2015, 7:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38577: Added synchronous validation for Call in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 9:25 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Added validation for Call protobuf message in Agent /api/v1/executor endpoint.


Diffs (updated)
-

  src/CMakeLists.txt 891f951 
  src/Makefile.am 8aa4566 
  src/slave/http.cpp f9cf7bb 
  src/slave/validation.hpp PRE-CREATION 
  src/slave/validation.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Chi Zhang]

> On Sept. 29, 2015, 9:42 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/socket.cpp, lines 79-81
> > 
> >
> > We're using `s.get()` above here, so `owned` should never be `true`?

owned is determined upon entering the function? (we just reuse the variable 
name if s is none)


- Chi


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


On Sept. 29, 2015, 8:30 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38809/
> ---
> 
> (Updated Sept. 29, 2015, 8:30 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3519
> https://issues.apache.org/jira/browse/MESOS-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> socket: refactor to use Option and fix file descriptor leaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 817cb37 
>   3rdparty/libprocess/src/socket.cpp 5879423 
> 
> Diff: https://reviews.apache.org/r/38809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38452: Updated C++ style guide for namespace usage

[Ben Mahler]

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

Ship it!


Thanks for your patience! Just a couple of cleanups and I can get this 
committed for you.


docs/mesos-c++-style-guide.md (lines 41 - 45)


To mirror the google style guide, can we place this before 'Naming'?



docs/mesos-c++-style-guide.md (line 45)


Add the following trailing sentence:

```
... you want to use. These should only be present at the top of the .cpp 
file.
```


- Ben Mahler


On Sept. 29, 2015, 11:04 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38452/
> ---
> 
> (Updated Sept. 29, 2015, 11:04 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3450
> https://issues.apache.org/jira/browse/MESOS-3450
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated C++ style guide for namespace usage
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 
> 
> Diff: https://reviews.apache.org/r/38452/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38452: Updated C++ style guide for namespace usage

[Guangya Liu]

> On 九月 29, 2015, 11:30 p.m., Ben Mahler wrote:
> > docs/mesos-c++-style-guide.md, lines 41-45
> > 
> >
> > To mirror the google style guide, can we place this before 'Naming'?

Done


> On 九月 29, 2015, 11:30 p.m., Ben Mahler wrote:
> > docs/mesos-c++-style-guide.md, line 45
> > 
> >
> > Add the following trailing sentence:
> > 
> > ```
> > ... you want to use. These should only be present at the top of the 
> > .cpp file.
> > ```

Done


- Guangya


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


On 九月 29, 2015, 11:43 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38452/
> ---
> 
> (Updated 九月 29, 2015, 11:43 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3450
> https://issues.apache.org/jira/browse/MESOS-3450
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated C++ style guide for namespace usage
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 
> 
> Diff: https://reviews.apache.org/r/38452/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38452: Updated C++ style guide for namespace usage

[Guangya Liu]

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

(Updated 九月 29, 2015, 11:43 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Updated C++ style guide for namespace usage


Diffs (updated)
-

  docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38618, 38577, 38844]

All tests passed.

- Mesos ReviewBot


On Sept. 29, 2015, 9:27 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> ---
> 
> (Updated Sept. 29, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 37540: Add perf event API

[Cong Wang]

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

(Updated Sept. 30, 2015, 12:12 a.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Address review comments


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


Repository: mesos


Description
---

Abstract Linux kernel perf event API and provide API to collect schedule events.


Diffs (updated)
-

  src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
  src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
  src/tests/containerizer/perf_tests.cpp 
ed5212ee31b8aa47339b8b8fab184bbdf85be82a 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

[Jie Yu]

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

(Updated Sept. 29, 2015, 11:12 p.m.)


Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Added support for getting shared and slave mount peer group ID.


Diffs (updated)
-

  src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
  src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
  src/tests/containerizer/fs_tests.cpp 34d3c41045373326fb6096031cd1daef0e7b203a 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38858: Ensured that slave's work_dir is a shared mount in its own peer group when LinuxFilesystemIsolator is used.

[Jie Yu]

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

(Updated Sept. 29, 2015, 11:13 p.m.)


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


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Ensured that slave's work_dir is a shared mount in its own peer group when 
LinuxFilesystemIsolator is used.

This addressed the TODO in the current code.


Diffs (updated)
-

  src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8fa929f12838044762be82287e1b714406d9 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
41d098c7d66eba0befc3708c6660a6a3e5f0d2de 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38858: Ensured that slave's work_dir is a shared mount in its own peer group when LinuxFilesystemIsolator is used.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38855, 38858]

All tests passed.

- Mesos ReviewBot


On Sept. 29, 2015, 7:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38858/
> ---
> 
> (Updated Sept. 29, 2015, 7:48 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that slave's work_dir is a shared mount in its own peer group when 
> LinuxFilesystemIsolator is used.
> 
> This addressed the TODO in the current code.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8fa929f12838044762be82287e1b714406d9 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 41d098c7d66eba0befc3708c6660a6a3e5f0d2de 
> 
> Diff: https://reviews.apache.org/r/38858/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 9:27 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Adding test for invalid status update 'TASK_STAGING'


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38747: Adding digest utilities

[Timothy Chen]

> On Sept. 29, 2015, 9:35 p.m., Ben Mahler wrote:
> > Tim, could you also get a libprocess maintainer to review this?

Volunteered you :)


- Timothy


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


On Sept. 29, 2015, 6:41 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 29, 2015, 6:41 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256, SHA512 implementation.
> 
> 
> Diffs
> -
> 
>   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 38074: Calculate schedule latency with trace events

[Cong Wang]

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

(Updated Sept. 30, 2015, 12:15 a.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Cleanup and rebase


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


Repository: mesos


Description
---

Finally, calculate schedule latency with the sched trace events, and add it to 
our statistics


Diffs (updated)
-

  include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp 
1f722ef3ef7ab7fce5542d4affae961d6cec2406 
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
03035dfbfb84470ba39ed9ecfd1eb73890e3f784 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 

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


Testing
---

manual tests


Thanks,

Cong Wang

Re: Review Request 37541: Add trace event API

[Cong Wang]

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

(Updated Sept. 30, 2015, 12:14 a.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Cleanup and rebase


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs (updated)
-

  src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
  src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/perf_tests.cpp 
ed5212ee31b8aa47339b8b8fab184bbdf85be82a 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

[Jiang Yan Xu]

> On Sept. 29, 2015, 2:09 p.m., Timothy Chen wrote:
> > src/linux/fs.cpp, line 156
> > 
> >
> > Do you know in what situations will the conversion fail? Also for 
> > master: as well.
> > I wonder if we should return Result instead of ignoring the error.
> 
> Jie Yu wrote:
> It shouldn't unless the kernel has a bug. Maybe a CHECK is more 
> appropriate?

+1 for CHECK


- Jiang Yan


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


On Sept. 29, 2015, 12:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 12:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Joris Van Remoortere]

> On Sept. 29, 2015, 9:42 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/socket.cpp, lines 79-81
> > 
> >
> > We're using `s.get()` above here, so `owned` should never be `true`?
> 
> Chi Zhang wrote:
> owned is determined upon entering the function? (we just reuse the 
> variable name if s is none)

I see, it gets re-assigned.


- Joris


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


On Sept. 29, 2015, 8:30 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38809/
> ---
> 
> (Updated Sept. 29, 2015, 8:30 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3519
> https://issues.apache.org/jira/browse/MESOS-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> socket: refactor to use Option and fix file descriptor leaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 817cb37 
>   3rdparty/libprocess/src/socket.cpp 5879423 
> 
> Diff: https://reviews.apache.org/r/38809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

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

[Joris Van Remoortere]

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

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 38855: Added support for getting shared and slave mount peer group ID.

[Timothy Chen]

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



src/linux/fs.cpp (line 156)


Do you know in what situations will the conversion fail? Also for master: 
as well.
I wonder if we should return Result instead of ignoring the error.


- Timothy Chen


On Sept. 29, 2015, 7:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

[Jiang Yan Xu]

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

Ship it!


Modulo


src/linux/fs.hpp (line 174)


s/resides/resides in/


- Jiang Yan Xu


On Sept. 29, 2015, 12:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 12:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38747: Adding digest utilities

[Ben Mahler]

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


Tim, could you also get a libprocess maintainer to review this?

- Ben Mahler


On Sept. 29, 2015, 6:41 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> ---
> 
> (Updated Sept. 29, 2015, 6:41 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHA256, SHA512 implementation.
> 
> 
> Diffs
> -
> 
>   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 38837: CMake: Disable agent build on Windows.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38825, 38826, 38827, 38837]

All tests passed.

- Mesos ReviewBot


On Sept. 29, 2015, 8:21 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38837/
> ---
> 
> (Updated Sept. 29, 2015, 8:21 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Disable slave build on Windows.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 
> 
> Diff: https://reviews.apache.org/r/38837/diff/
> 
> 
> Testing
> ---
> 
> # On OS X 10.10, Ubuntu 14.04, Ubuntu 15.04
> 
> cmake .. -DENABLE_LIBEVENT=1 && make -j 8
> cmake .. && make -j 8
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38858: Ensured that slave's work_dir is a shared mount in its own peer group when LinuxFilesystemIsolator is used.

[Jie Yu]

> On Sept. 29, 2015, 9:04 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 126
> > 
> >
> > If it's already mounted (so it's in /etc/mtab) there's no reason to 
> > shell out; you can use fs::mount().

True, but given that we already shell out for the bind mount, I just want to be 
consistent.


- Jie


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


On Sept. 29, 2015, 7:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38858/
> ---
> 
> (Updated Sept. 29, 2015, 7:48 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that slave's work_dir is a shared mount in its own peer group when 
> LinuxFilesystemIsolator is used.
> 
> This addressed the TODO in the current code.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8fa929f12838044762be82287e1b714406d9 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 41d098c7d66eba0befc3708c6660a6a3e5f0d2de 
> 
> Diff: https://reviews.apache.org/r/38858/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38858: Ensured that slave's work_dir is a shared mount in its own peer group when LinuxFilesystemIsolator is used.

[Jie Yu]

> On Sept. 29, 2015, 9:04 p.m., Ian Downes wrote:
> > src/tests/containerizer/filesystem_isolator_tests.cpp, lines 926-930
> > 
> >
> > This doesn't verify the work_dir *is* bind mounted, just that *if* it 
> > is then it's a shared mount...

We don't care if it is a bind mount or not. As long as its a shared mount in 
its own peer group, we are ok with that.


- Jie


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


On Sept. 29, 2015, 7:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38858/
> ---
> 
> (Updated Sept. 29, 2015, 7:48 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that slave's work_dir is a shared mount in its own peer group when 
> LinuxFilesystemIsolator is used.
> 
> This addressed the TODO in the current code.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8fa929f12838044762be82287e1b714406d9 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 41d098c7d66eba0befc3708c6660a6a3e5f0d2de 
> 
> Diff: https://reviews.apache.org/r/38858/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38858: Ensured that slave's work_dir is a shared mount in its own peer group when LinuxFilesystemIsolator is used.

[Jie Yu]

> On Sept. 29, 2015, 9:04 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, lines 88-94
> > 
> >
> > This has appeared in multiple places, perhaps refactor this into a 
> > helper?

Yeah, I think having a 'find' method for mount table sounds like a better 
solution. I'll add a TODO here.


- Jie


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


On Sept. 29, 2015, 7:48 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38858/
> ---
> 
> (Updated Sept. 29, 2015, 7:48 p.m.)
> 
> 
> Review request for mesos, Timothy Chen and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured that slave's work_dir is a shared mount in its own peer group when 
> LinuxFilesystemIsolator is used.
> 
> This addressed the TODO in the current code.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8fa929f12838044762be82287e1b714406d9 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 41d098c7d66eba0befc3708c6660a6a3e5f0d2de 
> 
> Diff: https://reviews.apache.org/r/38858/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38855: Added support for getting shared and slave mount peer group ID.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Sept. 29, 2015, 11:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38855/
> ---
> 
> (Updated Sept. 29, 2015, 11:12 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3539
> https://issues.apache.org/jira/browse/MESOS-3539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for getting shared and slave mount peer group ID.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
>   src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
>   src/tests/containerizer/fs_tests.cpp 
> 34d3c41045373326fb6096031cd1daef0e7b203a 
> 
> Diff: https://reviews.apache.org/r/38855/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38870: Fixed a bug related to reviewboard_url option in post-reviews.py.

[Jie Yu]

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

(Updated Sept. 30, 2015, 12:36 a.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description (updated)
---

Fixed a bug related to reviewboard_url option in post-reviews.py.

We append sys.argv[1:] to rbt post, and --reviewboard_url is not a valid option 
for rbt post.


Diffs
-

  support/post-reviews.py 78201ee79dc528a0cfdd0b89cc0a191be3d5846c 

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


Testing
---

Posted this review using
support/post-reviews.py --server=https://reviews.apache.org/


Thanks,

Jie Yu

Review Request 38870: Fixed a bug related to reviewboard_url option in post-reviews.py.

[Jie Yu]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Fixed a bug related to reviewboard_url option in post-reviews.py.


Diffs
-

  support/post-reviews.py 78201ee79dc528a0cfdd0b89cc0a191be3d5846c 

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


Testing
---

Posted this review using
support/post-reviews.py --server=https://reviews.apache.org/


Thanks,

Jie Yu

Re: Review Request 38878: Added test for the Subscribe->Subscribed workflow for the Executor HTTP API

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 九月 30, 2015, 3:40 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38878/
> ---
> 
> (Updated 九月 30, 2015, 3:40 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a basic test to validate the implementation for 
> Subscribe->Subscribed workflow on the `api/v1/executor` endpoint on Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 31938c295367686d444f8a8aa2c43d2696b481ec 
> 
> Diff: https://reviews.apache.org/r/38878/diff/
> 
> 
> Testing
> ---
> 
> make check. Would add more agent recovery tests/executor reconnect tests in a 
> separate patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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(), 
> > ::allocate);
> > 
> > result.await();
> > ```
> > 
> > However; the allocate function would need to return something other 
> > than `void` such as `Nothing` for that to work.

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?


- James


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


On Sept. 25, 2015, 9:46 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38649/
> ---
> 
> (Updated Sept. 25, 2015, 9:46 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 38452: Updated C++ style guide for namespace usage

[Ben Mahler]

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



docs/mesos-c++-style-guide.md (lines 41 - 43)


It looks like we should mirror the structure of Google's C++ style guide 
here to make the differences more clear.

So these belong in: 'Scoping' -> 'Namespaces'

See: 
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Namespaces

Can you add a 'Scoping' section above and have 'Namespaces' as the 
subsection with the content you've added here?


- Ben Mahler


On Sept. 24, 2015, 2:20 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38452/
> ---
> 
> (Updated Sept. 24, 2015, 2:20 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3450
> https://issues.apache.org/jira/browse/MESOS-3450
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated C++ style guide for namespace usage
> 
> 
> Diffs
> -
> 
>   docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 
> 
> Diff: https://reviews.apache.org/r/38452/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

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

[Joris Van Remoortere]

---
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.


Changes
---

moved includes around.


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 (updated)
-

  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 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Klaus Ma]

> On Sept. 29, 2015, 10:43 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h, line 40
> > 
> >
> > Just to confirm: this file is generated using bundled protobuf, right? 
> > Could you please put the `protoc` version in the RR description for 
> > posterity?

Yes, I used `protoc` built from our source code.


> On Sept. 29, 2015, 10:43 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto, line 80
> > 
> >
> > s/MessageArray/ArrayMessage
> > 
> > A high level question: why wouldn't you use `SimpleMessage` instead? 
> > Proto->JSON conversion for a single message is already checked in the 
> > different test, you want to check the conversion for collections. I would 
> > say, using `SimpleMessage` can reduce the amount of test code needed.

Agree, it'll reduce the LOC (line of code).


> On Sept. 29, 2015, 10:43 a.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 773-774
> > 
> >
> > Could you please help me understand why this check is necessary? IIUC, 
> > compiler ensures `T` is a protobuf becasue you pass `elem` of type `T` to 
> > `JSON::protobuf()`.
> > 
> > AFAIK, the only reason to do this check is to prohibit arguments like 
> > `google::protobuf::RepeatedPtrField`.
> >  Do you want to express this? I don't think it is necessary, because 
> > `JSON::Array` filled with `JSON::Array` is fine.
> > 
> > @MPark, what'd you say?

To me, this's a safe guard to make sure this function is not abused.
For 
`google::protobuf::RepeatedPtrField`, I 
think we need to define a message to include 
`google::protobuf::RepeatedPtrField`, the proto file looks like:

message MyMessage
{
message Item {
string value = 1;
}
repeated Item values = 1;
}


- Klaus


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


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.

[Alex Clemmer]

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



src/Makefile.am (line 489)


Note that if review #38827 goes in before this, it would be great to add 
this line to the CMakeLists file in src/


- Alex Clemmer


On Sept. 30, 2015, 12:09 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, 12:09 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 38875: Moved the framework/executor exists check before Subscribe call in Agent

[Guangya Liu]

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



src/slave/http.cpp (line 292)


Can you please add some comments for when this if condition will be hit? I 
see that the scheduler API is not handling such logic.


- Guangya Liu


On 九月 30, 2015, 3:39 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38875/
> ---
> 
> (Updated 九月 30, 2015, 3:39 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored the `api/v1/executor` endpoint to do executor/framework 
> exists(`NULL`) checks before `Subscribe` since the slave explicitly knows the 
> `frameworkID/executorID` when it launches it. Also modified tests to first 
> spawn a dummy `cat` process and then make a `Subscribe` call to ensure that 
> the Agent knows about the Framework/Executor and not reject it via 
> `BadRequest` due to the `NULL` checks.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/tests/executor_http_api_tests.cpp 
> 31938c295367686d444f8a8aa2c43d2696b481ec 
> 
> Diff: https://reviews.apache.org/r/38875/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 38878: Added test for the Subscribe->Subscribed workflow for the Executor HTTP API

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


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


Repository: mesos


Description
---

This change adds a basic test to validate the implementation for 
Subscribe->Subscribed workflow on the `api/v1/executor` endpoint on Agent.


Diffs
-

  src/tests/executor_http_api_tests.cpp 
31938c295367686d444f8a8aa2c43d2696b481ec 

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


Testing
---

make check. Would add more agent recovery tests/executor reconnect tests in a 
separate patch.


Thanks,

Anand Mazumdar

Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


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


Repository: mesos


Description
---

This change adds the functionality for executors to `Subscribe` via the 
`api/v1/executor` endpoint. It also stores a marker file as part of the 
`Subscribe` call if framework `checkpointing` is enabled. This can then be used 
by the agent when recovering to wait for reconnecting back with the executor.

Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
`CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
the `Subscribe` call.


Diffs
-

  src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
  src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 38875: Moved the framework/executor exists check before Subscribe call in Agent

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


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


Repository: mesos


Description
---

Refactored the `api/v1/executor` endpoint to do executor/framework 
exists(`NULL`) checks before `Subscribe` since the slave explicitly knows the 
`frameworkID/executorID` when it launches it. Also modified tests to first 
spawn a dummy `cat` process and then make a `Subscribe` call to ensure that the 
Agent knows about the Framework/Executor and not reject it via `BadRequest` due 
to the `NULL` checks.


Diffs
-

  src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
  src/tests/executor_http_api_tests.cpp 
31938c295367686d444f8a8aa2c43d2696b481ec 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 38876: Added functionality to store a marker file to denote HTTP based executors

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


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


Repository: mesos


Description
---

This change adds the relevant functionality to `src/slave/paths.cpp/hpp` to 
store a marker file to denote HTTP based executors. We create the file when 
`checkpointing` is enabled as part of handling the `Subscribe` request. This is 
then used by the agent when recovering to ascertain if the executor was 
connected via `HTTP` before the agent restart.


Diffs
-

  src/slave/paths.hpp f743fb4b1ca278fade9134e0ae8f6a6450d4a977 
  src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 38874: Refactored executor struct in Agent for the Executor HTTP API

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


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


Repository: mesos


Description
---

This change refactors the Executor struct on Agent and adds support for 
Executors to connect via the `api/v1/executor` endpoint on Agent. This is 
similar to the change done in Master for the Scheduler HTTP API.


Diffs
-

  src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 38870: Fixed a bug related to reviewboard_url option in post-reviews.py.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 30, 2015, 12:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38870/
> ---
> 
> (Updated Sept. 30, 2015, 12:36 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug related to reviewboard_url option in post-reviews.py.
> 
> We append sys.argv[1:] to rbt post, and --reviewboard_url is not a valid 
> option for rbt post.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 78201ee79dc528a0cfdd0b89cc0a191be3d5846c 
> 
> Diff: https://reviews.apache.org/r/38870/diff/
> 
> 
> Testing
> ---
> 
> Posted this review using
> support/post-reviews.py --server=https://reviews.apache.org/
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 38873: Added helper functions for evolving old style executor messages to V1 Executor Events

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


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


Repository: mesos


Description
---

This change implements helper functions for evolving old style Executor 
messages in `src/messages/messages.proto` to V1 Executor Events defined in 
`include/mesos/v1/executor/executor.proto`. This is needed for MESOS-3480


Diffs
-

  src/internal/evolve.hpp 9babac3ccbfb2bf9a3989a3ae20cf96e5f3a2903 
  src/internal/evolve.cpp 625706e089984b32d8298a2eacf2f8af2bca931e 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 38873: Added helper functions for evolving old style executor messages to V1 Executor Events

[Guangya Liu]

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



src/internal/evolve.hpp (line 46)


This should be removed?


- Guangya Liu


On 九月 30, 2015, 3:38 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38873/
> ---
> 
> (Updated 九月 30, 2015, 3:38 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3480
> https://issues.apache.org/jira/browse/MESOS-3480
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change implements helper functions for evolving old style Executor 
> messages in `src/messages/messages.proto` to V1 Executor Events defined in 
> `include/mesos/v1/executor/executor.proto`. This is needed for MESOS-3480
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 9babac3ccbfb2bf9a3989a3ae20cf96e5f3a2903 
>   src/internal/evolve.cpp 625706e089984b32d8298a2eacf2f8af2bca931e 
> 
> Diff: https://reviews.apache.org/r/38873/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38874: Refactored executor struct in Agent for the Executor HTTP API

[Guangya Liu]

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



src/slave/slave.cpp (line 5325)


Can you please add some comments here? Just curious why here the state is 
REGISTERING? Thanks.


- Guangya Liu


On 九月 30, 2015, 3:39 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38874/
> ---
> 
> (Updated 九月 30, 2015, 3:39 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3480
> https://issues.apache.org/jira/browse/MESOS-3480
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change refactors the Executor struct on Agent and adds support for 
> Executors to connect via the `api/v1/executor` endpoint on Agent. This is 
> similar to the change done in Master for the Scheduler HTTP API.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38874/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38074: Calculate schedule latency with trace events

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37540, 37541, 38074]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 12:15 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38074/
> ---
> 
> (Updated Sept. 30, 2015, 12:15 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Finally, calculate schedule latency with the sched trace events, and add it 
> to our statistics
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp 
> 1f722ef3ef7ab7fce5542d4affae961d6cec2406 
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 03035dfbfb84470ba39ed9ecfd1eb73890e3f784 
>   src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
> 
> Diff: https://reviews.apache.org/r/38074/diff/
> 
> 
> Testing
> ---
> 
> manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 38860: Linked more doc pages from home.md.

[Adam B]

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

Ship it!


It took a while to track down some of these 2-character changes in raw RB, but 
I've reviewed and it all looks good. There are a couple of minor changes, but 
I'll just make those before I commit.


docs/deploy-scripts.md (line 16)


s/configure Mesos use/configure Mesos to use/



docs/home.md (line 32)


We might want to move other "advanced" features in here like authn/z, SSL, 
containerizers, rate limiting, network-monitoring, and maintenance. But we can 
do that later, as needed.



docs/home.md (line 55)


Let's move this below the general Mesos Modules link


- Adam B


On Sept. 29, 2015, 1:37 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38860/
> ---
> 
> (Updated Sept. 29, 2015, 1:37 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also made various cleanups and fixes throughout the docs.
> 
> 
> Diffs
> -
> 
>   docs/allocation-module.md 4ba5ba8c27cccebf0e5fc5064fdc1d6af05d38f0 
>   docs/app-framework-development-guide.md 
> bada7fea8af545f18876cae584f67c9422f45eaf 
>   docs/attributes-resources.md 1ea5dde7cf32af5a0ab93510642863cc952f9780 
>   docs/deploy-scripts.md 58b3a69042746ccdc6515ad652eac50b9c67e8eb 
>   docs/fetcher-cache-internals.md d17b41e7113ce72679734fc0bed16614011b6917 
>   docs/fetcher.md 638f5305a24fe7dbf459c937872a295e8edd5d44 
>   docs/home.md f8c2b62c486dfbce310705de51038247701cf54a 
>   docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 
>   docs/oversubscription.md b1a6c9979025ce33190792a07b8e9618c518430e 
>   docs/persistent-volume.md ae5b0e53997c9cb2a8bab22e0109e6768124f82b 
>   docs/reconciliation.md b50d692d415cf677d23051fdd8a2fddbd4c4d134 
>   docs/scheduler_http_api.md cfac5105109519dcb275e50671906382960ff39b 
>   docs/tools.md 09619f2c162b1765e7718fc314e77b0f77148b97 
> 
> Diff: https://reviews.apache.org/r/38860/diff/
> 
> 
> Testing
> ---
> 
> Previewed via https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

[Adam B]

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

Ship it!


Still looks good. I'll commit this now.


docs/submitting-a-patch.md (line 63)


There is no MESOS-01, only MESOS-1.


- Adam B


On Sept. 29, 2015, 11:19 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38468/
> ---
> 
> (Updated Sept. 29, 2015, 11:19 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added suggestion that new contributors seek to find a shepherd for their work
> before their begin writing code. Made a few other improvements.
> 
> 
> Diffs
> -
> 
>   docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
>   docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 
> 
> Diff: https://reviews.apache.org/r/38468/diff/
> 
> 
> Testing
> ---
> 
> Viewed markdown with https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38870: Fixed a bug related to reviewboard_url option in post-reviews.py.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [38870]

Failed command: ./support/apply-review.sh -n -r 38870

Error:
 2015-09-30 02:50:52 URL:https://reviews.apache.org/r/38870/diff/raw/ [954/954] 
-> "38870.patch" [1]
error: patch failed: support/post-reviews.py:98
error: support/post-reviews.py: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 30, 2015, 12:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38870/
> ---
> 
> (Updated Sept. 30, 2015, 12:36 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug related to reviewboard_url option in post-reviews.py.
> 
> We append sys.argv[1:] to rbt post, and --reviewboard_url is not a valid 
> option for rbt post.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 78201ee79dc528a0cfdd0b89cc0a191be3d5846c 
> 
> Diff: https://reviews.apache.org/r/38870/diff/
> 
> 
> Testing
> ---
> 
> Posted this review using
> support/post-reviews.py --server=https://reviews.apache.org/
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Anand Mazumdar]

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


LGTM ! Just some minor comments to clean up the tests around 
parameterizing/scoping.


src/tests/executor_http_api_tests.cpp (line 450)


s/TEST_F/TEST_P

Is there a reason why this test is not parameterized ?



src/tests/executor_http_api_tests.cpp (line 468)






src/tests/executor_http_api_tests.cpp (lines 468 - 483)


s/responseSubscribe/response

Can we use scoping here for each of the 3 different scenarios :

{
  Call call;
  blah blah;
}

{
  Call call;
  blah blah;
}
...



src/tests/executor_http_api_tests.cpp (line 515)


s/TEST_F/TEST_P and make this test parametrized. Is there a reason why this 
is not ?

s/InvalidCallUpdate/StatusUpdateCallFailedValidation to be more descriptive



src/tests/executor_http_api_tests.cpp (line 531)


// We send a valid Call::Update message with inconsistent executor_id 
between Call::executor_id and Call::Update::TaskInfo::executor_id. This should 
result in failed validation.

^^ How does this sound to you ?



src/tests/executor_http_api_tests.cpp (line 561)


Nit: // We send a valid Call::Update message with a TASK_STAGING status 
update. This should fail validation.

(Mainly because after parameterizing the test this won't always be a 
protobuf message.)



src/tests/executor_http_api_tests.cpp (lines 563 - 573)


Scope this similar to my earlier comment:

{
  blah blah;
}


- Anand Mazumdar


On Sept. 29, 2015, 9:27 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> ---
> 
> (Updated Sept. 29, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38869]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 12:09 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, 12:09 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 38878: Added test for the Subscribe->Subscribed workflow for the Executor HTTP API

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38873, 38874, 38875, 38876, 38877, 38878]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 3:40 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38878/
> ---
> 
> (Updated Sept. 30, 2015, 3:40 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a basic test to validate the implementation for 
> Subscribe->Subscribed workflow on the `api/v1/executor` endpoint on Agent.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 31938c295367686d444f8a8aa2c43d2696b481ec 
> 
> Diff: https://reviews.apache.org/r/38878/diff/
> 
> 
> Testing
> ---
> 
> make check. Would add more agent recovery tests/executor reconnect tests in a 
> separate patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38876: Added functionality to store a marker file to denote HTTP based executors

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 九月 30, 2015, 3:39 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38876/
> ---
> 
> (Updated 九月 30, 2015, 3:39 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the relevant functionality to `src/slave/paths.cpp/hpp` to 
> store a marker file to denote HTTP based executors. We create the file when 
> `checkpointing` is enabled as part of handling the `Subscribe` request. This 
> is then used by the agent when recovering to ascertain if the executor was 
> connected via `HTTP` before the agent restart.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp f743fb4b1ca278fade9134e0ae8f6a6450d4a977 
>   src/slave/paths.cpp fb77e64a6da017d8c9a00916d8935b670da0d374 
> 
> Diff: https://reviews.apache.org/r/38876/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38577: Added synchronous validation for Call in Agent

[Anand Mazumdar]

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


Thanks Isabel, this looks pretty neat now. Just some minor cleanup comments.


src/slave/validation.cpp (line 58)


Nit: const TaskStatus& status = call.update().status();



src/slave/validation.cpp (line 65)


s/in Update/in TaskStatus



src/slave/validation.cpp (lines 70 - 77)


Should we be explicitly validating that the source is always 
SOURCE_EXECUTOR i.e. something like ?

if (state.source() != TaskStatus::SOURCE_EXECUTOR) {
// Failed Validation
}

...

if (status.state() == TASK_STAGING) {
// Failed Validation
}


- Anand Mazumdar


On Sept. 29, 2015, 9:25 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38577/
> ---
> 
> (Updated Sept. 29, 2015, 9:25 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
> https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added validation for Call protobuf message in Agent /api/v1/executor endpoint.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 891f951 
>   src/Makefile.am 8aa4566 
>   src/slave/http.cpp f9cf7bb 
>   src/slave/validation.hpp PRE-CREATION 
>   src/slave/validation.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38577/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

[Guangya Liu]

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



src/slave/slave.hpp (line 169)


Do you mind switch the first two parameters as following, this can 
highlight that this is an http call.

void subscribe(
HttpConnection http,
const executor::Call::Subscribe& subscribe,
Framework* framework,
Executor* executor);



src/slave/slave.cpp (line 2385)


What about using "Received subscription request for HTTP executor xxx" to 
keep consistent with scheduler log?



src/slave/slave.cpp (line 2387)


How to handle the case if executor is in RECOVRING state? The logic is 
different for register and reregister with RECOVERING state.


- Guangya Liu


On 九月 30, 2015, 3:40 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated 九月 30, 2015, 3:40 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38837: CMake: Disable agent build on Windows.

[Alex Clemmer]

> On Sept. 29, 2015, 5:06 p.m., Joseph Wu wrote:
> > Can you explain why you're disabling this part?  (Is it simply not 
> > implemented yet, so you're temporarily skipping it?)

Mainly because the agent doesn't actually compile on Windows. I meant to put 
this up for review awhile ago but forgot. I'll open this up again to Windows in 
a few weeks when we are ready to do it for real.


- Alex


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


On Sept. 29, 2015, 5:17 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38837/
> ---
> 
> (Updated Sept. 29, 2015, 5:17 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Disable slave build on Windows.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 
> 
> Diff: https://reviews.apache.org/r/38837/diff/
> 
> 
> Testing
> ---
> 
> # On OS X 10.10, Ubuntu 14.04, Ubuntu 15.04
> 
> cmake .. -DENABLE_LIBEVENT=1 && make -j 8
> cmake .. && make -j 8
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38827: CMake: Expanded support for compiling much of the master.

[Alex Clemmer]

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

(Updated Sept. 29, 2015, 5:17 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Expanded support for compiling much of the master.


Diffs
-

  cmake/CompilationConfigure.cmake 98a08ee99299c873577b0b2a93a153f82ef75b29 
  src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 

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


Testing (updated)
---

# On OS X 10.10, Ubuntu 14.04, Ubuntu 15.04

cmake .. -DENABLE_LIBEVENT=1 && make -j 8
cmake .. && make -j 8


Thanks,

Alex Clemmer

Re: Review Request 38837: CMake: Disable agent build on Windows.

[Alex Clemmer]

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

(Updated Sept. 29, 2015, 5:17 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Disable slave build on Windows.


Diffs
-

  CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 

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


Testing (updated)
---

# On OS X 10.10, Ubuntu 14.04, Ubuntu 15.04

cmake .. -DENABLE_LIBEVENT=1 && make -j 8
cmake .. && make -j 8


Thanks,

Alex Clemmer

Review Request 38855: Added support for getting shared and slave mount peer group ID.

[Jie Yu]

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

Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added support for getting shared and slave mount peer group ID.


Diffs
-

  src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
  src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
  src/tests/containerizer/fs_tests.cpp 34d3c41045373326fb6096031cd1daef0e7b203a 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

[Neil Conway]

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

(Updated Sept. 29, 2015, 6:19 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.


Changes
---

Address review suggestions.


Repository: mesos


Description
---

Added suggestion that new contributors seek to find a shepherd for their work
before their begin writing code. Made a few other improvements.


Diffs (updated)
-

  docs/reporting-a-bug.md 6c7f74719a8586f0608eb0f0f77d15c8d534321d 
  docs/submitting-a-patch.md 754a16f9b43630880f0f6c4a8e8e2f5e081b0a87 

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


Testing (updated)
---

Viewed markdown with https://github.com/mesosphere/mesos-website-container


Thanks,

Neil Conway

Re: Review Request 38825: CMake: Plumb `ENABLE_LIBEVENT` through libprocess config scripts.

[Alex Clemmer]

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

(Updated Sept. 29, 2015, 5:16 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


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


Repository: mesos


Description
---

CMake: Plumb `ENABLE_LIBEVENT` through libprocess config scripts.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
31d77d5629dba7ee0d1a4ceb7b60899f6047165f 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
9c369e2c94abcd2dfa463f98fb3b6527033064ab 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
076c8836d9d53b8bc69330a46a7e1d3039aa476b 

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


Testing (updated)
---

# On OS X 10.10, Ubuntu 14.04, Ubuntu 15.04

cmake .. -DENABLE_LIBEVENT=1 && make -j 8
cmake .. && make -j 8


Thanks,

Alex Clemmer

Re: Review Request 38826: CMake: Added support for libevent for Unix builds.

[Alex Clemmer]

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

(Updated Sept. 29, 2015, 5:16 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


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


Repository: mesos


Description
---

CMake: Added support for libevent for Unix builds.


Diffs
-

  CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 
  src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
  src/slave/cmake/SlaveConfigure.cmake 230e574f3ddc6aa12cc74105de7201a33dd7cdab 

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


Testing (updated)
---

# On OS X 10.10, Ubuntu 14.04, Ubuntu 15.04

cmake .. -DENABLE_LIBEVENT=1 && make -j 8
cmake .. && make -j 8


Thanks,

Alex Clemmer

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Chi Zhang]

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

(Updated Sept. 29, 2015, 6:11 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

socket: refactor to use Option and fix file descriptor leaks.


Diffs (updated)
-

  3rdparty/libprocess/include/process/socket.hpp 817cb37 
  3rdparty/libprocess/src/socket.cpp 5879423 

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


Testing
---

make check


Thanks,

Chi Zhang

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. 29, 2015, 6:41 p.m.)


Review request for mesos, Gilbert Song and Timothy Chen.


Changes
---

style fixes.


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 38855: Added support for getting shared and slave mount peer group ID.

[Jie Yu]

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

(Updated Sept. 29, 2015, 7:09 p.m.)


Review request for mesos, Ian Downes, Timothy Chen, and Jiang Yan Xu.


Changes
---

Added const for the methods.


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


Repository: mesos


Description
---

Added support for getting shared and slave mount peer group ID.


Diffs (updated)
-

  src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 
  src/linux/fs.cpp 8631d892ed6c132d6a9dc2031c2ca040623e9acc 
  src/tests/containerizer/fs_tests.cpp 34d3c41045373326fb6096031cd1daef0e7b203a 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38828: docker: containerizer: fixed double-closing of STDIN in error handling.

[Ben Mahler]

> On Sept. 29, 2015, 1:17 a.m., Timothy Chen wrote:
> > src/slave/containerizer/docker.cpp, lines 926-928
> > 
> >
> > Hey I think we should be doing the same behavior as MesosContainerizer 
> > since the executor behaves the same, and I think yes the extra close is not 
> > necessary.

Great, I'll get this committed then.


- Ben


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


On Sept. 29, 2015, 12:50 a.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38828/
> ---
> 
> (Updated Sept. 29, 2015, 12:50 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: mesos-3519
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The slave will destroy executors that are not launched successfully, which 
> will
> close all open file descriptors, thus the extra close is not needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp efa37266368ee12fea9134b35ebc5047a2820f94 
> 
> Diff: https://reviews.apache.org/r/38828/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Review Request 38858: Ensured that slave's work_dir is a shared mount in its own peer group when LinuxFilesystemIsolator is used.

[Jie Yu]

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

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


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


Repository: mesos


Description
---

Ensured that slave's work_dir is a shared mount in its own peer group when 
LinuxFilesystemIsolator is used.

This addressed the TODO in the current code.


Diffs
-

  src/slave/containerizer/isolators/filesystem/linux.cpp 
8fa929f12838044762be82287e1b714406d9 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
41d098c7d66eba0befc3708c6660a6a3e5f0d2de 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38328: Added a helper to get stat.st_dev in stout.

[Jie Yu]

> On Sept. 29, 2015, 6:32 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/stat.hpp, line 134
> > 
> >
> > Hm.. looks like this file needs a pass to phrase the errors as "Failed 
> > to invoke stat" to be consistent with our other messages, also we should 
> > avoiding printing the path since the caller will do that when composing the 
> > error message, yeah?

Just wanted to be consistent with the rest in this file. Will follow up with a 
patch to cleanup the error message.


> On Sept. 29, 2015, 6:32 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/stat.hpp, lines 
> > 129-138
> > 
> >
> > I believe you also need an UNIMPLEMENTED version inside 
> > windows/stat.hpp.

yeah, good catch.


- Jie


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


On Sept. 12, 2015, 6:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38328/
> ---
> 
> (Updated Sept. 12, 2015, 6:36 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
> https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper to get stat.st_dev in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/stat.hpp 
> c7084f1d3672f5610de1cb47e275cb67a9cac1d5 
> 
> Diff: https://reviews.apache.org/r/38328/diff/
> 
> 
> Testing
> ---
> 
> trivial.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38809: socket: refactor to use Option and fix file descriptor leaks.

[Ben Mahler]

> On Sept. 29, 2015, 12:47 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/socket.cpp, lines 42-44
> > 
> >
> > Just as an aside, it's unfortunate the caller has to do this, we should 
> > consider doing the same thing that is done in os/posix/open.hpp with 
> > O_CLOEXEC:
> > 
> > 
> > https://github.com/apache/mesos/blob/5aa050bfaa7338b010b1a522406bde0f15015259/3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/open.hpp#L33
> 
> Chi Zhang wrote:
> I actually think this is not too bad -- a very staightforward if-else. 
> 
> I can probably break it up and save one network::socket call -- not sure 
> if that's worth the added complexity, but I can do it in another patch if 
> you'd like to see that?

No don't do it right now, just an aside. While it's straightforward here, every 
user of network::socket is going to have this #ifdef which complicates the 
calling code, it got out of hand for os::open which is why we provided 
O_CLOEXEC for all platforms.


- Ben


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


On Sept. 29, 2015, 6:11 p.m., Chi Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38809/
> ---
> 
> (Updated Sept. 29, 2015, 6:11 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3519
> https://issues.apache.org/jira/browse/MESOS-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> socket: refactor to use Option and fix file descriptor leaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 817cb37 
>   3rdparty/libprocess/src/socket.cpp 5879423 
> 
> Diff: https://reviews.apache.org/r/38809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 8:05 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38577: Added synchronous validation for Call in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 8:15 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.


Changes
---

Added validation.cpp to CMakelist


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


Repository: mesos


Description
---

Added validation for Call protobuf message in Agent /api/v1/executor endpoint.


Diffs (updated)
-

  src/CMakeLists.txt 891f951 
  src/Makefile.am 8aa4566 
  src/slave/http.cpp f9cf7bb 
  src/slave/validation.hpp PRE-CREATION 
  src/slave/validation.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 38826: CMake: Added support for libevent for Unix builds.

[Alex Clemmer]

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

(Updated Sept. 29, 2015, 8:21 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


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


Repository: mesos


Description
---

CMake: Added support for libevent for Unix builds.


Diffs (updated)
-

  CMakeLists.txt c866f6d85d629ca9c3a987fa2c652bc63a7ff1be 
  src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
  src/slave/cmake/SlaveConfigure.cmake 230e574f3ddc6aa12cc74105de7201a33dd7cdab 

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


Testing
---

# On OS X 10.10, Ubuntu 14.04, Ubuntu 15.04

cmake .. -DENABLE_LIBEVENT=1 && make -j 8
cmake .. && make -j 8


Thanks,

Alex Clemmer

Re: Review Request 38825: CMake: Plumb `ENABLE_LIBEVENT` through libprocess config scripts.

[Alex Clemmer]

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

(Updated Sept. 29, 2015, 8:21 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


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


Repository: mesos


Description
---

CMake: Plumb `ENABLE_LIBEVENT` through libprocess config scripts.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
31d77d5629dba7ee0d1a4ceb7b60899f6047165f 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
9c369e2c94abcd2dfa463f98fb3b6527033064ab 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
076c8836d9d53b8bc69330a46a7e1d3039aa476b 

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


Testing
---

# On OS X 10.10, Ubuntu 14.04, Ubuntu 15.04

cmake .. -DENABLE_LIBEVENT=1 && make -j 8
cmake .. && make -j 8


Thanks,

Alex Clemmer

Re: Review Request 38844: Added unit tests for Call validation in Agent

[Isabel Jimenez]

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

(Updated Sept. 29, 2015, 8:25 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Unit tests for Call validation in Agent.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
---

make check


Thanks,

Isabel Jimenez