Re: Review Request 44706: Implemented isolate() method of "network/cni" isolator.

[Qian Zhang]

> On March 21, 2016, 11:57 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 328-336
> > 
> >
> > why do we need this dispatch ? The dispatch is to itself, so seems a 
> > bit odd. Can we invoke the `subprocess` for the plugin in _connect directly 
> > ?
> > 
> > Basically not sure why we need connect & _connect.
> 
> Qian Zhang wrote:
> The idea is similar with how `MesosContainerizer` call each isolator: 
> https://github.com/apache/mesos/blob/0.28.0/src/slave/containerizer/mesos/containerizer.cpp#L1171:L1181
> 
> I'd like to handle each call to a CNI plugin in a separate libprocess 
> dispatch event, so that's why I call `connect` via `dispatch`.
> 
> Avinash sridharan wrote:
> In the example you gave the `isolator` does a dispatch on an isolator 
> process. So the `MesosContainerizer` effectively does a dispatch on a 
> separate libprocess thread, which is the intended behavior. Here it seems a 
> bit odd that dispatch is scheduling an event on itself. Functionally nothing 
> wrong, but this is not idiomatic and we should avoid this.

Actually, when I wrote these code, instead of using `dispatch()`, I was 
thinking to directly call `subprocess()` to invoke CNI plugin in the 
`foreachvalue(NetworkInfo& networkInfo, infos[containerId]->networkInfos)` 
loop, and then use `defer()` to checkpoint the output return by the CNI plugin. 
However, the problem with this approach is, say a container wants to join two 
CNI networks (e.g., net1 and net2), if `subprocess()` for net1 succeeds, but 
`subprocess()` for net2 fails for a reason (it will return an `Error`), in this 
case we need to directly return a `Failure` in `isolate()`, but the issue is no 
one will be responsible for checkpointing the output for net1 which will cause 
IP leak since when we cleanup this container, we will not call CNI plugin to 
disconnect the container from net1 due to no checkpointed data. That's why I'd 
like to handle each call to a CNI plugin in a separate libprocess dispatch 
event, then the failure of one call will not affect any other calls.


- Qian


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


