Re: Review Request 38639: Add os::clone function in stout.

[Kapil Arya]

> On Sept. 22, 2015, 2:58 p.m., Kapil Arya wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp, line 73
> > 
> >
> > We should delete the stack only if CLONE_VM was not specified in the 
> > flags or at least have a comment mentioning the fact that this call is 
> > creating a new process and not a new thread. Having a CLONE_VM will create 
> > a new thread, not a new process.
> 
> Jie Yu wrote:
> Good call! +1
> 
> Jie Yu wrote:
> Or we can disallow CLONE_VM since if the user wants to create a thread, 
> he/she should use pthread.

That should work too and is better in my opinion.


- Kapil


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


On Sept. 22, 2015, 2:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38639/
> ---
> 
> (Updated Sept. 22, 2015, 2:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::clone function in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> b994b13941628947c9d12b8baae155d5da1ec7bd 
> 
> Diff: https://reviews.apache.org/r/38639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38625: Keep clone syscall consistent.

[haosdent huang]

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

(Updated Sept. 22, 2015, 6:37 p.m.)


Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, Michael 
Park, and Cong Wang.


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


Repository: mesos


Description
---

Keep clone syscall consistent.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 
  src/tests/containerizer/launch_tests.cpp 
dbe891cd004a8f8b4be252b1bd496fc5d4cc5923 
  src/tests/containerizer/ns_tests.cpp 3a22938a31b717568d38090c57ca37e440e77274 

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


Testing
---

sudo make -j8 check


Thanks,

haosdent huang

Re: Review Request 38639: Add os::clone function in stout.

[Jie Yu]

> On Sept. 22, 2015, 6:58 p.m., Kapil Arya wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp, line 73
> > 
> >
> > We should delete the stack only if CLONE_VM was not specified in the 
> > flags or at least have a comment mentioning the fact that this call is 
> > creating a new process and not a new thread. Having a CLONE_VM will create 
> > a new thread, not a new process.
> 
> Jie Yu wrote:
> Good call! +1

Or we can disallow CLONE_VM since if the user wants to create a thread, he/she 
should use pthread.


- Jie


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


On Sept. 22, 2015, 6:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38639/
> ---
> 
> (Updated Sept. 22, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::clone function in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> b994b13941628947c9d12b8baae155d5da1ec7bd 
> 
> Diff: https://reviews.apache.org/r/38639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Joseph Wu]

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

Ship it!



3rdparty/libprocess/cmake/ProcessConfigure.cmake (lines 59 - 65)


What do you think about moving this `if` block into `Versions.cmake`?

Pro: Versions are kept in one place.  If we unilaterally update to glog 
0.3.4, we'll only need to update it in one place.

Con: Logic in the version file, which we don't really have a precedent for.


- Joseph Wu


On Sept. 20, 2015, 8:23 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 20, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38625: Replace clone system call with os::clone

[haosdent huang]

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

(Updated Sept. 22, 2015, 6:52 p.m.)


Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, Michael 
Park, and Cong Wang.


Changes
---

Update desc.


Summary (updated)
-

Replace clone system call with os::clone


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


Repository: mesos


Description (updated)
---

Replace clone system call with os::clone.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 
  src/tests/containerizer/launch_tests.cpp 
dbe891cd004a8f8b4be252b1bd496fc5d4cc5923 
  src/tests/containerizer/ns_tests.cpp 3a22938a31b717568d38090c57ca37e440e77274 

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


Testing
---

sudo make -j8 check


Thanks,

haosdent huang

Re: Review Request 38639: Add os::clone function in stout.

[Jie Yu]

> On Sept. 22, 2015, 6:58 p.m., Kapil Arya wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp, line 73
> > 
> >
> > We should delete the stack only if CLONE_VM was not specified in the 
> > flags or at least have a comment mentioning the fact that this call is 
> > creating a new process and not a new thread. Having a CLONE_VM will create 
> > a new thread, not a new process.

Good call! +1


- Jie


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


On Sept. 22, 2015, 6:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38639/
> ---
> 
> (Updated Sept. 22, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::clone function in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> b994b13941628947c9d12b8baae155d5da1ec7bd 
> 
> Diff: https://reviews.apache.org/r/38639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38568: Maintenance Primitives: Fix the formatting of the user doc.

[Joseph Wu]

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

(Updated Sept. 22, 2015, 11:51 a.m.)


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


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


Repository: mesos


Description
---

Also adds a link to this document from the documentation home page.

The website's markdown renderer is slightly different than the Gist renderer.
This fixes the markdown to show correctly on the website.


Diffs
-

  docs/home.md 3dff97ba35fafe8fca7e18866f0c7e8d526022e1 
  docs/maintenance.md 465dcfb72777611c2a4be2aa38c8cd7e869a2baa 

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


Testing
---

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


Thanks,

Joseph Wu

Re: Review Request 38639: Add os::clone function in stout.

[Jie Yu]

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp (line 52)


in fact, can you rename 'namespaces' to 'flags'?


- Jie Yu


On Sept. 22, 2015, 6:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38639/
> ---
> 
> (Updated Sept. 22, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::clone function in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> b994b13941628947c9d12b8baae155d5da1ec7bd 
> 
> Diff: https://reviews.apache.org/r/38639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38625: Replace clone system call with os::clone

[Jie Yu]

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

Ship it!



src/tests/containerizer/ns_tests.cpp (lines 169 - 171)


Maybe

```
pid_t pid = os::clone(
lambda::bind(, NULL),
nstype.get() | SIGCHLD);
```



src/tests/containerizer/ns_tests.cpp (lines 223 - 225)


Ditto.


- Jie Yu


On Sept. 22, 2015, 6:52 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38625/
> ---
> 
> (Updated Sept. 22, 2015, 6:52 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, Michael 
> Park, and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace clone system call with os::clone.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
>   src/tests/containerizer/launch_tests.cpp 
> dbe891cd004a8f8b4be252b1bd496fc5d4cc5923 
>   src/tests/containerizer/ns_tests.cpp 
> 3a22938a31b717568d38090c57ca37e440e77274 
> 
> Diff: https://reviews.apache.org/r/38625/diff/
> 
> 
> Testing
> ---
> 
> sudo make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38625: Replace clone system call with os::clone

[haosdent huang]

> On Sept. 22, 2015, 4:51 p.m., Cong Wang wrote:
> > Why not introduce a wrapper for clone() to hide the 'stack'?
> 
> haosdent huang wrote:
> Sure, let me update again.

@wangcong I finish update the patch. Could you help review this again? Thank 
you very much.


- haosdent


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


On Sept. 22, 2015, 6:52 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38625/
> ---
> 
> (Updated Sept. 22, 2015, 6:52 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, Michael 
> Park, and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace clone system call with os::clone.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
>   src/tests/containerizer/launch_tests.cpp 
> dbe891cd004a8f8b4be252b1bd496fc5d4cc5923 
>   src/tests/containerizer/ns_tests.cpp 
> 3a22938a31b717568d38090c57ca37e440e77274 
> 
> Diff: https://reviews.apache.org/r/38625/diff/
> 
> 
> Testing
> ---
> 
> sudo make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38639: Add os::clone function in stout.

[Kapil Arya]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp (line 73)


We should delete the stack only if CLONE_VM was not specified in the flags 
or at least have a comment mentioning the fact that this call is creating a new 
process and not a new thread. Having a CLONE_VM will create a new thread, not a 
new process.


- Kapil Arya


On Sept. 22, 2015, 2:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38639/
> ---
> 
> (Updated Sept. 22, 2015, 2:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::clone function in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> b994b13941628947c9d12b8baae155d5da1ec7bd 
> 
> Diff: https://reviews.apache.org/r/38639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38569: Made slave's work_dir a shared mount in LinuxFilesystemIsolator.

[Cong Wang]

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



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


This only checks if it is mounted, but not if it is bind mounted. Not sure 
if this is correct when work_dir itself is a non-bind mounted directory.



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


Not sure if you need to undo the conditional previous bind mount.



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


Ditto



src/tests/environment.cpp (line 47)


This one looks unnecessary.


- Cong Wang


On Sept. 21, 2015, 9:23 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38569/
> ---
> 
> (Updated Sept. 21, 2015, 9:23 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3483
> https://issues.apache.org/jira/browse/MESOS-3483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made slave's work_dir a shared mount in LinuxFilesystemIsolator.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> f1e6f7519bdeeff7790fff63e7a9cb3075001758 
>   src/tests/environment.cpp e40cde23daba25b0b61b567ee73682c67e7acbdc 
>   src/tests/utils.cpp 498c9aa1442c4e937a74364073b2267dde6ffcb2 
> 
> Diff: https://reviews.apache.org/r/38569/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 38600: Introduced a typedef to make consistent use of case-insentitive http headers easier.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Many places in the code were not using case insensitive header names for http. 
This adds a typedef to make it easier.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 
  3rdparty/libprocess/src/tests/process_tests.cpp 
1023500ac2e3c55be955c4686e7f720eba6b4cc9 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38597: Added URL within http::Request.

[Ben Mahler]

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

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


Repository: mesos


Description
---

Now that URL is a URI-reference, we can implement the TODO to replace the raw 
fields in Request with a URL.


Diffs
-

  3rdparty/libprocess/include/process/firewall.hpp 
b1abfb29fe2db07d991e9a6f655345720faef863 
  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/include/process/system.hpp 
264d948c2c000c6cb176601735bd4ea0d6fcbc77 
  3rdparty/libprocess/src/decoder.hpp 67a5135f302153e376e8dfe8db82aa0b15449389 
  3rdparty/libprocess/src/help.cpp d358b93005506dad53aac815908cca59e6a53118 
  3rdparty/libprocess/src/logging.cpp db76abe08bc74c4d7444e112f32a5d11a236d229 
  3rdparty/libprocess/src/metrics/metrics.cpp 
943ba63b01fd982185bc33679bba5cc3fcc086fc 
  3rdparty/libprocess/src/process.cpp 4afa30569b4d235637b49a624602e6b199c32e0e 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
97c81b8e61a75771e5bf7d46505cec4e0c0f404a 
  3rdparty/libprocess/src/tests/decoder_tests.cpp 
6994fa96d33209f9a367b8c3bb09b0d050023fad 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38599: Consistent use of case insensitive keys for HTTP headers.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

A number of places in the code were not using case insensitive header keys, 
these are the fixes within mesos that leverage the typedef from 
[r/38600](https://reviews.apache.org/r/38600).


Diffs
-

  src/scheduler/scheduler.cpp ee146eb81a8ad72bc04c62d1e223de6aa27b351d 
  src/slave/containerizer/provisioners/docker/registry_client.cpp 
b262ef031e0373ee009273e50a16d0a58ed83e8e 
  src/tests/http_api_tests.cpp 79381edade8fa092118baab63f85a83b4ca2 
  src/tests/master_maintenance_tests.cpp 
44785057f129a3e6a69f399f7d6db59d9d5c2e91 
  src/tests/reservation_endpoints_tests.cpp 
398a2e11cb334095f16b9e5529e6715e633d2c6f 
  src/tests/teardown_tests.cpp daf748c1e128dc296b075fd7152dd9393def9d65 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38596: Updated http::URL to be a URI-reference.

[Ben Mahler]

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

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


Repository: mesos


Description
---

This updates URL to be a URI-reference (see [RFC 3986 
4.1](https://tools.ietf.org/html/rfc3986#section-4.1)), similarly to how other 
http libraries store a URL in a Request object (see the go libraries for 
[Request](http://golang.org/pkg/net/http/#Request) and 
[URL](http://golang.org/pkg/net/url/#URL) as an example).

The motivation for this change is to enable cleanup of the http utilities in 
libprocess. By storing a full URL inside the Request, we can obtain the address 
from the request itself, rather than having to pass the address alongside.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38598: Updates to reflect addition of URL in http::Request.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

These are the necessary updates to mesos from 
[r/38597](https://reviews.apache.org/r/38597).


Diffs
-

  src/files/files.cpp 1d4b1e6d2fcd99321084515f538aa357f8d9b305 
  src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
  src/master/registrar.cpp d81560a90f039fc159fb8201f82076dca5a46ca6 
  src/slave/http.cpp 101aa06e981eaa43bf6c68268b057f3d15e33e2e 
  src/slave/monitor.cpp 8d8b4226ecb3de5ee12984d6fb65228feba49ddc 
  src/tests/fetcher_cache_tests.cpp b709b1eedeb880bc815e0742dc604d93828e593f 

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


Testing
---

make check


Thanks,

Ben Mahler

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

> On Sept. 21, 2015, 11:22 p.m., Jie Yu wrote:
> > src/Makefile.am, line 491
> > 
> >
> > Similar to master/resgistry.proto, we should put this proto file under
> > 
> > `slave/containerizer/provisioner/docker.proto`
> > or
> > `slave/containerizer/provisioner/docker/message.proto`
> > ?

This is actually gone now.


> On Sept. 21, 2015, 11:22 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioner/docker/message.proto, line 21
> > 
> >
> > Remove .docker?

I added the docker namespace since it's pretty specific to just docker and is 
in the same namespace of the docker provisioner. 
Any reason why we should move it up?


- Timothy


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


On Sept. 21, 2015, 5:50 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 21, 2015, 5:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 834f69a 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e31a418 
>   src/slave/flags.cpp add4196 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

> On Sept. 21, 2015, 11:22 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioner/docker/paths.hpp, line 40
> > 
> >
> > What's this?

No longer needed with the shared provisioner change, thanks!


> On Sept. 21, 2015, 11:22 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioner/docker/store.cpp, line 228
> > 
> >
> > Why do you need the 'Nothing()' here?

Just so we can chain callbacks.


- Timothy


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


On Sept. 21, 2015, 5:50 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 21, 2015, 5:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 834f69a 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e31a418 
>   src/slave/flags.cpp add4196 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38584: Added a few filesystem isolation and provisioning metrics.

[Timothy Chen]

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



src/slave/containerizer/isolators/filesystem/linux.hpp (line 138)


I actually like new rootfs more, but open to either.



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


Add a blank line above



src/slave/containerizer/provisioner/backends/bind.cpp (line 194)


Why only when errno == EBUSY?


- Timothy Chen


On Sept. 22, 2015, 12:58 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38584/
> ---
> 
> (Updated Sept. 22, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3466
> https://issues.apache.org/jira/browse/MESOS-3466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a few filesystem isolation and provisioning metrics.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 041af7b51cc7da933a929d3beb05c419321ddd73 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> e821b27d5cba324e434132d884d25a526c11a9c5 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> d853b49891360fbc2eb46f2e7859fa296fd36beb 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 213f8a6cbcfa198920dbb83c42e58e8aef6d6946 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e2eb049ae477578caeedaa4251f7ddf9028d3fb5 
> 
> Diff: https://reviews.apache.org/r/38584/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

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

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

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


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 

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


Testing
---

make check


Thanks,

Ben Mahler

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 九月 22, 2015, 6:05 a.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38564/
> ---
> 
> (Updated 九月 22, 2015, 6:05 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new callback enabling custom attribute discovery logic
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 2353602c8ac8b89e3fa86bda7d1e262a3debef80 
>   src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
>   src/hook/manager.hpp a517c05855b07831a9728c1191243d114781b70a 
>   src/hook/manager.cpp 691976e94d5ac9fe4f8ba583214f24900d14248c 
>   src/slave/slave.cpp 29865ece00fa8bff3054a7f8c87cbf93403405db 
>   src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 
> 
> Diff: https://reviews.apache.org/r/38564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Review Request 38606: Added ability to check if the streaming decoder is writing to a body pipe.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

This is necessary to know when the decoder has finished writing the latest 
response, and is needed for the subsequent patch that adds an http::Connection.


Diffs
-

  3rdparty/libprocess/src/decoder.hpp 67a5135f302153e376e8dfe8db82aa0b15449389 
  3rdparty/libprocess/src/tests/decoder_tests.cpp 
6994fa96d33209f9a367b8c3bb09b0d050023fad 

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


Testing
---

Updated the streaming tests.


Thanks,

Ben Mahler

Review Request 38601: Added a TODO to keep Request.keepAlive consistent with the 'Connection' header value.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

The boolean field is a bit unfortunate as it may not be consistent with the 
'Connection' header.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 

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


Testing
---

N/A


Thanks,

Ben Mahler

Review Request 38604: Re-enabled decoder test for case insensitive headers.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/src/tests/decoder_tests.cpp 
6994fa96d33209f9a367b8c3bb09b0d050023fad 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38605: Improved streaming decoder tests by checking the failed state.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/src/tests/decoder_tests.cpp 
6994fa96d33209f9a367b8c3bb09b0d050023fad 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38607: Pulled out an encode function for http::Request encoding.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Now we have all the information within Request necessary to encode it, so this 
extracts encoding into a function to clean up the code.


Diffs
-

  3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 

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


Testing
---

make check


Thanks,

Ben Mahler

Review Request 38603: Refactored http::internal::Request to take a Request object.

[Ben Mahler]

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

Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

This is a step towards simplifying the complicated overloads for http::get / 
http::post as well.


Diffs
-

  3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 

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


Testing
---

make check


Thanks,

Ben Mahler

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

[Ben Mahler]

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

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


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


Repository: mesos


Description
---

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


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
fbd6cf7967173495875a8ea90ed28ade88b982a2 
  3rdparty/libprocess/src/http.cpp 9ad613a16c379b6d76a9a0f8d6160fe23a182fd4 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 

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


Testing
---

Added tests.


Thanks,

Ben Mahler

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

> On Sept. 21, 2015, 11:22 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioner/docker/store.cpp, line 256
> > 
> >
> > According to your current design, Store is supposed to be agnostic to 
> > which puller is being used, right? Therefore, there should be local puller 
> > specific code in Store.
> > 
> > Shouldn't the puller interface returns a map: imageId -> 
> > path_to_fs_layer and the Store just looks at the paths and move them to 
> > getImageLayerRootfsPath?

I thought about this, but I also want to know the dependencies among layers. So 
decided that the simplest is to return the vector of layer IDs, but just expect 
the layers to be prepared under the staging directory with the right structure. 
I've commented on that in the puller interface.


- Timothy


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


On Sept. 21, 2015, 5:50 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 21, 2015, 5:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 834f69a 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/flags.hpp e31a418 
>   src/slave/flags.cpp add4196 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[haosdent huang]

> On Sept. 22, 2015, 6:21 a.m., Cong Wang wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 182
> > 
> >
> > You forgot to remove this line?
> 
> haosdent huang wrote:
> sorry, my fault.

Let me create a new patch.


- haosdent


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


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[haosdent huang]

> On Sept. 22, 2015, 6:21 a.m., Cong Wang wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 182
> > 
> >
> > You forgot to remove this line?

sorry, my fault.


- haosdent


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


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Guangya Liu]

> On 九月 21, 2015, 10:08 p.m., Guangya Liu wrote:
> > I think that you need make this patch depend on 
> > https://reviews.apache.org/r/38279/ to make this works.
> 
> Felix Abecassis wrote:
> Thank you, I hope it's fixed now.

I see that this patch is depending on https://reviews.apache.org/r/38279/ , but 
from review borad, I see that it is depending on 38517, is that right?


- Guangya


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


On 九月 22, 2015, 1:40 a.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38564/
> ---
> 
> (Updated 九月 22, 2015, 1:40 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new callback enabling custom attribute discovery logic
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 2353602c8ac8b89e3fa86bda7d1e262a3debef80 
>   src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
>   src/hook/manager.hpp a517c05855b07831a9728c1191243d114781b70a 
>   src/hook/manager.cpp 691976e94d5ac9fe4f8ba583214f24900d14248c 
>   src/slave/slave.cpp 29865ece00fa8bff3054a7f8c87cbf93403405db 
>   src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 
> 
> Diff: https://reviews.apache.org/r/38564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Felix Abecassis]

> On Sept. 21, 2015, 10:08 p.m., Guangya Liu wrote:
> > I think that you need make this patch depend on 
> > https://reviews.apache.org/r/38279/ to make this works.
> 
> Felix Abecassis wrote:
> Thank you, I hope it's fixed now.
> 
> Guangya Liu wrote:
> I see that this patch is depending on https://reviews.apache.org/r/38279/ 
> , but from review borad, I see that it is depending on 38517, is that right?

Somehow, the post-review script deleted the dependency on 38279.


- Felix


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


On Sept. 22, 2015, 6:05 a.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38564/
> ---
> 
> (Updated Sept. 22, 2015, 6:05 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new callback enabling custom attribute discovery logic
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 2353602c8ac8b89e3fa86bda7d1e262a3debef80 
>   src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
>   src/hook/manager.hpp a517c05855b07831a9728c1191243d114781b70a 
>   src/hook/manager.cpp 691976e94d5ac9fe4f8ba583214f24900d14248c 
>   src/slave/slave.cpp 29865ece00fa8bff3054a7f8c87cbf93403405db 
>   src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 
> 
> Diff: https://reviews.apache.org/r/38564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[Cong Wang]

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



src/slave/containerizer/linux_launcher.cpp (line 182)


You forgot to remove this line?


There are two more places calling clone() in a similar way:  
src/tests/containerizer/launch_tests.cpp, src/tests/containerizer/ns_tests.cpp. 
Not sure if you need to fix them too.

- Cong Wang


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 38634: Added Systemd environment check to LinuxLauncher.

[Joris Van Remoortere]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
Nielsen, Thomas Rampelberg, and Timothy Chen.


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


Repository: mesos


Description
---

This will be used to perform Systemd specific actions.


Diffs
-

  src/slave/containerizer/linux_launcher.hpp 
f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 

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


Testing
---


Thanks,

Joris Van Remoortere

Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

[Joris Van Remoortere]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
Nielsen, Thomas Rampelberg, and Timothy Chen.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/containerizer/linux_launcher.hpp 
f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Jie Yu]

> On Sept. 21, 2015, 11:22 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioner/docker/store.cpp, line 256
> > 
> >
> > According to your current design, Store is supposed to be agnostic to 
> > which puller is being used, right? Therefore, there should be local puller 
> > specific code in Store.
> > 
> > Shouldn't the puller interface returns a map: imageId -> 
> > path_to_fs_layer and the Store just looks at the paths and move them to 
> > getImageLayerRootfsPath?
> 
> Timothy Chen wrote:
> I thought about this, but I also want to know the dependencies among 
> layers. So decided that the simplest is to return the vector of layer IDs, 
> but just expect the layers to be prepared under the staging directory with 
> the right structure. I've commented on that in the puller interface.

Have you looked at LinkedHashMap?


- Jie


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


On Sept. 22, 2015, 8:22 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 22, 2015, 8:22 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e224060 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp e31a418 
>   src/slave/flags.cpp add4196 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Review Request 38636: Assign executors to `mesos_executor.slice` in LinuxLauncher on Systemd.

[Joris Van Remoortere]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
Nielsen, Thomas Rampelberg, and Timothy Chen.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 

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


Testing
---


Thanks,

Joris Van Remoortere

Review Request 38637: Added recovery warnings for LinuxLauncher on Systemd.

[Joris Van Remoortere]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
Nielsen, Thomas Rampelberg, and Timothy Chen.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 

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


Testing
---

make check
on ubuntu 15.04 with systemd 219 verified that pids get assigned to the 
`mesos_executor.slice`, and operator is warned about lack of proper resource 
isolation upon recovery if the executor pid is no longer in the slice.


Thanks,

Joris Van Remoortere

Re: Review Request 38625: Keep clone syscall consistent.

[haosdent huang]

> On Sept. 22, 2015, 4:51 p.m., Cong Wang wrote:
> > Why not introduce a wrapper for clone() to hide the 'stack'?

Sure, let me update again.


- haosdent


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


On Sept. 22, 2015, 2:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38625/
> ---
> 
> (Updated Sept. 22, 2015, 2:01 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, Michael 
> Park, and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Keep clone syscall consistent.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
>   src/tests/containerizer/launch_tests.cpp 
> dbe891cd004a8f8b4be252b1bd496fc5d4cc5923 
>   src/tests/containerizer/ns_tests.cpp 
> 3a22938a31b717568d38090c57ca37e440e77274 
> 
> Diff: https://reviews.apache.org/r/38625/diff/
> 
> 
> Testing
> ---
> 
> sudo make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38584: Added a few filesystem isolation and provisioning metrics.

[Jojy Varghese]

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



src/slave/containerizer/isolators/filesystem/linux.hpp (line 141)


is this proper accordng to style guide (Since this is a public method)?



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


const?



src/slave/containerizer/provisioner/provisioner.cpp (line 414)


const?



src/slave/containerizer/provisioner/provisioner.cpp (line 429)


const?


- Jojy Varghese


On Sept. 22, 2015, 12:58 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38584/
> ---
> 
> (Updated Sept. 22, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3466
> https://issues.apache.org/jira/browse/MESOS-3466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a few filesystem isolation and provisioning metrics.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 041af7b51cc7da933a929d3beb05c419321ddd73 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> e821b27d5cba324e434132d884d25a526c11a9c5 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> d853b49891360fbc2eb46f2e7859fa296fd36beb 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 213f8a6cbcfa198920dbb83c42e58e8aef6d6946 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e2eb049ae477578caeedaa4251f7ddf9028d3fb5 
> 
> Diff: https://reviews.apache.org/r/38584/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38625: Keep clone syscall consistent.

[Cong Wang]

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


Why not introduce a wrapper for clone() to hide the 'stack'?

- Cong Wang


On Sept. 22, 2015, 2:01 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38625/
> ---
> 
> (Updated Sept. 22, 2015, 2:01 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, Michael 
> Park, and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Keep clone syscall consistent.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
>   src/tests/containerizer/launch_tests.cpp 
> dbe891cd004a8f8b4be252b1bd496fc5d4cc5923 
>   src/tests/containerizer/ns_tests.cpp 
> 3a22938a31b717568d38090c57ca37e440e77274 
> 
> Diff: https://reviews.apache.org/r/38625/diff/
> 
> 
> Testing
> ---
> 
> sudo make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38630: Fixed order for header includes

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38630]

All tests passed.

- Mesos ReviewBot


On Sept. 22, 2015, 4:16 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38630/
> ---
> 
> (Updated Sept. 22, 2015, 4:16 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Trivial change to fix the include order for internal files noticed while 
> working on the Executor API
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 7a54fadcd7651352d01a0db0d802b9665806730a 
> 
> Diff: https://reviews.apache.org/r/38630/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38632: Fix printing of JSON numbers to be valid JSON for web browsers.

[Joseph Wu]

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

(Updated Sept. 22, 2015, 10:58 a.m.)


Review request for mesos, Artem Harutyunyan, Isabel Jimenez, and Joris Van 
Remoortere.


Summary (updated)
-

Fix printing of JSON numbers to be valid JSON for web browsers.


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


Repository: mesos


Description (updated)
---

Printing an integer as a JSON double looked like "1." which is an "unterminated 
fractional number".
Instead, the printed value should be "1.0".


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
e36f4b9aed50a75767d29306cb8d4974c4995156 
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
88b6147fa47ebfcb6000de5b2f0891d4438d26bf 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
6f154cd5ab12e1d1dd64e56574d2186ae53ee66e 

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


Testing
---

`make check` (OSX clang 7.0)

Manual check: Opened Mesos master web UI and a browser console (Firefox).  
Verified that no JSON parsing error appeared.


Thanks,

Joseph Wu

Re: Review Request 38632: Fix printing of JSON numbers to be valid JSON for web browsers.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Sept. 22, 2015, 5:58 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38632/
> ---
> 
> (Updated Sept. 22, 2015, 5:58 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Isabel Jimenez, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3490
> https://issues.apache.org/jira/browse/MESOS-3490
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Printing an integer as a JSON double looked like "1." which is an 
> "unterminated fractional number".
> Instead, the printed value should be "1.0".
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
> e36f4b9aed50a75767d29306cb8d4974c4995156 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 88b6147fa47ebfcb6000de5b2f0891d4438d26bf 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 
> 6f154cd5ab12e1d1dd64e56574d2186ae53ee66e 
> 
> Diff: https://reviews.apache.org/r/38632/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OSX clang 7.0)
> 
> Manual check: Opened Mesos master web UI and a browser console (Firefox).  
> Verified that no JSON parsing error appeared.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38584: Added a few filesystem isolation and provisioning metrics.

[Jiang Yan Xu]

> On Sept. 21, 2015, 9:56 p.m., Jie Yu wrote:
> > src/slave/containerizer/isolators/filesystem/linux.hpp, line 138
> > 
> >
> > `containers_changing_rootfs`?

Keeping `containers_new_rootfs` because I think it should be a shorthand for 
`containers_that_are_using_a_changed_rootfs` or 
`containers_that_are_using_a_new_rootfs`. And indeed a *new* rootfs is created 
for this container.


- Jiang Yan


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


On Sept. 21, 2015, 5:58 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38584/
> ---
> 
> (Updated Sept. 21, 2015, 5:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3466
> https://issues.apache.org/jira/browse/MESOS-3466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a few filesystem isolation and provisioning metrics.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 041af7b51cc7da933a929d3beb05c419321ddd73 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> e821b27d5cba324e434132d884d25a526c11a9c5 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> d853b49891360fbc2eb46f2e7859fa296fd36beb 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 213f8a6cbcfa198920dbb83c42e58e8aef6d6946 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e2eb049ae477578caeedaa4251f7ddf9028d3fb5 
> 
> Diff: https://reviews.apache.org/r/38584/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38584: Added a few filesystem isolation and provisioning metrics.

[Jiang Yan Xu]

> On Sept. 21, 2015, 11:09 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner/backends/bind.cpp, line 194
> > 
> >
> > Why only when errno == EBUSY?

Because other errors are not masked by the backend and propagted uptowards and 
will be reflected in other metrics.

It's no big deal and we'll likely deprecated these some time later. 

Currently we need these to see how often we get errors due to race condition 
between containers cleaning up their mount table and the host destroying these 
folders.


- Jiang Yan


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


On Sept. 21, 2015, 5:58 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38584/
> ---
> 
> (Updated Sept. 21, 2015, 5:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3466
> https://issues.apache.org/jira/browse/MESOS-3466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a few filesystem isolation and provisioning metrics.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 041af7b51cc7da933a929d3beb05c419321ddd73 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> e821b27d5cba324e434132d884d25a526c11a9c5 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> d853b49891360fbc2eb46f2e7859fa296fd36beb 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 213f8a6cbcfa198920dbb83c42e58e8aef6d6946 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e2eb049ae477578caeedaa4251f7ddf9028d3fb5 
> 
> Diff: https://reviews.apache.org/r/38584/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38584: Added a few filesystem isolation and provisioning metrics.

[Jiang Yan Xu]

> On Sept. 22, 2015, 9:19 a.m., Jojy Varghese wrote:
> > src/slave/containerizer/isolators/filesystem/linux.hpp, line 141
> > 
> >
> > is this proper accordng to style guide (Since this is a public method)?

It's under the private section and the convention used for metrics gauges. e.g. 
https://github.com/apache/mesos/blob/master/src/master/master.hpp#L1237


> On Sept. 22, 2015, 9:19 a.m., Jojy Varghese wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, line 839
> > 
> >
> > const?

You mean "const PID& isolator"? It's fixed 
already. Thanks!


> On Sept. 22, 2015, 9:19 a.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioner/provisioner.cpp, line 435
> > 
> >
> > const?

What being const?


> On Sept. 22, 2015, 9:19 a.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioner/provisioner.cpp, line 419
> > 
> >
> > const?

Yeah I guess it won't hurt to use const but the entire usage and lifecyle of 
this variable is within 5 lines I didn't feel the need. For more complex logic, 
for sure. Thanks!


- Jiang Yan


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


On Sept. 21, 2015, 5:58 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38584/
> ---
> 
> (Updated Sept. 21, 2015, 5:58 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3466
> https://issues.apache.org/jira/browse/MESOS-3466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a few filesystem isolation and provisioning metrics.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 041af7b51cc7da933a929d3beb05c419321ddd73 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> e821b27d5cba324e434132d884d25a526c11a9c5 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> d853b49891360fbc2eb46f2e7859fa296fd36beb 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 213f8a6cbcfa198920dbb83c42e58e8aef6d6946 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e2eb049ae477578caeedaa4251f7ddf9028d3fb5 
> 
> Diff: https://reviews.apache.org/r/38584/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 38637: Added recovery warnings for LinuxLauncher on Systemd.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38634, 38635, 38636, 38637]

All tests passed.

- Mesos ReviewBot


On Sept. 22, 2015, 5:29 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38637/
> ---
> 
> (Updated Sept. 22, 2015, 5:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
> Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
> 
> Diff: https://reviews.apache.org/r/38637/diff/
> 
> 
> Testing
> ---
> 
> make check
> on ubuntu 15.04 with systemd 219 verified that pids get assigned to the 
> `mesos_executor.slice`, and operator is warned about lack of proper resource 
> isolation upon recovery if the executor pid is no longer in the slice.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Jie Yu]

> On Sept. 21, 2015, 11:22 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioner/docker/message.proto, line 21
> > 
> >
> > Remove .docker?
> 
> Timothy Chen wrote:
> I added the docker namespace since it's pretty specific to just docker 
> and is in the same namespace of the docker provisioner. 
> Any reason why we should move it up?

You already have 'Docker' in DockerImage, docker::DockerImage does not read 
well. Either remove .docker or remove Docker.


- Jie


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


On Sept. 22, 2015, 8:22 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 22, 2015, 8:22 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e224060 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp e31a418 
>   src/slave/flags.cpp add4196 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Neil Conway]

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


For the sake of repro'ing, maybe you could add a sleep before waiting on the 
future? Obviously not something we want in the actual patch though.

- Neil Conway


On Sept. 22, 2015, 8:46 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38645/
> ---
> 
> (Updated Sept. 22, 2015, 8:46 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This showed up on ASF CI. From the logs:
> 
> `I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
> Then 
> `../../src/tests/executor_http_api_tests.cpp:290: Failure`
> `Failed to wait 15secs for __recover`
> 
> Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing 
> it before starting the slave. In some cases, slave would have already 
> recovered by the time we invoke `FUTURE_DISPATCH` leading to the flakiness.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 9dbc5191b5950df2faa693720f3740e97c7df758 
> 
> Diff: https://reviews.apache.org/r/38645/diff/
> 
> 
> Testing
> ---
> 
> I was not able to reproduce it before or after this change but looking at the 
> logs it is quite obvious what the issue was. Ran in a loop 100 times.
> 
> ASF CI error log: 
> https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38637: Added recovery warnings for LinuxLauncher on Systemd.

[Niklas Nielsen]

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


Can we add tests and user docs describing the issue and new behavior, how to 
setup systemd's policies etc?

- Niklas Nielsen


On Sept. 22, 2015, 10:29 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38637/
> ---
> 
> (Updated Sept. 22, 2015, 10:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
> Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
> 
> Diff: https://reviews.apache.org/r/38637/diff/
> 
> 
> Testing
> ---
> 
> make check
> on ubuntu 15.04 with systemd 219 verified that pids get assigned to the 
> `mesos_executor.slice`, and operator is warned about lack of proper resource 
> isolation upon recovery if the executor pid is no longer in the slice.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38639: Add os::clone function in stout.

[haosdent huang]

> On Sept. 22, 2015, 6:55 p.m., Cong Wang wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp, line 52
> > 
> >
> > s/namespaces/flags/, because SIG* can be or'ed with namespaces. Or even 
> > better, pass namespaces separately with signals as two different parameters?

Because I see ::clone only have one param, I just change s/namespaces/flags/ 
here.


- haosdent


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


On Sept. 23, 2015, 1:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38639/
> ---
> 
> (Updated Sept. 23, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::clone function in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> b994b13941628947c9d12b8baae155d5da1ec7bd 
> 
> Diff: https://reviews.apache.org/r/38639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38158: Refactored Value::Ranges coalesce().

[Michael Park]

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



src/common/values.cpp (lines 141 - 154)


This seems like an awkward `else if` and `else` style. Could we rearrange 
the comments a little to get the common structure back? For example,

```cpp
if (...) {
  ...
} else if (range.start > current.start) {
  // If we are starting farther ahead, then there are 2 cases:
  // Ranges are overlapping and we can merge them.
  if (range.start <= currentEnd + 1) {
...
  } else { // No overlap and we are adding a new range.
...
  }
}



src/common/values.cpp (lines 268 - 269)


This fits in one line


- Michael Park


On Sept. 22, 2015, 8:55 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38158/
> ---
> 
> (Updated Sept. 22, 2015, 8:55 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3051
> https://issues.apache.org/jira/browse/MESOS-3051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal of this refactoring was to reuse the Ranges objects as much as 
> possible, as prior there was substantial time spend in allocation/destruction 
> (MESOS-3051).
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
>   src/tests/resources_tests.cpp 0318885336409f7cc9dbd4a3daa9b52db197bbd1 
>   src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 
> 
> Diff: https://reviews.apache.org/r/38158/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38137]

All tests passed.

- Mesos ReviewBot


On Sept. 22, 2015, 10:15 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38137/
> ---
> 
> (Updated Sept. 22, 2015, 10:15 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang 
> Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Joining all the commits around provisioner and local store into one review so 
> it's easier to review, as patches
> are changing code on top of each other.
> 
> All the commits are going to committed together.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e224060 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
>   src/slave/containerizer/provisioner/store.cpp 35d1199 
>   src/slave/flags.hpp e31a418 
>   src/slave/flags.cpp add4196 
>   src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 
> 
> Diff: https://reviews.apache.org/r/38137/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 38639: Add os::clone function in stout.

[haosdent huang]

> On Sept. 22, 2015, 6:58 p.m., Kapil Arya wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp, line 73
> > 
> >
> > We should delete the stack only if CLONE_VM was not specified in the 
> > flags or at least have a comment mentioning the fact that this call is 
> > creating a new process and not a new thread. Having a CLONE_VM will create 
> > a new thread, not a new process.
> 
> Jie Yu wrote:
> Good call! +1
> 
> Jie Yu wrote:
> Or we can disallow CLONE_VM since if the user wants to create a thread, 
> he/she should use pthread.
> 
> Kapil Arya wrote:
> That should work too and is better in my opinion.

Thanks a lot!


- haosdent


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


On Sept. 23, 2015, 1:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38639/
> ---
> 
> (Updated Sept. 23, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::clone function in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> b994b13941628947c9d12b8baae155d5da1ec7bd 
> 
> Diff: https://reviews.apache.org/r/38639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

[Joris Van Remoortere]

> On Sept. 23, 2015, 12:48 a.m., Niklas Nielsen wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 197
> > 
> >
> > Wouldn't the idomatic way be to use stout's file abstractions?

Which ones are you referring to specifically? This is the pattern used in the 
cgroups files.
I did abstract some of this away into a systemd namespace. Please see my 
upcoming reviews.


- Joris


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


On Sept. 22, 2015, 5:29 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> ---
> 
> (Updated Sept. 22, 2015, 5:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
> Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38653: Maintenance Primitives: Expose Accept/Decline responses for inverse offers through /maintenance/status

[Qian Zhang]

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



src/master/http.cpp (line 1775)


s/master->machines[id]/machine


- Qian Zhang


On Sept. 23, 2015, 7:24 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38653/
> ---
> 
> (Updated Sept. 23, 2015, 7:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3489
> https://issues.apache.org/jira/browse/MESOS-3489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a `getInverseOfferStatuses` method to the allocator, which returns some 
> `InverseOfferStatus` objects, grouped by Agent and Framework.
> 
> Changes the `/maintenance/status` endpoint to return this additional 
> information about draining machines.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.proto 
> ee01c5d29b792ca660c9dff90ea1c3f93ac6a492 
>   include/mesos/master/allocator.hpp 2fd00ca4f0ce6efa5ffb0af33bc4e791d66c04cc 
>   include/mesos/master/allocator.proto 
> b42f19def20ac3144e07144aece0da6873888b02 
>   src/common/protobuf_utils.hpp 3817c6a3374b2e8f333784261a0c8edabf854fd9 
>   src/common/protobuf_utils.cpp 4dc58fed315d99fd9cdde49e91eab1f4947ef046 
>   src/master/allocator/mesos/allocator.hpp 
> dca256598a19ccc83885af202554dfe21e3e6095 
>   src/master/allocator/mesos/hierarchical.hpp 
> 4ec08fd4f897fb0a4acb22a06ca69175ef7b9b55 
>   src/master/http.cpp 3e44b06eafa73c5eb4cd8cd90d9e7f14b3fc4e59 
>   src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
>   src/tests/master_maintenance_tests.cpp 
> c5277a13bd42e7e5d3c298f51823f12d31a6325f 
>   src/tests/mesos.hpp ff241cca567870f6dfd85bbe835754a4156c8874 
> 
> Diff: https://reviews.apache.org/r/38653/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OSX clang 7.0)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38650: Enabled '--no-cache' for docker images build on ASF CI

[Greg Mann]

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

Ship it!


Ship It!

- Greg Mann


On Sept. 22, 2015, 11:42 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38650/
> ---
> 
> (Updated Sept. 22, 2015, 11:42 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-3491
> https://issues.apache.org/jira/browse/MESOS-3491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Docker images should be built with --no-cache=true flag to prevent outdated 
> image layers.
> 
> 
> Diffs
> -
> 
>   support/jenkins_build.sh 8451c05 
> 
> Diff: https://reviews.apache.org/r/38650/diff/
> 
> 
> Testing
> ---
> 
> Ran this patch on the ASF CI
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 38634: Added Systemd environment check to LinuxLauncher.

[Artem Harutyunyan]

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



src/linux/systemd.cpp (line 38)


I agree with Nik, stat() would work just fine here.


- Artem Harutyunyan


On Sept. 22, 2015, 6:48 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38634/
> ---
> 
> (Updated Sept. 22, 2015, 6:48 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
> Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used to perform Systemd specific actions.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e22406018d6b1d900c3f0ba1b11b13910ee9307c 
>   src/linux/systemd.hpp PRE-CREATION 
>   src/linux/systemd.cpp PRE-CREATION 
>   src/slave/containerizer/linux_launcher.hpp 
> f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
> 
> Diff: https://reviews.apache.org/r/38634/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

[Niklas Nielsen]

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



src/slave/containerizer/linux_launcher.cpp (line 197)


Wouldn't the idomatic way be to use stout's file abstractions?


- Niklas Nielsen


On Sept. 22, 2015, 10:29 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38635/
> ---
> 
> (Updated Sept. 22, 2015, 10:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
> Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
> 
> Diff: https://reviews.apache.org/r/38635/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38158: Refactored Value::Ranges coalesce().

[Michael Park]

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


I've got some clean-up suggestions, I think `lhs` and `rhs` variables should be 
renamed to `result` and `source` or something along those lines.


src/common/values.cpp (line 45)


`s/operator<< (/operator<<(`



src/common/values.cpp (line 109)


`s/protbuf/protobuf/`



src/common/values.cpp (line 111)


Why not pass by value?



src/common/values.cpp (lines 122 - 123)


How about `return std::tie(lhs.start, lhs.end) < std::tie(rhs.start, 
rhs.end);`?



src/common/values.cpp (lines 129 - 130)


How about `Range current = ranges.front();`,
and using `current.{start,end}` rather than `current{Start,End}`?



src/common/values.cpp (lines 155 - 156)


`ranges[count - 1] = current;` if you take the above suggestion.



src/common/values.cpp (lines 158 - 159)


`current = range;` if you take the above suggestion.



src/common/values.cpp (lines 170 - 186)


Could we simply `Resize` and not worry about `DeleteSubrange`, `Reserve`, 
and `add_range`?

```cpp
result->mutable_range()->Resize(count, {});
```



src/common/values.cpp (line 198)


Can we just take a `Value::Ranges` here rather than 
`initializer_list`?

It looks like the only place we actually use this is in `operator+`, for 
which we can just follow the pattern for `operator-`.

```cpp
coalesce(, left);
return result += result;
```

I think it would also clean up this code significantly, as we wouldn't need 
the `rangesSum` loop, The `fill` function wouldn't have to be factored out, 
wouldn't need the `offset` math, etc.


- Michael Park


On Sept. 22, 2015, 8:55 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38158/
> ---
> 
> (Updated Sept. 22, 2015, 8:55 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3051
> https://issues.apache.org/jira/browse/MESOS-3051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal of this refactoring was to reuse the Ranges objects as much as 
> possible, as prior there was substantial time spend in allocation/destruction 
> (MESOS-3051).
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
>   src/tests/resources_tests.cpp 0318885336409f7cc9dbd4a3daa9b52db197bbd1 
>   src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 
> 
> Diff: https://reviews.apache.org/r/38158/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38625: Replace clone system call with os::clone

[haosdent huang]

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

(Updated Sept. 23, 2015, 1:25 a.m.)


Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, Michael 
Park, and Cong Wang.


Changes
---

Update according @jieyu's reviews.


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


Repository: mesos


Description
---

Replace clone system call with os::clone.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 
  src/tests/containerizer/launch_tests.cpp 
dbe891cd004a8f8b4be252b1bd496fc5d4cc5923 
  src/tests/containerizer/ns_tests.cpp 3a22938a31b717568d38090c57ca37e440e77274 

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


Testing
---

sudo make -j8 check


Thanks,

haosdent huang

Re: Review Request 38637: Added recovery warnings for LinuxLauncher on Systemd.

[Joris Van Remoortere]

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

(Updated Sept. 23, 2015, 1:48 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
Nielsen, Thomas Rampelberg, and Timothy Chen.


Changes
---

Refactored based on BenH's comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 

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


Testing
---

make check
on ubuntu 15.04 with systemd 219 verified that pids get assigned to the 
`mesos_executor.slice`, and operator is warned about lack of proper resource 
isolation upon recovery if the executor pid is no longer in the slice.


Thanks,

Joris Van Remoortere

Re: Review Request 38634: Added Systemd environment check to LinuxLauncher.

[Joris Van Remoortere]

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

(Updated Sept. 23, 2015, 1:48 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
Nielsen, Thomas Rampelberg, and Timothy Chen.


Changes
---

Refactored based on BenH's comments.


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


Repository: mesos


Description
---

This will be used to perform Systemd specific actions.


Diffs (updated)
-

  src/Makefile.am e22406018d6b1d900c3f0ba1b11b13910ee9307c 
  src/linux/systemd.hpp PRE-CREATION 
  src/linux/systemd.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.hpp 
f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 38636: Assign executors to `mesos_executor.slice` in LinuxLauncher on Systemd.

[Joris Van Remoortere]

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

(Updated Sept. 23, 2015, 1:48 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
Nielsen, Thomas Rampelberg, and Timothy Chen.


Changes
---

Refactored based on BenH's comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

[Joris Van Remoortere]

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

(Updated Sept. 23, 2015, 1:48 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
Nielsen, Thomas Rampelberg, and Timothy Chen.


Changes
---

Refactored based on BenH's comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/linux/systemd.hpp PRE-CREATION 
  src/linux/systemd.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.hpp 
f6112c98ffcc46ebcaf5581e821d5481d2f6b494 
  src/slave/containerizer/linux_launcher.cpp 
fd0ffcf838a745ccd458d57821d358eceb85be26 
  src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
  src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 38639: Add os::clone function in stout.

[haosdent huang]

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

(Updated Sept. 23, 2015, 1:26 a.m.)


Review request for mesos, Jie Yu and Cong Wang.


Changes
---

Update according reviews.


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


Repository: mesos


Description
---

Add os::clone function in stout.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
b994b13941628947c9d12b8baae155d5da1ec7bd 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 38158: Refactored Value::Ranges coalesce().

[Joris Van Remoortere]

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

Ship it!


After addressing the outstanding issues.

- Joris Van Remoortere


On Sept. 22, 2015, 8:55 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38158/
> ---
> 
> (Updated Sept. 22, 2015, 8:55 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3051
> https://issues.apache.org/jira/browse/MESOS-3051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The goal of this refactoring was to reuse the Ranges objects as much as 
> possible, as prior there was substantial time spend in allocation/destruction 
> (MESOS-3051).
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
>   src/tests/resources_tests.cpp 0318885336409f7cc9dbd4a3daa9b52db197bbd1 
>   src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 
> 
> Diff: https://reviews.apache.org/r/38158/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38653: Maintenance Primitives: Expose Accept/Decline responses for inverse offers through /maintenance/status

[Artem Harutyunyan]

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


Great job, Joris!

- Artem Harutyunyan


On Sept. 22, 2015, 4:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38653/
> ---
> 
> (Updated Sept. 22, 2015, 4:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3489
> https://issues.apache.org/jira/browse/MESOS-3489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a `getInverseOfferStatuses` method to the allocator, which returns some 
> `InverseOfferStatus` objects, grouped by Agent and Framework.
> 
> Changes the `/maintenance/status` endpoint to return this additional 
> information about draining machines.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.proto 
> ee01c5d29b792ca660c9dff90ea1c3f93ac6a492 
>   include/mesos/master/allocator.hpp 2fd00ca4f0ce6efa5ffb0af33bc4e791d66c04cc 
>   include/mesos/master/allocator.proto 
> b42f19def20ac3144e07144aece0da6873888b02 
>   src/common/protobuf_utils.hpp 3817c6a3374b2e8f333784261a0c8edabf854fd9 
>   src/common/protobuf_utils.cpp 4dc58fed315d99fd9cdde49e91eab1f4947ef046 
>   src/master/allocator/mesos/allocator.hpp 
> dca256598a19ccc83885af202554dfe21e3e6095 
>   src/master/allocator/mesos/hierarchical.hpp 
> 4ec08fd4f897fb0a4acb22a06ca69175ef7b9b55 
>   src/master/http.cpp 3e44b06eafa73c5eb4cd8cd90d9e7f14b3fc4e59 
>   src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
>   src/tests/master_maintenance_tests.cpp 
> c5277a13bd42e7e5d3c298f51823f12d31a6325f 
>   src/tests/mesos.hpp ff241cca567870f6dfd85bbe835754a4156c8874 
> 
> Diff: https://reviews.apache.org/r/38653/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OSX clang 7.0)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38650: Enabled '--no-cache' for docker images build on ASF CI

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38650]

All tests passed.

- Mesos ReviewBot


On Sept. 22, 2015, 11:42 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38650/
> ---
> 
> (Updated Sept. 22, 2015, 11:42 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-3491
> https://issues.apache.org/jira/browse/MESOS-3491
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Docker images should be built with --no-cache=true flag to prevent outdated 
> image layers.
> 
> 
> Diffs
> -
> 
>   support/jenkins_build.sh 8451c05 
> 
> Diff: https://reviews.apache.org/r/38650/diff/
> 
> 
> Testing
> ---
> 
> Ran this patch on the ASF CI
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 37967: Added Non-Freezeer Task Killer. 36620

[Cong Wang]

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


High level comment: why not just use pid namespace to kill processes since we 
already have it?


src/linux/cgroups.hpp (line 758)


Over documented, this is private, so you can just document it in its 
definition and remove this one.



src/linux/cgroups.cpp (line 981)


s/verified/error/, otherwise reads odd.



src/linux/cgroups.cpp (line 1631)


s/chain/statuses/


- Cong Wang


On Sept. 22, 2015, 7:43 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37967/
> ---
> 
> (Updated Sept. 22, 2015, 7:43 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Non-Freezeer Task Killer. 36620
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 204c53038a1ccfa693f4f2293488cff8cdd60835 
>   src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 
> 
> Diff: https://reviews.apache.org/r/37967/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38625: Replace clone system call with os::clone

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38639, 38625]

All tests passed.

- Mesos ReviewBot


On Sept. 22, 2015, 6:52 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38625/
> ---
> 
> (Updated Sept. 22, 2015, 6:52 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, Michael 
> Park, and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace clone system call with os::clone.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
>   src/tests/containerizer/launch_tests.cpp 
> dbe891cd004a8f8b4be252b1bd496fc5d4cc5923 
>   src/tests/containerizer/ns_tests.cpp 
> 3a22938a31b717568d38090c57ca37e440e77274 
> 
> Diff: https://reviews.apache.org/r/38625/diff/
> 
> 
> Testing
> ---
> 
> sudo make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

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

(Updated Sept. 22, 2015, 10:15 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang Yan 
Xu.


Repository: mesos


Description
---

Joining all the commits around provisioner and local store into one review so 
it's easier to review, as patches
are changing code on top of each other.

All the commits are going to committed together.


Diffs (updated)
-

  src/Makefile.am e224060 
  src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/store.cpp 35d1199 
  src/slave/flags.hpp e31a418 
  src/slave/flags.cpp add4196 
  src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38568: Maintenance Primitives: Fix the formatting of the user doc.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Sept. 22, 2015, 6:51 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38568/
> ---
> 
> (Updated Sept. 22, 2015, 6:51 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3492
> https://issues.apache.org/jira/browse/MESOS-3492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also adds a link to this document from the documentation home page.
> 
> The website's markdown renderer is slightly different than the Gist renderer.
> This fixes the markdown to show correctly on the website.
> 
> 
> Diffs
> -
> 
>   docs/home.md 3dff97ba35fafe8fca7e18866f0c7e8d526022e1 
>   docs/maintenance.md 465dcfb72777611c2a4be2aa38c8cd7e869a2baa 
> 
> Diff: https://reviews.apache.org/r/38568/diff/
> 
> 
> Testing
> ---
> 
> Rendered with: https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Anand Mazumdar]

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

(Updated Sept. 22, 2015, 8:46 p.m.)


Review request for mesos, Isabel Jimenez and Vinod Kone.


Repository: mesos


Description (updated)
---

This showed up on ASF CI. From the logs:

`I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
Then 
`../../src/tests/executor_http_api_tests.cpp:290: Failure`
`Failed to wait 15secs for __recover`

Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing it 
before starting the slave. In some cases, slave would have already recovered by 
the time we invoke `FUTURE_DISPATCH` leading to the flakiness.


Diffs
-

  src/tests/executor_http_api_tests.cpp 
9dbc5191b5950df2faa693720f3740e97c7df758 

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


Testing (updated)
---

I was not able to reproduce it before or after this change but looking at the 
logs it is quite obvious what the issue was. Ran in a loop 100 times.

ASF CI error log: 
https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes


Thanks,

Anand Mazumdar

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Anand Mazumdar]

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

(Updated Sept. 22, 2015, 9:37 p.m.)


Review request for mesos, Isabel Jimenez and Vinod Kone.


Changes
---

update testing done


Repository: mesos


Description
---

This showed up on ASF CI. From the logs:

`I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
Then 
`../../src/tests/executor_http_api_tests.cpp:290: Failure`
`Failed to wait 15secs for __recover`

Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing it 
before starting the slave. In some cases, slave would have already recovered by 
the time we invoke `FUTURE_DISPATCH` leading to the flakiness.


Diffs
-

  src/tests/executor_http_api_tests.cpp 
9dbc5191b5950df2faa693720f3740e97c7df758 

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


Testing (updated)
---

Introduced a sleep before `AWAIT_READY`, the test failed. After this change 
with the sleep it still passed.

Ran in a loop 100 times.

ASF CI error log: 
https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes


Thanks,

Anand Mazumdar

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Neil Conway]

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

Ship it!


Ship It!

- Neil Conway


On Sept. 22, 2015, 9:37 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38645/
> ---
> 
> (Updated Sept. 22, 2015, 9:37 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This showed up on ASF CI. From the logs:
> 
> `I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
> Then 
> `../../src/tests/executor_http_api_tests.cpp:290: Failure`
> `Failed to wait 15secs for __recover`
> 
> Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing 
> it before starting the slave. In some cases, slave would have already 
> recovered by the time we invoke `FUTURE_DISPATCH` leading to the flakiness.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 9dbc5191b5950df2faa693720f3740e97c7df758 
> 
> Diff: https://reviews.apache.org/r/38645/diff/
> 
> 
> Testing
> ---
> 
> Introduced a sleep before `AWAIT_READY`, the test failed. After this change 
> with the sleep it still passed.
> 
> Ran in a loop 100 times.
> 
> ASF CI error log: 
> https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Anand Mazumdar]

> On Sept. 22, 2015, 9:09 p.m., Neil Conway wrote:
> > For the sake of repro'ing, maybe you could add a sleep before waiting on 
> > the future? Obviously not something we want in the actual patch though.

Thanks Neil, that worked. Updated the `Testing Done` section with the details 
now. Should have spent more time reproducing it then just leaving it to 
inference from the error logs.


- Anand


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


On Sept. 22, 2015, 8:46 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38645/
> ---
> 
> (Updated Sept. 22, 2015, 8:46 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This showed up on ASF CI. From the logs:
> 
> `I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
> Then 
> `../../src/tests/executor_http_api_tests.cpp:290: Failure`
> `Failed to wait 15secs for __recover`
> 
> Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing 
> it before starting the slave. In some cases, slave would have already 
> recovered by the time we invoke `FUTURE_DISPATCH` leading to the flakiness.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 9dbc5191b5950df2faa693720f3740e97c7df758 
> 
> Diff: https://reviews.apache.org/r/38645/diff/
> 
> 
> Testing
> ---
> 
> I was not able to reproduce it before or after this change but looking at the 
> logs it is quite obvious what the issue was. Ran in a loop 100 times.
> 
> ASF CI error log: 
> https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[James Peach]

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

Review request for mesos.


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 38158: Refactored Value::Ranges coalesce().

[Joerg Schad]

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

(Updated Sept. 22, 2015, 8:55 p.m.)


Review request for mesos, Bernd Mathiske, Joris Van Remoortere, and Till 
Toenshoff.


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


Repository: mesos


Description
---

The goal of this refactoring was to reuse the Ranges objects as much as 
possible, as prior there was substantial time spend in allocation/destruction 
(MESOS-3051).


Diffs (updated)
-

  src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f 
  src/tests/resources_tests.cpp 0318885336409f7cc9dbd4a3daa9b52db197bbd1 
  src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 

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


Testing
---

make check


Thanks,

Joerg Schad

Review Request 38646: Added WIP note for Executor endpoint in changelog

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

WIP Note for the 0.25 release that `api/v1/executor` endpoint is incomplete.


Diffs
-

  CHANGELOG 885ab46ce90b60ae51c5ffbd3578c848c73321a7 

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


Testing
---


Thanks,

Anand Mazumdar

Re: Review Request 38517: Make attributes.hpp public

[Connor Doyle]

> On Sept. 22, 2015, 12:40 a.m., Connor Doyle wrote:
> > Ship It!
> 
> Connor Doyle wrote:
> Please re-run the post-reviews so the patch applies cleanly 
> (`src/slave/http.cpp` was concurrently modified).
> 
> Felix Abecassis wrote:
> Done. Please verify.

Verified (patch applied, make, make check).


- Connor


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


On Sept. 22, 2015, 1:26 a.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38517/
> ---
> 
> (Updated Sept. 22, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Connor Doyle and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is required in order to enable callback hooks that can modify attributes 
> of a slave during initialization.
> 
> 
> Diffs
> -
> 
>   include/mesos/attributes.hpp PRE-CREATION 
>   src/Makefile.am e224060 
>   src/common/attributes.hpp 2a7efbd 
>   src/common/attributes.cpp f713bb5 
>   src/common/http.hpp 058baa6 
>   src/common/http.cpp aaef10b 
>   src/common/type_utils.cpp 22118b4 
>   src/master/http.cpp 3e44b06 
>   src/slave/http.cpp 12a4d39 
>   src/slave/slave.hpp 7a54fad 
>   src/tests/attributes_tests.cpp ded6120 
>   src/tests/registrar_tests.cpp 5131b57 
> 
> Diff: https://reviews.apache.org/r/38517/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Review Request 38645: Fixed Flaky Executor HTTP tests

[Anand Mazumdar]

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

Review request for mesos, Isabel Jimenez and Vinod Kone.


Repository: mesos


Description
---

This showed up on ASF. From the logs:

`I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
Then 
`../../src/tests/executor_http_api_tests.cpp:290: Failure`
`Failed to wait 15secs for __recover`

Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing it 
before starting the slave. In some cases, slave would have already recovered by 
the time we invoke `FUTURE_DISPATCH` leading to the flakiness.


Diffs
-

  src/tests/executor_http_api_tests.cpp 
9dbc5191b5950df2faa693720f3740e97c7df758 

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


Testing
---

I was not able to reproduce it before or after this change but looking at the 
logs it is quite obvious what the issue was. Ran in a loop 100 times.

ASF error log: 
https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes


Thanks,

Anand Mazumdar

Re: Review Request 38517: Make attributes.hpp public

[Niklas Nielsen]

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

Ship it!


Ship It!

- Niklas Nielsen


On Sept. 21, 2015, 6:26 p.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38517/
> ---
> 
> (Updated Sept. 21, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Connor Doyle and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is required in order to enable callback hooks that can modify attributes 
> of a slave during initialization.
> 
> 
> Diffs
> -
> 
>   include/mesos/attributes.hpp PRE-CREATION 
>   src/Makefile.am e224060 
>   src/common/attributes.hpp 2a7efbd 
>   src/common/attributes.cpp f713bb5 
>   src/common/http.hpp 058baa6 
>   src/common/http.cpp aaef10b 
>   src/common/type_utils.cpp 22118b4 
>   src/master/http.cpp 3e44b06 
>   src/slave/http.cpp 12a4d39 
>   src/slave/slave.hpp 7a54fad 
>   src/tests/attributes_tests.cpp ded6120 
>   src/tests/registrar_tests.cpp 5131b57 
> 
> Diff: https://reviews.apache.org/r/38517/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Review Request 38648: Hack the allocator interface to expose the batch allocation internals.

[James Peach]

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

Review request for mesos.


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


Repository: mesos


Description
---

Hack the allocator interface to expose the batch allocation internals.


Diffs
-

  src/master/allocator/mesos/allocator.hpp 
dca256598a19ccc83885af202554dfe21e3e6095 
  src/master/allocator/mesos/hierarchical.hpp 
4ec08fd4f897fb0a4acb22a06ca69175ef7b9b55 

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


Testing
---

make check && make bench


Thanks,

James Peach

Review Request 38660: Restore Linux/Posix launcher creation logic for MesosContainerizer.

[Kapil Arya]

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

Review request for mesos, haosdent huang and Jie Yu.


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


Repository: mesos


Description
---

The current logic uses LinuxLauncher if running as root. However, this causes
problems with cgroups freezer when we launch multiple Agents inside the test
suite.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
b4a77e734706f40dc1d2110939d4f33fcbd1fd8c 
  src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
  src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 

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


Testing
---

sudo make check currently works for all but one test. Here is the failing case:

```
[--] 1 test from LimitedCpuIsolatorTest
[ RUN  ] LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
../../src/tests/containerizer/isolator_tests.cpp:742: Failure
Value of: usage.get().processes()
  Actual: 2
Expected: 1U
Which is: 1
../../src/tests/containerizer/isolator_tests.cpp:743: Failure
Value of: usage.get().threads()
  Actual: 2
Expected: 1U
Which is: 1
[  FAILED  ] LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids (187 ms)
[--] 1 test from LimitedCpuIsolatorTest (188 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (199 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids

 1 FAILED TEST
```

I am still investigating the failure but thought of putting out the review to 
get some comments in the meanwhile (and some possible hints :-)).


Thanks,

Kapil Arya

Re: Review Request 38653: Maintenance Primitives: Expose Accept/Decline responses for inverse offers through /maintenance/status

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38653]

All tests passed.

- Mesos ReviewBot


On Sept. 22, 2015, 11:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38653/
> ---
> 
> (Updated Sept. 22, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3489
> https://issues.apache.org/jira/browse/MESOS-3489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a `getInverseOfferStatuses` method to the allocator, which returns some 
> `InverseOfferStatus` objects, grouped by Agent and Framework.
> 
> Changes the `/maintenance/status` endpoint to return this additional 
> information about draining machines.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.proto 
> ee01c5d29b792ca660c9dff90ea1c3f93ac6a492 
>   include/mesos/master/allocator.hpp 2fd00ca4f0ce6efa5ffb0af33bc4e791d66c04cc 
>   include/mesos/master/allocator.proto 
> b42f19def20ac3144e07144aece0da6873888b02 
>   src/common/protobuf_utils.hpp 3817c6a3374b2e8f333784261a0c8edabf854fd9 
>   src/common/protobuf_utils.cpp 4dc58fed315d99fd9cdde49e91eab1f4947ef046 
>   src/master/allocator/mesos/allocator.hpp 
> dca256598a19ccc83885af202554dfe21e3e6095 
>   src/master/allocator/mesos/hierarchical.hpp 
> 4ec08fd4f897fb0a4acb22a06ca69175ef7b9b55 
>   src/master/http.cpp 3e44b06eafa73c5eb4cd8cd90d9e7f14b3fc4e59 
>   src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
>   src/tests/master_maintenance_tests.cpp 
> c5277a13bd42e7e5d3c298f51823f12d31a6325f 
>   src/tests/mesos.hpp ff241cca567870f6dfd85bbe835754a4156c8874 
> 
> Diff: https://reviews.apache.org/r/38653/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OSX clang 7.0)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38637: Added recovery warnings for LinuxLauncher on Systemd.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [38634, 38635]

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

Error:
 2015-09-23 03:49:49 URL:https://reviews.apache.org/r/38635/diff/raw/ 
[9385/9385] -> "38635.patch" [1]
38635.patch:232: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Successfully applied: Create and Start `mesos_executor.slice` in LinuxLauncher 
on Systemd.

See summary.


Review: https://reviews.apache.org/r/38635
src/linux/systemd.cpp:205: new blank line at EOF.
Failed to commit patch

- Mesos ReviewBot


On Sept. 23, 2015, 1:48 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38637/
> ---
> 
> (Updated Sept. 23, 2015, 1:48 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas 
> Nielsen, Thomas Rampelberg, and Timothy Chen.
> 
> 
> Bugs: MESOS-3425
> https://issues.apache.org/jira/browse/MESOS-3425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
> 
> Diff: https://reviews.apache.org/r/38637/diff/
> 
> 
> Testing
> ---
> 
> make check
> on ubuntu 15.04 with systemd 219 verified that pids get assigned to the 
> `mesos_executor.slice`, and operator is warned about lack of proper resource 
> isolation upon recovery if the executor pid is no longer in the slice.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38399: Add ACLs for the maintenance HTTP endpoints

[Zhiwei Chen]

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

(Updated Sept. 23, 2015, 1:16 p.m.)


Review request for mesos, Alexander Rojas, Artem Harutyunyan, Joris Van 
Remoortere, Joseph Wu, and Till Toenshoff.


Bugs: mesos-
https://issues.apache.org/jira/browse/mesos-


Repository: mesos


Description (updated)
---

Add ACLs for the maintenance HTTP endpoints

The design doc: 
https://docs.google.com/document/d/1sG28ASH-5MDygR2igackOb5x-Lt5nXyIUMcwrrbk4cE/edit?usp=sharing


Diffs
-

  docs/authentication.md 1c22c5416caf66b28238fc181a255e51ed16d867 
  docs/authorization.md 2a5e74782754e446184a297f91112e9f94077896 
  include/mesos/authorizer/authorizer.hpp 
d667a52f90f970a313580446a5a006cec4b5e25b 
  include/mesos/authorizer/authorizer.proto 
86bbb45f9d91b4098a262e3e50a793f3bb39497e 
  src/authorizer/local/authorizer.hpp 32de102fd588f029882ef121ca83a7410c65 
  src/authorizer/local/authorizer.cpp 6d7da87731a438c2180cf91003e09d4aa5a1c773 
  src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
  src/master/http.cpp f7ce9aa56b453c1d37b99dd836d956ab292ab62e 
  src/master/master.hpp d48ef7c0da8978a5e02e69e055ff010585b20ceb 
  src/tests/master_maintenance_tests.cpp 
44785057f129a3e6a69f399f7d6db59d9d5c2e91 
  src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 

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


Testing
---

make check


Thanks,

Zhiwei Chen

Re: Review Request 38639: Add os::clone function in stout.

[Cong Wang]

> On Sept. 22, 2015, 6:55 p.m., Cong Wang wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp, line 52
> > 
> >
> > s/namespaces/flags/, because SIG* can be or'ed with namespaces. Or even 
> > better, pass namespaces separately with signals as two different parameters?
> 
> haosdent huang wrote:
> Because I see ::clone only have one param, I just change 
> s/namespaces/flags/ here.

I meant:

clone(...int flags, int signal)
{
::clone(... flags | siganl, ...);
}


- Cong


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


On Sept. 23, 2015, 1:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38639/
> ---
> 
> (Updated Sept. 23, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::clone function in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> b994b13941628947c9d12b8baae155d5da1ec7bd 
> 
> Diff: https://reviews.apache.org/r/38639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38625: Replace clone system call with os::clone

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38639, 38625]

All tests passed.

- Mesos ReviewBot


On Sept. 23, 2015, 1:25 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38625/
> ---
> 
> (Updated Sept. 23, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, Michael 
> Park, and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace clone system call with os::clone.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> fd0ffcf838a745ccd458d57821d358eceb85be26 
>   src/tests/containerizer/launch_tests.cpp 
> dbe891cd004a8f8b4be252b1bd496fc5d4cc5923 
>   src/tests/containerizer/ns_tests.cpp 
> 3a22938a31b717568d38090c57ca37e440e77274 
> 
> Diff: https://reviews.apache.org/r/38625/diff/
> 
> 
> Testing
> ---
> 
> sudo make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38653: Maintenance Primitives: Expose Accept/Decline responses for inverse offers through /maintenance/status

[Joris Van Remoortere]

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



src/master/http.cpp (line 1751)


Can you add a brief comment about the inverse offer statuses getting 
cleared if the master fails over

Here and around line 1772 where we implement the transformation.



src/tests/master_maintenance_tests.cpp (line 1231)


new line after we wrap the previous
same below.


- Joris Van Remoortere


On Sept. 22, 2015, 11:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38653/
> ---
> 
> (Updated Sept. 22, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3489
> https://issues.apache.org/jira/browse/MESOS-3489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a `getInverseOfferStatuses` method to the allocator, which returns some 
> `InverseOfferStatus` objects, grouped by Agent and Framework.
> 
> Changes the `/maintenance/status` endpoint to return this additional 
> information about draining machines.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.proto 
> ee01c5d29b792ca660c9dff90ea1c3f93ac6a492 
>   include/mesos/master/allocator.hpp 2fd00ca4f0ce6efa5ffb0af33bc4e791d66c04cc 
>   include/mesos/master/allocator.proto 
> b42f19def20ac3144e07144aece0da6873888b02 
>   src/common/protobuf_utils.hpp 3817c6a3374b2e8f333784261a0c8edabf854fd9 
>   src/common/protobuf_utils.cpp 4dc58fed315d99fd9cdde49e91eab1f4947ef046 
>   src/master/allocator/mesos/allocator.hpp 
> dca256598a19ccc83885af202554dfe21e3e6095 
>   src/master/allocator/mesos/hierarchical.hpp 
> 4ec08fd4f897fb0a4acb22a06ca69175ef7b9b55 
>   src/master/http.cpp 3e44b06eafa73c5eb4cd8cd90d9e7f14b3fc4e59 
>   src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
>   src/tests/master_maintenance_tests.cpp 
> c5277a13bd42e7e5d3c298f51823f12d31a6325f 
>   src/tests/mesos.hpp ff241cca567870f6dfd85bbe835754a4156c8874 
> 
> Diff: https://reviews.apache.org/r/38653/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OSX clang 7.0)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38660: Restore Linux/Posix launcher creation logic for MesosContainerizer.

[haosdent huang]

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


Hi, @kapil The flaky test you metioned is a known issue. 
[MESOS-3293](https://issues.apache.org/jira/browse/MESOS-3293) and 
[Patch](https://reviews.apache.org/r/38454/) I think you could ignore the flaky 
test result.

- haosdent huang


On Sept. 23, 2015, 4:21 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38660/
> ---
> 
> (Updated Sept. 23, 2015, 4:21 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3498
> https://issues.apache.org/jira/browse/MESOS-3498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current logic uses LinuxLauncher if running as root. However, this causes
> problems with cgroups freezer when we launch multiple Agents inside the test
> suite.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b4a77e734706f40dc1d2110939d4f33fcbd1fd8c 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
> 
> Diff: https://reviews.apache.org/r/38660/diff/
> 
> 
> Testing
> ---
> 
> sudo make check currently works for all but one test. Here is the failing 
> case:
> 
> ```
> [--] 1 test from LimitedCpuIsolatorTest
> [ RUN  ] LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> ../../src/tests/containerizer/isolator_tests.cpp:742: Failure
> Value of: usage.get().processes()
>   Actual: 2
> Expected: 1U
> Which is: 1
> ../../src/tests/containerizer/isolator_tests.cpp:743: Failure
> Value of: usage.get().threads()
>   Actual: 2
> Expected: 1U
> Which is: 1
> [  FAILED  ] LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids (187 ms)
> [--] 1 test from LimitedCpuIsolatorTest (188 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (199 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] LimitedCpuIsolatorTest.ROOT_CGROUPS_Pids_and_Tids
> 
>  1 FAILED TEST
> ```
> 
> I am still investigating the failure but thought of putting out the review to 
> get some comments in the meanwhile (and some possible hints :-)).
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 38639: Add os::clone function in stout.

[Cong Wang]

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp (line 138)


This whitespace change is not supposed in this patch.


- Cong Wang


On Sept. 23, 2015, 1:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38639/
> ---
> 
> (Updated Sept. 23, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Cong Wang.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add os::clone function in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> b994b13941628947c9d12b8baae155d5da1ec7bd 
> 
> Diff: https://reviews.apache.org/r/38639/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38645: Fixed Flaky Executor HTTP tests

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38645]

All tests passed.

- Mesos ReviewBot


On Sept. 22, 2015, 9:37 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38645/
> ---
> 
> (Updated Sept. 22, 2015, 9:37 p.m.)
> 
> 
> Review request for mesos, Isabel Jimenez and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This showed up on ASF CI. From the logs:
> 
> `I0922 17:31:49.819221 28463 slave.cpp:4104] Finished recovery”`
> Then 
> `../../src/tests/executor_http_api_tests.cpp:290: Failure`
> `Failed to wait 15secs for __recover`
> 
> Instead of doing a `FUTURE_DISPATCH` after `StartSlave()` we should be doing 
> it before starting the slave. In some cases, slave would have already 
> recovered by the time we invoke `FUTURE_DISPATCH` leading to the flakiness.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 9dbc5191b5950df2faa693720f3740e97c7df758 
> 
> Diff: https://reviews.apache.org/r/38645/diff/
> 
> 
> Testing
> ---
> 
> Introduced a sleep before `AWAIT_READY`, the test failed. After this change 
> with the sleep it still passed.
> 
> Ran in a loop 100 times.
> 
> ASF CI error log: 
> https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu%3A14.04,label_exp=docker%7C%7CHadoop/839/changes
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 36620: Added Non-Freezeer Task Killer.

[Jie Yu]

> On Sept. 1, 2015, 2:52 p.m., Mesos ReviewBot wrote:
> > Patch looks great!
> > 
> > Reviews applied: [36612, 36620]
> > 
> > All tests passed.
> 
> Timothy Chen wrote:
> Jie does this look good to you?

Should we close this given https://reviews.apache.org/r/37967/?


- Jie


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


On Sept. 1, 2015, 2:12 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36620/
> ---
> 
> (Updated Sept. 1, 2015, 2:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Till Toenshoff, and Timothy Chen.
> 
> 
> Bugs: MESOS-3086
> https://issues.apache.org/jira/browse/MESOS-3086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Non-Freezeer Task Killer.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 204c53038a1ccfa693f4f2293488cff8cdd60835 
>   src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f 
> 
> Diff: https://reviews.apache.org/r/36620/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> + manual tests
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

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

(Updated Sept. 22, 2015, 11:38 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

minor style changes.


Repository: mesos


Description
---

First iteration with basic functionality.


Diffs (updated)
-

  src/Makefile.am 2286366852b272c808490b6bd87ac9e894de57ac 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38653: Maintenance Primitives: Expose Accept/Decline responses for inverse offers through /maintenance/status

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 九月 22, 2015, 11:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38653/
> ---
> 
> (Updated 九月 22, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3489
> https://issues.apache.org/jira/browse/MESOS-3489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a `getInverseOfferStatuses` method to the allocator, which returns some 
> `InverseOfferStatus` objects, grouped by Agent and Framework.
> 
> Changes the `/maintenance/status` endpoint to return this additional 
> information about draining machines.
> 
> 
> Diffs
> -
> 
>   include/mesos/maintenance/maintenance.proto 
> ee01c5d29b792ca660c9dff90ea1c3f93ac6a492 
>   include/mesos/master/allocator.hpp 2fd00ca4f0ce6efa5ffb0af33bc4e791d66c04cc 
>   include/mesos/master/allocator.proto 
> b42f19def20ac3144e07144aece0da6873888b02 
>   src/common/protobuf_utils.hpp 3817c6a3374b2e8f333784261a0c8edabf854fd9 
>   src/common/protobuf_utils.cpp 4dc58fed315d99fd9cdde49e91eab1f4947ef046 
>   src/master/allocator/mesos/allocator.hpp 
> dca256598a19ccc83885af202554dfe21e3e6095 
>   src/master/allocator/mesos/hierarchical.hpp 
> 4ec08fd4f897fb0a4acb22a06ca69175ef7b9b55 
>   src/master/http.cpp 3e44b06eafa73c5eb4cd8cd90d9e7f14b3fc4e59 
>   src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
>   src/tests/master_maintenance_tests.cpp 
> c5277a13bd42e7e5d3c298f51823f12d31a6325f 
>   src/tests/mesos.hpp ff241cca567870f6dfd85bbe835754a4156c8874 
> 
> Diff: https://reviews.apache.org/r/38653/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OSX clang 7.0)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>