Re: Review Request 68929: Fixed the nested container launch failure on the agent upgrade case.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68929 was successfully built and tested.

Reviews applied: `['68929']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2421/mesos-review-68929

- Mesos Reviewbot Windows


On Oct. 5, 2018, 12:37 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68929/
> ---
> 
> (Updated Oct. 5, 2018, 12:37 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-9295
> https://issues.apache.org/jira/browse/MESOS-9295
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the nested container launch failure on the agent upgrade case.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 11dfbab182487c45fa494c41e31f777272b4b5d0 
> 
> 
> Diff: https://reviews.apache.org/r/68929/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 68929: Fixed the nested container launch failure on the agent upgrade case.

[Gilbert Song]

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

Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Fixed the nested container launch failure on the agent upgrade case.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
11dfbab182487c45fa494c41e31f777272b4b5d0 


Diff: https://reviews.apache.org/r/68929/diff/1/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 68923: Updated Docker library to avoid 'os::killtree()' when discarding.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68923 was successfully built and tested.

Reviews applied: `['68923']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2420/mesos-review-68923

- Mesos Reviewbot Windows


On Oct. 4, 2018, 10:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68923/
> ---
> 
> (Updated Oct. 4, 2018, 10:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-9283
> https://issues.apache.org/jira/browse/MESOS-9283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the Docker library to consistently use the
> overload of subprocess() which runs its binary via exec()
> rather than through a shell. This makes it safe to use
> os::kill() rather than os::killtree() when discarding
> these commands, which this patch also accomplishes.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 25d9ca662fa5d99b32c668a5fdfc75584132cc38 
>   src/docker/docker.cpp fb39f7480045c225096e07d7d55cd3aa7b870bc5 
> 
> 
> Diff: https://reviews.apache.org/r/68923/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*DOCKER*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 68924: Updated the Docker library to avoid 'os::killtree()'.

[Greg Mann]

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

(Updated Oct. 4, 2018, 10:14 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

This patch updates the Docker library to consistently use the
overload of `subprocess()` which runs its binary via `exec()`
rather than through a shell. This makes it safe to use
`os::kill()` rather than `os::killtree()` when discarding
these commands, which this patch also accomplishes.


Diffs
-

  src/docker/docker.cpp fb39f7480045c225096e07d7d55cd3aa7b870bc5 


Diff: https://reviews.apache.org/r/68924/diff/1/


Testing
---

`sudo bin/mesos-tests.sh --gtest_filter="*DOCKER*"`


Thanks,

Greg Mann

Re: Review Request 68923: Updated Docker library to avoid 'os::killtree()' when discarding.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 4, 2018, 10:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68923/
> ---
> 
> (Updated Oct. 4, 2018, 10:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-9283
> https://issues.apache.org/jira/browse/MESOS-9283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the Docker library to consistently use the
> overload of subprocess() which runs its binary via exec()
> rather than through a shell. This makes it safe to use
> os::kill() rather than os::killtree() when discarding
> these commands, which this patch also accomplishes.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 25d9ca662fa5d99b32c668a5fdfc75584132cc38 
>   src/docker/docker.cpp fb39f7480045c225096e07d7d55cd3aa7b870bc5 
> 
> 
> Diff: https://reviews.apache.org/r/68923/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*DOCKER*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 68923: Updated Docker library to avoid 'os::killtree()' when discarding.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 4, 2018, 3:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68923/
> ---
> 
> (Updated Oct. 4, 2018, 3:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-9283
> https://issues.apache.org/jira/browse/MESOS-9283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the Docker library to consistently use the
> overload of subprocess() which runs its binary via exec()
> rather than through a shell. This makes it safe to use
> os::kill() rather than os::killtree() when discarding
> these commands, which this patch also accomplishes.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 25d9ca662fa5d99b32c668a5fdfc75584132cc38 
>   src/docker/docker.cpp fb39f7480045c225096e07d7d55cd3aa7b870bc5 
> 
> 
> Diff: https://reviews.apache.org/r/68923/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*DOCKER*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 68923: Updated Docker library to avoid 'os::killtree()' when discarding.

[Greg Mann]

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

(Updated Oct. 4, 2018, 10:14 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Summary (updated)
-

Updated Docker library to avoid 'os::killtree()' when discarding.


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


Repository: mesos


Description (updated)
---

This patch updates the Docker library to consistently use the
overload of subprocess() which runs its binary via exec()
rather than through a shell. This makes it safe to use
os::kill() rather than os::killtree() when discarding
these commands, which this patch also accomplishes.


Diffs (updated)
-

  src/docker/docker.hpp 25d9ca662fa5d99b32c668a5fdfc75584132cc38 
  src/docker/docker.cpp fb39f7480045c225096e07d7d55cd3aa7b870bc5 


Diff: https://reviews.apache.org/r/68923/diff/2/

Changes: https://reviews.apache.org/r/68923/diff/1-2/


Testing
---

`sudo bin/mesos-tests.sh --gtest_filter="*DOCKER*"`


Thanks,

Greg Mann

Re: Review Request 68866: Waited for TEARDOWN response in v1 Java scheduler shim.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Oct. 4, 2018, 10:02 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68866/
> ---
> 
> (Updated Oct. 4, 2018, 10:02 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker.
> 
> 
> Bugs: MESOS-9274
> https://issues.apache.org/jira/browse/MESOS-9274
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Destruction of the library is not always under our control. For
> example, the JVM can call JNI `finalize()` if the Java scheduler
> nullifies its reference to the V1Mesos library immediately after
> sending `TEARDOWN`. We want to make sure that `TEARDOWN` is sent
> before the Mesos library is destructed.
> 
> 
> Diffs
> -
> 
>   src/java/jni/org_apache_mesos_v1_scheduler_V1Mesos.cpp 
> 2a5fbd79ac7bad933067cd96e38186849af8edc4 
> 
> 
> Diff: https://reviews.apache.org/r/68866/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` on various Linux distro
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 68865: Put `TerminateEvent` at the end of the queue in the Mesos library.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Oct. 4, 2018, 9:38 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68865/
> ---
> 
> (Updated Oct. 4, 2018, 9:38 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker.
> 
> 
> Bugs: MESOS-9274
> https://issues.apache.org/jira/browse/MESOS-9274
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is to ensure all pending dispatches are processed. However
> multistage events, e.g., `Call`, might still be dropped, because
> a continuation (stage) of such event can be dispatched after the
> termination event.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 471152945d6af660c8983324b38702d872657f89 
> 
> 
> Diff: https://reviews.apache.org/r/68865/diff/2/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/68866/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 68916: Moved libevent_openssl validation into libevent.m4.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Oct. 3, 2018, 7:32 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68916/
> ---
> 
> (Updated Oct. 3, 2018, 7:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Combines all libevent related header and library checks into
> libevent.m4.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac c3e39643924dd4581d46e3d4e1b89b409d823e88 
>   3rdparty/libprocess/m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68916/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68906: Fixed ssl build specific incompatiblity with libevent later than 2.1.5.

[James Peach]

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


Ship it!




Looks good. The only question I have is whether we should also add a call to 
enable events on the new SSL sockaet at the end of 
`LibeventSSLSocketImpl::accept_SSL_callback` after we call `bufferevent_setcb` 
on it.

- James Peach


On Oct. 3, 2018, 11:06 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68906/
> ---
> 
> (Updated Oct. 3, 2018, 11:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-9265
> https://issues.apache.org/jira/browse/MESOS-9265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Up until and including libevent 2.1.5 (beta) the buffer events EV_READ
> and EV_WRITE were enabled by default in the openssl related socket
> setup.
> 
> That got changed by f4b6284b8393dbabf389ddce734a30f4cdeffa17 within
> libevent; the default expectancy of read and write buffer events got
> removed.
> 
> The changes in libevent in turn caused our implementation on top of it
> to fail, typically directly after accept and connect calls.
> 
> The libevent maintainer Azat Khuzhin has generously been helping us out
> by debugging and fixing the incomplete setup within libprocess.
> 
> This patch is based on Azat's suggestions from https://goo.gl/4CK3PD.
> It has been validated against libevent 2.0.22, 2.1.8 and 2.2 (alpha)
> on macOS 10.14.1 and Ubuntu 16.04.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 436b38913e43233e352cf9783da8fafefa42226f 
> 
> 
> Diff: https://reviews.apache.org/r/68906/diff/2/
> 
> 
> Testing
> ---
> 
> make check with and without bundled libevent.
> 
> Tested combinations were:
> macOS 10.14.1 - libevent 2.2
> macOS 10.14.1 - libevent 2.1.8
> macOS 10.14.1 - libevent 2.0.22 
> Ubuntu 16.04  - libevent 2.1.8
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68915: Moved libevent_openssl validation into libevent.m4.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Oct. 3, 2018, 7:32 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68915/
> ---
> 
> (Updated Oct. 3, 2018, 7:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Combines all libevent related header and library checks into
> libevent.m4.
> 
> 
> Diffs
> -
> 
>   configure.ac 505201a1ce84834b574067650993ce51cf1e1d5d 
>   m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68915/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68914: Updated libevent linkage to adhere to best practices.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Oct. 3, 2018, 4:51 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68914/
> ---
> 
> (Updated Oct. 3, 2018, 4:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-9222
> https://issues.apache.org/jira/browse/MESOS-9222
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> libevent is a combination of libevent_core and libevent_extra. Mesos
> and libprocess do not make use of any libevent_extra functionality.
> 
> Additionally, linking against libevent is against best practices.
> 
> Replaces use of libevent with libevent_core in both libevent.m4 as
> well as the libprocess linkage.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 22b1395ce69f4ac630367427542fa1ad4c88b50b 
>   3rdparty/libprocess/m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68914/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68913: Updated libevent linkage to adhere to best practices.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Oct. 3, 2018, 4:50 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68913/
> ---
> 
> (Updated Oct. 3, 2018, 4:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-9222
> https://issues.apache.org/jira/browse/MESOS-9222
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> libevent is a combination of libevent_core and libevent_extra. Mesos
> and libprocess do not make use of any libevent_extra functionality.
> 
> Additionally, linking against libevent is against best practices.
> 
> Replaces use of libevent with libevent_core in libevent.m4,
> FindLIBEVENT.cmake as well as the native Python egg linkage.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/FindLIBEVENT.cmake d6a1e65a2660fa96f50c08298b1e61ac42d85cee 
>   m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
>   src/python/native_common/ext_modules.py.in 
> df10b5c9131011b0a6b6cb83bceb6d7444aa35eb 
> 
> 
> Diff: https://reviews.apache.org/r/68913/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68905: Removed version check from libevent dependency tracking from libprocess.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Oct. 3, 2018, 1:31 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68905/
> ---
> 
> (Updated Oct. 3, 2018, 1:31 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-9265
> https://issues.apache.org/jira/browse/MESOS-9265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed version check from libevent dependency tracking from libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac c3e39643924dd4581d46e3d4e1b89b409d823e88 
>   3rdparty/libprocess/m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68905/diff/1/
> 
> 
> Testing
> ---
> 
> make check on macOS 10.14.1 with and without bundled libevent.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68904: Removed version check from libevent dependency tracking.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Oct. 3, 2018, 1:30 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68904/
> ---
> 
> (Updated Oct. 3, 2018, 1:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-9265
> https://issues.apache.org/jira/browse/MESOS-9265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed version check from libevent dependency tracking.
> 
> 
> Diffs
> -
> 
>   configure.ac 505201a1ce84834b574067650993ce51cf1e1d5d 
>   m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68904/diff/1/
> 
> 
> Testing
> ---
> 
> make check on macOS 10.14.1 with and without bundled libevent.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68924: Updated the Docker library to avoid 'os::killtree()'.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 4, 2018, 5:26 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68924/
> ---
> 
> (Updated Oct. 4, 2018, 5:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-9283
> https://issues.apache.org/jira/browse/MESOS-9283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the Docker library to consistently use the
> overload of `subprocess()` which runs its binary via `exec()`
> rather than through a shell. This makes it safe to use
> `os::kill()` rather than `os::killtree()` when discarding
> these commands, which this patch also accomplishes.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp fb39f7480045c225096e07d7d55cd3aa7b870bc5 
> 
> 
> Diff: https://reviews.apache.org/r/68924/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*DOCKER*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 68923: Updated 'Docker::inspect()' to avoid 'os::killtree()'.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 4, 2018, 5:22 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68923/
> ---
> 
> (Updated Oct. 4, 2018, 5:22 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-9283
> https://issues.apache.org/jira/browse/MESOS-9283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When futures returned by `Docker::inspect()` are discarded
> frequently, agent node performance can be negatively
> impacted due to the `os::killtree()` call in the discard
> handler.
> 
> This patch avoids running `docker inspect` commands through
> a shell so that it's safe to use `os::kill()` when
> discarding the returned futures.
> 
> This change is being made independently from similar
> changes to the rest of the Docker library so that it can be
> more easily backported to previous versions, since issues
> related to `Docker::inspect()` in particular have been
> observed.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 25d9ca662fa5d99b32c668a5fdfc75584132cc38 
>   src/docker/docker.cpp fb39f7480045c225096e07d7d55cd3aa7b870bc5 
> 
> 
> Diff: https://reviews.apache.org/r/68923/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*DOCKER*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 68924: Updated the Docker library to avoid 'os::killtree()'.

[Greg Mann]

> On Oct. 4, 2018, 6:25 a.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some of the unit tests failed. Please check the relevant logs.
> > 
> > Reviews applied: `['68923', '68924']`
> > 
> > Failed command: `Start-MesosCITesting`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2418/mesos-review-68924
> > 
> > Relevant logs:
> > 
> > - 
> > [mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2418/mesos-review-68924/logs/mesos-tests.log):
> > 
> > ```
> > I1004 06:25:30.318265 18864 executor.cpp:805] Shutting down
> > I1004 06:25:30.318265 18864 executor.cpp:918] Sending SIGTERM to process 
> > tree at pid 1106:25:30.315294 16068 slave.cpp:6640] Shutting down executor 
> > '4a510516-6cf6-475f-ad54-6e5202dee38a' of framework 
> > 455c5a23-b91c-4314-89df-13fed3c13608- at executor(1)@192.10.1.5:50375
> > I1004 06:25:30.318265 19344 master.cpp:11030] Removing task 
> > 4a510516-6cf6-475f-ad54-6e5202dee38a with resources cpus(allocated: *):4; 
> > mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
> > *):[31000-32000] of framework 455c5a23-b91c-4314-89df-13fed3c13608- on 
> > agent 455c5a23-b91c-4314-89df-13fed3c13608-S0 at 
> > slave(461)@192.10.1.5:64964 
> > (windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
> > I1004 06:25:30.318265 18956 slave.cpp:909] Agent terminating
> > W1004 06:25:30.318265 18956 slave.cpp:3917] Ignoring shutdown framework 
> > 455c5a23-b91c-4314-89df-13fed3c13608- because it is terminating
> > I1004 06:25:30.320267 16328 master.cpp:1251] Agent 
> > 455c5a23-b91c-4314-89df-13fed3c13608-S0 at slave(461)@192.10.1.5:64964 
> > (windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) 
> > disconnected
> > I1004 06:25:30.320267 16328 master.cpp:3267] Disconnecting agent 
> > 455c5a23-b91c-4314-89df-13fed3c13608-S0 at slave(461)@192.10.1.5:64964 
> > (windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
> > I1004 06:25:30.321262 16328 master.cpp:3286] Deactivating agent 
> > 455c5a23-b91c-4314-89df-13fed3c13608-S0 at slave(461)@192.10.1.5:64964 
> > (windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
> > I1004 06:25:30.321262 10404 hierarchical.cpp:359] Removed framework 
> > 455c5a23-b91c-4314-89df-13fed3c13608-
> > I1004 06:25:30.321262 10404 hierarchical.cpp:803] Agent 
> > 455c5a23-b91c-4314-89df-13fed3c13608-S0 deactivated
> > I1004 06:25:30.322278 10404 containerizer.cpp:2455] Destroying container 
> > 0b2a5b04-0cc8-455c-bc6e-3305a0ad3a7a in RUNNING state
> > I1004 06:25:30.322278 10404 containerizer.cpp:3122] Transitioning the state 
> > of container 0b2a5b04-0cc8-455c-bc6e-3305a0ad3a7a from RUNNING to DESTROYING
> > I1004 06:25:30.323292 10404 launcher.cpp:166] Asked to destroy container 
> > 0b2a5b04-0cc8-455c-bc6e-3305a0ad3a7a
> > I1004 06:25:30.366261 16328 contai[   OK ] 
> > IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (586 ms)
> > [--] 1 test from IsolationFlag/MemoryIsolatorTest (604 ms total)
> > 
> > [--] Global test environment tear-down
> > [==] 1051 tests from 103 test cases ran. (483700 ms total)
> > [  PASSED  ] 1050 tests.
> > [  FAILED  ] 1 test, listed below:
> > [  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
> > 
> >  1 FAILED TEST
> >   YOU HAVE 232 DISABLED TESTS
> > 
> > nerizer.cpp:2961] Container 0b2a5b04-0cc8-455c-bc6e-3305a0ad3a7a has exited
> > I1004 06:25:30.394275 18956 master.cpp:1093] Master terminating
> > I1004 06:25:30.396296 19016 hierarchical.cpp:645] Removed agent 
> > 455c5a23-b91c-4314-89df-13fed3c13608-S0
> > I1004 06:25:30.715306  8752 process.cpp:926] Stopped the socket accept loop
> > ```

Hmm... disconcerting that a Docker test failed on this chain, but I don't think 
it's related:
```
[ RUN  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
'hadoop' is not recognized as an internal or external command,
operable program or batch file.
d:\dcos\mesos\mesos\src\tests\uri_fetcher_tests.cpp(358): error: 
(fetcher.get()->fetch(uri, dir)).failure(): Collect failed: Unexpected 'curl' 
output: 
d:\dcos\mesos\mesos\3rdparty\stout\include\stout\tests\utils.hpp(68): error: 
os::rmdir(sandbox.get()): The process cannot access the file because it is 
being used by another process.


[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage (2415 ms)
```


- Greg


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


On Oct. 4, 2018, 5:26 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68924/
> ---
> 
> (Updated Oct. 4, 2018, 5:26 a.m.)
> 
> 

Re: Review Request 68813: Added support for `Option` / `Option`.

[Michael Park]

> On Sept. 23, 2018, 4:31 a.m., Benjamin Bannier wrote:
> > I am not convinced we should add this. The alternative of using e.g., an 
> > `Option` or `Option` seems to not only produce correct 
> > behavior (even when wrapping a ptr to `const`), but also caution users 
> > enough that noting here protects against dangling references or performs 
> > any reference lifetime extension. While this seems redundant in the case of 
> > `Option` where one could just return a `nullptr` for `None` values, such a 
> > pattern would translate seemlessly to e.g., `Try` or `Result`, and the 
> > behavior of empty case could be solved by documentation wherever we would 
> > return such a type. It would also avoid unusual semantics around assignment 
> > or comparision, and would e.g., continue to support hashing (the type 
> > proposed here does not support `hash`).
> > 
> > I'd suggest to drop this patch and instead use wrappers around pointers if 
> > we really want to provide such behavior in lieu of e.g., `contains` checks 
> > and returning naked values.
> 
> Benjamin Mahler wrote:
> > performs any reference lifetime extension
> 
> Can't we just delete the rvalue reference constructor to prevent the user 
> from trying to extend lifetime? This seems to be what boost did?
> 
> 
> https://www.boost.org/doc/libs/1_68_0/libs/optional/doc/html/boost_optional/tutorial/optional_references.html
> 
> > It would also avoid unusual semantics around assignment or comparision
> 
> Isn't this patch already avoiding these by disabling them?
> 
> > I'd suggest to drop this patch and instead use wrappers around pointers 
> if we really want to provide such behavior in lieu of e.g., contains checks 
> and returning naked values.
> 
> Are you suggesting code like this?
> 
> ```
> Option value = hashmap.get(key);
> 
> if (value.isSome()) {
>   (*value)->foo();
> }
> ```
> 
> This doesn't feel quite a clean as:
> 
> ```
> T& value = hashmap.at(key);
> 
> // use value.
> 
> // Now, I'm not assuming the key is present, so naturally,
> // I get an optional reference instead of the reference:
> Option value = hashmap.get(key);
> 
> if (value.isSome()) {
>   value->foo();
> }
> ```
> 
> Benjamin Bannier wrote:
> > Can't we just delete the rvalue reference constructor to prevent the 
> user from trying to extend lifetime?
> 
> Consider e.g.,
> 
> ```
> hashmap fun();
> Option value = fun().get(key); // Allowed if `hashmap::get` not 
> forbidden for `&&`.
> ```
> 
> To make such code safe any function returning an `Option` would need 
> to be disabled for rvalue `this` values explicitly; there seems there is 
> nothing we can do in `Option`'s definition to make this safe in general. 
> I am not sure we would be able to prevent bad code slipping in across the 
> board in normal human on human reviews.
> 
> >> It would also avoid unusual semantics around assignment or comparision
> 
> > Isn't this patch already avoiding these by disabling them?
> 
> Yes, it does. What I meant is that the way these new `Option` values 
> can be handled is suprisingly different from "normal" `Option` values; 
> they e.g., cannot be compared against each other or values of the wrapped 
> type, or cannot be put into `set`s or `hashmap`s (the latter could likely be 
> fixed). We loose some of the power that `Option` brings because due to its 
> value semantics. I am unsure there is a lot benefit left at that point.
> 
> > Are you suggesting code like this?
> > [...]
> 
> I was suggesting to use the existing
> ```
> // Pretty safe `value`; some dangerous patterns likely recognizable.
> if (hashmap.contains(key)) {
>   T& value = hashmap.at(key);
>   value->foo();
> }
> ```
> or if we really wanted to expose reference semantics with all the extra 
> rope it brings
> ```
> // Completely unsafe `value`; type leaves no illusion on safety,
> // users and reviewers hopefully on alert.
> Option value = hashmap.get(key);
> 
> if (value.isSome()) {
>   value.get()->foo();
> }
> ```
> 
> My _I am not convinced_ was sincere as I am really not sure whether these 
> are strong enough arguments or just me being change-averse. Maybe @mpark has 
> some insights he can share from the trenches of `variant` becoming 
> forbidden.

I think one thing to consider is what we want `Option` to be.
Is it a temporary stand-in for `std::optional` until it can be replaced?
or are we unsatisfied with the `std::optional` design and we want `Option`
to actually be a different thing? IIRC, even for `Option`, assignment
semantics are deviate from `std::optional`.

---

As for the standard, we __need__ to answer questions around what equality
and assignment means for references. We don't have to 

Re: Review Request 68866: Waited for TEARDOWN response in v1 Java scheduler shim.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68866 was successfully built and tested.

Reviews applied: `['68865', '68866']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2419/mesos-review-68866

- Mesos Reviewbot Windows


On Oct. 4, 2018, 10:02 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68866/
> ---
> 
> (Updated Oct. 4, 2018, 10:02 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker.
> 
> 
> Bugs: MESOS-9274
> https://issues.apache.org/jira/browse/MESOS-9274
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Destruction of the library is not always under our control. For
> example, the JVM can call JNI `finalize()` if the Java scheduler
> nullifies its reference to the V1Mesos library immediately after
> sending `TEARDOWN`. We want to make sure that `TEARDOWN` is sent
> before the Mesos library is destructed.
> 
> 
> Diffs
> -
> 
>   src/java/jni/org_apache_mesos_v1_scheduler_V1Mesos.cpp 
> 2a5fbd79ac7bad933067cd96e38186849af8edc4 
> 
> 
> Diff: https://reviews.apache.org/r/68866/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` on various Linux distro
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 68866: Waited for TEARDOWN response in v1 Java scheduler shim.

[Alexander Rukletsov]

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

(Updated Oct. 4, 2018, 10:02 a.m.)


Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker.


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


Repository: mesos


Description
---

Destruction of the library is not always under our control. For
example, the JVM can call JNI `finalize()` if the Java scheduler
nullifies its reference to the V1Mesos library immediately after
sending `TEARDOWN`. We want to make sure that `TEARDOWN` is sent
before the Mesos library is destructed.


Diffs (updated)
-

  src/java/jni/org_apache_mesos_v1_scheduler_V1Mesos.cpp 
2a5fbd79ac7bad933067cd96e38186849af8edc4 


Diff: https://reviews.apache.org/r/68866/diff/4/

Changes: https://reviews.apache.org/r/68866/diff/3-4/


Testing
---

`make check` on various Linux distro


Thanks,

Alexander Rukletsov

Re: Review Request 68919: Removed unbundling by default for libevent on macOS when using CMake.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Oct. 3, 2018, 10:55 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68919/
> ---
> 
> (Updated Oct. 3, 2018, 10:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unbundling by default for libevent on macOS when using CMake.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake a820c867b41d2120525c5a32128812ae527ffeba 
> 
> 
> Diff: https://reviews.apache.org/r/68919/diff/1/
> 
> 
> Testing
> ---
> 
> macos: cmake .. -DENABLE_SSL=ON -DENABLE_LIBEVENT=ON  
> -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl -DCMAKE_EXPORT_COMPILE_COMMANDS=1 
> -G Ninja && ninja check -j12
> macos: cmake .. -DUNBUNDLED_LIBEVENT=ON -DENABLE_SSL=ON -DENABLE_LIBEVENT=ON 
> -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl -DCMAKE_EXPORT_COMPILE_COMMANDS=1 
> -G Ninja && ninja check -j12
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68865: Put `TerminateEvent` at the end of the queue in the Mesos library.

[Alexander Rukletsov]

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

(Updated Oct. 4, 2018, 9:38 a.m.)


Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

This is to ensure all pending dispatches are processed. However
multistage events, e.g., `Call`, might still be dropped, because
a continuation (stage) of such event can be dispatched after the
termination event.


Diffs (updated)
-

  src/scheduler/scheduler.cpp 471152945d6af660c8983324b38702d872657f89 


Diff: https://reviews.apache.org/r/68865/diff/2/

Changes: https://reviews.apache.org/r/68865/diff/1-2/


Testing
---

See https://reviews.apache.org/r/68866/


Thanks,

Alexander Rukletsov

Re: Review Request 68866: Waited for TEARDOWN response in v1 Java scheduler shim.

[Alexander Rukletsov]

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

(Updated Oct. 4, 2018, 9:42 a.m.)


Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker.


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


Repository: mesos


Description
---

Destruction of the library is not always under our control. For
example, the JVM can call JNI `finalize()` if the Java scheduler
nullifies its reference to the V1Mesos library immediately after
sending `TEARDOWN`. We want to make sure that `TEARDOWN` is sent
before the Mesos library is destructed.


Diffs (updated)
-

  src/java/jni/org_apache_mesos_v1_scheduler_V1Mesos.cpp 
2a5fbd79ac7bad933067cd96e38186849af8edc4 


Diff: https://reviews.apache.org/r/68866/diff/3/

Changes: https://reviews.apache.org/r/68866/diff/2-3/


Testing
---

`make check` on various Linux distro


Thanks,

Alexander Rukletsov

Re: Review Request 68866: Waited for TEARDOWN response in v1 Java scheduler shim.

[Alexander Rukletsov]

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

(Updated Oct. 4, 2018, 9:39 a.m.)


Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

Destruction of the library is not always under our control. For
example, the JVM can call JNI `finalize()` if the Java scheduler
nullifies its reference to the V1Mesos library immediately after
sending `TEARDOWN`. We want to make sure that `TEARDOWN` is sent
before the Mesos library is destructed.


Diffs (updated)
-

  src/java/jni/org_apache_mesos_v1_scheduler_V1Mesos.cpp 
2a5fbd79ac7bad933067cd96e38186849af8edc4 


Diff: https://reviews.apache.org/r/68866/diff/2/

Changes: https://reviews.apache.org/r/68866/diff/1-2/


Testing (updated)
---

`make check` on various Linux distro


Thanks,

Alexander Rukletsov

Re: Review Request 68866: Waited for TEARDOWN response in v1 Java scheduler shim.

[Alexander Rukletsov]

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

(Updated Oct. 4, 2018, 9:33 a.m.)


Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker.


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


Repository: mesos


Description (updated)
---

Destruction of the library is not always under our control. For
example, the JVM can call JNI `finalize()` if the Java scheduler
nullifies its reference to the V1Mesos library immediately after
sending `TEARDOWN`. We want to make sure that `TEARDOWN` is sent
before the Mesos library is destructed.


Diffs
-

  src/java/jni/org_apache_mesos_v1_scheduler_V1Mesos.cpp 
2a5fbd79ac7bad933067cd96e38186849af8edc4 


Diff: https://reviews.apache.org/r/68866/diff/1/


Testing
---

`make check`on various Linux distro


Thanks,

Alexander Rukletsov

Re: Review Request 68865: Put `TerminateEvent` at the end of the queue in the Mesos library.

[Alexander Rukletsov]

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

(Updated Oct. 4, 2018, 9:07 a.m.)


Review request for mesos, Gastón Kleiman, Greg Mann, and Nicholas Parker.


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


Repository: mesos


Description (updated)
---

This is to ensure all pending dispatches are processed. However
multistage events, e.g., `Call`, might still be dropped, because
a continuation (stage) of such event can be dispatched after the
termination event.


Diffs
-

  src/scheduler/scheduler.cpp 471152945d6af660c8983324b38702d872657f89 


Diff: https://reviews.apache.org/r/68865/diff/1/


Testing
---

See https://reviews.apache.org/r/68866/


Thanks,

Alexander Rukletsov

Re: Review Request 68903: Avoid deadlock-prone blocking in master's parallel endpoint serving.

[Alexander Rukletsov]

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




src/master/http.cpp
Lines 2433-2440 (patched)


The interesting scenario is when the only available working thread is the 
one currently attached to the master actor. In this scenario, is the 
expectation here that each `process::async(worker)` will be spawned and 
executed on this single worker thread before master actor proceeds to launching 
the last worker?


- Alexander Rukletsov


On Oct. 3, 2018, 12:07 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68903/
> ---
> 
> (Updated Oct. 3, 2018, 12:07 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The approach taken to serve endpoints in parallel was to call
> `process::async` for each request and block until they finish.
> Blocking in this case assumes that there will be enough additional
> non-blocked worker threads to execute all the requests. When the
> number of blocking Processes is equal or greater than the number
> of worker threads, deadlock become possible. The parallel endpoint
> serving approach added another blocking Process.
> 
> While we could make blocking safe (see MESOS-8256) by, for example,
> detaching a worker thread prior to blocking (a la golang), such a
> change is more involved.
> 
> In lieu of making blocking safe, this patch updates the Master to
> block only when it's guaranteed to be safe. This is accomplished
> by:
> 
>   (1) using a "work queue" (just a shared atomic "next index") and
>   having workers pop items from this "queue",
>   (2) we execute one of the workers on the master Process directly,
>   (3) when this worker completes, we then know that all requests
>   are either processed or being processed (i.e. the `async`
>   `Process` is executing on a worker thread),
>   (4) the master can then block on a count down latch, knowing
>   that it will unblock because of (3).
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d912789bcfdcf6eaefc29dba7b878fa4e02f9276 
> 
> 
> Diff: https://reviews.apache.org/r/68903/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68905: Removed version check from libevent dependency tracking from libprocess.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Oct. 3, 2018, 3:31 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68905/
> ---
> 
> (Updated Oct. 3, 2018, 3:31 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-9265
> https://issues.apache.org/jira/browse/MESOS-9265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed version check from libevent dependency tracking from libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac c3e39643924dd4581d46e3d4e1b89b409d823e88 
>   3rdparty/libprocess/m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68905/diff/1/
> 
> 
> Testing
> ---
> 
> make check on macOS 10.14.1 with and without bundled libevent.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68914: Updated libevent linkage to adhere to best practices.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Oct. 3, 2018, 6:51 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68914/
> ---
> 
> (Updated Oct. 3, 2018, 6:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-9222
> https://issues.apache.org/jira/browse/MESOS-9222
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> libevent is a combination of libevent_core and libevent_extra. Mesos
> and libprocess do not make use of any libevent_extra functionality.
> 
> Additionally, linking against libevent is against best practices.
> 
> Replaces use of libevent with libevent_core in both libevent.m4 as
> well as the libprocess linkage.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 22b1395ce69f4ac630367427542fa1ad4c88b50b 
>   3rdparty/libprocess/m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68914/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68916: Moved libevent_openssl validation into libevent.m4.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Oct. 3, 2018, 9:32 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68916/
> ---
> 
> (Updated Oct. 3, 2018, 9:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Combines all libevent related header and library checks into
> libevent.m4.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac c3e39643924dd4581d46e3d4e1b89b409d823e88 
>   3rdparty/libprocess/m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68916/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68915: Moved libevent_openssl validation into libevent.m4.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Oct. 3, 2018, 9:32 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68915/
> ---
> 
> (Updated Oct. 3, 2018, 9:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Combines all libevent related header and library checks into
> libevent.m4.
> 
> 
> Diffs
> -
> 
>   configure.ac 505201a1ce84834b574067650993ce51cf1e1d5d 
>   m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68915/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68913: Updated libevent linkage to adhere to best practices.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Oct. 3, 2018, 6:50 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68913/
> ---
> 
> (Updated Oct. 3, 2018, 6:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-9222
> https://issues.apache.org/jira/browse/MESOS-9222
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> libevent is a combination of libevent_core and libevent_extra. Mesos
> and libprocess do not make use of any libevent_extra functionality.
> 
> Additionally, linking against libevent is against best practices.
> 
> Replaces use of libevent with libevent_core in libevent.m4,
> FindLIBEVENT.cmake as well as the native Python egg linkage.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/FindLIBEVENT.cmake d6a1e65a2660fa96f50c08298b1e61ac42d85cee 
>   m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
>   src/python/native_common/ext_modules.py.in 
> df10b5c9131011b0a6b6cb83bceb6d7444aa35eb 
> 
> 
> Diff: https://reviews.apache.org/r/68913/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68906: Fixed ssl build specific incompatiblity with libevent later than 2.1.5.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Oct. 4, 2018, 1:06 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68906/
> ---
> 
> (Updated Oct. 4, 2018, 1:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-9265
> https://issues.apache.org/jira/browse/MESOS-9265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Up until and including libevent 2.1.5 (beta) the buffer events EV_READ
> and EV_WRITE were enabled by default in the openssl related socket
> setup.
> 
> That got changed by f4b6284b8393dbabf389ddce734a30f4cdeffa17 within
> libevent; the default expectancy of read and write buffer events got
> removed.
> 
> The changes in libevent in turn caused our implementation on top of it
> to fail, typically directly after accept and connect calls.
> 
> The libevent maintainer Azat Khuzhin has generously been helping us out
> by debugging and fixing the incomplete setup within libprocess.
> 
> This patch is based on Azat's suggestions from https://goo.gl/4CK3PD.
> It has been validated against libevent 2.0.22, 2.1.8 and 2.2 (alpha)
> on macOS 10.14.1 and Ubuntu 16.04.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 436b38913e43233e352cf9783da8fafefa42226f 
> 
> 
> Diff: https://reviews.apache.org/r/68906/diff/2/
> 
> 
> Testing
> ---
> 
> make check with and without bundled libevent.
> 
> Tested combinations were:
> macOS 10.14.1 - libevent 2.2
> macOS 10.14.1 - libevent 2.1.8
> macOS 10.14.1 - libevent 2.0.22 
> Ubuntu 16.04  - libevent 2.1.8
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68904: Removed version check from libevent dependency tracking.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Oct. 3, 2018, 3:30 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68904/
> ---
> 
> (Updated Oct. 3, 2018, 3:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James Peach.
> 
> 
> Bugs: MESOS-9265
> https://issues.apache.org/jira/browse/MESOS-9265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed version check from libevent dependency tracking.
> 
> 
> Diffs
> -
> 
>   configure.ac 505201a1ce84834b574067650993ce51cf1e1d5d 
>   m4/libevent.m4 3a7fcad7d0c2d967fb308714c4b1f631c988001b 
> 
> 
> Diff: https://reviews.apache.org/r/68904/diff/1/
> 
> 
> Testing
> ---
> 
> make check on macOS 10.14.1 with and without bundled libevent.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 68924: Updated the Docker library to avoid 'os::killtree()'.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['68923', '68924']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2418/mesos-review-68924

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2418/mesos-review-68924/logs/mesos-tests.log):

```
I1004 06:25:30.318265 18864 executor.cpp:805] Shutting down
I1004 06:25:30.318265 18864 executor.cpp:918] Sending SIGTERM to process tree 
at pid 1106:25:30.315294 16068 slave.cpp:6640] Shutting down executor 
'4a510516-6cf6-475f-ad54-6e5202dee38a' of framework 
455c5a23-b91c-4314-89df-13fed3c13608- at executor(1)@192.10.1.5:50375
I1004 06:25:30.318265 19344 master.cpp:11030] Removing task 
4a510516-6cf6-475f-ad54-6e5202dee38a with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 455c5a23-b91c-4314-89df-13fed3c13608- on 
agent 455c5a23-b91c-4314-89df-13fed3c13608-S0 at slave(461)@192.10.1.5:64964 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1004 06:25:30.318265 18956 slave.cpp:909] Agent terminating
W1004 06:25:30.318265 18956 slave.cpp:3917] Ignoring shutdown framework 
455c5a23-b91c-4314-89df-13fed3c13608- because it is terminating
I1004 06:25:30.320267 16328 master.cpp:1251] Agent 
455c5a23-b91c-4314-89df-13fed3c13608-S0 at slave(461)@192.10.1.5:64964 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I1004 06:25:30.320267 16328 master.cpp:3267] Disconnecting agent 
455c5a23-b91c-4314-89df-13fed3c13608-S0 at slave(461)@192.10.1.5:64964 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1004 06:25:30.321262 16328 master.cpp:3286] Deactivating agent 
455c5a23-b91c-4314-89df-13fed3c13608-S0 at slave(461)@192.10.1.5:64964 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1004 06:25:30.321262 10404 hierarchical.cpp:359] Removed framework 
455c5a23-b91c-4314-89df-13fed3c13608-
I1004 06:25:30.321262 10404 hierarchical.cpp:803] Agent 
455c5a23-b91c-4314-89df-13fed3c13608-S0 deactivated
I1004 06:25:30.322278 10404 containerizer.cpp:2455] Destroying container 
0b2a5b04-0cc8-455c-bc6e-3305a0ad3a7a in RUNNING state
I1004 06:25:30.322278 10404 containerizer.cpp:3122] Transitioning the state of 
container 0b2a5b04-0cc8-455c-bc6e-3305a0ad3a7a from RUNNING to DESTROYING
I1004 06:25:30.323292 10404 launcher.cpp:166] Asked to destroy container 
0b2a5b04-0cc8-455c-bc6e-3305a0ad3a7a
I1004 06:25:30.366261 16328 contai[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (586 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (604 ms total)

[--] Global test environment tear-down
[==] 1051 tests from 103 test cases ran. (483700 ms total)
[  PASSED  ] 1050 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage

 1 FAILED TEST
  YOU HAVE 232 DISABLED TESTS

nerizer.cpp:2961] Container 0b2a5b04-0cc8-455c-bc6e-3305a0ad3a7a has exited
I1004 06:25:30.394275 18956 master.cpp:1093] Master terminating
I1004 06:25:30.396296 19016 hierarchical.cpp:645] Removed agent 
455c5a23-b91c-4314-89df-13fed3c13608-S0
I1004 06:25:30.715306  8752 process.cpp:926] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Oct. 4, 2018, 5:26 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68924/
> ---
> 
> (Updated Oct. 4, 2018, 5:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-9283
> https://issues.apache.org/jira/browse/MESOS-9283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the Docker library to consistently use the
> overload of `subprocess()` which runs its binary via `exec()`
> rather than through a shell. This makes it safe to use
> `os::kill()` rather than `os::killtree()` when discarding
> these commands, which this patch also accomplishes.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp fb39f7480045c225096e07d7d55cd3aa7b870bc5 
> 
> 
> Diff: https://reviews.apache.org/r/68924/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*DOCKER*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>