On March 23, 2016, 10:22 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44706/
> ---
> 
> (Updated March 23, 2016, 10:22 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented isolate() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0bd7a978306488b687a7e2eeeb8a5c9766d43548 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3878a7e85fe24175b3bd5e3a6268cf32a07f2d8b 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 9552312f9ba1e4df6564cfb737cc41e041cf4407 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44706/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45166: Fixed flaky `MasterTest.SlavesEndpointTwoSlaves`.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 23, 2016, 8:14 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45166/
> ---
> 
> (Updated March 23, 2016, 8:14 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Neil Conway.
> 
> 
> Bugs: MESOS-4984
> https://issues.apache.org/jira/browse/MESOS-4984
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We were not correctly waiting for the master to register the first
> slave before making a call to the `/slaves` endpoint. There was this
> possible race:
> 
> - Slave1 is started.
> - Slave2 is started.
> - Slave2 sends register message to master.
> - Slave2 re-tries the register message.
> - Master registers slave2.
> - Master resends register acknowledgment to slave2.
> - The test thinks that both expectations i.e. `FUTURE_PROTOBUF`
> have completed.
> - Test makes call to `/slaves` endpoint and sees 1 slave has only registered. 
> The test fails.
> - Master registers slave1.
> 
> Reordering the `AWAIT_READY` calls to explicitly wait for
> `slave1` to register first should fix the flakiness.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp d34ba0bdd71efd261850d8c205c16cecb701ac7c 
> 
> Diff: https://reviews.apache.org/r/45166/diff/
> 
> 
> Testing
> ---
> 
> Ran it in a loop. Previously used to fail quite often.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 33174: Fix for docker not configuring CFS quotas correctly

[Jie Yu]

> On March 16, 2016, 11:08 p.m., Jie Yu wrote:
> > This patch breaks all the ROOT DOCKER tests in our internal CI. I've 
> > reverted it for now. Can you do a sudo make check with docker?
> 
> Steve Niemitz wrote:
> ok, I see what the problem is here.  The issue is with the 
> mesos-docker-executor code path (when launching a task w/ a TaskInfo, 
> launchExecutorProcess).  Because the code path only waits for the 
> mesos-docker-executor process to launch, the inspect in update() fails since 
> the docker container hasn't launched yet.
> 
> I'm not sure what the best solution is here, I'll play around with some 
> options.
> 
> Steve Niemitz wrote:
> Also, interestingly enough, if you remove the check in update() the 
> checks if the resources have changed, even without my patch it breaks because 
> the container isn't there yet, so this is even now just kind of accidentally 
> working.

Yeah, looks like the whole 'update' is problematic for the command task case. 
I'll think about that tomorrow to see if there is a clean solution. 
`docker->inspect` supports a retry interval (optional), basically, it'll wait 
until the container is up. But the problem with that is: if there is error in 
the executor, it'll wait forever. Maybe we could setup a timeout for that.


- Jie


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


On March 16, 2016, 8:10 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33174/
> ---
> 
> (Updated March 16, 2016, 8:10 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2617
> https://issues.apache.org/jira/browse/MESOS-2617
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix for docker containerizer not configuring CFS quotas correctly.
> 
> It would be nice to refactor all this isolation code in a way that can be 
> shared between all containerizers, as this is basically just copied from the 
> CgroupsCpushareIsolator, but that's a much bigger undertaking.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 79cd955e9c241becff52cc4bbef81dcc16802ee7 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
> 
> Diff: https://reviews.apache.org/r/33174/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 45265: Plugged in dvd isolator into agent.

[Guangya Liu]

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

(Updated 三月 24, 2016, 6:13 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Plugged in dvd isolator into agent.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
e7f7e7fd1304e14dbfaab8b53cea16efc0417911 
  src/slave/containerizer/mesos/isolators/docker/dvd.cpp PRE-CREATION 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 45259: Added Appc provisioner integration test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45259]

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

- Mesos ReviewBot


On March 24, 2016, 1:05 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45259/
> ---
> 
> (Updated March 24, 2016, 1:05 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4818
> https://issues.apache.org/jira/browse/MESOS-4818
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Appc provisioner integration test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 23a5b1059b4d9fde1e4a1aab5cd4fa6d05862332 
> 
> Diff: https://reviews.apache.org/r/45259/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45249: Added new '/files' endpoints tests using authentication.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44678, 44703, 44515, 44523, 44553, 44554, 45088, 45248, 45249]

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

- Mesos ReviewBot


On March 24, 2016, 1:05 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45249/
> ---
> 
> (Updated March 24, 2016, 1:05 a.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
> 
> 
> Bugs: MESOS-4956
> https://issues.apache.org/jira/browse/MESOS-4956
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New tests were added to the FilesTests, and existing tests were modified, to 
> probe the endpoints' behavior with and without HTTP authentication. Tests 
> were also added for `/flags/debug`, as there were no pre-existing tests for 
> that endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e 
> 
> Diff: https://reviews.apache.org/r/45249/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on OSX and CentOS 7.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 45272: Test update zookeeper version.

[haosdent huang]

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

Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test update zookeeper version.


Diffs
-

  3rdparty/Makefile.am 49aa55741d76aa88c8fbb526f18908312bb0c717 
  3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
  3rdparty/versions.am c2dae2fb521b12344b93bf771dd5497ba8d446c3 
  3rdparty/zookeeper-3.4.8.patch PRE-CREATION 
  src/examples/java/test-log.in 4c8547aaa115779ae7cec58edde01ab85feeb1b1 
  src/python/native_common/ext_modules.py.in 
c335bd83024bc07b6243dd59d775e7f29adc7520 
  src/tests/zookeeper_test_server.cpp 0dc041fef8973d35114b9f76a6a4002853884670 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45269: Test upload zookeeper tar.

[haosdent huang]

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

(Updated March 24, 2016, 4:02 a.m.)


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description (updated)
---

Test upload zookeeper tar.


Diffs (updated)
-

  3rdparty/zookeeper-3.4.5.tar.gz 1a547fe17a6fad86012f855d3c4cc38fed4899fc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45269: Test upload zookeeper tar.

[haosdent huang]

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

(Updated March 24, 2016, 3:56 a.m.)


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description (updated)
---

Test upload zookeeper tar. Only used for test and discussion.


Diffs
-

  3rdparty/zookeeper-3.4.5.tar.gz 1a547fe17a6fad86012f855d3c4cc38fed4899fc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45267: Fixed a memory leak in process::subprocess.

[Klaus Ma]

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




3rdparty/libprocess/src/subprocess.cpp (line 456)


Sugguest to add a NOTE that, the envp is environment->size() + 1, so the 
last NULL ptr is not handled (delete).


- Klaus Ma


On March 24, 2016, 11:41 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45267/
> ---
> 
> (Updated March 24, 2016, 11:41 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5021
> https://issues.apache.org/jira/browse/MESOS-5021
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 16c327db6fde1a1bac92d9e8f7cc80e57b028c1a 
> 
> Diff: https://reviews.apache.org/r/45267/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 45267: Fixed a memory leak in process::subprocess.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On March 24, 2016, 11:41 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45267/
> ---
> 
> (Updated March 24, 2016, 11:41 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5021
> https://issues.apache.org/jira/browse/MESOS-5021
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 16c327db6fde1a1bac92d9e8f7cc80e57b028c1a 
> 
> Diff: https://reviews.apache.org/r/45267/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 45267: Fixed a memory leak in process::subprocess.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 24, 2016, 3:41 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45267/
> ---
> 
> (Updated March 24, 2016, 3:41 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5021
> https://issues.apache.org/jira/browse/MESOS-5021
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 16c327db6fde1a1bac92d9e8f7cc80e57b028c1a 
> 
> Diff: https://reviews.apache.org/r/45267/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 44138: Windows:[1/2] Lifted socket API into Stout.

[Daniel Pravat]

> On March 23, 2016, 12:40 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/socket.hpp, line 
> > 35
> > 
> >
> > Should `EINPROGRESS` be in here?
> > would a retry not cause an `EALREADY`?
> 
> Daniel Pravat wrote:
> The error from the socket operations is dependent on the operation and 
> the environement conditions. ::connect will initialy return EINPROGRESS and 
> the subseqential call to connect will return EALREADY. Since the original 
> code was not handling EALREADY we may assume the connect is happening before 
> the second call (or we don't connect if takes too much time).

Based on offline discussion the test for EINPROGRESS after `::connect` has been 
extracted in its own method.


- Daniel


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


On March 24, 2016, 3:17 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44138/
> ---
> 
> (Updated March 24, 2016, 3:17 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows:[1/2] Lifted socket API into Stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/socket.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/socket.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/socket.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44138/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build/execute
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Review Request 45269: Test upload zookeeper tar.

[haosdent huang]

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

Review request for mesos.


Summary (updated)
-

Test upload zookeeper tar.


Repository: mesos


Description (updated)
---

Test upload zookeeper tar.


Diffs (updated)
-

  3rdparty/zookeeper-3.4.5.tar.gz 1a547fe17a6fad86012f855d3c4cc38fed4899fc 

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


Testing
---


Thanks,

haosdent huang

Review Request 45267: Fixed a memory leak in process::subprocess.

[Ben Mahler]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/src/subprocess.cpp 
16c327db6fde1a1bac92d9e8f7cc80e57b028c1a 

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


Testing
---

make check


Thanks,

Ben Mahler

Re: Review Request 44138: Windows:[1/2] Lifted socket API into Stout.

[Daniel Pravat]

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

(Updated March 24, 2016, 3:17 a.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
Clemmer, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Windows:[1/2] Lifted socket API into Stout.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/socket.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/socket.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/socket.hpp 
PRE-CREATION 

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


Testing
---

OSX: make check
Windows: build/execute


Thanks,

Daniel Pravat

Re: Review Request 44670: Added master_detector and master_contender flags.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44287, 44288, 44543, 44544, 44545, 44546, 44547, 44289, 
44669, 44670]

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

- Mesos ReviewBot


On March 23, 2016, 11:06 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44670/
> ---
> 
> (Updated March 23, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master_detector and master_contender flags allow modules to be
> used for specifying the MasterContender and MasterDetector
> implementations to use.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp f8d2cc4c6c8dab00e34ca737dbcb5b9ca3870d6d 
>   src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 
>   src/master/main.cpp 61210d9f275d4073967c3468179307cf09e88551 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp b77afa956834bb5b1f85301d7a5f386ab9da41e3 
>   src/slave/main.cpp 33a1af84aeb079224b15e92caf97bcf081ea4646 
> 
> Diff: https://reviews.apache.org/r/44670/diff/
> 
> 
> Testing
> ---
> 
> In addition to all unit tests passing, we are currently using this 
> functionality in our environment with a custom consensus stack. In our world, 
> we have a C++ plugin that calls out to an HTTP REST service (implemented in 
> Java/Scala, not that it matters).
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Review Request 45266: Add zookeeper-3.4.8.tar.gz.

[haosdent huang]

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

Review request for mesos.


Repository: mesos


Description
---

Add zookeeper-3.4.8.tar.gz.


Diffs
-

  3rdparty/zookeeper-3.4.8.tar.gz PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Review Request 45265: Plugged in dvd isolator into agent.

[Guangya Liu]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Plugged in dvd isolator into agent.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
e7f7e7fd1304e14dbfaab8b53cea16efc0417911 
  src/slave/containerizer/mesos/isolators/docker/dvd.cpp PRE-CREATION 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Qian Zhang]

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

(Updated March 24, 2016, 10:34 a.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented prepare() method of "network/cni" isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
3878a7e85fe24175b3bd5e3a6268cf32a07f2d8b 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
9552312f9ba1e4df6564cfb737cc41e041cf4407 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 44661: Deprecated the `docker_stop_timeout` flag.

[Ben Mahler]

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


Fix it, then Ship it!




Thanks!


CHANGELOG (line 6)


this is in another review, but s/log/long/



CHANGELOG (line 7)


this is in another review, but s/htting/hitting/



CHANGELOG (lines 8 - 9)


It is the executor's responsiblitity to enforce kill policies


- Ben Mahler


On March 23, 2016, 11:26 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44661/
> ---
> 
> (Updated March 23, 2016, 11:26 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-4910
> https://issues.apache.org/jira/browse/MESOS-4910
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead, a combination of `executor_shutdown_grace_period`
> agent flag and task kill policies should be used.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1fadaa1819c9b1a801752b1c0ddd572b82e6c170 
>   docs/configuration.md 73ee8fa7a77a6ceaf44e1f4c54ab4027a7109258 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
>   src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
>   src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 
> 
> Diff: https://reviews.apache.org/r/44661/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44660: Used `KillPolicy` and shutdown grace period in docker executor.

[Ben Mahler]

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


Fix it, then Ship it!





include/mesos/mesos.proto (lines 359 - 360)


`s/-t/--time/` here and elsewhere?



src/docker/executor.cpp (lines 200 - 205)


This is for backwards compatibility, right? I wouldn't expect the shutdown 
timeout to impact the kill timeout since no shutdown is occuring, so that might 
surprise others and its worth saying why we do this.


- Ben Mahler


On March 23, 2016, 11:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44660/
> ---
> 
> (Updated March 23, 2016, 11:25 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The docker executor determines how much time it allots the
> underlying container to clean up (via passing the timeout to
> the docker daemon) based on both optional task's `KillPolicy`
> and optional `shutdown_grace_period` field in `ExecutorInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
>   include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 
>   src/docker/executor.cpp afc769d0887e3842106e4c350e94c95c8ffc085e 
> 
> Diff: https://reviews.apache.org/r/44660/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 45217: Implemented docker volume driver isolator interface.

[Guangya Liu]

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

(Updated 三月 24, 2016, 2:25 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Implemented docker volume driver isolator interface.


Diffs (updated)
-

  src/CMakeLists.txt 0bd7a978306488b687a7e2eeeb8a5c9766d43548 
  src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
  src/slave/containerizer/mesos/isolators/docker/dvd.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/dvd.cpp PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45000: MESOS-3902: [Updated] Fix location header in redirect from non-leader.

[Ashwin Murthy]

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

(Updated March 24, 2016, 2:24 a.m.)


Review request for mesos and Vinod Kone.


Summary (updated)
-

MESOS-3902: [Updated] Fix location header in redirect from non-leader.


Repository: mesos


Description (updated)
---

MESOS-3902: [Final] Fix location header in redirect from non-leader. Fixed to 
append the path from the original URL.


Diffs (updated)
-

  src/master/http.cpp 97e4b0ce3286540788e3e6c5b484687b83e000f6 

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


Testing (updated)
---

This has been tested end to end with 3 local mesos masters and zk. Valided the 
location header is now correct.


Thanks,

Ashwin Murthy

Re: Review Request 45214: Updated protobuf to support external storage.

[Guangya Liu]

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

(Updated 三月 24, 2016, 2:24 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Updated protobuf to support external storage.


Diffs (updated)
-

  include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
  include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 44994: Added a test for executor shutdown grace period.

[Ben Mahler]

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


Fix it, then Ship it!





src/tests/slave_tests.cpp (line 3267)


s/TimedOut/Timeout/



src/tests/slave_tests.cpp (lines 3308 - 3309)


AWAIT_EXPECT_EQ


- Ben Mahler


On March 23, 2016, 11:22 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44994/
> ---
> 
> (Updated March 23, 2016, 11:22 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4949
> https://issues.apache.org/jira/browse/MESOS-4949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 
> 
> Diff: https://reviews.apache.org/r/44994/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `make check`
> `GTEST_FILTER="*ExecutorShutdownGracePeriod*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 45000: MESOS-3902: Fix in location header during redirect from non-leader.

[Ashwin Murthy]

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

(Updated March 24, 2016, 2:19 a.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

MESOS-3902: [Updated] Fix location header in redirect from non-leader. 
Addressed comments from Vinod and Ben. Removed the scheme but added the path 
from the original request to the location header. Preserved the link to the RFC 
in the comments.


Diffs
-

  src/master/http.cpp bfea8fa811c24d37b2d7a8109728e1f40217e02f 

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


Testing (updated)
---

This has been tested end to end


Thanks,

Ashwin Murthy

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Qian Zhang]

> On March 24, 2016, 5:04 a.m., Cong Wang wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 228
> > 
> >
> > You always have ifIndex==0 here, it is never changed. So either you 
> > miss some code or it is not needed at all.

Thanks for catching this! Will fix it soon.


- Qian


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


On March 23, 2016, 1:10 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 23, 2016, 1:10 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3878a7e85fe24175b3bd5e3a6268cf32a07f2d8b 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 9552312f9ba1e4df6564cfb737cc41e041cf4407 
> 
> Diff: https://reviews.apache.org/r/44514/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45067: Updated the long-lived-framework example.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45067]

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

- Mesos ReviewBot


On March 23, 2016, 10:50 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45067/
> ---
> 
> (Updated March 23, 2016, 10:50 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `long-lived-framework` enough options to run outside 
> of the build environment.
> 
> This also updates:
> 
> * The style of the framework code.
> * Gives the `ExecutorInfo` some resources (needed for some cgroups isolators).
> * Restricts the framework to one agent.  Otherwise, it would grab a small 
> chunk of every machine in the cluster.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/45067/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran this on the master node on a Mesos cluster:
> ```
> ./long-lived-framework --master=zk://localhost:2181/mesos 
> --executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; 
> --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && 
> ./long-lived-executor"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45186: Implemented user specified system config files support.

[Guangya Liu]

> On 三月 23, 2016, 6:51 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, lines 231-233
> > 
> >
> > Put this check to the block of 
> > if (flags.system_config_files.isSome()) {
> > } ?
> 
> Gilbert Song wrote:
> It can be a case that any of the four default sys config files missing on 
> the host for some os env, so we check all files which will be bind mounted 
> here.

Just curious for which system/os does the four default sys config files will 
not exist?


- Guangya


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


On 三月 23, 2016, 12:20 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45186/
> ---
> 
> (Updated 三月 23, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3003
> https://issues.apache.org/jira/browse/MESOS-3003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented user specified system config files support.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> f97a9a92895387a9d504810a2ae971cfb5d3dbb4 
> 
> Diff: https://reviews.apache.org/r/45186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> tested manually with mesos-execute to cat /etc/resolv.conf from an alpine 
> image.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45186: Implemented user specified system config files support.

[Guangya Liu]

> On 三月 23, 2016, 6:46 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, lines 204-205
> > 
> >
> > what about:
> > 
> > foreach (
> > const string& file,
> > strings::split(flags.system_config_files.get(), ",")) {
> 
> Gilbert Song wrote:
> Thanks for mentioning this. We are currently supporting both formats. We 
> may prefer the original one here since that format is used around this part 
> of codes.

Yes, but we did have patches 
https://reviews.apache.org/r/44653/diff/5#index_header to address such issue, 
so it would be great if we can follow this format ;-)


- Guangya


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


On 三月 23, 2016, 12:20 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45186/
> ---
> 
> (Updated 三月 23, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3003
> https://issues.apache.org/jira/browse/MESOS-3003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented user specified system config files support.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> f97a9a92895387a9d504810a2ae971cfb5d3dbb4 
> 
> Diff: https://reviews.apache.org/r/45186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> tested manually with mesos-execute to cat /etc/resolv.conf from an alpine 
> image.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44660: Used `KillPolicy` and shutdown grace period in docker executor.

[Ben Mahler]

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



Sorry I forgot to publish some stale comments from before, these are partial 
and may be out-of-date but figured you may find them useful.


include/mesos/mesos.proto (line 360)


For docker executor-less tasks?



include/mesos/mesos.proto (line 361)


To be more helpful s/-t/--time/



include/mesos/mesos.proto (line 1247)


ditto here, the user doesn't know about the docker executor



include/mesos/v1/mesos.proto (lines 358 - 1244)


Ditto here.



include/mesos/v1/mesos.proto (line 361)


To be more helpful s/-t/--time/



src/docker/executor.cpp (lines 216 - 242)


It's not immediately clear to me why the approach taken here is different 
from the approach taken in the command executor:

In the command executor, the kill policy overrides the shutdown grace 
period. In the docker executor, we still take the minimum of the kill policy 
and the shutdown grace period.

After thinking about it, I assume it's because you want to respect the 
--docker_stop_timeout flag when it's set to a smaller value than the kill 
policy? If so, please document why we have the minimum logic, because in the 
command executor there is an assumption that we never need to take a minimum of 
the shutdown grace period and the kill policy.



src/docker/executor.cpp (line 222)


Let's say --time to be more clear than -t



src/docker/executor.cpp (lines 236 - 239)


I added this TODO for killTask, can you help by clarifying that the 
shutdown arrives after a kill task?



src/docker/executor.cpp (line 630)


It doesn't crash it just exits :)



src/docker/executor.cpp (line 631)


removing flags is also difficult :)


- Ben Mahler


On March 23, 2016, 11:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44660/
> ---
> 
> (Updated March 23, 2016, 11:25 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The docker executor determines how much time it allots the
> underlying container to clean up (via passing the timeout to
> the docker daemon) based on both optional task's `KillPolicy`
> and optional `shutdown_grace_period` field in `ExecutorInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
>   include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 
>   src/docker/executor.cpp afc769d0887e3842106e4c350e94c95c8ffc085e 
> 
> Diff: https://reviews.apache.org/r/44660/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 45000: MESOS-3902: Fix in location header during redirect from non-leader.

[Ashwin Murthy]

> On March 20, 2016, 11:43 p.m., Vinod Kone wrote:
> > Can you confirm that you did a manual test of this (using curl and multiple 
> > masters w/ ZK)?

Vinod, validated the fix. Here is the o/p from testing with 3 local mesos 
masters and zookeeper

$ curl -v -X POST -H "Content-Type: application/json" --data @body.json 
http://localhost:5050/api/v1/scheduler
*   Trying ::1...
* connect to ::1 port 5050 failed: Connection refused
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 5050 (#0)
> POST /api/v1/scheduler HTTP/1.1
> Host: localhost:5050
> User-Agent: curl/7.43.0
> Accept: */*
> Content-Type: application/json
> Content-Length: 120
> 
* upload completely sent off: 120 out of 120 bytes
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 24 Mar 2016 02:01:08 GMT
< Location: //localhost:5052/master/api/v1/scheduler    On March 20, 2016, 11:43 p.m., Vinod Kone wrote:
> > src/master/http.cpp, lines 970-972
> > 
> >
> > looks like this fits in one line, why the wrapping?
> > 
> > ```
> >   return TemporaryRedirect(
> >   "//" + hostname.get() + ":" + stringify(info.port()) + 
> > request.url.path);
> > ```

I guess I can make it fit


- Ashwin


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


On March 20, 2016, 11:03 p.m., Ashwin Murthy wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45000/
> ---
> 
> (Updated March 20, 2016, 11:03 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3902: [Updated] Fix location header in redirect from non-leader. 
> Addressed comments from Vinod and Ben. Removed the scheme but added the path 
> from the original request to the location header. Preserved the link to the 
> RFC in the comments.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp bfea8fa811c24d37b2d7a8109728e1f40217e02f 
> 
> Diff: https://reviews.apache.org/r/45000/diff/
> 
> 
> Testing
> ---
> 
> Was trying to write unit test in scheduler http api test but it turns out 
> multi master tests cannot be written at this point. Need to manually verify 
> this by setting up multiple masters with ZK.
> 
> 
> Thanks,
> 
> Ashwin Murthy
> 
>

Re: Review Request 44378: Upgrade libev to 4.22 to support PowerPC LE platform.

[Zhiwei Chen]

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

(Updated March 24, 2016, 10:02 a.m.)


Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
and Vinod Kone.


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


Repository: mesos


Description
---

Upgrade libev to 4.22 to support PowerPC LE platform.


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
  3rdparty/libprocess/3rdparty/CMakeLists.txt 
6b07aefc58a1daa235b35e83832e47d1878e2e94 
  3rdparty/libprocess/3rdparty/libev-4.15.patch 
bbd83e6928e6caba3bc5a9739823d60923cfaa2a 
  3rdparty/libprocess/3rdparty/libev-4.15.tar.gz 
4c282b573aa9331fd16197ef286faf323b6515eb 
  3rdparty/libprocess/3rdparty/libev-4.22.patch PRE-CREATION 
  3rdparty/libprocess/3rdparty/libev-4.22.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
98195b8eb60b2673d610d8ab7ea31103f137debf 
  LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
  src/python/native_common/ext_modules.py.in 
c335bd83024bc07b6243dd59d775e7f29adc7520 

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


Testing
---

sudo make dist check

sudo ./src/mesos-tests --benchmark

sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build


Thanks,

Zhiwei Chen

Re: Review Request 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

[fan du]

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

(Updated 三月 24, 2016, 1:54 a.m.)


Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.


Changes
---

Fix coding sytle by adding blank line.


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


Repository: mesos


Description
---

Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
  src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
  src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 

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


Testing
---

ChangLog:

v3:
  - Move the couting out of common code path to http endpoint and master accept 
call separately to reflect its logic.

v2:
  - Documenting those metrics
  - Add test code for MetricsTest as suggested by Guangya
  - post-review.py does not update original 
RR(https://reviews.apache.org/r/44058/), but only create a new one even if I 
rebased.


Tests:
1. make check GTEST_FILTER="MetricsTest.Master" on Centos-7 
(3.10.0-123.el7.x86_640)

[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from MetricsTest
[ RUN  ] MetricsTest.Master
[   OK ] MetricsTest.Master (211 ms)
[--] 1 test from MetricsTest (211 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (244 ms total)
[  PASSED  ] 1 test

2. Verify its functionality with 'reserve' http endpoint as an test case

# curl http://ipdc02-kvm-guest2:5050/metrics/snapshot | python -mjson.tool | 
grep reserve
"master/messages_reserve_resource": 0.0,
"master/messages_unreserve_resource": 0.0,


# curl -i -d slaveId=6250553a-2f39-4a92-9073-4618d130f433-S1  -d resources='[ { 
"name": "cpus", "type": "SCALAR","scalar": { "value": 1 
},"reservation":{"principal": "XiaoHaHa"}}  ]' -X POST  
ipdc02-kvm-guest2:5050/master/reserve
HTTP/1.1 200 OK
Date: Fri, 26 Feb 2016 19:59:01 GMT
Content-Length: 0

# curl http://ipdc02-kvm-guest2:5050/metrics/snapshot  | python -mjson.tool | 
grep reserve
"master/messages_reserve_resource": 1.0,
"master/messages_unreserve_resource": 0.0,


Thanks,

fan du

Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.

[Jojy Varghese]

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

(Updated March 24, 2016, 1:34 a.m.)


Review request for mesos, Jie Yu and Neil Conway.


Changes
---

review addressed.


Repository: mesos


Description
---

According to the documentation for fts_open, either FTS_PHYSICAL or FTS_LOGICAL
SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want
to resolve the symlink targets while deleting them.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
cbc97596cd8ed1e6d4261568fd0086c86e063232 
  3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
5bd154e40c6b45e74967b620141cc373bdee 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 44257: Upgrade protobuf to 2.6.1 to support PowerPC LE platform.

[Zhiwei Chen]

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

(Updated March 24, 2016, 9:28 a.m.)


Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
and Vinod Kone.


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


Repository: mesos


Description
---

Upgrade protobuf to 2.6.1 to support PowerPC LE platform.


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
  3rdparty/libprocess/3rdparty/protobuf-2.5.0.tar.gz 
e600ac57be4c88efb5f146e4b3ec226d8f685033 
  3rdparty/libprocess/3rdparty/protobuf-2.6.1.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/README.md 
c534835db7baca1138791f2c700e95ff73052d85 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 
3d1f13082a65f9b1694ee7c65ba0cec131c18c5a 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 
fb11b1147b3a1872f60e90d0691723f9b2985427 
  3rdparty/libprocess/3rdparty/versions.am 
98195b8eb60b2673d610d8ab7ea31103f137debf 
  LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
  configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 
  src/java/mesos.pom.in 7615d61eb6fedfa0ead785cd360946c56ccf80af 
  src/python/interface/setup.py.in d73996734c3a3c70c3a6c0c697bb6733c241c091 
  src/python/native_common/ext_modules.py.in 
c335bd83024bc07b6243dd59d775e7f29adc7520 
  src/python/protocol/setup.py.in 4c50fbbf1ce11c4c42c848364523225ee7ea5a3b 

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


Testing
---

sudo make dist check

sudo ./src/mesos-tests --benchmark

sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build


Thanks,

Zhiwei Chen

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support PowerPC LE platform.

[haosdent huang]

> On March 22, 2016, 2:35 a.m., haosdent huang wrote:
> > Hi, zhiwei. Thank you for your patch. But seems it still doesn't contain 
> > the binary file correctly after I apply this patch.
> > 
> > ```
> > diff --git a/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz 
> > b/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz
> > new file mode 100644
> > index 
> > ..06528d5a456d6aa8dcee18159392d2ed4e5f3866
> > Binary files /dev/null and 
> > b/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz differ
> > ```
> 
> Zhiwei Chen wrote:
> Yes, I told Vinod in another patch, I think he can solve this issue. 
> Thanks.
> 
> Vinod Kone wrote:
> Looks like you haven't uploaded the tar.gz files? Can you upload them 
> manually to reviewboard if support/post-reviews.py or rbt doesn't support 
> binary file uploads?
> 
> haosdent huang wrote:
> Hi, @vinodkone As I metioned to zhiwei before, he need apply this patch 
> to his rbtools if he want upload the binary files. 
> https://reviews.apache.org/r/40053/diff/3/ is a example that could upload 
> binary file sucessfully. :-)
> 
> Zhiwei Chen wrote:
> Thanks, I thought the Mesos committer may have a better solution for this.
> 
> Joseph Wu wrote:
> You can also just generate the diff via:
> ```
> git diff --binary 
> ```
> and upload that diff via the web UI.
> 
> Zhiwei Chen wrote:
> Thanks Joseph, this is very helpful.

Actually I ask rbtools maintainers about the binary support in review boards 
before. They said would add a setting item `GIT_DIFF_INCLUDES_BINARIES` or 
`GIT_DIFF_EXTRA_FLAGS` in the future, but they have not yet finished it. Hope 
we would not suck on binary patches after they done.


- haosdent


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


On March 24, 2016, 1:13 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44372/
> ---
> 
> (Updated March 24, 2016, 1:13 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4805
> https://issues.apache.org/jira/browse/MESOS-4805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade http-parser to 2.6.1 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 6b07aefc58a1daa235b35e83832e47d1878e2e94 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> ddf7e3d9bf76d4a03c33f02d52ec29812aef8509 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch 
> f9fac12437a6bedc66353fda1ce9c0d7a383225a 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz 
> b811b63ce0ad6d71d9d296fed76656c023c76fc5 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   3rdparty/libprocess/Makefile.am ac8cc8d29baccf6e3a17367540ddd1f28585ef6d 
>   3rdparty/libprocess/src/decoder.hpp 
> a20b5ba8fc50d834573d253948645cc863f030dd 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
> 
> Diff: https://reviews.apache.org/r/44372/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Jojy Varghese]

> On March 20, 2016, 2:33 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > This comment seems incorrect:
> > 
> > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as 
> > well as `FTS_COMFOLLOW`.
> > 
> > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for 
> > `FTS_SLNONE` to be returned?
> 
> Jojy Varghese wrote:
> I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for 
> FTS_SLNONE to be returned.
> 
> Jie Yu wrote:
> Can you do a simple test to validate that (see if FTS_SLNONE is hit or 
> not)? My understanding is that FTS_SLNONE will still be hit if we're using 
> FTS_PHYSICAL.
> 
> Cong Wang wrote:
> Looking at 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901
>  I think Jojy is correct here, I don't see FTS_SLNONE could be set without 
> FTS_LOGICAL or FTS_COMFOLLOW.
> 
> Jojy Varghese wrote:
> @jie. I tested it already and verified that FTS_SLNONE  is not hit (using 
> logs).
> 
> Neil Conway wrote:
> Can we add a unit test anyway, please? This is corner-case behavior that 
> we should be validating we handle correctly, regardless of whether 
> `FTS_SLNONE` is actually set.
> 
> Jojy Varghese wrote:
> I have added unit tests in patch https://reviews.apache.org/r/45002/. 
> Would love to hear more test ideas.
> 
> Neil Conway wrote:
> I'd like a test case that covers `rmdir` of a symbolic link that points 
> to a non-existent file/directory; as far as I can see, the existing tests do 
> not cover this case.

What do you think about the test case `RemoveDirectoryWithNoTargetSymbolicLink` 
in the test file?


- Jojy


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


On March 24, 2016, 1:10 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 24, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support PowerPC LE platform.

[Zhiwei Chen]

> On March 22, 2016, 10:35 a.m., haosdent huang wrote:
> > Hi, zhiwei. Thank you for your patch. But seems it still doesn't contain 
> > the binary file correctly after I apply this patch.
> > 
> > ```
> > diff --git a/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz 
> > b/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz
> > new file mode 100644
> > index 
> > ..06528d5a456d6aa8dcee18159392d2ed4e5f3866
> > Binary files /dev/null and 
> > b/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz differ
> > ```
> 
> Zhiwei Chen wrote:
> Yes, I told Vinod in another patch, I think he can solve this issue. 
> Thanks.
> 
> Vinod Kone wrote:
> Looks like you haven't uploaded the tar.gz files? Can you upload them 
> manually to reviewboard if support/post-reviews.py or rbt doesn't support 
> binary file uploads?
> 
> haosdent huang wrote:
> Hi, @vinodkone As I metioned to zhiwei before, he need apply this patch 
> to his rbtools if he want upload the binary files. 
> https://reviews.apache.org/r/40053/diff/3/ is a example that could upload 
> binary file sucessfully. :-)
> 
> Zhiwei Chen wrote:
> Thanks, I thought the Mesos committer may have a better solution for this.
> 
> Joseph Wu wrote:
> You can also just generate the diff via:
> ```
> git diff --binary 
> ```
> and upload that diff via the web UI.

Thanks Joseph, this is very helpful.


- Zhiwei


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


On March 24, 2016, 9:13 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44372/
> ---
> 
> (Updated March 24, 2016, 9:13 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4805
> https://issues.apache.org/jira/browse/MESOS-4805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade http-parser to 2.6.1 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 6b07aefc58a1daa235b35e83832e47d1878e2e94 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> ddf7e3d9bf76d4a03c33f02d52ec29812aef8509 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch 
> f9fac12437a6bedc66353fda1ce9c0d7a383225a 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz 
> b811b63ce0ad6d71d9d296fed76656c023c76fc5 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   3rdparty/libprocess/Makefile.am ac8cc8d29baccf6e3a17367540ddd1f28585ef6d 
>   3rdparty/libprocess/src/decoder.hpp 
> a20b5ba8fc50d834573d253948645cc863f030dd 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
> 
> Diff: https://reviews.apache.org/r/44372/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support PowerPC LE platform.

[Joseph Wu]

> On March 21, 2016, 7:35 p.m., haosdent huang wrote:
> > Hi, zhiwei. Thank you for your patch. But seems it still doesn't contain 
> > the binary file correctly after I apply this patch.
> > 
> > ```
> > diff --git a/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz 
> > b/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz
> > new file mode 100644
> > index 
> > ..06528d5a456d6aa8dcee18159392d2ed4e5f3866
> > Binary files /dev/null and 
> > b/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz differ
> > ```
> 
> Zhiwei Chen wrote:
> Yes, I told Vinod in another patch, I think he can solve this issue. 
> Thanks.
> 
> Vinod Kone wrote:
> Looks like you haven't uploaded the tar.gz files? Can you upload them 
> manually to reviewboard if support/post-reviews.py or rbt doesn't support 
> binary file uploads?
> 
> haosdent huang wrote:
> Hi, @vinodkone As I metioned to zhiwei before, he need apply this patch 
> to his rbtools if he want upload the binary files. 
> https://reviews.apache.org/r/40053/diff/3/ is a example that could upload 
> binary file sucessfully. :-)
> 
> Zhiwei Chen wrote:
> Thanks, I thought the Mesos committer may have a better solution for this.

You can also just generate the diff via:
```
git diff --binary 
```
and upload that diff via the web UI.


- Joseph


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


On March 23, 2016, 6:13 p.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44372/
> ---
> 
> (Updated March 23, 2016, 6:13 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4805
> https://issues.apache.org/jira/browse/MESOS-4805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade http-parser to 2.6.1 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 6b07aefc58a1daa235b35e83832e47d1878e2e94 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> ddf7e3d9bf76d4a03c33f02d52ec29812aef8509 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch 
> f9fac12437a6bedc66353fda1ce9c0d7a383225a 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz 
> b811b63ce0ad6d71d9d296fed76656c023c76fc5 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   3rdparty/libprocess/Makefile.am ac8cc8d29baccf6e3a17367540ddd1f28585ef6d 
>   3rdparty/libprocess/src/decoder.hpp 
> a20b5ba8fc50d834573d253948645cc863f030dd 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
> 
> Diff: https://reviews.apache.org/r/44372/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support PowerPC LE platform.

[Zhiwei Chen]

> On March 22, 2016, 10:35 a.m., haosdent huang wrote:
> > Hi, zhiwei. Thank you for your patch. But seems it still doesn't contain 
> > the binary file correctly after I apply this patch.
> > 
> > ```
> > diff --git a/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz 
> > b/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz
> > new file mode 100644
> > index 
> > ..06528d5a456d6aa8dcee18159392d2ed4e5f3866
> > Binary files /dev/null and 
> > b/3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz differ
> > ```
> 
> Zhiwei Chen wrote:
> Yes, I told Vinod in another patch, I think he can solve this issue. 
> Thanks.
> 
> Vinod Kone wrote:
> Looks like you haven't uploaded the tar.gz files? Can you upload them 
> manually to reviewboard if support/post-reviews.py or rbt doesn't support 
> binary file uploads?
> 
> haosdent huang wrote:
> Hi, @vinodkone As I metioned to zhiwei before, he need apply this patch 
> to his rbtools if he want upload the binary files. 
> https://reviews.apache.org/r/40053/diff/3/ is a example that could upload 
> binary file sucessfully. :-)

Thanks, I thought the Mesos committer may have a better solution for this.


- Zhiwei


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


On March 24, 2016, 9:13 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44372/
> ---
> 
> (Updated March 24, 2016, 9:13 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4805
> https://issues.apache.org/jira/browse/MESOS-4805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade http-parser to 2.6.1 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 6b07aefc58a1daa235b35e83832e47d1878e2e94 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> ddf7e3d9bf76d4a03c33f02d52ec29812aef8509 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch 
> f9fac12437a6bedc66353fda1ce9c0d7a383225a 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz 
> b811b63ce0ad6d71d9d296fed76656c023c76fc5 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   3rdparty/libprocess/Makefile.am ac8cc8d29baccf6e3a17367540ddd1f28585ef6d 
>   3rdparty/libprocess/src/decoder.hpp 
> a20b5ba8fc50d834573d253948645cc863f030dd 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
> 
> Diff: https://reviews.apache.org/r/44372/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Neil Conway]

> On March 20, 2016, 2:33 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > This comment seems incorrect:
> > 
> > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as 
> > well as `FTS_COMFOLLOW`.
> > 
> > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for 
> > `FTS_SLNONE` to be returned?
> 
> Jojy Varghese wrote:
> I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for 
> FTS_SLNONE to be returned.
> 
> Jie Yu wrote:
> Can you do a simple test to validate that (see if FTS_SLNONE is hit or 
> not)? My understanding is that FTS_SLNONE will still be hit if we're using 
> FTS_PHYSICAL.
> 
> Cong Wang wrote:
> Looking at 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901
>  I think Jojy is correct here, I don't see FTS_SLNONE could be set without 
> FTS_LOGICAL or FTS_COMFOLLOW.
> 
> Jojy Varghese wrote:
> @jie. I tested it already and verified that FTS_SLNONE  is not hit (using 
> logs).
> 
> Neil Conway wrote:
> Can we add a unit test anyway, please? This is corner-case behavior that 
> we should be validating we handle correctly, regardless of whether 
> `FTS_SLNONE` is actually set.
> 
> Jojy Varghese wrote:
> I have added unit tests in patch https://reviews.apache.org/r/45002/. 
> Would love to hear more test ideas.

I'd like a test case that covers `rmdir` of a symbolic link that points to a 
non-existent file/directory; as far as I can see, the existing tests do not 
cover this case.


- Neil


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


On March 24, 2016, 1:10 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 24, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.

[Neil Conway]

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




3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp (line 268)


`os::touch` would be a bit more readable, I think.


- Neil Conway


On March 24, 2016, 1:07 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45002/
> ---
> 
> (Updated March 24, 2016, 1:07 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to the documentation for fts_open, either FTS_PHYSICAL or 
> FTS_LOGICAL
> SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want
> to resolve the symlink targets while deleting them.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 5bd154e40c6b45e74967b620141cc373bdee 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 23a5b1059b4d9fde1e4a1aab5cd4fa6d05862332 
> 
> Diff: https://reviews.apache.org/r/45002/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support PowerPC LE platform.

[Zhiwei Chen]

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

(Updated March 24, 2016, 9:13 a.m.)


Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
and Vinod Kone.


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


Repository: mesos


Description
---

Upgrade http-parser to 2.6.1 to support PowerPC LE platform.


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
  3rdparty/libprocess/3rdparty/CMakeLists.txt 
6b07aefc58a1daa235b35e83832e47d1878e2e94 
  3rdparty/libprocess/3rdparty/Makefile.am 
ddf7e3d9bf76d4a03c33f02d52ec29812aef8509 
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch 
f9fac12437a6bedc66353fda1ce9c0d7a383225a 
  3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz 
b811b63ce0ad6d71d9d296fed76656c023c76fc5 
  3rdparty/libprocess/3rdparty/versions.am 
98195b8eb60b2673d610d8ab7ea31103f137debf 
  3rdparty/libprocess/Makefile.am ac8cc8d29baccf6e3a17367540ddd1f28585ef6d 
  3rdparty/libprocess/src/decoder.hpp a20b5ba8fc50d834573d253948645cc863f030dd 
  LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 

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


Testing
---

sudo make dist check

sudo ./src/mesos-tests --benchmark

sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build


Thanks,

Zhiwei Chen

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Jojy Varghese]

> On March 20, 2016, 2:33 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > This comment seems incorrect:
> > 
> > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as 
> > well as `FTS_COMFOLLOW`.
> > 
> > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for 
> > `FTS_SLNONE` to be returned?
> 
> Jojy Varghese wrote:
> I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for 
> FTS_SLNONE to be returned.
> 
> Jie Yu wrote:
> Can you do a simple test to validate that (see if FTS_SLNONE is hit or 
> not)? My understanding is that FTS_SLNONE will still be hit if we're using 
> FTS_PHYSICAL.
> 
> Cong Wang wrote:
> Looking at 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901
>  I think Jojy is correct here, I don't see FTS_SLNONE could be set without 
> FTS_LOGICAL or FTS_COMFOLLOW.
> 
> Jojy Varghese wrote:
> @jie. I tested it already and verified that FTS_SLNONE  is not hit (using 
> logs).
> 
> Neil Conway wrote:
> Can we add a unit test anyway, please? This is corner-case behavior that 
> we should be validating we handle correctly, regardless of whether 
> `FTS_SLNONE` is actually set.

I have added unit tests in patch https://reviews.apache.org/r/45002/. Would 
love to hear more test ideas.


- Jojy


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


On March 24, 2016, 1:10 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 24, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Jojy Varghese]

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

(Updated March 24, 2016, 1:10 a.m.)


Review request for mesos, Jie Yu and Neil Conway.


Changes
---

review addressed.


Repository: mesos


Description
---

Fixed rmdir comment for FTS_SLNONE as per coding guidelines.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
cbc97596cd8ed1e6d4261568fd0086c86e063232 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Neil Conway]

> On March 20, 2016, 2:33 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > This comment seems incorrect:
> > 
> > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as 
> > well as `FTS_COMFOLLOW`.
> > 
> > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for 
> > `FTS_SLNONE` to be returned?
> 
> Jojy Varghese wrote:
> I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for 
> FTS_SLNONE to be returned.
> 
> Jie Yu wrote:
> Can you do a simple test to validate that (see if FTS_SLNONE is hit or 
> not)? My understanding is that FTS_SLNONE will still be hit if we're using 
> FTS_PHYSICAL.
> 
> Cong Wang wrote:
> Looking at 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901
>  I think Jojy is correct here, I don't see FTS_SLNONE could be set without 
> FTS_LOGICAL or FTS_COMFOLLOW.
> 
> Jojy Varghese wrote:
> @jie. I tested it already and verified that FTS_SLNONE  is not hit (using 
> logs).

Can we add a unit test anyway, please? This is corner-case behavior that we 
should be validating we handle correctly, regardless of whether `FTS_SLNONE` is 
actually set.


- Neil


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


On March 18, 2016, 12:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 18, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.

[Jojy Varghese]

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

(Updated March 24, 2016, 1:07 a.m.)


Review request for mesos, Jie Yu and Neil Conway.


Changes
---

review addressed.


Repository: mesos


Description
---

According to the documentation for fts_open, either FTS_PHYSICAL or FTS_LOGICAL
SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want
to resolve the symlink targets while deleting them.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
cbc97596cd8ed1e6d4261568fd0086c86e063232 
  3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
5bd154e40c6b45e74967b620141cc373bdee 
  src/tests/containerizer/provisioner_appc_tests.cpp 
23a5b1059b4d9fde1e4a1aab5cd4fa6d05862332 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 45249: Added new '/files' endpoints tests using authentication.

[Greg Mann]

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

(Updated March 24, 2016, 1:05 a.m.)


Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.


Changes
---

Removed unnecessary tests.


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


Repository: mesos


Description
---

New tests were added to the FilesTests, and existing tests were modified, to 
probe the endpoints' behavior with and without HTTP authentication. Tests were 
also added for `/flags/debug`, as there were no pre-existing tests for that 
endpoint.


Diffs (updated)
-

  src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e 

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


Testing
---

`sudo make check` was used to test on OSX and CentOS 7.


Thanks,

Greg Mann

Review Request 45259: Added Appc provisioner integration test.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added Appc provisioner integration test.


Diffs
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
23a5b1059b4d9fde1e4a1aab5cd4fa6d05862332 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 45248: Added authentication to the '/files' endpoints.

[Greg Mann]

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

(Updated March 24, 2016, 12:58 a.m.)


Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.


Changes
---

Moved `Files` initialization into initialization lists.


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


Repository: mesos


Description
---

The `Files` constructor was modified to accept an `Option& 
authenticationRealm`, allowing the master or agent to set the authentication 
realm during initialization. The master/agent test helpers were modified to 
pass the appropriate authentication realm to `Files`, and the GC tests were 
modified to authenticate when hitting the 'files/browse' endpoint.


Diffs (updated)
-

  src/files/files.hpp 7b65a0a4fbc591fdf22b6f88d6c74034bd8ab599 
  src/files/files.cpp bcde9d552ac26e2eb702e5d686574f69e640f14d 
  src/master/main.cpp 61210d9f275d4073967c3468179307cf09e88551 
  src/slave/main.cpp 33a1af84aeb079224b15e92caf97bcf081ea4646 
  src/tests/cluster.hpp 06424dd741aed2261a926429bb0fc7dea141c11b 
  src/tests/gc_tests.cpp 42059b2d6544f360cdc9230fe6ed33a11a15bc50 
  src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
  src/tests/mesos.cpp 90aef6bfe619dc0acdb4ccba6a7180482dd13ce5 

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


Testing
---

`sudo make check` was used to test on OSX and CentOS 7.

The master and agent binaries were run both with and without HTTP 
authentication, and tested for the expected behavior at the `/files/*` 
endpoints.


Thanks,

Greg Mann

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Jie Yu]

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


Fix it, then Ship it!





3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp (line 71)


as we don't set `FTS_COMFOLLOW` or `FTS_LOGICAL`.


- Jie Yu


On March 18, 2016, 12:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 18, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Jojy Varghese]

> On March 20, 2016, 2:33 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > This comment seems incorrect:
> > 
> > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as 
> > well as `FTS_COMFOLLOW`.
> > 
> > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for 
> > `FTS_SLNONE` to be returned?
> 
> Jojy Varghese wrote:
> I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for 
> FTS_SLNONE to be returned.
> 
> Jie Yu wrote:
> Can you do a simple test to validate that (see if FTS_SLNONE is hit or 
> not)? My understanding is that FTS_SLNONE will still be hit if we're using 
> FTS_PHYSICAL.
> 
> Cong Wang wrote:
> Looking at 
> https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901
>  I think Jojy is correct here, I don't see FTS_SLNONE could be set without 
> FTS_LOGICAL or FTS_COMFOLLOW.

@jie. I tested it already and verified that FTS_SLNONE  is not hit (using logs).


- Jojy


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


On March 18, 2016, 12:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 18, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44656: Introduced `KillPolicy` protobuf.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 11:27 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Describes a kill policy for a task. Currently does not express
different policies (e.g. hitting HTTP endpoints), only controls
how long to wait between graceful and forcible task kill.


Diffs (updated)
-

  CHANGELOG 1fadaa1819c9b1a801752b1c0ddd572b82e6c170 
  include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
  include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44661: Deprecated the `docker_stop_timeout` flag.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 11:26 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Instead, a combination of `executor_shutdown_grace_period`
agent flag and task kill policies should be used.


Diffs (updated)
-

  CHANGELOG 1fadaa1819c9b1a801752b1c0ddd572b82e6c170 
  docs/configuration.md 73ee8fa7a77a6ceaf44e1f4c54ab4027a7109258 
  src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
  src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
  src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
  src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44660: Used `KillPolicy` and shutdown grace period in docker executor.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 11:25 p.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

The docker executor determines how much time it allots the
underlying container to clean up (via passing the timeout to
the docker daemon) based on both optional task's `KillPolicy`
and optional `shutdown_grace_period` field in `ExecutorInfo`.


Diffs (updated)
-

  include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
  include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 
  src/docker/executor.cpp afc769d0887e3842106e4c350e94c95c8ffc085e 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44657: Used `KillPolicy` and shutdown grace period in command executor.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 11:25 p.m.)


Review request for mesos, Ben Mahler and Gilbert Song.


Changes
---

Added a test.


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


Repository: mesos


Description
---

The command executor determines how much time it allots the
underlying task to clean up (effectively how long to wait for
the task to comply to SIGTERM before sending SIGKILL) based
on both optional task's `KillPolicy` and optional
`shutdown_grace_period` field in `ExecutorInfo`.

Manual testing was performed to ensure newly introduced protobuf
fields are respected. To do that, "mesos-execute" was modified to
support `KillPolicy` and `CommandInfo.shell=false`. To simulate a
task that does not exit in the allotted period, a tiny app
(https://github.com/rukletsov/unresponsive-process) that ignores
SIGTERM was used.


Diffs (updated)
-

  src/launcher/executor.cpp 2df62f09637b55c63ae6fe5d5a70b8debc02fbe2 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 

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


Testing
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="*CommandTaskWithKillPolicy*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure`

To test the newly introduced protobuf fields, I've modified "mesos-execute" to
support these fields. I've inserted these lines in `resourceOffers()` method:
```
task.mutable_kill_policy()->mutable_grace_period()->
set_nanoseconds(Seconds().ns());
```

I've also implemented a "--no-shell" flag to support
setting `CommandInfo.shell` to `false`. An example command used in testing:
```
./src/mesos-execute --master=127.0.0.1:5050 --name=test 
--command="/Users/alex/bin/unresponsive_process" --no-shell
```

In order to test the signal escalation path, which is what the command executor
implements currently for kill policy, I've created a tiny application that
ignores SIGTERM (https://github.com/rukletsov/unresponsive-process ).

Test 1.1:
=
`KillPolicy.grace_period` is **not set**, `shell=false`, framework shutdown.
Framework is asked to shutdown several seconds after the task is started. This
results in agent asking executor to shutdown. From the agent's logs, we see that
the command executor sends a TASK_KILLED update in approx. **3 seconds** (the
default value is **5 seconds**, but we deduct some buffer to make sure the task 
can
be reaped and TASK_KILLED is sent) after it has been asked to shut down:
```
I0323 17:24:48.704677 418881536 slave.cpp:2080] Asked to shut down framework 
9eb233b5-dfe8-4030-a989-57f31ec0e3aa- by master@127.0.0.1:5050
I0323 17:24:48.704710 418881536 slave.cpp:2105] Shutting down framework 
9eb233b5-dfe8-4030-a989-57f31ec0e3aa-
I0323 17:24:48.704756 418881536 slave.cpp:4216] Shutting down executor 'test' 
of framework 9eb233b5-dfe8-4030-a989-57f31ec0e3aa- at 
executor(1)@192.168.178.24:50933
I0323 17:24:51.817987 419418112 slave.cpp:3003] Handling status update 
TASK_KILLED (UUID: 9cf799dd-3492-42c2-9b43-fcb7283ebec9) for task test of 
framework 9eb233b5-dfe8-4030-a989-57f31ec0e3aa- from 
executor(1)@192.168.178.24:50933
```

Excerpt from the executor's log we see, that `CommandExecutor::escalated()` has
been called in 3 seconds:
```
Shutting down
Sending SIGTERM to process tree at pid 23632
9276662428399220356
Sent SIGTERM to the following process trees:
[
--- 23632 /Users/alex/bin/unresponsive_process
]
10137513391590427691
12741826412569908689
Process 23632 did not terminate after 3secs, sending SIGKILL to process tree at 
23632
16972857197258033356
Killed the following process trees:
[
--- 23632 /Users/alex/bin/unresponsive_process
]
Command terminated with signal Killed: 9 (pid: 23632)
```

Test 1.2:
=
`KillPolicy.grace_period` is **not set**, `shell=true`, framework shutdown.
The only difference to test 1.1 is `shell=true`. Similar behavior is observed.

Test 2.1:
=
`KillPolicy.grace_period` set to **0s**, `shell=false`, framework shutdown.
Similar to test 1.1, but the escalation timeout is set to 0s (no deductions
because the kill policy is set explicitly; the grace period in containerizer
should be adjusted to approx. 2 seconds):
```
I0323 17:38:07.650616 189734912 slave.cpp:2080] Asked to shut down framework 
0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a- by master@127.0.0.1:5050
I0323 17:38:07.650647 189734912 slave.cpp:2105] Shutting down framework 
0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a-
I0323 17:38:07.650686 189734912 slave.cpp:4216] Shutting down executor 'test' 
of framework 0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a- at 
executor(1)@192.168.178.24:51836
I0323 17:38:07.823477 187052032 slave.cpp:3003] Handling status update 
TASK_KILLED (UU

Re: Review Request 44707: Added validation for task's kill policy.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 11:24 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b 
  src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 
  src/tests/master_validation_tests.cpp 
8d0070a1b8e8dcc7fe6360f8c6c6182ba9edef7d 

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


Testing
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="TaskValidationTest.KillPolicyGracePeriodIsNonNegative" 
./bin/mesos-tests.sh --gtest_repeat=100 --gtest_break_on_failure`


Thanks,

Alexander Rukletsov

Re: Review Request 45040: Added a test for task's kill policy.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 11:24 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 

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


Testing
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="*TaskKillPolicy*" ./bin/mesos-tests.sh --gtest_repeat=100 
--gtest_break_on_failure`


Thanks,

Alexander Rukletsov

Re: Review Request 45151: Updated FrameworkInfo::Capability::Type enum for upgradability.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45151]

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

- Mesos ReviewBot


On March 23, 2016, 5:58 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45151/
> ---
> 
> (Updated March 23, 2016, 5:58 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-4997
> https://issues.apache.org/jira/browse/MESOS-4997
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Per [MESOS-4977](https://issues.apache.org/jira/browse/MESOS-4977), having a 
> required enum field is problematic for supporting adding new enum values.
> 
> This updates FrameworkInfo::Capability to support the addition of new 
> capabilities while allowing old masters to deserialize the new messages.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
>   include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 
> 
> Diff: https://reviews.apache.org/r/45151/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> manually validated the upgrade / backwards compatibility of enum fields
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 44994: Added a test for executor shutdown grace period.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 11:22 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
  src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 

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


Testing
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="*ExecutorShutdownGracePeriod*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure`


Thanks,

Alexander Rukletsov

Re: Review Request 44655: Made `shutdown_grace_period` configurable in `ExecutorInfo`.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 11:15 p.m.)


Review request for mesos, Ben Mahler and Gilbert Song.


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


Repository: mesos


Description
---

If `ExecutorInfo.shutdown_grace_period` is set, the executor
driver uses it, otherwise it falls back to the environment
variable `MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD`.


Diffs (updated)
-

  CHANGELOG 1fadaa1819c9b1a801752b1c0ddd572b82e6c170 
  docs/configuration.md 73ee8fa7a77a6ceaf44e1f4c54ab4027a7109258 
  include/mesos/executor/executor.proto 
ae211194a44e0bf2fadc79e833881e45ea3eb2c2 
  include/mesos/mesos.proto b965f5a6b1aff28feb2ab088c30ea927b9247403 
  include/mesos/v1/executor/executor.proto 
36a2b3f9bc3aaa524f655b9e686a6d33512e6aaa 
  include/mesos/v1/mesos.proto d2ab6ed187ec0d0e5dbac92607b612bb55b1a682 
  src/slave/containerizer/containerizer.cpp 
f6fc7863d0c215611f170dc0c89aa229407b5137 
  src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 

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


Testing
---

The complete chain was tested. See https://reviews.apache.org/r/44662/.


Thanks,

Alexander Rukletsov

Re: Review Request 44657: Used `KillPolicy` and shutdown grace period in command executor.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 11:01 p.m.)


Review request for mesos, Ben Mahler and Gilbert Song.


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


Repository: mesos


Description
---

The command executor determines how much time it allots the
underlying task to clean up (effectively how long to wait for
the task to comply to SIGTERM before sending SIGKILL) based
on both optional task's `KillPolicy` and optional
`shutdown_grace_period` field in `ExecutorInfo`.

Manual testing was performed to ensure newly introduced protobuf
fields are respected. To do that, "mesos-execute" was modified to
support `KillPolicy` and `CommandInfo.shell=false`. To simulate a
task that does not exit in the allotted period, a tiny app
(https://github.com/rukletsov/unresponsive-process) that ignores
SIGTERM was used.


Diffs
-

  src/launcher/executor.cpp 2df62f09637b55c63ae6fe5d5a70b8debc02fbe2 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 

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


Testing (updated)
---

On Mac OS 10.10.4:
`make check`
`GTEST_FILTER="*CommandTaskWithKillPolicy*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure`

To test the newly introduced protobuf fields, I've modified "mesos-execute" to
support these fields. I've inserted these lines in `resourceOffers()` method:
```
task.mutable_kill_policy()->mutable_grace_period()->
set_nanoseconds(Seconds().ns());
```

I've also implemented a "--no-shell" flag to support
setting `CommandInfo.shell` to `false`. An example command used in testing:
```
./src/mesos-execute --master=127.0.0.1:5050 --name=test 
--command="/Users/alex/bin/unresponsive_process" --no-shell
```

In order to test the signal escalation path, which is what the command executor
implements currently for kill policy, I've created a tiny application that
ignores SIGTERM (https://github.com/rukletsov/unresponsive-process ).

Test 1.1:
=
`KillPolicy.grace_period` is **not set**, `shell=false`, framework shutdown.
Framework is asked to shutdown several seconds after the task is started. This
results in agent asking executor to shutdown. From the agent's logs, we see that
the command executor sends a TASK_KILLED update in approx. **3 seconds** (the
default value is **5 seconds**, but we deduct some buffer to make sure the task 
can
be reaped and TASK_KILLED is sent) after it has been asked to shut down:
```
I0323 17:24:48.704677 418881536 slave.cpp:2080] Asked to shut down framework 
9eb233b5-dfe8-4030-a989-57f31ec0e3aa- by master@127.0.0.1:5050
I0323 17:24:48.704710 418881536 slave.cpp:2105] Shutting down framework 
9eb233b5-dfe8-4030-a989-57f31ec0e3aa-
I0323 17:24:48.704756 418881536 slave.cpp:4216] Shutting down executor 'test' 
of framework 9eb233b5-dfe8-4030-a989-57f31ec0e3aa- at 
executor(1)@192.168.178.24:50933
I0323 17:24:51.817987 419418112 slave.cpp:3003] Handling status update 
TASK_KILLED (UUID: 9cf799dd-3492-42c2-9b43-fcb7283ebec9) for task test of 
framework 9eb233b5-dfe8-4030-a989-57f31ec0e3aa- from 
executor(1)@192.168.178.24:50933
```

Excerpt from the executor's log we see, that `CommandExecutor::escalated()` has
been called in 3 seconds:
```
Shutting down
Sending SIGTERM to process tree at pid 23632
9276662428399220356
Sent SIGTERM to the following process trees:
[
--- 23632 /Users/alex/bin/unresponsive_process
]
10137513391590427691
12741826412569908689
Process 23632 did not terminate after 3secs, sending SIGKILL to process tree at 
23632
16972857197258033356
Killed the following process trees:
[
--- 23632 /Users/alex/bin/unresponsive_process
]
Command terminated with signal Killed: 9 (pid: 23632)
```

Test 1.2:
=
`KillPolicy.grace_period` is **not set**, `shell=true`, framework shutdown.
The only difference to test 1.1 is `shell=true`. Similar behavior is observed.

Test 2.1:
=
`KillPolicy.grace_period` set to **0s**, `shell=false`, framework shutdown.
Similar to test 1.1, but the escalation timeout is set to 0s (no deductions
because the kill policy is set explicitly; the grace period in containerizer
should be adjusted to approx. 2 seconds):
```
I0323 17:38:07.650616 189734912 slave.cpp:2080] Asked to shut down framework 
0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a- by master@127.0.0.1:5050
I0323 17:38:07.650647 189734912 slave.cpp:2105] Shutting down framework 
0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a-
I0323 17:38:07.650686 189734912 slave.cpp:4216] Shutting down executor 'test' 
of framework 0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a- at 
executor(1)@192.168.178.24:51836
I0323 17:38:07.823477 187052032 slave.cpp:3003] Handling status update 
TASK_KILLED (UUID: 56aad190-2ea2-4bb2-8d61-7a49ff42d4ba) for task test of 
framework 0dbc9e01-89ef-4cff-8c64-cdb3b46e9

Re: Review Request 45067: Updated the long-lived-framework example.

[Joseph Wu]

> On March 18, 2016, 6:35 p.m., Neil Conway wrote:
> > src/examples/long_lived_framework.cpp, line 135
> > 
> >
> > Don't we need `TaskState_Name` here?

Yeah, for some reason, I thought that used an internal header.  Added it back.


- Joseph


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


On March 23, 2016, 3:50 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45067/
> ---
> 
> (Updated March 23, 2016, 3:50 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `long-lived-framework` enough options to run outside 
> of the build environment.
> 
> This also updates:
> 
> * The style of the framework code.
> * Gives the `ExecutorInfo` some resources (needed for some cgroups isolators).
> * Restricts the framework to one agent.  Otherwise, it would grab a small 
> chunk of every machine in the cluster.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/45067/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran this on the master node on a Mesos cluster:
> ```
> ./long-lived-framework --master=zk://localhost:2181/mesos 
> --executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; 
> --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && 
> ./long-lived-executor"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45067: Updated the long-lived-framework example.

[Joseph Wu]

> On March 18, 2016, 11:42 p.m., haosdent huang wrote:
> > src/examples/long_lived_framework.cpp, line 168
> > 
> >
> > I think use
> > ```
> >   if (flags.master.isNone()) {
> > EXIT(EXIT_FAILURE)
> >   << flags.usage("Missing required option --master");
> >   }
> > ```
> > to keep consistent with current codebase would be better

The flag validation lambdas are somewhat newer stylistically, but they are the 
recommended way of adding flag validation now.

As the for the `EXIT`, we exit below:
```
  if (load.isError()) {
EXIT(1) << flags.usage(load.error());
  }
```
(I'll change the `1` to `EXIT_FAILURE`.)


> On March 18, 2016, 11:42 p.m., haosdent huang wrote:
> > src/examples/long_lived_framework.cpp, line 177
> > 
> >
> > Should use `build_dir`?

Ah, yes.  Missed that :)


- Joseph


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


On March 23, 2016, 3:50 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45067/
> ---
> 
> (Updated March 23, 2016, 3:50 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `long-lived-framework` enough options to run outside 
> of the build environment.
> 
> This also updates:
> 
> * The style of the framework code.
> * Gives the `ExecutorInfo` some resources (needed for some cgroups isolators).
> * Restricts the framework to one agent.  Otherwise, it would grab a small 
> chunk of every machine in the cluster.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 289a0b9dd3d1ce30f20dd9bb381126bff30c 
> 
> Diff: https://reviews.apache.org/r/45067/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran this on the master node on a Mesos cluster:
> ```
> ./long-lived-framework --master=zk://localhost:2181/mesos 
> --executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; 
> --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && 
> ./long-lived-executor"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45067: Updated the long-lived-framework example.

[Joseph Wu]

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

(Updated March 23, 2016, 3:50 p.m.)


Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Kevin Klues.


Changes
---

A couple more updates to make this framework play nicer on an actual cluster.  
Notably filters and a self-imposed agent restriction.


Repository: mesos


Description (updated)
---

This gives the example `long-lived-framework` enough options to run outside of 
the build environment.

This also updates:

* The style of the framework code.
* Gives the `ExecutorInfo` some resources (needed for some cgroups isolators).
* Restricts the framework to one agent.  Otherwise, it would grab a small chunk 
of every machine in the cluster.
* Adds filters for declined offers.


Diffs (updated)
-

  src/examples/long_lived_framework.cpp 
289a0b9dd3d1ce30f20dd9bb381126bff30c 

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


Testing
---

make check

Ran this on the master node on a Mesos cluster:
```
./long-lived-framework --master=zk://localhost:2181/mesos 
--executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; 
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./long-lived-executor"
```


Thanks,

Joseph Wu

Re: Review Request 45092: Fixed containerizer potential race destroy while provisioning.

[Jie Yu]

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


Ship it!





src/slave/containerizer/mesos/containerizer.cpp (line 754)


I added comments for you about why `containers_.contains(containerId)`.


- Jie Yu


On March 23, 2016, 9:17 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45092/
> ---
> 
> (Updated March 23, 2016, 9:17 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Shuai Lin, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4985
> https://issues.apache.org/jira/browse/MESOS-4985
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed containerizer potential race destroy while provisioning.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ef6a6752a6656e97be9f48bd4d2d060d1f9cb46 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> f3ca32b2d8b8ef9dcfa8f20d9ceaff48b6598a66 
> 
> Diff: https://reviews.apache.org/r/45092/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 45249: Added new '/files' endpoints tests using authentication.

[Greg Mann]

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

Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.


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


Repository: mesos


Description
---

New tests were added to the FilesTests, and existing tests were modified, to 
probe the endpoints' behavior with and without HTTP authentication. Tests were 
also added for `/flags/debug`, as there were no pre-existing tests for that 
endpoint.


Diffs
-

  src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e 

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


Testing
---

`sudo make check` was used to test on OSX and CentOS 7.


Thanks,

Greg Mann

Review Request 45248: Added authentication to the '/files' endpoints.

[Greg Mann]

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

Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.


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


Repository: mesos


Description
---

The `Files` constructor was modified to accept an `Option& 
authenticationRealm`, allowing the master or agent to set the authentication 
realm during initialization. The master/agent test helpers were modified to 
pass the appropriate authentication realm to `Files`, and the GC tests were 
modified to authenticate when hitting the 'files/browse' endpoint.


Diffs
-

  src/files/files.hpp 7b65a0a4fbc591fdf22b6f88d6c74034bd8ab599 
  src/files/files.cpp bcde9d552ac26e2eb702e5d686574f69e640f14d 
  src/master/main.cpp 61210d9f275d4073967c3468179307cf09e88551 
  src/slave/main.cpp 33a1af84aeb079224b15e92caf97bcf081ea4646 
  src/tests/cluster.hpp 06424dd741aed2261a926429bb0fc7dea141c11b 
  src/tests/gc_tests.cpp 42059b2d6544f360cdc9230fe6ed33a11a15bc50 
  src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 

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


Testing
---

`sudo make check` was used to test on OSX and CentOS 7.

The master and agent binaries were run both with and without HTTP 
authentication, and tested for the expected behavior at the `/files/*` 
endpoints.


Thanks,

Greg Mann

Re: Review Request 44946: Add utility functions to manipulate XFS project quotas.

[Jiang Yan Xu]

> On March 23, 2016, 10:39 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp, line 119
> > 
> >
> > I know I asked this previoiusly but what's the difference between this 
> > and:
> > 
> > ```
> > fs_disk_quota_t quota;
> > ```
> > 
> > If it's the same, then the more common syntax is preferred because this 
> > syntax reads like the 1st member of the struct has special some 
> > meaning/usage.
> 
> James Peach wrote:
> This is a ``C`` zero-initialization. I can either do this or ``memset`` 
> the structure to zero, to get the same result.

Forgot this is a C struct. Alright.


- Jiang Yan


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


On March 22, 2016, 4:24 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44946/
> ---
> 
> (Updated March 22, 2016, 4:24 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add utility functions to manipulate XFS project quotas.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/isolators/disk/xfs/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44946/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual verification. Tests in subsequent patches.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45184: Added test for mounting host system config files.

[Gilbert Song]

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

(Updated March 23, 2016, 2:50 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


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


Repository: mesos


Description
---

Added test for mounting host system config files.


Diffs (updated)
-

  src/tests/containerizer/runtime_isolator_tests.cpp 
9f3b0b08da7cebba722062a9932fae1b5f825efb 

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


Testing
---

make check

sudo ./bin/mesos-test.sh


Thanks,

Gilbert Song

Re: Review Request 45183: Implemented mounting host system config files to container.

[Gilbert Song]

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

(Updated March 23, 2016, 2:49 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


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


Repository: mesos


Description
---

Implemented mounting host system config files to container.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
90179119ef297855091dad3fe969aa79810bf209 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
f97a9a92895387a9d504810a2ae971cfb5d3dbb4 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 44657: Used `KillPolicy` and shutdown grace period in command executor.

[Alexander Rukletsov]

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

(Updated March 23, 2016, 9:45 p.m.)


Review request for mesos, Ben Mahler and Gilbert Song.


Changes
---

Manually tested the `killTask` path.


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


Repository: mesos


Description
---

The command executor determines how much time it allots the
underlying task to clean up (effectively how long to wait for
the task to comply to SIGTERM before sending SIGKILL) based
on both optional task's `KillPolicy` and optional
`shutdown_grace_period` field in `ExecutorInfo`.

Manual testing was performed to ensure newly introduced protobuf
fields are respected. To do that, "mesos-execute" was modified to
support `KillPolicy` and `CommandInfo.shell=false`. To simulate a
task that does not exit in the allotted period, a tiny app
(https://github.com/rukletsov/unresponsive-process) that ignores
SIGTERM was used.


Diffs
-

  src/launcher/executor.cpp 2df62f09637b55c63ae6fe5d5a70b8debc02fbe2 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 

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


Testing (updated)
---

To test the newly introduced protobuf fields, I've modified "mesos-execute" to
support these fields. I've inserted these lines in `resourceOffers()` method:
```
task.mutable_kill_policy()->mutable_grace_period()->
set_nanoseconds(Seconds().ns());
```

I've also implemented a "--no-shell" flag to support
setting `CommandInfo.shell` to `false`. An example command used in testing:
```
./src/mesos-execute --master=127.0.0.1:5050 --name=test 
--command="/Users/alex/bin/unresponsive_process" --no-shell
```

In order to test the signal escalation path, which is what the command executor
implements currently for kill policy, I've created a tiny application that
ignores SIGTERM (https://github.com/rukletsov/unresponsive-process ).

Test 1.1:
=
`KillPolicy.grace_period` is **not set**, `shell=false`, framework shutdown.
Framework is asked to shutdown several seconds after the task is started. This
results in agent asking executor to shutdown. From the agent's logs, we see that
the command executor sends a TASK_KILLED update in approx. **3 seconds** (the
default value is **5 seconds**, but we deduct some buffer to make sure the task 
can
be reaped and TASK_KILLED is sent) after it has been asked to shut down:
```
I0323 17:24:48.704677 418881536 slave.cpp:2080] Asked to shut down framework 
9eb233b5-dfe8-4030-a989-57f31ec0e3aa- by master@127.0.0.1:5050
I0323 17:24:48.704710 418881536 slave.cpp:2105] Shutting down framework 
9eb233b5-dfe8-4030-a989-57f31ec0e3aa-
I0323 17:24:48.704756 418881536 slave.cpp:4216] Shutting down executor 'test' 
of framework 9eb233b5-dfe8-4030-a989-57f31ec0e3aa- at 
executor(1)@192.168.178.24:50933
I0323 17:24:51.817987 419418112 slave.cpp:3003] Handling status update 
TASK_KILLED (UUID: 9cf799dd-3492-42c2-9b43-fcb7283ebec9) for task test of 
framework 9eb233b5-dfe8-4030-a989-57f31ec0e3aa- from 
executor(1)@192.168.178.24:50933
```

Excerpt from the executor's log we see, that `CommandExecutor::escalated()` has
been called in 3 seconds:
```
Shutting down
Sending SIGTERM to process tree at pid 23632
9276662428399220356
Sent SIGTERM to the following process trees:
[
--- 23632 /Users/alex/bin/unresponsive_process
]
10137513391590427691
12741826412569908689
Process 23632 did not terminate after 3secs, sending SIGKILL to process tree at 
23632
16972857197258033356
Killed the following process trees:
[
--- 23632 /Users/alex/bin/unresponsive_process
]
Command terminated with signal Killed: 9 (pid: 23632)
```

Test 1.2:
=
`KillPolicy.grace_period` is **not set**, `shell=true`, framework shutdown.
The only difference to test 1.1 is `shell=true`. Similar behavior is observed.

Test 2.1:
=
`KillPolicy.grace_period` set to **0s**, `shell=false`, framework shutdown.
Similar to test 1.1, but the escalation timeout is set to 0s (no deductions
because the kill policy is set explicitly; the grace period in containerizer
should be adjusted to approx. 2 seconds):
```
I0323 17:38:07.650616 189734912 slave.cpp:2080] Asked to shut down framework 
0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a- by master@127.0.0.1:5050
I0323 17:38:07.650647 189734912 slave.cpp:2105] Shutting down framework 
0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a-
I0323 17:38:07.650686 189734912 slave.cpp:4216] Shutting down executor 'test' 
of framework 0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a- at 
executor(1)@192.168.178.24:51836
I0323 17:38:07.823477 187052032 slave.cpp:3003] Handling status update 
TASK_KILLED (UUID: 56aad190-2ea2-4bb2-8d61-7a49ff42d4ba) for task test of 
framework 0dbc9e01-89ef-4cff-8c64-cdb3b46e9b2a- from 
executor(1)@192.168.178.24:51836
```

Excerpt from the executor's log we s

Re: Review Request 45186: Implemented user specified system config files support.

[Gilbert Song]

> On March 22, 2016, 11:46 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, lines 204-205
> > 
> >
> > what about:
> > 
> > foreach (
> > const string& file,
> > strings::split(flags.system_config_files.get(), ",")) {

Thanks for mentioning this. We are currently supporting both formats. We may 
prefer the original one here since that format is used around this part of 
codes.


> On March 22, 2016, 11:46 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, line 221
> > 
> >
> > I thihk that we also need to make sure the file exist before put it to 
> > systemConfig, otherwise, the bind mount will be failed.

They are checked together in the loop below. Yes, we should be careful about 
this.


- Gilbert


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


On March 22, 2016, 5:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45186/
> ---
> 
> (Updated March 22, 2016, 5:20 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3003
> https://issues.apache.org/jira/browse/MESOS-3003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented user specified system config files support.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> f97a9a92895387a9d504810a2ae971cfb5d3dbb4 
> 
> Diff: https://reviews.apache.org/r/45186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> tested manually with mesos-execute to cat /etc/resolv.conf from an alpine 
> image.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45186: Implemented user specified system config files support.

[Gilbert Song]

> On March 22, 2016, 11:51 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, lines 231-233
> > 
> >
> > Put this check to the block of 
> > if (flags.system_config_files.isSome()) {
> > } ?

It can be a case that any of the four default sys config files missing on the 
host for some os env, so we check all files which will be bind mounted here.


- Gilbert


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


On March 22, 2016, 5:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45186/
> ---
> 
> (Updated March 22, 2016, 5:20 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-3003
> https://issues.apache.org/jira/browse/MESOS-3003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented user specified system config files support.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> f97a9a92895387a9d504810a2ae971cfb5d3dbb4 
> 
> Diff: https://reviews.apache.org/r/45186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> tested manually with mesos-execute to cat /etc/resolv.conf from an alpine 
> image.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45093: Added mesos containerizer test DestroyWhileProvisioning.

[Gilbert Song]

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

(Updated March 23, 2016, 2:23 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, Shuai Lin, and Timothy 
Chen.


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


Repository: mesos


Description
---

Added mesos containerizer test DestroyWhileProvisioning.


Diffs (updated)
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
f3ca32b2d8b8ef9dcfa8f20d9ceaff48b6598a66 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 45092: Fixed containerizer potential race destroy while provisioning.

[Gilbert Song]

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

(Updated March 23, 2016, 2:17 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, Shuai Lin, and Timothy 
Chen.


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


Repository: mesos


Description
---

Fixed containerizer potential race destroy while provisioning.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
3ef6a6752a6656e97be9f48bd4d2d060d1f9cb46 
  src/slave/containerizer/mesos/containerizer.cpp 
ee7a265975323ca891114a286357c8e42901560c 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
f3ca32b2d8b8ef9dcfa8f20d9ceaff48b6598a66 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 45243: WIP: Added a metric for querying the number offer filters for a role.

[Benjamin Bannier]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Added a metric for querying the number offer filters for a role.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
e4604f4da59166da24709a68b8cd4e56bf55f97f 
  src/master/allocator/mesos/hierarchical.cpp 
39a290d0db2c22e179a8f933b1a78e3a2dcefdc3 
  src/master/allocator/mesos/metrics.hpp 
f8c6949f18400f4bef9c12c8d0fe0b74cfa37439 
  src/master/allocator/mesos/metrics.cpp 
2f3012a5bd40a4100e790e1d373832015c80b993 
  src/tests/hierarchical_allocator_tests.cpp 
e9cfcfc0ad8b0b89bbac459b7db39183f6c189be 

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


Testing
---

make check (OS X, stock clang, not optimized)


Thanks,

Benjamin Bannier

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Cong Wang]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 228)


You always have ifIndex==0 here, it is never changed. So either you miss 
some code or it is not needed at all.


- Cong Wang


On March 23, 2016, 5:10 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 23, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3878a7e85fe24175b3bd5e3a6268cf32a07f2d8b 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 9552312f9ba1e4df6564cfb737cc41e041cf4407 
> 
> Diff: https://reviews.apache.org/r/44514/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Cong Wang]

> On March 11, 2016, 6:19 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 228
> > 
> >
> > Can there be a use case where you want multiple NICs to be attached to 
> > the same network? Servers use this configuration when they want to utilize 
> > NIC bonding. To aggregate the bandwidth available on the NICs
> 
> Qian Zhang wrote:
> Good point! I think it makes sense that user enables NIC bonding by 
> creating a bond device (e.g., bond0) as the master of the normal ethernet 
> devices (e.g., eth0 and eth1), and both eth0 and eth1 are set up by CNI 
> plugin and get assigned IP from CNI plugin in the same subnet. My only 
> concern is, how to configure the IP for bond0, maybe just use IP of either 
> eth0 or eth1 as its IP?
> 
> Avinash sridharan wrote:
> I think for the time being we can leave the behavior as is. To support 
> something like NIC bonding, I think we will need to introduce the concept of 
> `repeated` devices in `NetworkInfo`, which is definitely out of the scope of 
> this work.

Bonding is a L2 device as a whole, therefore you should just set IP address on 
the bonding master, its slaves don't need IP addresses.


- Cong


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


On March 23, 2016, 5:10 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 23, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3878a7e85fe24175b3bd5e3a6268cf32a07f2d8b 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 9552312f9ba1e4df6564cfb737cc41e041cf4407 
> 
> Diff: https://reviews.apache.org/r/44514/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44709: Allowed unknown flags in command and docker executors.

[Alexander Rukletsov]

> On March 18, 2016, 11:32 p.m., Jie Yu wrote:
> > src/docker/executor.cpp, lines 588-590
> > 
> >
> > This is more like a question: are you talking about the case where the 
> > agent binary is updated, while the executor binary is not?

Exactly! Is it not clear from the comment or you think it is rather unprobable?


- Alexander


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


On March 18, 2016, 5:24 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44709/
> ---
> 
> (Updated March 18, 2016, 5:24 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We allow unknown flags to ensure that when a new flag is added
> in Mesos version N, agent of version N works with the executor
> of version N-1, i.e. the latter does not segfault when observing
> a newly introduced flag.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp afc769d0887e3842106e4c350e94c95c8ffc085e 
>   src/launcher/executor.cpp 4149f084b0b234fb995df04e9d1ca704feec082e 
> 
> Diff: https://reviews.apache.org/r/44709/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44994: Added a test for executor shutdown grace period.

[Alexander Rukletsov]

> On March 18, 2016, 11:24 p.m., Ben Mahler wrote:
> > src/tests/slave_tests.cpp, line 3264
> > 
> >
> > You don't need a settle here, AWAIT_READY will settle if the clock is 
> > paused.
> 
> Alexander Rukletsov wrote:
> I feel that this is not obvious, that settle will happen in `AWAIT_READY` 
> if clock is paused. Moreover, when we don't await, we have to explicitly 
> settle, which—I think—makes it harder for folks to follow the test and reason 
> when settle is necessary, because sometimes a necessary settle is implicit 
> (inside await). Hence I tend to put explicit settles when I feel this makes 
> the code more obvious. Do you have a strong preference?
> 
> Ben Mahler wrote:
> Think about it this way, if you put a `settle()` before the 
> `AWAIT_READY`, then the implication is that you don't need `AWAIT_READY` at 
> all, you could just do `ASSERT_TRUE(f.isReady())` because you've done the 
> necessary settling explicitly. This is why it seems strange to have a 
> `settle` immediately preceed an `AWAIT_READY`, its the job of `AWAIT_READY` 
> to wait until the future is ready (which means doing the right thing when the 
> clock is paused, otherwise we would have to write a lot of `settle` calls).

I see, makes sense. Thanks Ben!


- Alexander


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


On March 22, 2016, 5:07 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44994/
> ---
> 
> (Updated March 22, 2016, 5:07 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4949
> https://issues.apache.org/jira/browse/MESOS-4949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 
> 
> Diff: https://reviews.apache.org/r/44994/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `make check`
> `GTEST_FILTER="*ExecutorShutdownGracePeriod*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 45242: Fixed invalid HTML in upgrades documentation.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On March 23, 2016, 7:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45242/
> ---
> 
> (Updated March 23, 2016, 7:52 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed invalid HTML in upgrades documentation.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md f8bd8a2fcf1b38eaed1bd77e8a9dd3e3370008e5 
> 
> Diff: https://reviews.apache.org/r/45242/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 45240: Fixed typo in comment.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On March 23, 2016, 7:50 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45240/
> ---
> 
> (Updated March 23, 2016, 7:50 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was introduced in acb514c.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/metrics.hpp 
> f8c6949f18400f4bef9c12c8d0fe0b74cfa37439 
> 
> Diff: https://reviews.apache.org/r/45240/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 45241: Fixed invalid HTML in monitoring documentation.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On March 23, 2016, 7:50 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45241/
> ---
> 
> (Updated March 23, 2016, 7:50 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed invalid HTML in monitoring documentation.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 263d3ba6822de385722fee1f5b8275b8344ba6e7 
> 
> Diff: https://reviews.apache.org/r/45241/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44854: Added validation for executor's shutdown grace period.

[Alexander Rukletsov]

> On March 23, 2016, 6:49 p.m., Ben Mahler wrote:
> > src/tests/master_validation_tests.cpp, lines 1175-1176
> > 
> >
> > Why the explicit detector?

After recent test harness refactoring, there is no `StartSlave()` overload 
creating detector implicitly.


- Alexander


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


On March 22, 2016, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44854/
> ---
> 
> (Updated March 22, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4949
> https://issues.apache.org/jira/browse/MESOS-4949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 
>   src/tests/master_validation_tests.cpp 
> 8d0070a1b8e8dcc7fe6360f8c6c6182ba9edef7d 
> 
> Diff: https://reviews.apache.org/r/44854/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `make check`
> `GTEST_FILTER="TaskValidationTest.ExecutorShutdownGracePeriodIsNonNegative" 
> ./bin/mesos-tests.sh --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44760: Windows: Fixed non-blocking connect.

[Daniel Pravat]

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

(Updated March 23, 2016, 8:49 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
Clemmer, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Changes
---

Extracting EINPROGRESS test in OS dependent function


Repository: mesos


Description
---

Windows: Fixed non-blocking connect.


Diffs (updated)
-

  3rdparty/libprocess/include/process/network.hpp 
7d203f0ff1cdb3145bc2b914f8bd606203878f09 
  3rdparty/libprocess/src/poll_socket.cpp 
6e6634b4b352e3723096521843546cf56ec6dd8b 

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


Testing
---

OSX: make check
Windows: build/run


Thanks,

Daniel Pravat

Re: Review Request 44138: Windows:[1/2] Lifted socket API into Stout.

[Daniel Pravat]

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

(Updated March 23, 2016, 8:47 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
Clemmer, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Windows:[1/2] Lifted socket API into Stout.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/socket.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/socket.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/socket.hpp 
PRE-CREATION 

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


Testing
---

OSX: make check
Windows: build/execute


Thanks,

Daniel Pravat

Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.

[Cong Wang]

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



Not related to your patch, but it also makes sense to add FTS_NOSTAT flag to 
speed up? We don't need statp anyway.

- Cong Wang


On March 18, 2016, 12:16 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45002/
> ---
> 
> (Updated March 18, 2016, 12:16 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to the documentation for fts_open, either FTS_PHYSICAL or 
> FTS_LOGICAL
> SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want
> to resolve the symlink targets while deleting them.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> daa46e05d113fd62ea9dad042ec41aaab28ad003 
> 
> Diff: https://reviews.apache.org/r/45002/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45002: Added FTS_PHYSICAL option to fts_open for rmdir.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On March 18, 2016, 12:16 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45002/
> ---
> 
> (Updated March 18, 2016, 12:16 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to the documentation for fts_open, either FTS_PHYSICAL or 
> FTS_LOGICAL
> SHOULD be provided. We need FTS_PHYSICAL for the case of rmdir as we dont want
> to resolve the symlink targets while deleting them.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> daa46e05d113fd62ea9dad042ec41aaab28ad003 
> 
> Diff: https://reviews.apache.org/r/45002/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Cong Wang]

> On March 20, 2016, 2:33 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > This comment seems incorrect:
> > 
> > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as 
> > well as `FTS_COMFOLLOW`.
> > 
> > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for 
> > `FTS_SLNONE` to be returned?
> 
> Jojy Varghese wrote:
> I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for 
> FTS_SLNONE to be returned.
> 
> Jie Yu wrote:
> Can you do a simple test to validate that (see if FTS_SLNONE is hit or 
> not)? My understanding is that FTS_SLNONE will still be hit if we're using 
> FTS_PHYSICAL.

Looking at 
https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fts.c;h=0c5abfcdd660871d876751fba351ab25b921e88a;hb=HEAD#l901
 I think Jojy is correct here, I don't see FTS_SLNONE could be set without 
FTS_LOGICAL or FTS_COMFOLLOW.


- Cong


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


On March 18, 2016, 12:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 18, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 45242: Fixed invalid HTML in upgrades documentation.

[Benjamin Bannier]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Fixed invalid HTML in upgrades documentation.


Diffs
-

  docs/upgrades.md f8bd8a2fcf1b38eaed1bd77e8a9dd3e3370008e5 

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


Testing
---


Thanks,

Benjamin Bannier

Review Request 45240: Fixed typo in comment.

[Benjamin Bannier]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

This was introduced in acb514c.


Diffs
-

  src/master/allocator/mesos/metrics.hpp 
f8c6949f18400f4bef9c12c8d0fe0b74cfa37439 

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


Testing
---


Thanks,

Benjamin Bannier

Review Request 45241: Fixed invalid HTML in monitoring documentation.

[Benjamin Bannier]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Fixed invalid HTML in monitoring documentation.


Diffs
-

  docs/monitoring.md 263d3ba6822de385722fee1f5b8275b8344ba6e7 

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


Testing
---


Thanks,

Benjamin Bannier

Re: Review Request 43569: Updated log message if container not found.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43015, 43569]

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

- Mesos ReviewBot


On March 23, 2016, 1:50 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43569/
> ---
> 
> (Updated March 23, 2016, 1:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated log message if container not found.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
> 
> Diff: https://reviews.apache.org/r/43569/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45093: Added mesos containerizer test DestroyWhileProvisioning.

[Jie Yu]

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


Fix it, then Ship it!





src/tests/containerizer/mesos_containerizer_tests.cpp (line 821)


Space after `)`



src/tests/containerizer/mesos_containerizer_tests.cpp (line 835)


Can you add a `using` clause instead:
```
using mesos::internal::slave::ProvisionInfo;
```



src/tests/containerizer/mesos_containerizer_tests.cpp (line 863)


Use UUID::random()



src/tests/containerizer/mesos_containerizer_tests.cpp (line 902)


Remove the namespace qualifiers


- Jie Yu


On March 21, 2016, 5:51 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45093/
> ---
> 
> (Updated March 21, 2016, 5:51 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Shuai Lin, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4985
> https://issues.apache.org/jira/browse/MESOS-4985
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added mesos containerizer test DestroyWhileProvisioning.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> f3ca32b2d8b8ef9dcfa8f20d9ceaff48b6598a66 
> 
> Diff: https://reviews.apache.org/r/45093/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44994: Added a test for executor shutdown grace period.

[Ben Mahler]

> On March 18, 2016, 11:24 p.m., Ben Mahler wrote:
> > src/tests/slave_tests.cpp, line 3264
> > 
> >
> > You don't need a settle here, AWAIT_READY will settle if the clock is 
> > paused.
> 
> Alexander Rukletsov wrote:
> I feel that this is not obvious, that settle will happen in `AWAIT_READY` 
> if clock is paused. Moreover, when we don't await, we have to explicitly 
> settle, which—I think—makes it harder for folks to follow the test and reason 
> when settle is necessary, because sometimes a necessary settle is implicit 
> (inside await). Hence I tend to put explicit settles when I feel this makes 
> the code more obvious. Do you have a strong preference?

Think about it this way, if you put a `settle()` before the `AWAIT_READY`, then 
the implication is that you don't need `AWAIT_READY` at all, you could just do 
`ASSERT_TRUE(f.isReady())` because you've done the necessary settling 
explicitly. This is why it seems strange to have a `settle` immediately preceed 
an `AWAIT_READY`, its the job of `AWAIT_READY` to wait until the future is 
ready (which means doing the right thing when the clock is paused, otherwise we 
would have to write a lot of `settle` calls).


> On March 18, 2016, 11:24 p.m., Ben Mahler wrote:
> > src/tests/slave_tests.cpp, line 3357
> > 
> >
> > Why do you resume?
> 
> Alexander Rukletsov wrote:
> I think we *should* resume the clock before exiting the test. I've heard 
> plans to even add a check in our test harness that verified the clock is not 
> paused.

This is similar to having to do an explicit Shutdown() at the end of the test. 
If the test fails, the Shutdown() (or Clock::resume()) better be called! 
Looking around it seems like we have this in a bunch of places, which is 
unfortunate because if an assertion fails the clock is left paused. I thought 
we ensured it was resumed but I can't find that code, so I'm ok with leaving 
this in.


- Ben


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


On March 22, 2016, 5:07 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44994/
> ---
> 
> (Updated March 22, 2016, 5:07 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4949
> https://issues.apache.org/jira/browse/MESOS-4949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 
> 
> Diff: https://reviews.apache.org/r/44994/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `make check`
> `GTEST_FILTER="*ExecutorShutdownGracePeriod*" ./bin/mesos-tests.sh 
> --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 45003: Fixed rmdir comment for FTS_SLNONE as per coding guidelines.

[Jie Yu]

> On March 20, 2016, 2:33 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > This comment seems incorrect:
> > 
> > (1) The relevant parameters are `FTS_PHYSICAL` vs. `FTS_LOGICAL`, as 
> > well as `FTS_COMFOLLOW`.
> > 
> > (2) Since we _do_ set `FTS_PHYSICAL`, isn't it possible for 
> > `FTS_SLNONE` to be returned?
> 
> Jojy Varghese wrote:
> I believe FTS_LOGICAL or FTS_COMFOLLOW needs to be set in order for 
> FTS_SLNONE to be returned.

Can you do a simple test to validate that (see if FTS_SLNONE is hit or not)? My 
understanding is that FTS_SLNONE will still be hit if we're using FTS_PHYSICAL.


- Jie


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


On March 18, 2016, 12:17 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45003/
> ---
> 
> (Updated March 18, 2016, 12:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed rmdir comment for FTS_SLNONE as per coding guidelines.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
> 
> Diff: https://reviews.apache.org/r/45003/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44854: Added validation for executor's shutdown grace period.

[Ben Mahler]

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


Fix it, then Ship it!




Thanks for moving the test!


src/tests/master_validation_tests.cpp (lines 1175 - 1176)


Why the explicit detector?


- Ben Mahler


On March 22, 2016, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44854/
> ---
> 
> (Updated March 22, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4949
> https://issues.apache.org/jira/browse/MESOS-4949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 
>   src/tests/master_validation_tests.cpp 
> 8d0070a1b8e8dcc7fe6360f8c6c6182ba9edef7d 
> 
> Diff: https://reviews.apache.org/r/44854/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `make check`
> `GTEST_FILTER="TaskValidationTest.ExecutorShutdownGracePeriodIsNonNegative" 
> ./bin/mesos-tests.sh --gtest_repeat=100 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>