Re: Review Request 54365: Fixed indentation of a function argument in master.cpp.

[Guangya Liu]

> On 一月 10, 2017, 7:34 a.m., Guangya Liu wrote:
> > Ship It!

I have committed this already, but seems do not have permission to close this 
review, will close this when got permission.

commit 653fe55b3f2e6cd76567945dbbec4a84c03f13c2
Author: Jay Guo 
Date:   Tue Jan 10 15:32:05 2017 +0800

Fixed indentation of a function argument in master.cpp.

Fixed indentation of a function argument in master.cpp.

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


- Guangya


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


On 十二月 9, 2016, 9:38 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54365/
> ---
> 
> (Updated 十二月 9, 2016, 9:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Guangya Liu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed indentation of a function argument in master.cpp.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 353e6ea802e197b4456c1647f78d9984a50f1c9d 
> 
> Diff: https://reviews.apache.org/r/54365/diff/
> 
> 
> Testing
> ---
> 
> line 3902 should be aligned with other arguments.
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 54365: Fixed indentation of a function argument in master.cpp.

[Guangya Liu]

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


Ship it!




Ship It!

- Guangya Liu


On 十二月 9, 2016, 9:38 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54365/
> ---
> 
> (Updated 十二月 9, 2016, 9:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Guangya Liu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed indentation of a function argument in master.cpp.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 353e6ea802e197b4456c1647f78d9984a50f1c9d 
> 
> Diff: https://reviews.apache.org/r/54365/diff/
> 
> 
> Testing
> ---
> 
> line 3902 should be aligned with other arguments.
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 55362: Support asynchronous fetching of URIs.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [55362]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Jan. 10, 2017, 12:53 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55362/
> ---
> 
> (Updated Jan. 10, 2017, 12:53 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6601
> https://issues.apache.org/jira/browse/MESOS-6601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos fetcher fetches the URIs specified in CommandInfo serially,
> one after the other. If the task has too many URIs to be fetched
> then it delays the execution of the task.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 4456c28139966e42859cc6d2c79a1d90e83fb22f 
> 
> Diff: https://reviews.apache.org/r/55362/diff/
> 
> 
> Testing
> ---
> 
> make check on macOS.
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 54603: Replaced `int` with `int_fd` in mesos.

[Michael Park]

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

(Updated Jan. 9, 2017, 6:50 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Replaced `int` with `int_fd` in mesos.


Diffs (updated)
-

  include/mesos/slave/container_logger.hpp 
5781fecd08a2c92c3bc936598ca707a9a15f0e23 
  src/docker/docker.cpp b63b060ba1c1d39dc1702368cf667831edbd39bd 
  src/files/files.cpp 9c634ac928887b3f1a111f67ebb3fc5229c6fa16 
  src/slave/containerizer/fetcher.cpp ba2e118bdefa932aa4109452548faf6cafb3451f 
  src/slave/containerizer/mesos/containerizer.hpp 
9fcad933e1ec3b52d4dc366ba80d282deea04c0b 
  src/slave/containerizer/mesos/containerizer.cpp 
8b97dc4980e9581c21e59e290f2f736398b5b955 
  src/slave/containerizer/mesos/io/switchboard.cpp 
c444568dd478902afd79fe3c8960364fd68fabb5 
  src/slave/containerizer/mesos/io/switchboard_main.cpp 
87056c3f76525151834b4ff782e2009050941ccb 
  src/slave/containerizer/mesos/launch.hpp 
5bba13998e879afb490c44c427dc6844c1c3c38d 
  src/slave/containerizer/mesos/launch.cpp 
e482ab8bdfc358f695b87cda72ca59fb64cd8c4d 
  src/slave/state.cpp 6894875b37b7a34c8f3645087404584bbe982e6c 
  src/slave/status_update_manager.hpp 9fad0c51ec0fc00e134265850ad2236a585e46ec 
  src/slave/status_update_manager.cpp 056a684b52756d5c6309e7e2167a1532c4e60957 
  src/tests/containerizer/cgroups_tests.cpp 
0afaec6ae948cabf1472bf01103210d8f9809cb1 
  src/tests/containerizer/io_switchboard_tests.cpp 
e32df64c73df7f99f4c1a940db8807109be94696 
  src/tests/containerizer/memory_test_helper.cpp 
b94f0aac331da2e4f586245796176893195ad137 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
9a6bcb54f77f8343786f42e3cb511f2e95aaff5e 
  src/tests/credentials_tests.cpp ed50609cc5ed1937d2bcd782451245e0b5889ed5 
  src/tests/mesos.cpp adbb697c2bb908a584163dc9954b9e3138164433 
  src/tests/protobuf_io_tests.cpp 9263d8c2763cb30c93fd94be5344f83a6ce3e3b1 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 54602: Replaced `int` with `int_fd` in libprocess.

[Michael Park]

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

(Updated Jan. 9, 2017, 6:50 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Replaced `int` with `int_fd` in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/io.hpp 
f5489dc66adb136d9f53f98ac64c3fbe7831a1c2 
  3rdparty/libprocess/include/process/network.hpp 
8234765e23bb3d434da0b0f818fd42569d554ab8 
  3rdparty/libprocess/include/process/posix/subprocess.hpp 
a1054e788ef6a322901c262380aceab8192235ac 
  3rdparty/libprocess/include/process/socket.hpp 
87966155aa21328db51796b2ae0a883054c00457 
  3rdparty/libprocess/include/process/subprocess_base.hpp 
74a4bef0706334b4d3c1040a08a8921fbc300344 
  3rdparty/libprocess/include/process/windows/subprocess.hpp 
3bc7f1992d9c38dac2ec23d5bc57415f37d0318a 
  3rdparty/libprocess/src/encoder.hpp 1447d6f93c15b9f3d134507ecb0bda020a218a49 
  3rdparty/libprocess/src/http.cpp b5c38ce89b73788f7446e6c44fd99da6478b064d 
  3rdparty/libprocess/src/io.cpp 8aa3576a11ed5e998492b4020cb37a45ec708093 
  3rdparty/libprocess/src/libev_poll.cpp 
da2a78bd8aa4ed8c37bd2ca16d148b73112aa0e7 
  3rdparty/libprocess/src/libevent_poll.cpp 
0803ba33622c86df38b3efd4f1e3197edf93a0af 
  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
57eaf4f607d0628db466cc1a139772eeeaa51136 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
0e292a8b0d470f4e199db08f09a0c863d73c 
  3rdparty/libprocess/src/poll_socket.hpp 
89789e6bb91d79e4de1c4f4be3882df851845930 
  3rdparty/libprocess/src/poll_socket.cpp 
93ca37f105527fb225574107480114ee5ac74c76 
  3rdparty/libprocess/src/process.cpp f475fe78f801924f70f51fdc4ab190c2dbecd656 
  3rdparty/libprocess/src/socket.cpp b819503095261c77f42d6f20d1a4b2b6170fb4e1 
  3rdparty/libprocess/src/subprocess.cpp 
ad19b0896b4a2e9c60f573cc854c10c69e909e86 
  3rdparty/libprocess/src/subprocess_posix.cpp 
19271414f145d23f50ac07570c48782819f382b4 
  3rdparty/libprocess/src/subprocess_windows.cpp 
20cad52d4a4d7fc51487e150a849972eb19ed08e 
  3rdparty/libprocess/src/tests/io_tests.cpp 
466e343e6d775fe09debce119eae4fc4849b7006 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
a32d20e47f67d88bbe5928e0ddc129745c5f42e0 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
59c17692012ddfb540ecdd48560c73c42a15f061 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 54601: Replaced `int` with `int_fd` in stout.

[Michael Park]

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

(Updated Jan. 9, 2017, 6:49 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Replaced `int` with `int_fd` in stout.


Diffs (updated)
-

  3rdparty/stout/include/stout/net.hpp 4803aeb2fc1c89d42a02c631ef0450d5053ea8a2 
  3rdparty/stout/include/stout/os/mktemp.hpp 
2dfd35605eb4202a5475fe0e0d2f1fd27690a2de 
  3rdparty/stout/include/stout/os/open.hpp 
2a357926860b1523c51f12c7edee2babe6afbfa3 
  3rdparty/stout/include/stout/os/posix/socket.hpp 
041f00083c595c335146048c8685c4f96226b8e8 
  3rdparty/stout/include/stout/os/read.hpp 
d0b657db5b7dc0c1e11edc13fd6830a9f6273867 
  3rdparty/stout/include/stout/os/socket.hpp 
e9d9306e63aff2be083b4254fbf6f31c37edc424 
  3rdparty/stout/include/stout/os/sunos.hpp 
e0398d25c0a5d0b55934594dd59b2629957ab921 
  3rdparty/stout/include/stout/os/touch.hpp 
84d49bb8adc2612e380f037fd42c47166d55f593 
  3rdparty/stout/include/stout/os/windows/close.hpp 
9f1566bff19ee872418bce8a43a119c4f0a70a7c 
  3rdparty/stout/include/stout/os/windows/fcntl.hpp 
f331cd324bc609f5b8081fa86d66d9947daf04f6 
  3rdparty/stout/include/stout/os/windows/fsync.hpp 
e1c4868b02b320906f446af1d9371374e90651bc 
  3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
a7f53ad2e8735b515590af84c0efce3edcc1bebf 
  3rdparty/stout/include/stout/os/windows/read.hpp 
09190f97f33db5dfa023f937f8f2a4a62ed1ec15 
  3rdparty/stout/include/stout/os/windows/sendfile.hpp 
d6358cc02c1eea9298907da1f74eb7eeaeec7d21 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
17e3d564564abebf1d558b7a7a277aef3c87e5ae 
  3rdparty/stout/include/stout/os/windows/socket.hpp 
c787341181da53abc5a3b2eadc76c85fef181310 
  3rdparty/stout/include/stout/os/windows/write.hpp 
23488708ae366b8571bb8b4805f67d2054223fff 
  3rdparty/stout/include/stout/os/write.hpp 
9bb9fbd4593866b6193a1e253e65852da0ff5ed5 
  3rdparty/stout/include/stout/protobuf.hpp 
80cb20f40a7ddd4309d27973eef9fca9e4052b64 
  3rdparty/stout/include/stout/windows.hpp 
e641c46d033372e1b6c9f9c066b1ad4957d55088 
  3rdparty/stout/include/stout/windows/os.hpp 
5cd92545a49648e39e8eb7cf131895e9cfc97902 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
22460842c0db5dc5b6effbc2bdfce043ed47db6d 
  3rdparty/stout/tests/os/sendfile_tests.cpp 
17e10d92a0a004cb7ac83f4ff9c71844418a35b0 
  3rdparty/stout/tests/os_tests.cpp 30735e28a26ff713469711d63538676ed4e327d9 
  3rdparty/stout/tests/path_tests.cpp 8275e38a503e64d242da6be0fe6bd0a0c21869a3 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 54762: Introduced an `os::pipe` abstraction to stout.

[Michael Park]

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

(Updated Jan. 9, 2017, 6:49 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Introduced an `os::pipe` abstraction to stout.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
  3rdparty/stout/include/stout/os.hpp ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
  3rdparty/stout/include/stout/os/pipe.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/pipe.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/pipe.hpp PRE-CREATION 
  3rdparty/stout/include/stout/posix/os.hpp 
397c2d6b06ddb607d254eae8258da5151c5f57e0 
  3rdparty/stout/include/stout/windows/os.hpp 
5cd92545a49648e39e8eb7cf131895e9cfc97902 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 54592: Introduced an `os::lseek` abstraction in stout.

[Michael Park]

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

(Updated Jan. 9, 2017, 6:47 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Introduced an `os::lseek` abstraction in stout.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
  3rdparty/stout/include/stout/os.hpp ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
  3rdparty/stout/include/stout/os/lseek.hpp PRE-CREATION 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 54591: Introduced `WindowsFD` class which is analogous to an `int` in POSIX.

[Michael Park]

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

(Updated Jan. 9, 2017, 6:47 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

In POSIX the socket, pipe and a file are represented by the `int` type.
In Windows:
  - A socket is kept in a `SOCKET` type (64 bit wide)
  - A pipe or a WinAPI file descriptor in a `HANDLE` (64 bit wide)
  - A CRT file descriptor in an `int`

The `WindowsFD` class is a type that brings all of these things
together and behaves analogously to an `int` in POSIX.


Diffs (updated)
-

  3rdparty/stout/include/stout/os.hpp ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
  3rdparty/stout/include/stout/os/int_fd.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/fd.hpp PRE-CREATION 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 54590: Removed unused `peek` function.

[Michael Park]

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

(Updated Jan. 9, 2017, 6:47 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/include/process/io.hpp 
f5489dc66adb136d9f53f98ac64c3fbe7831a1c2 
  3rdparty/libprocess/src/io.cpp 8aa3576a11ed5e998492b4020cb37a45ec708093 
  3rdparty/libprocess/src/tests/io_tests.cpp 
466e343e6d775fe09debce119eae4fc4849b7006 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

[Michael Park]

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

(Updated Jan. 9, 2017, 6:47 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Introduced an `os::dup` abstraction in stout.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
  3rdparty/stout/include/stout/os.hpp ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
  3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 54232: Shutdown tasks of completed frameworks on agent re-registration.

[Neil Conway]

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

(Updated Jan. 10, 2017, 2:29 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Improve test case.


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


Repository: mesos


Description
---

Previously, if a framework completed (e.g., due to a teardown operation
or framework shutdown), any framework tasks running on partitioned
agents would not be shutdown when the agent re-registered. For tasks
that are not partition-aware, the task would be shutdown on agent
re-registration anyway. But for partition-aware tasks, this could lead
to orphan tasks.

Fix this by changing the master to shutdown such tasks when the agent
reregisters.

Note that if the master fails over between the time the framework
completes and a partitioned agent re-registers, any framework tasks
running on the agent will NOT be shutdown. This is a known bug; fixing
it requires persisting the framework shutdown operation to the registry
(MESOS-1719).


Diffs (updated)
-

  src/master/master.hpp 368ee1d5e97784fa54e0f141906405ee8f104317 
  src/master/master.cpp 39d203b0ef820a78c6871177e02c5051fd23ec70 
  src/tests/partition_tests.cpp 72013d1bfee275c6f3cb90173f0c408d55e0bc5d 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 55362: Support asynchronous fetching of URIs.

[Megha Sharma]

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

(Updated Jan. 10, 2017, 12:53 a.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Mesos fetcher fetches the URIs specified in CommandInfo serially,
one after the other. If the task has too many URIs to be fetched
then it delays the execution of the task.


Diffs (updated)
-

  src/launcher/fetcher.cpp 4456c28139966e42859cc6d2c79a1d90e83fb22f 

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


Testing
---

make check on macOS.


Thanks,

Megha Sharma

Re: Review Request 55296: Used `jsonify` in `operator<<` for `JSON::*` to reduce duplicate code.

[Michael Park]

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

(Updated Jan. 9, 2017, 4:36 p.m.)


Review request for mesos, Alexander Rojas and Joris Van Remoortere.


Changes
---

Addressed arojas' comment.


Repository: mesos


Description (updated)
---

The printing logic for `json.hpp` and `jsonify.hpp` are currently
duplicated. We can reduce this duplication by leveraging `jsonify` for
the implementation of `operator<<` for `JSON::*`. Since `JSON::Value`s
are not generally used for printing, there should be no performance
concerns here.


Diffs (updated)
-

  3rdparty/stout/include/stout/json.hpp 
62ce15274677112d142a3c829b4a9f06258c9e2c 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 55355: Fixed an FD leak in the IO switchboard.

[Jie Yu]

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


Ship it!




LGTM!

- Jie Yu


On Jan. 10, 2017, 12:02 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55355/
> ---
> 
> (Updated Jan. 10, 2017, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin 
> Mahler, Jie Yu, Joseph Wu, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the IO switchboard could leak file descriptors
> because it held a reference to its server socket within the
> socket's accept loop. This patch explicitly discards the
> future containing this reference to eliminate the leak.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 67c51678e87389ab08a81fe4832cffd3d83bdae9 
> 
> Diff: https://reviews.apache.org/r/55355/diff/
> 
> 
> Testing
> ---
> 
> Run:
> `bin/mesos-tests.sh --gtest_repeat=-1 
> --gtest_filter="IOSwitchboardServerTest.AttachInput"`
> 
> and then simultaneously execute `sudo lsof -p PID_of_test_process | wc -l` 
> repeatedly to ensure that the number of open FDs is not increasing.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 55355: Fixed an FD leak in the IO switchboard.

[Greg Mann]

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

(Updated Jan. 10, 2017, 12:02 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin Mahler, 
Jie Yu, Joseph Wu, and Kevin Klues.


Repository: mesos


Description
---

Previously, the IO switchboard could leak file descriptors
because it held a reference to its server socket within the
socket's accept loop. This patch explicitly discards the
future containing this reference to eliminate the leak.


Diffs (updated)
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
67c51678e87389ab08a81fe4832cffd3d83bdae9 

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


Testing
---

Run:
`bin/mesos-tests.sh --gtest_repeat=-1 
--gtest_filter="IOSwitchboardServerTest.AttachInput"`

and then simultaneously execute `sudo lsof -p PID_of_test_process | wc -l` 
repeatedly to ensure that the number of open FDs is not increasing.


Thanks,

Greg Mann

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

[Greg Mann]

> On Jan. 8, 2017, 3:22 a.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/src/libevent_ssl_socket.hpp, line 143
> > 
> >
> > I don't see any win in the name change from `recv_callback` to 
> > `perform_bev_read`. You also didn't delete the declaration of 
> > `recv_callback` above even though you changed the function name below.

I liked the name perform_bev_read because this function is no longer simply a 
continuation of the libevent callback, it also has a callsite in 
event_callback. I also find it confusing that the XXX_callback functions all 
have continuations with the same name in this file.

However, you make a good point regarding consistency. We could rename this 
function `_recv_callback`, which matches our usual pattern for continuations, 
and I could follow up with another patch to similarly change the names of the 
relevant `send_callback` and `event_callback` functions. Thoughts?


> On Jan. 8, 2017, 3:22 a.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/src/libevent_ssl_socket.hpp, line 155
> > 
> >
> > Was your inititon that `received_eof` was going to be protected by 
> > `synchronized (bev)`? I want to make sure there isn't a race with the IO 
> > thread setting `received_eof` in the `event_callback` and another thread 
> > reading and missing `received_eof` in `recv`. If you're confident that the 
> > `synchronized (bev)` is a sufficient barrier then let's just document that 
> > and maybe even move this up closer to `bufferevent* bev;` and specify that 
> > it's protected by `synchronized (bev)` which it gets automatically in any 
> > of the callbacks (i.e., `recv_callback`, `send_callback`, `event_callback`).

I originally did have received_eof explicitly protected by synchronized (bev), 
but as BenM mentioned in our offline discussion we should be safe without this 
since all accesses of received_eof occur in the event loop. I'll move this 
declaration and add a comment.


- Greg


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


On Dec. 15, 2016, 5:53 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53802/
> ---
> 
> (Updated Dec. 15, 2016, 5:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-6802
> https://issues.apache.org/jira/browse/MESOS-6802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, it was possible for an SSL socket to either:
> 1) Fail to receive an EOF if the EOF event was received when
>there was no pending recv() request.
> 2) Fail to receive all data sent on the sending side if an
>EOF event was received before all sent data was read.
> 
> This patch eliminates these race conditions to ensure reliable
> receipt of both sent data and EOFs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> 57eaf4f607d0628db466cc1a139772eeeaa51136 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 0e292a8b0d470f4e199db08f09a0c863d73c 
> 
> Diff: https://reviews.apache.org/r/53802/diff/
> 
> 
> Testing
> ---
> 
> Testing details are found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

[Greg Mann]

> On Jan. 9, 2017, 11:54 p.m., Greg Mann wrote:
> >

Whoops :)


- Greg


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


On Dec. 15, 2016, 5:53 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53802/
> ---
> 
> (Updated Dec. 15, 2016, 5:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-6802
> https://issues.apache.org/jira/browse/MESOS-6802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, it was possible for an SSL socket to either:
> 1) Fail to receive an EOF if the EOF event was received when
>there was no pending recv() request.
> 2) Fail to receive all data sent on the sending side if an
>EOF event was received before all sent data was read.
> 
> This patch eliminates these race conditions to ensure reliable
> receipt of both sent data and EOFs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> 57eaf4f607d0628db466cc1a139772eeeaa51136 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 0e292a8b0d470f4e199db08f09a0c863d73c 
> 
> Diff: https://reviews.apache.org/r/53802/diff/
> 
> 
> Testing
> ---
> 
> Testing details are found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

[Greg Mann]

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




3rdparty/libprocess/src/libevent_ssl_socket.hpp (line 143)


I liked the name `perform_bev_read` because this function is no longer 
simply a continuation of the libevent callback, it also has a callsite in 
`event_callback`. I also find it confusing that the `XXX_callback` functions 
all have continuations with the same name in this file.

However, you make a good point regarding consistency. We could rename this 
function `_recv_callback`, which matches our usual pattern for continuations, 
and I could follow up with another patch to similarly change the names of the 
relevant `send_callback` and `event_callback` functions. Thoughts?



3rdparty/libprocess/src/libevent_ssl_socket.hpp (line 155)


I originally did have `received_eof` explicitly protected by `synchronized 
(bev)`, but as BenM mentioned in our offline discussion we should be safe 
without this since all accesses of `received_eof` occur in the event loop. I'll 
move this declaration and add a comment.


- Greg Mann


On Dec. 15, 2016, 5:53 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53802/
> ---
> 
> (Updated Dec. 15, 2016, 5:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-6802
> https://issues.apache.org/jira/browse/MESOS-6802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, it was possible for an SSL socket to either:
> 1) Fail to receive an EOF if the EOF event was received when
>there was no pending recv() request.
> 2) Fail to receive all data sent on the sending side if an
>EOF event was received before all sent data was read.
> 
> This patch eliminates these race conditions to ensure reliable
> receipt of both sent data and EOFs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> 57eaf4f607d0628db466cc1a139772eeeaa51136 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 0e292a8b0d470f4e199db08f09a0c863d73c 
> 
> Diff: https://reviews.apache.org/r/53802/diff/
> 
> 
> Testing
> ---
> 
> Testing details are found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 55339: Removed unsupported 'friend' declaration.

[Michael Park]

> On Jan. 9, 2017, 2:09 p.m., Michael Park wrote:
> > 3rdparty/libprocess/include/process/loop.hpp, lines 216-219
> > 
> >
> > I think we can keep the constructor private, and just do:
> > 
> > ```
> > friend class Continue;
> > friend class Break;
> > ```

Oops. I was wrong about this.


- Michael


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


On Jan. 9, 2017, 5:43 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55339/
> ---
> 
> (Updated Jan. 9, 2017, 5:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-6895
> https://issues.apache.org/jira/browse/MESOS-6895
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The friend class declaration of nested, templated classes
> will raise an error when compiling with Clang.
> This is resolved by making the constructor of 'ControlFlow' public.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/loop.hpp 
> 53f62439752a6eb7cf870022e4965c9261fc3ba6 
> 
> Diff: https://reviews.apache.org/r/55339/diff/
> 
> 
> Testing
> ---
> 
> make check with
> ```
> $ clang --version
> Apple LLVM version 8.0.0 (clang-800.0.42.1)
> Target: x86_64-apple-darwin16.3.0
> ```
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 55355: Fixed an FD leak in the IO switchboard.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [55355]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Jan. 9, 2017, 10:55 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55355/
> ---
> 
> (Updated Jan. 9, 2017, 10:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin 
> Mahler, Jie Yu, Joseph Wu, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the IO switchboard could leak file descriptors
> because it held a reference to its server socket within the
> socket's accept loop. This patch explicitly discards the
> future containing this reference to eliminate the leak.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 67c51678e87389ab08a81fe4832cffd3d83bdae9 
> 
> Diff: https://reviews.apache.org/r/55355/diff/
> 
> 
> Testing
> ---
> 
> Run:
> `bin/mesos-tests.sh --gtest_repeat=-1 
> --gtest_filter="IOSwitchboardServerTest.AttachInput"`
> 
> and then simultaneously execute `sudo lsof -p PID_of_test_process | wc -l` 
> repeatedly to ensure that the number of open FDs is not increasing.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 55355: Fixed an FD leak in the IO switchboard.

[Kevin Klues]

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



I'd run this change my @jie.


src/slave/containerizer/mesos/io/switchboard.cpp (line 993)


Can you move this declaration just below the `inputConnected` declaration?

It seems to group better there for me.



src/slave/containerizer/mesos/io/switchboard.cpp (line 1265)


It's probably worth mentioning here why we grab a reference to the accept 
(i.e. so that we can discard it in `finalize()` so that they FD associated with 
the socket doesn't get leaked).


- Kevin Klues


On Jan. 9, 2017, 8:35 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55355/
> ---
> 
> (Updated Jan. 9, 2017, 8:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin 
> Mahler, Joseph Wu, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the IO switchboard could leak file descriptors
> because it held a reference to its server socket within the
> socket's accept loop. This patch explicitly discards the
> future containing this reference to eliminate the leak.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 67c51678e87389ab08a81fe4832cffd3d83bdae9 
> 
> Diff: https://reviews.apache.org/r/55355/diff/
> 
> 
> Testing
> ---
> 
> Run:
> `bin/mesos-tests.sh --gtest_repeat=-1 
> --gtest_filter="IOSwitchboardServerTest.AttachInput"`
> 
> and then simultaneously execute `sudo lsof -p PID_of_test_process | wc -l` 
> repeatedly to ensure that the number of open FDs is not increasing.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 54183: Improved management of unreachable and completed tasks in master.

[Neil Conway]

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

(Updated Jan. 9, 2017, 10:39 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Address review comments, unbreak test cases.


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


Repository: mesos


Description
---

Before partition-awareness, when an agent failed health checks, the
master removed the agent from the registry, marked all of its tasks
TASK_LOST, and moved them to the `completedTasks` list in the master's
memory. Although "lost" tasks might still be running, partitioned agents
would only be allowed to re-register if the master failed over, in which
case the `completedTasks` map would be emptied.

When partition-awareness was introduced, we initially followed the same
scheme, with the only difference that partition-aware tasks are marked
TASK_UNREACHABLE, not TASK_LOST.

This scheme has a few shortcomings. First, partition-aware tasks might
resume running when the partitioned agent re-registers. Second, we
re-added non-partition aware tasks when the agent re-registered but then
marked them completed when the framework is shutdown, resulting in two
entries in `completedTasks`.

This commit introduces a separate bounded map, `unreachableTasks`. These
tasks are reported separately via the HTTP endpoints, because they have
different semantics (unlike completed tasks, unreachable tasks can
resume running). The size of this map is limited by a new master flag,
`--max_unreachable_tasks_per_framework`. This commit also changes the
master to omit re-adding non-partition-aware tasks on re-registering
agents (unless the master has failed over): those tasks will shortly be
shutdown anyway.

Finally, this commit fixes a minor bug in the previous code: the
previous coding neglected to shutdown non-partition-aware frameworks
running on pre-1.0 Mesos agents that re-register with the master after
a network partition.


Diffs (updated)
-

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
  include/mesos/master/master.proto 03203c8ddd05876339b1c56789b7bb140499d49d 
  include/mesos/v1/master/master.proto f8edf39a68752c8601cece345f52bce8b9a8a68b 
  src/master/constants.hpp 900b69460de9280f30b16b86925e436054e080ca 
  src/master/flags.hpp 6a17b763dc76daa10073394f416b049e97a44238 
  src/master/flags.cpp 124bfbca6be9d5d60597b9a1793bd807202da8ec 
  src/master/http.cpp 75dcd6199dbfcca6260baed49b838a45312bb667 
  src/master/master.hpp 368ee1d5e97784fa54e0f141906405ee8f104317 
  src/master/master.cpp 0c2210bbcd0622f704497daf78fe959cde99ff7f 
  src/tests/partition_tests.cpp 72013d1bfee275c6f3cb90173f0c408d55e0bc5d 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 54183: Improved management of unreachable and completed tasks in master.

[Neil Conway]

> On Jan. 3, 2017, 10:25 p.m., Vinod Kone wrote:
> > src/master/master.hpp, lines 2634-2641
> > 
> >
> > this reads like `unreachableTasks` are completed tasks of PA 
> > frameworks. can you split this comment up and move the unreachable specific 
> > comments to #2644?

I added an additional comment describing the purpose of `unreachableTasks`, but 
I felt like it was useful to still keep a note in the `completedTasks` comment 
to describe which tasks are stored in `unreachableTasks` vs `completedTasks` 
when an agent is marked unreachable. Let me know if you think this is still 
confusing.


> On Jan. 3, 2017, 10:25 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 5496
> > 
> >
> > it's not necessary that the agent has failed over if we are here right?

Do you mean "it's not necessary that the master has failed over if we are here"?

If so, that's right: the master may or may not have re-registered in the time 
since the agent was marked unreachable. The comment talks about how these two 
cases are handled differently.


> On Jan. 3, 2017, 10:25 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 5507
> > 
> >
> > can we inline this?

We could, but personally I find it more readable to make it a separate 
function. The logic is a bit involved here (since we need to check two 
different data structures due to backward compatibility requirements), and 
there's already a lot going on in `Master::_reregisterSlave`.


> On Jan. 3, 2017, 10:25 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 5539
> > 
> >
> > what if the agent was marked unreachable by the previous master? do we 
> > still want to add all its tasks?

If the master has failed over, all tasks on the agent will be allowed to 
continue running (whether partition-aware or not) -- so that's why we re-add 
all tasks here.


> On Jan. 3, 2017, 10:25 p.m., Vinod Kone wrote:
> > src/tests/partition_tests.cpp, lines 1819-1847
> > 
> >
> > hmm. it is unfortunate that we send TASK_LOST followed by TASK_FINISHED 
> > and then send TASK_LOST on reconciliation. can you remind me why the master 
> > forwards status updates for unknown tasks? looks like it can just drop them 
> > if the reason for doing so is no longer valid.

I don't know why we forward status updates for unknown tasks. I can see it 
having some value, though -- in the scenario above, the first `TASK_LOST` just 
means the agent become unreachable. `TASK_FINISHED` tells the framework that 
the task actually did complete successfully. I can imagine situations where, 
say, the framework ignores `TASK_LOST` or uses a large timeout before assuming 
the task is "really lost", whereas `TASK_FINISHED` / `TASK_KILLED` / etc. is a 
more definitive signal.


> On Jan. 3, 2017, 10:25 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 5523
> > 
> >
> > not sure if this is worth making into a function?

I thought it was a bit more readable (against, `Master::_reregisterSlave` has a 
lot of stuff going on), but happy to change it if you disagree.


- Neil


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


On Dec. 18, 2016, 11:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54183/
> ---
> 
> (Updated Dec. 18, 2016, 11:29 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6619
> https://issues.apache.org/jira/browse/MESOS-6619
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before partition-awareness, when an agent failed health checks, the
> master removed the agent from the registry, marked all of its tasks
> TASK_LOST, and moved them to the `completedTasks` list in the master's
> memory. Although "lost" tasks might still be running, partitioned agents
> would only be allowed to re-register if the master failed over, in which
> case the `completedTasks` map would be emptied.
> 
> When partition-awareness was introduced, we initially followed the same
> scheme, with the only difference that partition-aware tasks are marked
> TASK_UNREACHABLE, not TASK_LOST.
> 
> This scheme has a few shortcomings. First, partition-aware tasks might
> resume running when the partitioned agent re-registers. Second, 

Review Request 55359: Consolidate update of allocations in `updateAllocation()`.

[Anindya Sinha]

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

Review request for mesos, Benjamin Mahler and Jiang Yan Xu.


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


Repository: mesos


Description
---

We handle shared resources for `LAUNCH` operations separately in the
`updateAllocation()` API to add additional copies of shared resources.
But the updates to the allocations in the allocator and sorters
can be consolidated together.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
91b1ec43940a788459f045ca4a4b82d4e8373bca 

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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Re: Review Request 45962: Updated a persistent volume test framework to include shared volumes.

[Anindya Sinha]

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

(Updated Jan. 9, 2017, 10:37 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Updated a persistent volume test framework to include shared volumes.


Diffs (updated)
-

  src/examples/persistent_volume_framework.cpp 
9d45bb496c4cf378af429b5aa970bf81450e482a 

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


Testing
---

New test framework for shared resources added.
Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 53096: Fix handling in shared count in total resources in the sorter.

[Anindya Sinha]

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

(Updated Jan. 9, 2017, 10:37 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

We maintain a single copy of shared resource in the role and quota
sorter's total resources. So, when we update these resources, we
remove the previous resources at this agent and add the new resources
at this agent.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
a6424d624864155e1c87a28a63b784512c5c8722 
  src/master/allocator/mesos/hierarchical.cpp 
91b1ec43940a788459f045ca4a4b82d4e8373bca 
  src/tests/persistent_volume_tests.cpp 
8198b6b5ad323d17835ba067c7ff3d34ef948125 

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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Re: Review Request 49571: Added a benchmark test for allocations.

[Anindya Sinha]

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

(Updated Jan. 9, 2017, 10:37 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Allocations test has the following resource configurations:
(1) REGULAR: Offers from every slave have regular resources.
(2) SHARED: Offers from every slave include a shared resource.
(3) REGULAR: Offers from every alternate slave contain only regular
resources; and offers from every other alternate slave contains
a shared resource.

This test is parameterized based on number of agents, number of
frameworks and resource configuration.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
1edd0ecc8a93cd41532e1cf3641f67c780ab23a5 
  src/tests/resources_utils.hpp 18dcca7f171102df8fe88f10785f70c5d1cf5b32 
  src/tests/resources_utils.cpp be1feb97be552af582dbba3a54fa5fa0714b3eb2 

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


Testing
---

All tests passed.

Allocations benchmark test results
==
Support of shared resources has a small impact (roughly 10%) on runtime 
performance in allocations as compared to HEAD (without shared resources). 
Also, there is no visible impact in performance when shared resources are added 
in the tests.

Following is a snapshot with 1000 agents and 200 frameworks.

With the patch (and no shared resources)

[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6907us
Added 1000 agents in 2.057098secs
round 0 allocate() took 1.689164secs to make 1000 offers
round 50 allocate() took 1.672373secs to make 1000 offers
round 100 allocate() took 1.680571secs to make 1000 offers
round 150 allocate() took 1.674683secs to make 1000 offers
round 199 allocate() took 1.671525secs to make 1000 offers

With the patch (and shared resources on all agents)
---
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/10
Using 1000 agents and 200 frameworks with resource type 1
Added 200 frameworks in 6888us
Added 1000 agents in 2.096218secs
round 0 allocate() took 1.704491secs to make 1000 offers
round 50 allocate() took 1.718623secs to make 1000 offers
round 100 allocate() took 1.716224secs to make 1000 offers
round 150 allocate() took 1.707343secs to make 1000 offers
round 199 allocate() took 1.727467secs to make 1000 offers

With the patch (and shared resources on alternate agents)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/11
Using 1000 agents and 200 frameworks with resource type 2
Added 200 frameworks in 7304us
Added 1000 agents in 2.071009secs
round 0 allocate() took 1.689045secs to make 1000 offers
round 50 allocate() took 1.691524secs to make 1000 offers
round 100 allocate() took 1.688873secs to make 1000 offers
round 150 allocate() took 1.688713secs to make 1000 offers
round 199 allocate() took 1.691223secs to make 1000 offers

Based on HEAD, with all regular resources (no shared resources in HEAD 
supported)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6801us
Added 1000 agents in 1.721447secs
round 0 allocate() took 1.502953secs to make 1000 offers
round 50 allocate() took 1.520157secs to make 1000 offers
round 100 allocate() took 1.517221secs to make 1000 offers
round 150 allocate() took 1.526446secs to make 1000 offers
round 199 allocate() took 1.538005secs to make 1000 offers


Thanks,

Anindya Sinha

Re: Review Request 55224: Removed unused namespace alias declarations in mesos.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 5, 2017, 12:07 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55224/
> ---
> 
> (Updated Jan. 5, 2017, 12:07 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Spotted with clang-tidy.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp cc9adfe6af18a0fc90be93c40590375f7bc00eaf 
>   src/launcher/posix/executor.cpp a29b31c14f23d0c60f1bffc5a98c21c2935f0ce0 
>   
> src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
>  b470f0c829ab398f3a45a0467ad25d6adfa17942 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> db2d267996959453f9896287320dca37440b499b 
>   src/tests/containerizer/capabilities_test_helper.cpp 
> 17f00b00656bdd553e2021771c4230910781762b 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 8b4b832a8fc022435b4d1ada8b8fd9c392ce685e 
>   src/tests/registrar_tests.cpp 0433b26f47d25e819533706fe1adbe210b5b6bcf 
>   src/tests/slave_validation_tests.cpp 
> 217e0edec7367e34836bf9b178853b18cbbf0632 
> 
> Diff: https://reviews.apache.org/r/55224/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux, plus internal CI.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55222: Avoided unnecessary copies in mesos.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 5, 2017, 12:05 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55222/
> ---
> 
> (Updated Jan. 5, 2017, 12:05 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Spotted with clang-tidy.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 05215ca66c04e4f1c8bcb0e5bf671a8a08ca0548 
>   src/examples/long_lived_framework.cpp 
> 03c26d921fba670655f3563009ecd3bed2e828f1 
>   src/master/contender/contender.cpp 3debf4730830eb48a97b404c14e34adb7e4c8f3d 
>   src/master/master.cpp 11c34a048586d30c6ac67be8638ed8fa81cc3f1f 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d9d5619e45ae1199fc91878f17a33b5647f48305 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 276b9003aa2671b9ca819e44fb1cd60ae9dff167 
>   src/slave/containerizer/mesos/provisioner/appc/cache.hpp 
> 1e10dc604b29e418beacad6475910dcf7f8899c0 
>   src/slave/containerizer/mesos/provisioner/appc/cache.cpp 
> 20a7030d864bc463322140937b7ae3dbef7d77a0 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4e3b67bbb989f9084dfdf4970839956dcb0caa0e 
>   src/tests/environment.cpp a683d8c221635f81906abbead1e01d2469850d93 
>   src/tests/mesos.hpp fa965150ae3e31b4d74dac57148a197402fd551e 
> 
> Diff: https://reviews.apache.org/r/55222/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux, plus internal CI.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55223: Removed unused namespace alias declarations in libprocess.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 5, 2017, 11:07 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55223/
> ---
> 
> (Updated Jan. 5, 2017, 11:07 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Spotted with clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 889a03444eaee7b5ad2be65bb414c30062d4a4f0 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d168ad15da4706fb9c3aa11a125b7dfb5987570d 
> 
> Diff: https://reviews.apache.org/r/55223/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux, plus internal CI.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55220: Cleaned up std::string usage in mesos.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 5, 2017, 12:04 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55220/
> ---
> 
> (Updated Jan. 5, 2017, 12:04 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid redundant initialization to empty string, use `char` overloads
> instead for single-character string literals.
> 
> Spotted with clang-tidy.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 7dbeefc8fc159f28dc1542ad6a761213c65ff226 
>   src/docker/docker.cpp 472cb1b4dc2b0ac65721c732fca8ec70a7470f47 
>   src/slave/containerizer/fetcher.cpp 
> 94cfa0e33adc622d77a75e256fb52fc75d3db18a 
>   src/state/zookeeper.cpp 9941f6b3f9da264d3b75d74edfc60afc740bc74e 
>   src/tests/environment.cpp a683d8c221635f81906abbead1e01d2469850d93 
>   src/v1/resources.cpp 859f7388035b855bf0cb60d5cbe746e6871bf833 
>   src/zookeeper/zookeeper.cpp e105377ed0ceba22b03c27ab1209e7a96c284066 
> 
> Diff: https://reviews.apache.org/r/55220/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux, plus internal CI.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55226: Cleaned up std::string usage in libprocess.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 5, 2017, 12:04 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55226/
> ---
> 
> (Updated Jan. 5, 2017, 12:04 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid redundant initialization to empty string, use `char` overloads
> instead for single-character string literals.
> 
> Spotted with clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp 97d1424be20e217401519c2bee79857bcf087023 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> b9825e8633f64c23e4b1ea904537cdc8da64ed5b 
> 
> Diff: https://reviews.apache.org/r/55226/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux, plus internal CI.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55221: Avoided needless copies in log tests.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 5, 2017, 11:06 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55221/
> ---
> 
> (Updated Jan. 5, 2017, 11:06 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also did some minor code cleanup.
> 
> Spotted with clang-tidy.
> 
> 
> Diffs
> -
> 
>   src/tests/log_tests.cpp 06efd1b68c713f3e121d0ba064dd10fd1171d284 
> 
> Diff: https://reviews.apache.org/r/55221/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux, plus internal CI.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55219: Cleaned up std::string usage in stout.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 5, 2017, 11:06 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55219/
> ---
> 
> (Updated Jan. 5, 2017, 11:06 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid redundant initialization to empty string, use `char` overloads
> instead for single-character string literals.
> 
> Spotted with clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> 61d17a9d50f2115ca2228ec4b05cdbb874c595c5 
>   3rdparty/stout/include/stout/gzip.hpp 
> 97891860dbdd7095d5498695ad0e97ad02e35d73 
>   3rdparty/stout/include/stout/os/mkdir.hpp 
> fe86864c8b480993c8f052f39b2fd3ece23798da 
>   3rdparty/stout/include/stout/path.hpp 
> d1092f6cccfb9dc53154ce9ce09414d57526a76d 
>   3rdparty/stout/tests/ip_tests.cpp 84eb75ee7e1e9ec04527ade3bb8aeba93301ab19 
> 
> Diff: https://reviews.apache.org/r/55219/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux, plus internal CI.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55218: Removed redundant `return` in libprocess.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 5, 2017, 11:05 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55218/
> ---
> 
> (Updated Jan. 5, 2017, 11:05 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Spotted with clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 2da9a32f48afcf06f960d51e19f898f6eebb4513 
> 
> Diff: https://reviews.apache.org/r/55218/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux, plus internal CI.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55339: Removed unsupported 'friend' declaration.

[Michael Park]

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


Fix it, then Ship it!





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


I think we can keep the constructor private, and just do:

```
friend class Continue;
friend class Break;
```


- Michael Park


On Jan. 9, 2017, 5:43 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55339/
> ---
> 
> (Updated Jan. 9, 2017, 5:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-6895
> https://issues.apache.org/jira/browse/MESOS-6895
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The friend class declaration of nested, templated classes
> will raise an error when compiling with Clang.
> This is resolved by making the constructor of 'ControlFlow' public.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/loop.hpp 
> 53f62439752a6eb7cf870022e4965c9261fc3ba6 
> 
> Diff: https://reviews.apache.org/r/55339/diff/
> 
> 
> Testing
> ---
> 
> make check with
> ```
> $ clang --version
> Apple LLVM version 8.0.0 (clang-800.0.42.1)
> Target: x86_64-apple-darwin16.3.0
> ```
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

[Kevin Klues]

> On Jan. 9, 2017, 8:24 p.m., Kevin Klues wrote:
> > What is the reason for this change? It seems much more intuitive for me to 
> > have a simple build directive that we simply remove once of the rename is 
> > complete, compared to a custom install-hook that creates a symlink.
> 
> James Peach wrote:
> The rationale is that we don't need to spend time and space building annd 
> installing an extra binary.

OK. Can you at least retain the comment for:
```
# TODO(tomxing): Remove this binary once the
# slave->agent rename is complete(MESOS-3782).
```

so taht we don't forget to remove the install-hook once the full rename is done.


- Kevin


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


On Jan. 9, 2017, 8:02 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54613/
> ---
> 
> (Updated Jan. 9, 2017, 8:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Alex Clemmer, Joseph Wu, Michael 
> Park, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6772
> https://issues.apache.org/jira/browse/MESOS-6772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Install a symlink rather than building mesos-slave twice.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
> 
> Diff: https://reviews.apache.org/r/54613/diff/
> 
> 
> Testing
> ---
> 
> Make install:
> ```
> [jpeach@jpeach mesos.git]$ ls -l /opt/mesos/sbin/
> total 10448
> -rwxr-xr-x 1 root root 5116104 Dec  9 15:27 mesos-agent*
> -rwxr-xr-x 1 root root 5539096 Dec  9 15:27 mesos-master*
> lrwxrwxrwx 1 root root  11 Dec  9 15:27 mesos-slave -> mesos-agent*
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 55296: Used `jsonify` in `operator<<` for `JSON::*` to reduce duplicate code.

[Michael Park]

> On Jan. 9, 2017, 8:52 a.m., Alexander Rojas wrote:
> > 3rdparty/stout/include/stout/json.hpp, line 703
> > 
> >
> > This needs either UNREACHABLE() or to return stream to avoid warnings.

There seems to be some issue with the git repo and reviewboard or something...
https://mesos.slack.com/archives/dev/p1483997930001234

I'll update this as soon as it's resolved.


- Michael


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


On Jan. 7, 2017, 2:02 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55296/
> ---
> 
> (Updated Jan. 7, 2017, 2:02 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The printing logic for `json.hpp` and `jsonify.hpp` are currently duplicated.
> We can reduce this duplication by leveraging `jsonify` for the implementation 
> of `operator<<` for `JSON::*`.
> Since `JSON::Value`s are not generally used for printing, there should be no 
> performance concerns here.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 62ce15274677112d142a3c829b4a9f06258c9e2c 
> 
> Diff: https://reviews.apache.org/r/55296/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 54573: Updated metrics counter during scheduler api calls.

[Benjamin Mahler]

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



Anand will shepherd these patches, can you add him as a reviewer?

- Benjamin Mahler


On Dec. 9, 2016, 4:40 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54573/
> ---
> 
> (Updated Dec. 9, 2016, 4:40 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Zhitao Li.
> 
> 
> Bugs: MESOS-6082
> https://issues.apache.org/jira/browse/MESOS-6082
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated metrics counter during  scheduler api calls.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp ac560d1fdd219d0de0c5d987a32a7112e149602f 
>   src/master/master.cpp 67f32229470da4cf7953881d1c5dcb99393002de 
> 
> Diff: https://reviews.apache.org/r/54573/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 55324: Included decoder error strings.

[Benjamin Mahler]

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



Any reason not to expose the error via the API so that the caller can log it? 
Right now it's only being exposed in one case (failure during body decoding) to 
the consumer of the body.


3rdparty/libprocess/src/decoder.hpp (lines 78 - 81)


Any reason for not exposing the error here as well?



3rdparty/libprocess/src/decoder.hpp (lines 92 - 95)


I was hoping that we could expose the error / failure via the API, for 
example we have a function like failed that returns an Option.



3rdparty/libprocess/src/decoder.hpp (lines 320 - 323)


Ditto here.



3rdparty/libprocess/src/decoder.hpp (lines 549 - 550)


It would be nice if the caller could get the error via the API, currently 
it's only exposed to the consumer of the body (if the failure occurs while 
decoding the body).


- Benjamin Mahler


On Jan. 8, 2017, 7:51 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55324/
> ---
> 
> (Updated Jan. 8, 2017, 7:51 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included decoder error strings.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 4c779d42548958e610142438a57529ccb4478053 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 
> 87563f4fcdfaa2a33c4482533ddff24e062e603a 
> 
> Diff: https://reviews.apache.org/r/55324/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 55157: Fixed a bug in the default executor around not committing suicide.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [55157]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Jan. 9, 2017, 7:37 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55157/
> ---
> 
> (Updated Jan. 9, 2017, 7:37 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6848
> https://issues.apache.org/jira/browse/MESOS-6848
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This bug is only observed when the task group contains a single task.
> The default executor was not committing suicide when this single task
> used to exit with a non-zero status code as per the default restart
> policy.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp fd9963996c82461b60888397989e309d51b60aaa 
>   src/tests/default_executor_tests.cpp 
> 340e8c8b36a6a3cc6e5bae021e19dfa7a54852c3 
> 
> Diff: https://reviews.apache.org/r/55157/diff/
> 
> 
> Testing
> ---
> 
> make check + added a test
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

[James Peach]

> On Jan. 9, 2017, 8:24 p.m., Kevin Klues wrote:
> > What is the reason for this change? It seems much more intuitive for me to 
> > have a simple build directive that we simply remove once of the rename is 
> > complete, compared to a custom install-hook that creates a symlink.

The rationale is that we don't need to spend time and space building annd 
installing an extra binary.


- James


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


On Jan. 9, 2017, 8:02 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54613/
> ---
> 
> (Updated Jan. 9, 2017, 8:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Alex Clemmer, Joseph Wu, Michael 
> Park, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6772
> https://issues.apache.org/jira/browse/MESOS-6772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Install a symlink rather than building mesos-slave twice.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
> 
> Diff: https://reviews.apache.org/r/54613/diff/
> 
> 
> Testing
> ---
> 
> Make install:
> ```
> [jpeach@jpeach mesos.git]$ ls -l /opt/mesos/sbin/
> total 10448
> -rwxr-xr-x 1 root root 5116104 Dec  9 15:27 mesos-agent*
> -rwxr-xr-x 1 root root 5539096 Dec  9 15:27 mesos-master*
> lrwxrwxrwx 1 root root  11 Dec  9 15:27 mesos-slave -> mesos-agent*
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 55355: Fixed an FD leak in the IO switchboard.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin Mahler, 
Joseph Wu, and Kevin Klues.


Repository: mesos


Description
---

Previously, the IO switchboard could leak file descriptors
because it held a reference to its server socket within the
socket's accept loop. This patch explicitly discards the
future containing this reference to eliminate the leak.


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
67c51678e87389ab08a81fe4832cffd3d83bdae9 

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


Testing
---

Run:
`bin/mesos-tests.sh --gtest_repeat=-1 
--gtest_filter="IOSwitchboardServerTest.AttachInput"`

and then simultaneously execute `sudo lsof -p PID_of_test_process | wc -l` 
repeatedly to ensure that the number of open FDs is not increasing.


Thanks,

Greg Mann

Re: Review Request 55355: Fixed an FD leak in the IO switchboard.

[Greg Mann]

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

(Updated Jan. 9, 2017, 8:35 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin Mahler, 
Joseph Wu, and Kevin Klues.


Repository: mesos


Description
---

Previously, the IO switchboard could leak file descriptors
because it held a reference to its server socket within the
socket's accept loop. This patch explicitly discards the
future containing this reference to eliminate the leak.


Diffs
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
67c51678e87389ab08a81fe4832cffd3d83bdae9 

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


Testing
---

Run:
`bin/mesos-tests.sh --gtest_repeat=-1 
--gtest_filter="IOSwitchboardServerTest.AttachInput"`

and then simultaneously execute `sudo lsof -p PID_of_test_process | wc -l` 
repeatedly to ensure that the number of open FDs is not increasing.


Thanks,

Greg Mann

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

[Kevin Klues]

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



What is the reason for this change? It seems much more intuitive for me to have 
a simple build directive that we simply remove once of the rename is complete, 
compared to a custom install-hook that creates a symlink.

- Kevin Klues


On Jan. 9, 2017, 8:02 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54613/
> ---
> 
> (Updated Jan. 9, 2017, 8:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Alex Clemmer, Joseph Wu, Michael 
> Park, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6772
> https://issues.apache.org/jira/browse/MESOS-6772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Install a symlink rather than building mesos-slave twice.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
> 
> Diff: https://reviews.apache.org/r/54613/diff/
> 
> 
> Testing
> ---
> 
> Make install:
> ```
> [jpeach@jpeach mesos.git]$ ls -l /opt/mesos/sbin/
> total 10448
> -rwxr-xr-x 1 root root 5116104 Dec  9 15:27 mesos-agent*
> -rwxr-xr-x 1 root root 5539096 Dec  9 15:27 mesos-master*
> lrwxrwxrwx 1 root root  11 Dec  9 15:27 mesos-slave -> mesos-agent*
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 55354: Cleaned up master logging code slightly.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 9, 2017, 7:44 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55354/
> ---
> 
> (Updated Jan. 9, 2017, 7:44 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6286
> https://issues.apache.org/jira/browse/MESOS-6286
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up master logging code slightly.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 11c34a048586d30c6ac67be8638ed8fa81cc3f1f 
> 
> Diff: https://reviews.apache.org/r/55354/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55307: Improved handling of agents that restart but never re-register.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 9, 2017, 7:44 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55307/
> ---
> 
> (Updated Jan. 9, 2017, 7:44 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6286
> https://issues.apache.org/jira/browse/MESOS-6286
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master expected that if an agent responds to pings, it will
> (eventually) register or re-register. However, if the agent hangs during
> recovery, that assumption does not hold: the agent will continue to
> respond to pings but won't attempt to re-register until recovery
> finishes.
> 
> To handle this case, the master now expects an agent to re-register
> within `agent_reregister_timeout` if the master -> agent socket breaks;
> if no re-registration is seen, the master will mark the agent
> unreachable. This is a "backup" to handle the case where recovery hangs,
> as explained above; more commonly, the agent will re-register (when it
> receives a ping and notices the master believes it is disconnected) or
> be marked unreachable because it fails to respond to pings.
> 
> 
> Diffs
> -
> 
>   CHANGELOG f9122fa67a5cb2094c8ab55624799bc0940fb0b0 
>   docs/configuration.md e4beb2d5a72f1c5f59b2e40f4984cc60b8437d9d 
>   src/master/flags.cpp 737290a42c532f2349009d0a451ce271d6f107b9 
>   src/master/master.hpp 57fc6e6f2995078df80f0aa52707727db802ede0 
>   src/master/master.cpp 11c34a048586d30c6ac67be8638ed8fa81cc3f1f 
>   src/slave/slave.cpp f8f2ccfadb9a00be17c0b552586aa5875b7cbb19 
>   src/tests/master_tests.cpp 1cf4c92b2474e18771459f877b2f3c49077e8a01 
>   src/tests/slave_tests.cpp d633a74d6b342239fbca0b44eec281eb315df5ff 
> 
> Diff: https://reviews.apache.org/r/55307/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran new tests a few thousand times on OSX and Linux VM to check for flakiness.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

[James Peach]

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

(Updated Jan. 9, 2017, 8:02 p.m.)


Review request for mesos, Benjamin Bannier, Alex Clemmer, Joseph Wu, Michael 
Park, Vinod Kone, and Jiang Yan Xu.


Changes
---

Make change suggested by bbannier.


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


Repository: mesos


Description
---

Install a symlink rather than building mesos-slave twice.


Diffs (updated)
-

  src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 

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


Testing
---

Make install:
```
[jpeach@jpeach mesos.git]$ ls -l /opt/mesos/sbin/
total 10448
-rwxr-xr-x 1 root root 5116104 Dec  9 15:27 mesos-agent*
-rwxr-xr-x 1 root root 5539096 Dec  9 15:27 mesos-master*
lrwxrwxrwx 1 root root  11 Dec  9 15:27 mesos-slave -> mesos-agent*
```


Thanks,

James Peach

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

[James Peach]

> On Dec. 20, 2016, 1:54 p.m., Benjamin Bannier wrote:
> > src/Makefile.am, line 2453
> > 
> >
> > Not yours, but since you touch this it would be great if you could make 
> > these independent commands (i.e., remove the ` &&` here).
> > 
> > I added more cleanups here, https://reviews.apache.org/r/54896/.

Done.


- James


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


On Jan. 9, 2017, 8:02 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54613/
> ---
> 
> (Updated Jan. 9, 2017, 8:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Alex Clemmer, Joseph Wu, Michael 
> Park, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6772
> https://issues.apache.org/jira/browse/MESOS-6772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Install a symlink rather than building mesos-slave twice.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
> 
> Diff: https://reviews.apache.org/r/54613/diff/
> 
> 
> Testing
> ---
> 
> Make install:
> ```
> [jpeach@jpeach mesos.git]$ ls -l /opt/mesos/sbin/
> total 10448
> -rwxr-xr-x 1 root root 5116104 Dec  9 15:27 mesos-agent*
> -rwxr-xr-x 1 root root 5539096 Dec  9 15:27 mesos-master*
> lrwxrwxrwx 1 root root  11 Dec  9 15:27 mesos-slave -> mesos-agent*
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 55157: Fixed a bug in the default executor around not committing suicide.

[Anand Mazumdar]

> On Jan. 9, 2017, 7:47 p.m., Vinod Kone wrote:
> > src/launcher/default_executor.cpp, lines 672-679
> > 
> >
> > shouldn't these be re-ordered? i'm assuming we don't need to call 
> > `__shutdown()` if a shutdown is already in progress?

As per our offline discussion, we can't reorder them as `shutdown()` doesn't 
invoke `__shutdown` implicitly to commit suicide. We wait for all our wait 
connections to return and commit suicide thereafter in `waited()`. This allows 
us to keep all the restart/kill policy logic in one place in `waited()`. 

I would add a comment in `shutdown()` in a separate review for posterity.


- Anand


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


On Jan. 9, 2017, 7:37 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55157/
> ---
> 
> (Updated Jan. 9, 2017, 7:37 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6848
> https://issues.apache.org/jira/browse/MESOS-6848
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This bug is only observed when the task group contains a single task.
> The default executor was not committing suicide when this single task
> used to exit with a non-zero status code as per the default restart
> policy.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp fd9963996c82461b60888397989e309d51b60aaa 
>   src/tests/default_executor_tests.cpp 
> 340e8c8b36a6a3cc6e5bae021e19dfa7a54852c3 
> 
> Diff: https://reviews.apache.org/r/55157/diff/
> 
> 
> Testing
> ---
> 
> make check + added a test
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 55157: Fixed a bug in the default executor around not committing suicide.

[Vinod Kone]

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




src/launcher/default_executor.cpp (lines 665 - 672)


shouldn't these be re-ordered? i'm assuming we don't need to call 
`__shutdown()` if a shutdown is already in progress?


- Vinod Kone


On Jan. 9, 2017, 7:37 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55157/
> ---
> 
> (Updated Jan. 9, 2017, 7:37 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6848
> https://issues.apache.org/jira/browse/MESOS-6848
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This bug is only observed when the task group contains a single task.
> The default executor was not committing suicide when this single task
> used to exit with a non-zero status code as per the default restart
> policy.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp fd9963996c82461b60888397989e309d51b60aaa 
>   src/tests/default_executor_tests.cpp 
> 340e8c8b36a6a3cc6e5bae021e19dfa7a54852c3 
> 
> Diff: https://reviews.apache.org/r/55157/diff/
> 
> 
> Testing
> ---
> 
> make check + added a test
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 55354: Cleaned up master logging code slightly.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Cleaned up master logging code slightly.


Diffs
-

  src/master/master.cpp 11c34a048586d30c6ac67be8638ed8fa81cc3f1f 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 55307: Improved handling of agents that restart but never re-register.

[Neil Conway]

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

(Updated Jan. 9, 2017, 7:44 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Address review comments.


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


Repository: mesos


Description
---

The master expected that if an agent responds to pings, it will
(eventually) register or re-register. However, if the agent hangs during
recovery, that assumption does not hold: the agent will continue to
respond to pings but won't attempt to re-register until recovery
finishes.

To handle this case, the master now expects an agent to re-register
within `agent_reregister_timeout` if the master -> agent socket breaks;
if no re-registration is seen, the master will mark the agent
unreachable. This is a "backup" to handle the case where recovery hangs,
as explained above; more commonly, the agent will re-register (when it
receives a ping and notices the master believes it is disconnected) or
be marked unreachable because it fails to respond to pings.


Diffs (updated)
-

  CHANGELOG f9122fa67a5cb2094c8ab55624799bc0940fb0b0 
  docs/configuration.md e4beb2d5a72f1c5f59b2e40f4984cc60b8437d9d 
  src/master/flags.cpp 737290a42c532f2349009d0a451ce271d6f107b9 
  src/master/master.hpp 57fc6e6f2995078df80f0aa52707727db802ede0 
  src/master/master.cpp 11c34a048586d30c6ac67be8638ed8fa81cc3f1f 
  src/slave/slave.cpp f8f2ccfadb9a00be17c0b552586aa5875b7cbb19 
  src/tests/master_tests.cpp 1cf4c92b2474e18771459f877b2f3c49077e8a01 
  src/tests/slave_tests.cpp d633a74d6b342239fbca0b44eec281eb315df5ff 

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


Testing
---

`make check`

Ran new tests a few thousand times on OSX and Linux VM to check for flakiness.


Thanks,

Neil Conway

Re: Review Request 55307: Improved handling of agents that restart but never re-register.

[Neil Conway]

> On Jan. 9, 2017, 3:17 a.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 1368-1370
> > 
> >
> > just like "health check time out", can this be succinct? maybe 
> > "re-registration time out"?

I think being a bit more verbose is warranted here -- e.g., pointing out that 
we previously observed the agent disconnecting, which is why we expected it to 
re-register.


> On Jan. 9, 2017, 3:17 a.m., Vinod Kone wrote:
> > src/tests/master_tests.cpp, lines 6105-6117
> > 
> >
> > Instead of advancing the clock for the recovery to finish, why not just 
> > not advance it. That way you don't have to do the registration drops either?
> > 
> > also, you only do `detector.appoint` way below; does agent even send 
> > re-registration messages without that here? i guess it does if the master 
> > pid didn't change?

Per offline discussion, we can't not-advance the clock because we need to 
advance the clock below to cause `agent_reregister_timeout` to expire. We also 
need to advance the clock if we want to trigger a master -> agent ping, which 
is a useful thing to do (since we want to verify that the agent continues to 
receive and respond to pings without having finished recovery).


- Neil


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


On Jan. 7, 2017, 9:13 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55307/
> ---
> 
> (Updated Jan. 7, 2017, 9:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6286
> https://issues.apache.org/jira/browse/MESOS-6286
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master expected that if an agent responds to pings, it will
> (eventually) register or re-register. However, if the agent hangs during
> recovery, that assumption does not hold: the agent will continue to
> respond to pings but won't attempt to re-register until recovery
> finishes.
> 
> To handle this case, the master now expects an agent to re-register
> within `agent_reregister_timeout` if the master -> agent socket breaks;
> if no re-registration is seen, the master will mark the agent
> unreachable. This is a "backup" to handle the case where recovery hangs,
> as explained above; more commonly, the agent will re-register (when it
> receives a ping and notices the master believes it is disconnected) or
> be marked unreachable because it fails to respond to pings.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md e4beb2d5a72f1c5f59b2e40f4984cc60b8437d9d 
>   src/master/flags.cpp 737290a42c532f2349009d0a451ce271d6f107b9 
>   src/master/master.hpp 57fc6e6f2995078df80f0aa52707727db802ede0 
>   src/master/master.cpp 11c34a048586d30c6ac67be8638ed8fa81cc3f1f 
>   src/slave/slave.cpp f8f2ccfadb9a00be17c0b552586aa5875b7cbb19 
>   src/tests/master_tests.cpp 1cf4c92b2474e18771459f877b2f3c49077e8a01 
>   src/tests/slave_tests.cpp d633a74d6b342239fbca0b44eec281eb315df5ff 
> 
> Diff: https://reviews.apache.org/r/55307/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Ran new tests a few thousand times on OSX and Linux VM to check for flakiness.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55157: Fixed a bug in the default executor around not committing suicide.

[Anand Mazumdar]

> On Jan. 9, 2017, 2:10 a.m., Vinod Kone wrote:
> > src/launcher/default_executor.cpp, lines 663-683
> > 
> >
> > How about
> > 
> > ```
> > // Check to see if the executor needs to shutdown.
> > 
> > // If the executor is already in the process of shutting down, return.
> > if (shuttingDown) {
> >   return;
> > }
> > 
> > // If there are no active containers, shutdown the executor.
> > if (containers.empty()) {
> >   shutdown();
> >   return;
> > }
> > 
> > // If this container exited with non-zero status, kill all the other 
> > containers,
> > // per the default policy.
> > if (taskState == TASK_FAILED) {
> > 
> >   // TODO(Anand): ...
> >   shutdown();
> > }
> > 
> > 
> > ```
> 
> Anand Mazumdar wrote:
> hmm,  I had refrained from splitting the no active containers check and 
> moving it above was because I still wanted to log why the single task pod 
> failed. With the above suggested snippet, we would need to do the logging for 
> both the cases where we return early if the task failed? Another alternative 
> can be that I return early when the executor is shutting down while keeping 
> the rest of the logic same:
> 
> ```cpp
> // Ignore if the executor is already in the process of shutting down.
> if (shuttingDown) {
>   return;
> }
> 
> // The default restart policy for a task group is to kill all the
> // remaining child containers if one of them terminated with a
> // non-zero exit code.
> if (taskState == TASK_FAILED) {
>   LOG(ERROR)
> << "Child container " << containerId << " terminated with status "
> << (status.isSome() ? WSTRINGIFY(status.get()) : "unknown");
> 
>   // Kill all the other active containers.
>   //
>   // TODO(anand): Invoke `kill()` once per active container
>   // instead of directly invoking `shutdown()`.
>   if (!containers.empty()) {
> shutdown();
> return;
>   }
> }
> 
> // Shutdown the executor if all the active child containers have 
> terminated.
> if (containers.empty()) {
>   __shutdown();
> }
> ```
> 
> What do you think?
> 
> Vinod Kone wrote:
> I think we can print the exit code/reason in the LOG(INFO) message at 
> #656. It is useful irrespective of task status?
> 
> I moved up `if (containers.empty())` check because it was not very 
> intuitive to me why you had a `!containers.empty()` check inside that if 
> statement. I had to read the `shutdown()` code to understand that. As a side 
> note, it sounds like `shutdown()` should have the logic to call 
> `__shutdown()` if containers is empty.

hmm, I couldn't think of any possible other task states where it might be 
useful other than `TASK_FAILED` to log the exit status. There might be some 
`TASK_KILLED` instances (OOM) where we might still be able to retrieve the 
status? But, neverthless it seemed useful to log the exit status reason.

Can you take a look at the review again?


- Anand


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


On Jan. 9, 2017, 7:37 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55157/
> ---
> 
> (Updated Jan. 9, 2017, 7:37 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6848
> https://issues.apache.org/jira/browse/MESOS-6848
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This bug is only observed when the task group contains a single task.
> The default executor was not committing suicide when this single task
> used to exit with a non-zero status code as per the default restart
> policy.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp fd9963996c82461b60888397989e309d51b60aaa 
>   src/tests/default_executor_tests.cpp 
> 340e8c8b36a6a3cc6e5bae021e19dfa7a54852c3 
> 
> Diff: https://reviews.apache.org/r/55157/diff/
> 
> 
> Testing
> ---
> 
> make check + added a test
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 55157: Fixed a bug in the default executor around not committing suicide.

[Anand Mazumdar]

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

(Updated Jan. 9, 2017, 7:37 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments from Vinod


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


Repository: mesos


Description
---

This bug is only observed when the task group contains a single task.
The default executor was not committing suicide when this single task
used to exit with a non-zero status code as per the default restart
policy.


Diffs (updated)
-

  src/launcher/default_executor.cpp fd9963996c82461b60888397989e309d51b60aaa 
  src/tests/default_executor_tests.cpp 340e8c8b36a6a3cc6e5bae021e19dfa7a54852c3 

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


Testing
---

make check + added a test


Thanks,

Anand Mazumdar

Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54996]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Jan. 9, 2017, 4:42 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
> 
> (Updated Jan. 9, 2017, 4:42 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently in the Linux launcher when the stack is allocated and prepared for 
> a call to clone() it is not properly aligned. This is not an issue for x86 or 
> x64 but for ARM64/AArch64 it is because of the requirement of having the 
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack 
> to have a 16 byte aligned stack, it is not enforced. An explanation of the 
> stack and requirements for ARM64 can be found here 
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
>  (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be 
> quad-word aligned.)
> 
> Additionally, the way that the stack is currently allocated and passed to 
> clone() accidentally chops off one entry, making a stack overflow using those 
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix 
> both the issue of the stack overflow issue as well as the SIGBUS crash. We 
> should also net better performance from having the stack aligned.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 530f1a55b 
>   src/linux/ns.hpp 77789717e 
> 
> Diff: https://reviews.apache.org/r/54996/diff/
> 
> 
> Testing
> ---
> 
> Built Mesos from source and am currently running it in a test cluster. 
> Launched both Docker and Mesos tasks via Marathon without any resulting crash 
> (initial crash only happened with Mesos containerizer + linux_launcher, not 
> with the posix_launcher).
> 
> `make check -j4` also passes with no errors.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 55327: Windows: Fixed hanging symlink bug in `os::rmdir`.

[Lior Zeno]

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




3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 97)


s/best-effor/best-effort


- Lior Zeno


On Jan. 8, 2017, 11:52 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55327/
> ---
> 
> (Updated Jan. 8, 2017, 11:52 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Windows implementation of `os::rmdir` will fail to delete "hanging"
> symlinks (i.e., symlinks whose targets do not exist). Note that on
> Windows this bug is specific to symlinks whose targets are _deleted_,
> since it is impossible to create a symlink whose target does not exist.
> 
> The primary issue that causes this problem is that it is very difficult
> to tell whether a symlink points at a directory or a file unless you
> resolve the symlink and determine whether the target is a directory or a
> file. In situations where the target does not exist, we can't use this
> information, and so `os::rmdir` occasionally mis-routes a symlink to
> (what was) a directory to a `::remove` call, which will fail with a
> cryptic error.
> 
> To fix this behavior, this commit will introduce code that simply tries
> to remove the reparse point with both `RemoveDirectory` and
> `DeleteFile`, and if either succeeds, we report success for the
> operation. This represents a "best effort"; in the case that the reparse
> point represents something more exotic than a symlink, we will still
> fail, but by choosing not to verify whether the target is a directory or
> a file, we simplify the code and still obtain the outcome of having
> deleted the directory.
> 
> This commit is the primary blocker for MESOS-6707, as deleting the Agent
> sandbox will sometimes cause us to delete the latest run directory for
> the executor before the symlinked `latest` directory itself. This causes
> the delete to fail, and then the GC tests to fail, since they tend to
> assert the directory does not exist.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> 4437484c068e9ef046e0be14683c97db447f2da1 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> 988d41b7fdd11cc96ce005671a7c62d1b5a3615d 
> 
> Diff: https://reviews.apache.org/r/55327/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Dec. 19, 2016, 4:11 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54613/
> ---
> 
> (Updated Dec. 19, 2016, 4:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Alex Clemmer, Joseph Wu, Michael 
> Park, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6772
> https://issues.apache.org/jira/browse/MESOS-6772
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Install a symlink rather than building mesos-slave twice.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 0f62ec70816e8b48e19d35036285656a6e7cd02b 
> 
> Diff: https://reviews.apache.org/r/54613/diff/
> 
> 
> Testing
> ---
> 
> Make install:
> ```
> [jpeach@jpeach mesos.git]$ ls -l /opt/mesos/sbin/
> total 10448
> -rwxr-xr-x 1 root root 5116104 Dec  9 15:27 mesos-agent*
> -rwxr-xr-x 1 root root 5539096 Dec  9 15:27 mesos-master*
> lrwxrwxrwx 1 root root  11 Dec  9 15:27 mesos-slave -> mesos-agent*
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 55302: Improved OneWayPartitionTest.MasterToSlave.

[Neil Conway]

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

(Updated Jan. 9, 2017, 6:24 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Fix typo in comment.


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


Repository: mesos


Description
---

Fix theoretical test flakiness (unlikely to be a problem in practice),
add more test expections, and cleanup code.


Diffs (updated)
-

  src/tests/partition_tests.cpp 3afdbba13a9753edce17b24ff6d1898d3cc6de62 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Zhitao Li]

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

(Updated Jan. 9, 2017, 5:59 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Fix for also recovering `ContainerConfig` for alive executor containers.


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


Repository: mesos


Description (updated)
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
9fcad933e1ec3b52d4dc366ba80d282deea04c0b 
  src/slave/containerizer/mesos/containerizer.cpp 
d9d5619e45ae1199fc91878f17a33b5647f48305 
  src/slave/containerizer/mesos/paths.hpp 
d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
  src/slave/containerizer/mesos/paths.cpp 
c1770cefe0287ce994eec6979db41201148ef3fb 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55296: Used `jsonify` in `operator<<` for `JSON::*` to reduce duplicate code.

[Alexander Rojas]

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




3rdparty/stout/include/stout/json.hpp (line 679)


This needs either UNREACHABLE() or to return stream to avoid warnings.


- Alexander Rojas


On Jan. 7, 2017, 11:02 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55296/
> ---
> 
> (Updated Jan. 7, 2017, 11:02 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The printing logic for `json.hpp` and `jsonify.hpp` are currently duplicated.
> We can reduce this duplication by leveraging `jsonify` for the implementation 
> of `operator<<` for `JSON::*`.
> Since `JSON::Value`s are not generally used for printing, there should be no 
> performance concerns here.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 62ce15274677112d142a3c829b4a9f06258c9e2c 
> 
> Diff: https://reviews.apache.org/r/55296/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.

[Aaron Wood]

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

(Updated Jan. 9, 2017, 4:42 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Currently in the Linux launcher when the stack is allocated and prepared for a 
call to clone() it is not properly aligned. This is not an issue for x86 or x64 
but for ARM64/AArch64 it is because of the requirement of having the stack 
aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have 
a 16 byte aligned stack, it is not enforced. An explanation of the stack and 
requirements for ARM64 can be found here 
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf 
(specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be 
quad-word aligned.)

Additionally, the way that the stack is currently allocated and passed to 
clone() accidentally chops off one entry, making a stack overflow using those 
missing 8 bytes a possibility. Fixing this while aligning the memory will fix 
both the issue of the stack overflow issue as well as the SIGBUS crash. We 
should also net better performance from having the stack aligned.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/linux.hpp 530f1a55b 
  src/linux/ns.hpp 77789717e 

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


Testing
---

Built Mesos from source and am currently running it in a test cluster. Launched 
both Docker and Mesos tasks via Marathon without any resulting crash (initial 
crash only happened with Mesos containerizer + linux_launcher, not with the 
posix_launcher).

`make check -j4` also passes with no errors.


Thanks,

Aaron Wood

Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.

[James Peach]

> On Jan. 7, 2017, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 72
> > 
> >
> > Please use errno error so that errno are in the error string.
> > 
> > `return ErrnoError("Memory allocation failed");`
> 
> Aaron Wood wrote:
> `posix_memalign` never sets `errno`. Shouldn't we just return `Error`?

You can do this:
```
int err = posix_memalign(...);
if (err) {
  return ErrnoError(err);
}
```


- James


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


On Jan. 9, 2017, 4:19 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
> 
> (Updated Jan. 9, 2017, 4:19 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently in the Linux launcher when the stack is allocated and prepared for 
> a call to clone() it is not properly aligned. This is not an issue for x86 or 
> x64 but for ARM64/AArch64 it is because of the requirement of having the 
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack 
> to have a 16 byte aligned stack, it is not enforced. An explanation of the 
> stack and requirements for ARM64 can be found here 
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
>  (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be 
> quad-word aligned.)
> 
> Additionally, the way that the stack is currently allocated and passed to 
> clone() accidentally chops off one entry, making a stack overflow using those 
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix 
> both the issue of the stack overflow issue as well as the SIGBUS crash. We 
> should also net better performance from having the stack aligned.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 530f1a55b 
>   src/linux/ns.hpp 77789717e 
> 
> Diff: https://reviews.apache.org/r/54996/diff/
> 
> 
> Testing
> ---
> 
> Built Mesos from source and am currently running it in a test cluster. 
> Launched both Docker and Mesos tasks via Marathon without any resulting crash 
> (initial crash only happened with Mesos containerizer + linux_launcher, not 
> with the posix_launcher).
> 
> `make check -j4` also passes with no errors.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.

[Aaron Wood]

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

(Updated Jan. 9, 2017, 4:19 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Currently in the Linux launcher when the stack is allocated and prepared for a 
call to clone() it is not properly aligned. This is not an issue for x86 or x64 
but for ARM64/AArch64 it is because of the requirement of having the stack 
aligned to a 16 byte boundary. While x86 and x64 also expect the stack to have 
a 16 byte aligned stack, it is not enforced. An explanation of the stack and 
requirements for ARM64 can be found here 
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf 
(specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be 
quad-word aligned.)

Additionally, the way that the stack is currently allocated and passed to 
clone() accidentally chops off one entry, making a stack overflow using those 
missing 8 bytes a possibility. Fixing this while aligning the memory will fix 
both the issue of the stack overflow issue as well as the SIGBUS crash. We 
should also net better performance from having the stack aligned.


Diffs
-

  3rdparty/stout/include/stout/os/linux.hpp 530f1a55b 
  src/linux/ns.hpp 77789717e 

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


Testing (updated)
---

Built Mesos from source and am currently running it in a test cluster. Launched 
both Docker and Mesos tasks via Marathon without any resulting crash (initial 
crash only happened with Mesos containerizer + linux_launcher, not with the 
posix_launcher).

`make check -j4` also passes with no errors.


Thanks,

Aaron Wood

Re: Review Request 54996: Fix SIGBUS crash on ARM64/AArch64.

[Aaron Wood]

> On Jan. 7, 2017, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp, line 72
> > 
> >
> > Please use errno error so that errno are in the error string.
> > 
> > `return ErrnoError("Memory allocation failed");`

`posix_memalign` never sets `errno`. Shouldn't we just return `Error`?


- Aaron


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


On Jan. 4, 2017, 9:55 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54996/
> ---
> 
> (Updated Jan. 4, 2017, 9:55 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6835
> https://issues.apache.org/jira/browse/MESOS-6835
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently in the Linux launcher when the stack is allocated and prepared for 
> a call to clone() it is not properly aligned. This is not an issue for x86 or 
> x64 but for ARM64/AArch64 it is because of the requirement of having the 
> stack aligned to a 16 byte boundary. While x86 and x64 also expect the stack 
> to have a 16 byte aligned stack, it is not enforced. An explanation of the 
> stack and requirements for ARM64 can be found here 
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
>  (specifically section 5.2.2.1 that says SP mod 16 = 0. The stack must be 
> quad-word aligned.)
> 
> Additionally, the way that the stack is currently allocated and passed to 
> clone() accidentally chops off one entry, making a stack overflow using those 
> missing 8 bytes a possibility. Fixing this while aligning the memory will fix 
> both the issue of the stack overflow issue as well as the SIGBUS crash. We 
> should also net better performance from having the stack aligned.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 530f1a55b 
>   src/linux/ns.hpp 77789717e 
> 
> Diff: https://reviews.apache.org/r/54996/diff/
> 
> 
> Testing
> ---
> 
> Built Mesos from source and am currently running it in a test cluster. 
> Launched both Docker and Mesos tasks via Marathon without any resulting crash 
> (initial crash only happened with Mesos containerizer + linux_launcher, not 
> with the posix_launcher).
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 55339: Removed unsupported 'friend' declaration.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [55339]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Jan. 9, 2017, 1:43 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55339/
> ---
> 
> (Updated Jan. 9, 2017, 1:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-6895
> https://issues.apache.org/jira/browse/MESOS-6895
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The friend class declaration of nested, templated classes
> will raise an error when compiling with Clang.
> This is resolved by making the constructor of 'ControlFlow' public.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/loop.hpp 
> 53f62439752a6eb7cf870022e4965c9261fc3ba6 
> 
> Diff: https://reviews.apache.org/r/55339/diff/
> 
> 
> Testing
> ---
> 
> make check with
> ```
> $ clang --version
> Apple LLVM version 8.0.0 (clang-800.0.42.1)
> Target: x86_64-apple-darwin16.3.0
> ```
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 52877: Fixed wrong float serialization in JSON in locales different from C.

[Alexander Rojas]

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

(Updated Jan. 9, 2017, 3:29 p.m.)


Review request for mesos, Adam B, Benjamin Bannier, Joris Van Remoortere, and 
Michael Park.


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


Repository: mesos


Description
---

Resolves an isse with the JSON serialization functions where floats
would use the process active locale as a decimal separator instead
of the JSON conformant period (`.`).


Diffs (updated)
-

  3rdparty/stout/include/stout/jsonify.hpp 
8220d001001e8b9f247c05be58534f1174611aad 

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


Testing
---

In order to test the different options the following benchmarking program was
written using [Google benchmark](https://github.com/google/benchmark):

```c++
#include 
#include 

#include 
#include 
#include 
#include 

static void BM_Jsonify(benchmark::State& state) {
  while (state.KeepRunning()) {
state.PauseTiming();

// Randomly generated set of numbers.
std::vector doubles = {
  54366462691.1798,
  3.35465250645312,
  3056184950.05953,
  74057564.7741182,
  280.445791893924,
  3446.63176368415,
  419012114.325581,
  3464212.51004162,
  1.45156507568354,
  13304750.4216248,
  7716457488648.00,
  700533630.650588,
  679.659801950981,
  307059152.688268,
  5615744.28063731,
  859.902033900705,
  293878.810967451,
  284685668.155903,
  722683811.462448,
  407.682284299325,
  9874834.88341080,
  4829649.14505646,
  3045544.72401146,
  1112707.08627010,
  8379539.79388719,
  3004161.89676627,
  382.79849617,
  3871151.73991937,
  4090119.26657417,
  4118699.88616345,
  2104416.18322520,
  9896898.63226234,
  5957851.08773457,
  3501068.52003430,
  7524714.36218293,
  4333874.01982647,
  9562008.06930384,
  3374957.45494027,
  5867075.07463260,
  815499.697450741,
  600936.470830026,
  9661425.72632153,
  6392256.71537575,
  7517969.33139398,
  9031912.03425553,
  5497593.85752735,
  815419.808032205,
  5098659.46057626,
  930160.667551563,
  5970944.98217500,
  6166534.92677787,
  3541537.67676275,
  1291933.13549156,
  9185094.72404290,
  4507338.03523123,
  9559754.89147696,
  6152898.39204769,
  2358966.41795446,
  6520510.92218183,
  2201757.78606032,
  4960487.80737309,
  1784969.91832029,
  3858390.23735070,
  1439952.27402359,
  6407199.91163080,
  6130379.95590661,
  6427890.23913244,
  2128879.29010338,
  8175291.42483598,
  1587278.27639235,
  7493343.68705307,
  4853439.25371342,
  2564845.15639735,
  1415661.63929173,
  6388168.79342734,
  3003424.90116775,
  6915390.10600792,
  7115390.65502377,
  5288515.90088063,
  1209208.86882085,
  4483111.91884606,
  5974377.97163572,
  5821054.89489766,
  8284136.84073623,
  1607044.34898051,
  3255087.31267763,
  2305369.43079747,
  1282392.57487249,
  4884797.49134467,
  5119420.62129117,
  6276725.07755672,
  3054999.92210194,
  3594116.58894982,
  6691568.49651968,
  265562.721872220,
  2864878.07276221,
  627299.902077148,
  5885179.44212665,
  7654144.98508934,
  590857.599685431
};

state.ResumeTiming();

benchmark::DoNotOptimize(jsonify(object));
  }
}

BENCHMARK(BM_Jsonify);

BENCHMARK_MAIN();
```

The program creates a JSON object with 50 floats in it, and then it creates
its string representation. Since the algorithms in `json.hpp` and `jsonify.hpp`
are similar but not the same, they were benchmarked as different. While the
original solutions are non solutions since they produce erroneous results if
localization is active, they constitute a baseline for comparison with the
alternatives. These are the benchmarks for the original algorithms and their
alternatives:

1. `json.hpp` default:

```c++
char buffer[50] {};
snprintf(
buffer,
sizeof(buffer),
"%#.*g",
std::numeric_limits::digits10,
number.value);

std::string trimmed = strings::trim(buffer, strings::SUFFIX, "0");
*stream_ << trimmed << (trimmed.back() == '.' ? "0" : "");
```

   Perfomance

```
Run on (8 X 2800 MHz CPU s)
2017-01-03 15:06:35
Benchmark   Time   CPU Iterations
-
BM_Jsonify985 ns986 ns 714227
```

2. `jsonify.hpp` default (it changes the call to `strings::trim` in order to
   avoid string copies:

```c++
char buffer[50] {};
const int size = snprintf(
buffer,
sizeof(buffer),
"%#.*g",
std::numeric_limits::digits10,
number.value);

int back 

Review Request 55339: Removed unsupported 'friend' declaration.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Michael Park.


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


Repository: mesos


Description
---

The friend class declaration of nested, templated classes
will raise an error when compiling with Clang.
This is resolved by making the constructor of 'ControlFlow' public.


Diffs
-

  3rdparty/libprocess/include/process/loop.hpp 
53f62439752a6eb7cf870022e4965c9261fc3ba6 

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


Testing
---

make check with
```
$ clang --version
Apple LLVM version 8.0.0 (clang-800.0.42.1)
Target: x86_64-apple-darwin16.3.0
```


Thanks,

Jan Schlicht

Re: Review Request 55319: Future::after memory leak fix.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Jan. 8, 2017, 8:45 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55319/
> ---
> 
> (Updated Jan. 8, 2017, 8:45 a.m.)
> 
> 
> Review request for mesos and Alexander Rojas.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Future::after memory leak fix.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 71171c6ab8cf9587734bb2da15b19296a5849e98 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> fbf69950efb3817f749851a8def73dd56afa3da1 
> 
> Diff: https://reviews.apache.org/r/55319/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 54825: Made sure process::Loop instances can only be created as shared_ptrs.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54825]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Jan. 9, 2017, 11:06 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54825/
> ---
> 
> (Updated Jan. 9, 2017, 11:06 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> process::Loop is a std::enable_shared_from_this. Using stack-allocated
> instances of classes derived from std::enable_shared_from_this is
> undefined behavior.
> 
> Prevent creation of stack-allocated process::Loop instances by hiding
> the constructors and providing a factory method to be used instead.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/loop.hpp 
> 53f62439752a6eb7cf870022e4965c9261fc3ba6 
> 
> Diff: https://reviews.apache.org/r/54825/diff/
> 
> 
> Testing
> ---
> 
> Observed only known test failures with:
> 
> * `make check` (OS X, clang-trunk, w/optimizations, SSL)
> * `make check` and `ROOT` tests on various Linux configurations in internal CI
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 55335: Backfilled required fields in TaskInfo in MesosContainerizer* tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [55334, 55335]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Jan. 9, 2017, 8:03 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55335/
> ---
> 
> (Updated Jan. 9, 2017, 8:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests were using a `TaskInfo` field which is missing required
> protobuf fields. After the dependeny patches begins to checkpoint
> `ContainerConfig`, the incomplete `TaskInfo` cannot be checkpointed,
> thus caused these tests to fail.
> 
> This patch backfills these fields to make these tests pass.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 039f197ece893fdf95c25fe60bb827fa2d7a3cbf 
> 
> Diff: https://reviews.apache.org/r/55335/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="MesosContainerizer*" make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 54825: Made sure process::Loop instances can only be created as shared_ptrs.

[Benjamin Bannier]

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

(Updated Jan. 9, 2017, 12:06 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

Rebased.


Repository: mesos


Description
---

process::Loop is a std::enable_shared_from_this. Using stack-allocated
instances of classes derived from std::enable_shared_from_this is
undefined behavior.

Prevent creation of stack-allocated process::Loop instances by hiding
the constructors and providing a factory method to be used instead.


Diffs (updated)
-

  3rdparty/libprocess/include/process/loop.hpp 
53f62439752a6eb7cf870022e4965c9261fc3ba6 

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


Testing
---

Observed only known test failures with:

* `make check` (OS X, clang-trunk, w/optimizations, SSL)
* `make check` and `ROOT` tests on various Linux configurations in internal CI


Thanks,

Benjamin Bannier

Re: Review Request 52382: Added stubs for OCI store.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Jan. 8, 2017, 11:32 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52382/
> ---
> 
> (Updated Jan. 8, 2017, 11:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stubs for OCI store.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c8d4260c03d8cdee1951a50d293e9fdabcd2cf84 
>   src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
>   src/slave/containerizer/mesos/provisioner/oci/store.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52382/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 55332: Added 'message.proto' for OCI store.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Jan. 8, 2017, 11:35 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55332/
> ---
> 
> (Updated Jan. 8, 2017, 11:35 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'message.proto' for OCI store.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c8d4260c03d8cdee1951a50d293e9fdabcd2cf84 
>   src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
>   src/slave/containerizer/mesos/provisioner/oci/message.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/message.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/55332/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 55331: Added 'OCI' message into 'Image' message.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Jan. 8, 2017, 11:33 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55331/
> ---
> 
> (Updated Jan. 8, 2017, 11:33 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'OCI' message into 'Image' message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ab68ff85c4af5d254779b30a7f27eda9fcb790eb 
>   include/mesos/v1/mesos.proto caefa239be6ead10b9a5fc91ba120ea9c8775313 
> 
> Diff: https://reviews.apache.org/r/55331/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 55333: Implemented the 'get()' method of OCI store.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52349, 55139, 55140, 52379, 54638, 52382, 54639, 55331, 
55332, 55333]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Jan. 9, 2017, 7:45 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55333/
> ---
> 
> (Updated Jan. 9, 2017, 7:45 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the 'get()' method of OCI store.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/oci/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/paths.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> a312ad953b406aa75506051593dcc1c27cdc93af 
> 
> Diff: https://reviews.apache.org/r/55333/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 54638: Added agent flags '--oci_discovery' and '--oci_discovery_prefix'.

[Gilbert Song]

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




src/slave/flags.hpp (lines 60 - 61)


As you may already observe, these two flags may be confusing. We may either 
find another name, or document them in the documentation patch later.



src/slave/flags.cpp (line 173)


What elas types we might have?


- Gilbert Song


On Jan. 3, 2017, 6:18 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54638/
> ---
> 
> (Updated Jan. 3, 2017, 6:18 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags '--oci_discovery' and '--oci_discovery_prefix'.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md e4beb2d5a72f1c5f59b2e40f4984cc60b8437d9d 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp 1eccea920338032173be4df6c374ec50dbd2eaf3 
> 
> Diff: https://reviews.apache.org/r/54638/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52379: Added agent flag '--oci_store_dir'.

[Gilbert Song]

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




src/slave/flags.cpp (line 174)


Why not consistent with docker/appc store?


- Gilbert Song


On Jan. 3, 2017, 6:17 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52379/
> ---
> 
> (Updated Jan. 3, 2017, 6:17 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flag '--oci_store_dir'.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md e4beb2d5a72f1c5f59b2e40f4984cc60b8437d9d 
>   docs/endpoints/slave/state.json.md 0f82c1926404e79b281b2ea5f4d0ca21323aeded 
>   docs/endpoints/slave/state.md b34459e8624f0b29e927ff79be7fc845ac88080b 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp 1eccea920338032173be4df6c374ec50dbd2eaf3 
>   src/slave/http.cpp b6e2d4f9190358d113b39140d116b8659ddbb49b 
> 
> Diff: https://reviews.apache.org/r/52379/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52349: Add protobuf messages for OCI image spec.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Jan. 8, 2017, 11:21 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52349/
> ---
> 
> (Updated Jan. 8, 2017, 11:21 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6681
> https://issues.apache.org/jira/browse/MESOS-6681
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add protobuf messages for OCI image spec.
> 
> 
> Diffs
> -
> 
>   include/mesos/oci/spec.hpp PRE-CREATION 
>   include/mesos/oci/spec.proto PRE-CREATION 
>   src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 
> 
> Diff: https://reviews.apache.org/r/52349/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 55335: Backfilled required fields in TaskInfo in MesosContainerizer* tests.

[Zhitao Li]

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

Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

These tests were using a `TaskInfo` field which is missing required
protobuf fields. After the dependeny patches begins to checkpoint
`ContainerConfig`, the incomplete `TaskInfo` cannot be checkpointed,
thus caused these tests to fail.

This patch backfills these fields to make these tests pass.


Diffs
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
039f197ece893fdf95c25fe60bb827fa2d7a3cbf 

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


Testing
---

GTEST_FILTER="MesosContainerizer*" make check


Thanks,

Zhitao Li

Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Zhitao Li]

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

Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

This patch includes the following changes:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
9fcad933e1ec3b52d4dc366ba80d282deea04c0b 
  src/slave/containerizer/mesos/containerizer.cpp 
d9d5619e45ae1199fc91878f17a33b5647f48305 
  src/slave/containerizer/mesos/paths.hpp 
d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
  src/slave/containerizer/mesos/paths.cpp 
c1770cefe0287ce994eec6979db41201148ef3fb 

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


Testing
---


Thanks,

Zhitao Li