Re: Review Request 45314: Updated `network::connect` to use the typeful `Try` error state.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45310, 45311, 45312, 45313, 45314]

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, 8:23 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45314/
> ---
> 
> (Updated March 24, 2016, 8:23 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/network.hpp 
> 9976257d2d13316062bc95a22ab564ca0df165e5 
>   3rdparty/libprocess/src/poll_socket.cpp 
> 6e6634b4b352e3723096521843546cf56ec6dd8b 
> 
> Diff: https://reviews.apache.org/r/45314/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

[Yong Tang]

> On March 25, 2016, 1:53 a.m., Anand Mazumdar wrote:
> > LGTM, just some minor code comments.

Thanks a lot for the review Anand. I just updated the review request with the 
fixes. Let me know if there are other issues.


- Yong


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


On March 25, 2016, 2:28 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45327/
> ---
> 
> (Updated March 25, 2016, 2:28 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5020
> https://issues.apache.org/jira/browse/MESOS-5020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix drops `404 Not Found` and `307 Temporary Redirect` in
> the scheduler library, instead of treating them as Event::ERROR.
> (MESOS-5020)
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 
> 
> Diff: https://reviews.apache.org/r/45327/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

[Yong Tang]

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

(Updated March 25, 2016, 2:28 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Address comment issues based on the feedback from Anand.


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


Repository: mesos


Description
---

This fix drops `404 Not Found` and `307 Temporary Redirect` in
the scheduler library, instead of treating them as Event::ERROR.
(MESOS-5020)


Diffs (updated)
-

  src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 

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


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

Re: Review Request 43935: Allow setting role in mesos-execute.

[Jian Qiu]

> On 三月 24, 2016, 7:57 p.m., Greg Mann wrote:
> > src/cli/execute.cpp, line 215
> > 
> >
> > s/sepcified/specified/

Sorry for the typo... please review again.


- Jian


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


On 三月 25, 2016, 1:54 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated 三月 25, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

[Jian Qiu]

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

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


Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.


Changes
---

Fix a typo in the comment.


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


Repository: mesos


Description
---

Allow setting role in mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

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


Testing
---

make & make check

start master.
./bin/mesos-master.sh --work_dir=/tmp/mesos

start slave.
./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
--resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"

running mesos-execute without specifying role succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --resources="cpus:1;mem:512"

running mesos-execute with role test succeeds.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test" --resources="cpus:2;mem:512"

running mesos-execute with role test1 fails.
./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
10" --role="test1" --resources="cpus:2;mem:512"


Thanks,

Jian Qiu

Re: Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

[Anand Mazumdar]

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


Fix it, then Ship it!




LGTM, just some minor code comments.


src/scheduler/scheduler.cpp (line 547)


Let's keep the wording consistent with the executor library.

```cpp
// This could happen if the master libprocess process has not yet set up 
HTTP routes.
```

Also was missing a period at the end.



src/scheduler/scheduler.cpp (line 554)


```cpp
// This could happen if the detector detects a new leading master before 
master itself realizes it (e.g., ZK watch delay).
```


- Anand Mazumdar


On March 24, 2016, 11:21 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45327/
> ---
> 
> (Updated March 24, 2016, 11:21 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5020
> https://issues.apache.org/jira/browse/MESOS-5020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix drops `404 Not Found` and `307 Temporary Redirect` in
> the scheduler library, instead of treating them as Event::ERROR.
> (MESOS-5020)
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 
> 
> Diff: https://reviews.apache.org/r/45327/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45304]

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, 6:42 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45304/
> ---
> 
> (Updated March 24, 2016, 6:42 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5015
> https://issues.apache.org/jira/browse/MESOS-5015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in executor.proto
> optional for the purpose of backwards compatibility. (MESOS-5015)
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 
> ae211194a44e0bf2fadc79e833881e45ea3eb2c2 
>   include/mesos/v1/executor/executor.proto 
> 36a2b3f9bc3aaa524f655b9e686a6d33512e6aaa 
>   src/examples/test_http_executor.cpp 
> 562b0acfd8555b9b773175f53defe0e7e2744641 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45304/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

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

[Jie Yu]

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




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


s/netInfoDir/networkInfoDir/



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


Can you include the 'networkInfoDir' in the error message?



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


No need for the dispatch here. You can just do:
```
foreachkey (const string& name, infos[.]->..) {
  futures.push_back(attach(containerId, name, target));
}
```

I would suggest we obtain NetworkInfo in 'attach' using containerId and 
name. We rarely use raw pointers.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 399 - 401)


YOu can combine these two statements:

```
return collect(futures)
  .then([]() { return Nothing(); });
```



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 410 - 420)


No need for this step. mkdir is recursive.



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


Why do you need to get os::environment()?



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


s/netConfig/networkConfig/



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 463 - 471)


We cannot read stdout and stderr after the process has finished. THis is 
becasuse the PIPE size is bounded. If the subprocess writes a lot of data, the 
write will block if there's no reader.

Please follow the same pattern in:
https://github.com/apache/mesos/blob/master/src/uri/fetchers/curl.cpp#L98


- Jie Yu


On March 23, 2016, 2: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, 2: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 45290: Added a test for the '/files/debug' endpoint.

[Greg Mann]

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

(Updated March 25, 2016, midnight)


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


Changes
---

Rebase.


Repository: mesos


Description
---

This endpoint previously had no test coverage, so a test was added that hits 
the endpoint and checks for the expected output.


Diffs (updated)
-

  src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e 

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


Testing
---

`GTEST_FILTER="FilesTest.AuthenticationTest" bin/mesos-tests.sh 
--gtest_repeat=1000 --gtest_break_on_failure=1` was used to test on OSX.


Thanks,

Greg Mann

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

[Greg Mann]

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

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


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


Changes
---

Removed the RAII wrapper :-) Manage the authenticator lifecycle in the test 
fixture setup/teardown.


Summary (updated)
-

Added a new '/files' endpoints test using authentication.


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


Repository: mesos


Description (updated)
---

A new test was added (FilesTest.AuthenticationTest) to the FilesTests, and 
existing tests were modified, to probe the endpoints' behavior with and without 
HTTP authentication.


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

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

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 24, 2016, 2:34 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 24, 2016, 2: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
> -
> 
>   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 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.
> 
> Jie Yu wrote:
> 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.
> 
> Steve Niemitz wrote:
> I thought about the inspect retry option, but it seems like it'll 
> introduce a lot more complexity (like you said, a timeout, etc).
> 
> How do you feel about commiting this w/ just the executor code path, and 
> fixing up the command task case in a new review?

Yeah, that sounds good to me.


- 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 44834: Documented task, executor, and volume IDs reuse is discouraged.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 15, 2016, 5:54 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44834/
> ---
> 
> (Updated March 15, 2016, 5:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2198
> https://issues.apache.org/jira/browse/MESOS-2198
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is generally a bad idea for (at least) two reasons:
> 
> (1) Determining when the previous entity with the ID has terminated
> can be non-trivial. Several frameworks have done this incorrectly
> in the past.
> 
> (2) When reusing IDs, logs can be more difficult to read because the
> same ID can refer to different entities at different times.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 56d456acfd35fa59f394b27d62f62772eec03f6a 
>   include/mesos/v1/mesos.proto 4fba77464bb052d27c424f7721397142850b1144 
> 
> Diff: https://reviews.apache.org/r/44834/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44500: Implemented passing image and user env var to command task.

[Gilbert Song]

> On March 16, 2016, 4:22 a.m., Benjamin Bannier wrote:
> > src/slave/containerizer/mesos/isolators/docker/runtime.cpp, line 160
> > 
> >
> > Does it makes sense to distinguish the sources of overwriting here and 
> > in l.179ff below?

They are different. This one is the environment from image, and the latter one 
is from the user. We have to prevent the case that image env are dulplicate 
themselves, and the case that image env duplicates user's.


- Gilbert


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


On March 24, 2016, 3:01 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44500/
> ---
> 
> (Updated March 24, 2016, 3:01 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing image and user env var to command task.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> f97a9a92895387a9d504810a2ae971cfb5d3dbb4 
> 
> Diff: https://reviews.apache.org/r/44500/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44500: Implemented passing image and user env var to command task.

[Gilbert Song]

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

(Updated March 24, 2016, 3:01 p.m.)


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


Repository: mesos


Description
---

Implemented passing image and user env var to command task.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
f97a9a92895387a9d504810a2ae971cfb5d3dbb4 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 45320: Made libprocess return "Accepted" for all libprocess requests.

[Neil Conway]

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

(Updated March 24, 2016, 9:58 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Tweak description.


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


Repository: mesos


Description (updated)
---

Previously, we returned "Accepted" only for libprocess requests that
were issued by non-libprocess clients (that is, client programs that
issued requests according to the libprocess protocol but aren't
themselves libprocess, such as mesos-go). The reason we didn't
return "Accepted" for requests sent by libprocess is that old
versions of libprocess (Mesos 0.18 and earlier) wouldn't handle such
responses correctly. Since we don't need to maintain compatibility
with such ancient versions of libprocess, we can now return
"Accepted" responses unconditionally.

Note that all recent versions of libprocess will ignore any response
that is received to a libprocess request, but that will change in
the near future.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4334422ec3964f8d079f570061eaf390d2 
  3rdparty/libprocess/src/tests/process_tests.cpp 
6b3aa1bcf20466cdf8f8249988b8b06dec27e5cd 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 45320: Made libprocess return "Accepted" for all libprocess requests.

[Neil Conway]

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

(Updated March 24, 2016, 9:57 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Previously, we returned "Accepted" only for libprocess requests that were issued
by non-libprocess clients (that is, client programs that issued requests
according to the libprocess protocol but aren't themselves libprocess, such as
mesos-go). The reason we didn't return "Accepted" for requests sent by
libprocess is that old versions of libprocess (Mesos 0.18 and earlier) wouldn't
handle such responses correctly. Since we don't need to maintain compatibility
with such ancient versions of libprocess, we can now return "Accepted" responses
unconditionally.

Note that all recent versions of libprocess will ignore any response that is
received to a libprocess request, but that will change in the near future.


Diffs
-

  3rdparty/libprocess/src/process.cpp feaffa4334422ec3964f8d079f570061eaf390d2 
  3rdparty/libprocess/src/tests/process_tests.cpp 
6b3aa1bcf20466cdf8f8249988b8b06dec27e5cd 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 44499: Added task_environment flag/used execvpe for command executor.

[Gilbert Song]

> On March 16, 2016, 4:22 a.m., Benjamin Bannier wrote:
> > src/launcher/executor.cpp, lines 470-471
> > 
> >
> > Any reason we cannot use the `CHECK` condition for the `if`?

Thanks, Benjamin! We do not always have taskEnvironment passed form runtime 
isolator, so it is not always ture.


- Gilbert


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


On March 24, 2016, 2:53 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44499/
> ---
> 
> (Updated March 24, 2016, 2:53 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4781
> https://issues.apache.org/jira/browse/MESOS-4781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added task_environment flag/used execvpe for command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 4e9b4d9820f7c2f4cb1b3e16e2f4c8c13500693f 
> 
> Diff: https://reviews.apache.org/r/44499/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 45320: Made libprocess return "Accepted" for all libprocess requests.

[Neil Conway]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Previously, we returned "Accepted" only for libprocess requests that were issued
by non-libprocess clients (that is, client programs that issued requests
according to the libprocess protocol but aren't themselves libprocess, such as
mesos-go). The reason we didn't return "Accepted" for requests sent by
libprocess is that old versions of libprocess (Mesos 0.18 and earlier) wouldn't
handle such responses correctly. Since we don't need to maintain compatibility
with such ancient versions of libprocess, we can now return "Accepted" responses
unconditionally.

Note that all recent versions of libprocess will ignore any response that is
received to a libprocess request, but that will change in the near future.


Diffs
-

  3rdparty/libprocess/src/process.cpp feaffa4334422ec3964f8d079f570061eaf390d2 
  3rdparty/libprocess/src/tests/process_tests.cpp 
6b3aa1bcf20466cdf8f8249988b8b06dec27e5cd 

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


Testing
---

make check


Thanks,

Neil Conway

Review Request 45319: Cleaned up various code in libprocess.

[Neil Conway]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Cleaned up various code in libprocess.


Diffs
-

  3rdparty/libprocess/src/process.cpp feaffa4334422ec3964f8d079f570061eaf390d2 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
e8bbc55f0f9aeabe7612a2ced5299cc01202b1f6 
  3rdparty/libprocess/src/tests/process_tests.cpp 
6b3aa1bcf20466cdf8f8249988b8b06dec27e5cd 

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


Testing
---

make check


Thanks,

Neil Conway

Review Request 45318: Added a comment to a utility function in libprocess.

[Neil Conway]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Added a comment to a utility function in libprocess.


Diffs
-

  3rdparty/libprocess/src/process.cpp feaffa4334422ec3964f8d079f570061eaf390d2 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 44499: Added task_environment flag/used execvpe for command executor.

[Gilbert Song]

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

(Updated March 24, 2016, 2:53 p.m.)


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


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


Repository: mesos


Description
---

Added task_environment flag/used execvpe for command executor.


Diffs (updated)
-

  src/launcher/executor.cpp 4e9b4d9820f7c2f4cb1b3e16e2f4c8c13500693f 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 44498: Forbid the executor to inherit from slave environment.

[Gilbert Song]

> On March 8, 2016, 10:02 p.m., Guangya Liu wrote:
> > The configuration.md should also be updated.
> 
> Gilbert Song wrote:
> Thanks, Guangya! Will do.
> 
> Note that this patch will be commited until 0.30 :)

Fixed. Thanks, Guangya!


- Gilbert


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


On March 24, 2016, 2:51 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44498/
> ---
> 
> (Updated March 24, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Forbid the executor to inherit from slave environment.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md b0f9cb53d4890f6de83ae4f3f07b5d51c66698fd 
>   src/slave/containerizer/containerizer.hpp 
> ff78b4d0fd4a3b862f6019fc757c16b7367cd3cf 
>   src/slave/containerizer/containerizer.cpp 
> 3556040c7f77dbf24436f14f3ce4a3d50733c88c 
>   src/slave/containerizer/docker.hpp 79cd955e9c241becff52cc4bbef81dcc16802ee7 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
>   src/slave/flags.cpp e831ce7748d87a05b4b862bb032b4ebc9a87621c 
> 
> Diff: https://reviews.apache.org/r/44498/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44498: Forbid the executor to inherit from slave environment.

[Gilbert Song]

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

(Updated March 24, 2016, 2:51 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Forbid the executor to inherit from slave environment.


Diffs (updated)
-

  docs/configuration.md b0f9cb53d4890f6de83ae4f3f07b5d51c66698fd 
  src/slave/containerizer/containerizer.hpp 
ff78b4d0fd4a3b862f6019fc757c16b7367cd3cf 
  src/slave/containerizer/containerizer.cpp 
3556040c7f77dbf24436f14f3ce4a3d50733c88c 
  src/slave/containerizer/docker.hpp 79cd955e9c241becff52cc4bbef81dcc16802ee7 
  src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
  src/slave/flags.cpp e831ce7748d87a05b4b862bb032b4ebc9a87621c 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 45317: Change Call and Event Type enums in scheduler.proto optional.

[Yong Tang]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This fix changes Call and Event Type enums in scheduler.proto
optional for the purpose of backwards compatibility. (MESOS-5014)


Diffs
-

  include/mesos/scheduler/scheduler.proto 
0049e1383f50574c3dad6a29b91811001694e82c 
  include/mesos/v1/scheduler/scheduler.proto 
09fafedd06837f2307fedc6fa0e7b4460b21e5b0 
  src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 

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


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

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, 8:33 p.m.)


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


Changes
---

Put the HTTP authenticator in an RAII wrapper.


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

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, 8:30 p.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
---

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 39021677389d7d0ed8bfd55f128d33041ed59522 
  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 543b2a7f3d1a4da28a5901fb238285cea6778c2b 

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 43911: Updated `/state` agent endpoint to use jsonify.

[Michael Park]

> On March 10, 2016, 7 p.m., Michael Park wrote:
> > src/slave/http.cpp, line 105
> > 
> >
> > If we were using `model` before, we need to maintain that.
> > 
> > We have a `json` defined for `CommandInfo` in `src/common/http.cpp`, we 
> > should add the declaration `void json(JSON::ObjectWriter* writer, const 
> > CommandInfo& command);` to `src/common/http.hpp`.
> > 
> > We can then use it like this:
> > 
> > ```
> > writer->field("command", task.command());
> > ```

This does not seem to have been addressed.


- Michael


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


On March 17, 2016, 9:33 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43911/
> ---
> 
> (Updated March 17, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/state` agent endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
> 
> Diff: https://reviews.apache.org/r/43911/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 45313: Introduced `WindowsSocketError` and factored out `WindowsErrorBase`.

[Michael Park]

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

Review request for mesos, Alex Clemmer and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
64102e1f31437d4271a1126e339fb2f33f0181b8 

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 45314: Updated `network::connect` to use the typeful `Try` error state.

[Michael Park]

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

Review request for mesos, Alex Clemmer and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/include/process/network.hpp 
9976257d2d13316062bc95a22ab564ca0df165e5 
  3rdparty/libprocess/src/poll_socket.cpp 
6e6634b4b352e3723096521843546cf56ec6dd8b 

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 45312: Captured the error code in `ErrnoError` and `WindowsError`.

[Michael Park]

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

Review request for mesos, Alex Clemmer and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/errorbase.hpp 
1e9db7e1cd2f411efb94893ba2c29bd0694e4ddc 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
64102e1f31437d4271a1126e339fb2f33f0181b8 

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


Testing
---

`make check`


Thanks,

Michael Park

Review Request 45310: Removed unnecessary constructors in `Try`.

[Michael Park]

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

Review request for mesos, Alex Clemmer and Joris Van Remoortere.


Repository: mesos


Description
---

Correctly constrained the templated constructor.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
c444c0118d39ee6a5da4618d7c62784464377280 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 43935: Allow setting role in mesos-execute.

[Greg Mann]

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


Fix it, then Ship it!





src/cli/execute.cpp (line 214)


s/sepcified/specified/


- Greg Mann


On March 22, 2016, 2:57 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 22, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

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

[Steve Niemitz]

> 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.
> 
> Jie Yu wrote:
> 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.

I thought about the inspect retry option, but it seems like it'll introduce a 
lot more complexity (like you said, a timeout, etc).

How do you feel about commiting this w/ just the executor code path, and fixing 
up the command task case in a new review?


- Steve


---
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 45280: Test upload binary file.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45280]

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, 11:10 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45280/
> ---
> 
> (Updated March 24, 2016, 11:10 a.m.)
> 
> 
> Review request for mesos and Zhiwei Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test upload binary file.
> 
> 
> Diffs
> -
> 
>   3rdparty/zookeeper-3.4.8.tar.gz PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45280/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Yong Tang]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This fix changes Call and Event Type enums in executor.proto
optional for the purpose of backwards compatibility. (MESOS-5015)


Diffs
-

  include/mesos/executor/executor.proto 
ae211194a44e0bf2fadc79e833881e45ea3eb2c2 
  include/mesos/v1/executor/executor.proto 
36a2b3f9bc3aaa524f655b9e686a6d33512e6aaa 
  src/examples/test_http_executor.cpp 562b0acfd8555b9b773175f53defe0e7e2744641 
  src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 

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


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

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

[Gilbert Song]

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



I am discarding these patches because they should be supported by network 
isolator. Thanks for reviewing guys!

- Gilbert Song


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 45278: Replace remain zookeeper version number.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On March 24, 2016, 9:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45278/
> ---
> 
> (Updated March 24, 2016, 9:38 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace remain zookeeper version number.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 49aa55741d76aa88c8fbb526f18908312bb0c717 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/versions.am c2dae2fb521b12344b93bf771dd5497ba8d446c3 
>   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/45278/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [37168]

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 22, 2016, 3:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 3:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 44993: Updated `TestContainerizer` to support default actions.

[Alexander Rukletsov]

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

(Updated March 24, 2016, 5:30 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

We use `ON_CALL` and `WillByDefault` here to specify the default
actions. This allows the tests to leverage the `DoDefault` action.
However, `ON_CALL` results in a "Uninteresting mock function call"
warning unless each test puts expectations in place. As a result,
we also use `EXPECT_CALL` and `WillRepeatedly` to get the best of
both worlds: the ability to use `DoDefault` and no warnings when
expectations are not explicit.


Diffs
-

  src/tests/containerizer.cpp c6772ce5908edaab6c3189a65e8446217d1c7c27 

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


Testing
---

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


Thanks,

Alexander Rukletsov

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

[Jiang Yan Xu]

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




src/slave/containerizer/mesos/isolators/disk/xfs/utils.cpp (line 305)


More strictly: `0u`.

Plus, it would be clearer is it is defined as a constant.

// A sentinel project ID that indicates that the file/directory is not 
associated with a valid project ID.
constexpr prid_t NON_PROJECT_ID = 0u;

As mentioned in another review, I think we can limit the use of this 
sentinel in the util.


- Jiang Yan Xu


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 44948: Add XFS disk resource isolator.

[Jiang Yan Xu]

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




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


IMO "xfs/disk" is probably better than "disk/xfs" (compared with 
"posix/disk") and "xfs/quota" is even better ("posix/quota" would have been 
more confusing but xfs being a filesystem, "xfs/quota" seem pretty clear to me 
what it means)



src/slave/containerizer/mesos/isolators/disk/xfs.hpp (line 68)


It fits in one line.



src/slave/containerizer/mesos/isolators/disk/xfs.hpp (line 80)


We usually don't omit argument names.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 95)


This method aborts early when a error occurs, I think it's useful to note 
this.

Plus, I still think what we need in the isolator is a generic filesystem 
walk functionality. I'll help with creating something that utilize `nftw`: 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/nftw.html



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 128)


`::dirfd` specially because you use dirfd as function arguments as well.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 156 - 157)


How does `openat` facilitate DFS better?



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 212 - 214)


This helper method tries to do two things and it continues to do the second 
even if the first fails but the error reflects the first if it fails.

It's a bit weird and we don't need this: we can do all this directly in:

```
Future XfsDiskIsolatorProcess::cleanup(
const ContainerID& containerId) {...}
```

And in `recover()` we can call `cleanup(containerId)` directly.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 247)


What's `nbytes`, number of bytes? We could simply call it `result`, to 
avoid name abbreviation.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 278)


We should check if the user is root and the filesystem `flags.work_dir` is 
on is XFS here.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 301)


To eliminate the need to use the sentinel value in the isolator we can add 
a 

```
bool validateProjectIds(IntervalSet);
```

to the utils.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 305)


s/process::Owned/Owned/



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 310 - 312)


Jie mentioned this too: it's customary for the isolators to take and store 
the flags directly, even if we have already derived `projectIds` from the flags.

For `workDir` because it can be used directly from `flags.work_dir` with no 
conversion, we'd just use that in recovery.

The argument for it is not strong but I would advocate for it.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 335)


Single quotes around `state.directory()`.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 337 - 352)


Jie mentioned this and I concur. The XFS API should interpret the sentinel 
value for us.

i.e.

```
Result getProjectId(directory);
```



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (line 350)


If this is not necessarily an error, I think LOG(WARNING) is more 
appropriate.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 366 - 367)


I don't think this is strictly true because the known orphans in the 
`orphans` list have not fully died yet and could in theory be manipulating 
files and causing race conditions that result in errors in cleanup operations.

I'd follow the commonly used pattern to cleanup only unknown orphans (i.e., 
sandboxes that are neither in `states` or `orphans`) here and leave the 
`orphans` cleanup for the containerizer once it successfully kills the cgroup.



src/slave/containerizer/mesos/isolators/disk/xfs.cpp (lines 381 - 382)


This is 

Re: Review Request 45185: Introduced an agent flag 'system_config_files'.

[Timothy Chen]

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




src/slave/flags.cpp (line 108)


I think we should also mention this only applies when image provisioner is 
enabled and used.


- Timothy Chen


On March 23, 2016, 12:20 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45185/
> ---
> 
> (Updated March 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
> ---
> 
> Introduced an agent flag 'system_config_files'.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 73ee8fa7a77a6ceaf44e1f4c54ab4027a7109258 
>   src/slave/flags.hpp 3067c128f1fded4f130f4d45f191584c2f30ad9c 
>   src/slave/flags.cpp ce028825ad99f54a231b4b18dde277b63aa0525c 
> 
> Diff: https://reviews.apache.org/r/45185/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Neil Conway]

> On March 23, 2016, 8:21 a.m., Deshi Xiao wrote:
> > can we ignore .md scan? the .md possible container non-ascii characters.

Are non-ASCII characters in .md files actually required? We should be able to 
use HTML character entities instead (e.g., ``) instead.


- Neil


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


On March 23, 2016, 2:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 23, 2016, 2:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent 
> huang, Neil Conway, and Deshi Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

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

[Timothy Chen]

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




src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 207)


We should use the path::absolute method here



src/slave/containerizer/mesos/isolators/docker/runtime.cpp (line 219)


Also log when we cannot find it.


- Timothy Chen


On March 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 March 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 43911: Updated `/state` agent endpoint to use jsonify.

[Michael Park]

> On March 24, 2016, 4:38 p.m., Michael Park wrote:
> > src/slave/http.cpp, line 96
> > 
> >
> > This should be marked `static`, right?

This is addressed in https://reviews.apache.org/r/44981/.


- Michael


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


On March 17, 2016, 9:33 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43911/
> ---
> 
> (Updated March 17, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/state` agent endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
> 
> Diff: https://reviews.apache.org/r/43911/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44981: Marked some private `json` functions as `static`.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On March 17, 2016, 9:33 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44981/
> ---
> 
> (Updated March 17, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Marked some private `json` functions as `static`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b47ab7cc86c0a56a81815a98bd63f37a1175ba7f 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
> 
> Diff: https://reviews.apache.org/r/44981/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44658: Removed unused signal escalation constant.

[Alexander Rukletsov]

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

(Updated March 24, 2016, 4:15 p.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/slave/constants.hpp d096cfec26aef16abbb3f9ef773373b765a22f68 

Diff: https://reviews.apache.org/r/44658/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 24, 2016, 4:18 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 (updated)
-

  include/mesos/mesos.proto 59f5d3aadd1cb32c1eb869451ad55d877bdeee28 
  include/mesos/v1/mesos.proto 6556a1c243d227fdb9af0d3c1063fed08d3f6c8b 
  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 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 24, 2016, 4:15 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 44854: Added validation for executor's shutdown grace period.

[Alexander Rukletsov]

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

(Updated March 24, 2016, 4:14 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/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 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 24, 2016, 4:15 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 (updated)
-

  src/launcher/executor.cpp 2df62f09637b55c63ae6fe5d5a70b8debc02fbe2 
  src/slave/slave.cpp 08ff1ed12e1da72d957b1f2dc726796137e72cda 
  src/tests/slave_tests.cpp 5f47a715b9ee6c9f552854a228aca376761f1155 

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 (UUID: 

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 24, 2016, 4:15 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 59f5d3aadd1cb32c1eb869451ad55d877bdeee28 
  include/mesos/v1/mesos.proto 6556a1c243d227fdb9af0d3c1063fed08d3f6c8b 

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 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 24, 2016, 4:15 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 bc68cf1e82cf2b1a64b8c8a2f923ccf94a4cb229 
  src/tests/slave_tests.cpp 5f47a715b9ee6c9f552854a228aca376761f1155 

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 44991: Enabled mocking on `TestContainerizer::destroy`.

[Alexander Rukletsov]

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

(Updated March 24, 2016, 4:14 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/containerizer.hpp 67fbe7fedbe170c3f22a2dcbb5aebf4195a5aabc 
  src/tests/containerizer.cpp c6772ce5908edaab6c3189a65e8446217d1c7c27 

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


Testing
---

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


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 24, 2016, 4:14 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 70686adeca239b5962652a17661c8b0d44ca6708 
  include/mesos/executor/executor.proto 
ae211194a44e0bf2fadc79e833881e45ea3eb2c2 
  include/mesos/mesos.proto 59f5d3aadd1cb32c1eb869451ad55d877bdeee28 
  include/mesos/v1/executor/executor.proto 
36a2b3f9bc3aaa524f655b9e686a6d33512e6aaa 
  include/mesos/v1/mesos.proto 6556a1c243d227fdb9af0d3c1063fed08d3f6c8b 
  src/slave/containerizer/containerizer.cpp 
f6fc7863d0c215611f170dc0c89aa229407b5137 
  src/slave/flags.cpp 71685cee58322e608139f7c344c2e954f7e14322 
  src/slave/slave.cpp 08ff1ed12e1da72d957b1f2dc726796137e72cda 

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 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 24, 2016, 4:13 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 (updated)
-

  CHANGELOG 1fadaa1819c9b1a801752b1c0ddd572b82e6c170 
  docs/configuration.md 70686adeca239b5962652a17661c8b0d44ca6708 
  src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
  src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
  src/slave/flags.hpp 9ee7f349cc2bb3fa76141645f4a06fad57664367 
  src/slave/flags.cpp 71685cee58322e608139f7c344c2e954f7e14322 

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 45270: Added spec protobut for external mount.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45214, 45217, 45265, 45270]

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, 7:04 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45270/
> ---
> 
> (Updated March 24, 2016, 7:04 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added spec protobut for external mount.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0bd7a978306488b687a7e2eeeb8a5c9766d43548 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/isolators/docker/dvd/spec.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/dvd/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45270/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44084: Windows: Forked os::rename().

[Alex Clemmer]

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

(Updated March 24, 2016, 2:59 p.m.)


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


Repository: mesos


Description
---

Windows: Forked os::rename().


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
7bd4bfbc2ec5922879dcefddc12137336b11be52 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rename.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/rename.hpp 
7b2215208bcaa5613426d8ede9cab21d1a084631 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rename.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 42551: Porting mesos on ppc64le.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42551]

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, 5:10 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42551/
> ---
> 
> (Updated March 24, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4312
> https://issues.apache.org/jira/browse/MESOS-4312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Porting mesos on ppc64le.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp 0df194207a66bf4c417349bebcc8912acbf5976e 
> 
> Diff: https://reviews.apache.org/r/42551/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX 10.10.5, Ubuntu 14.04.3 LTS ppc64le)
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 43883: 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/43883/
---

(Updated March 24, 2016, 2:36 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  docs/monitoring.md dcf19e6ce06b02373a2bd1af81a451a35743fa76 
  src/master/allocator/mesos/hierarchical.hpp 
e4604f4da59166da24709a68b8cd4e56bf55f97f 
  src/master/allocator/mesos/hierarchical.cpp 
39a290d0db2c22e179a8f933b1a78e3a2dcefdc3 
  src/master/allocator/mesos/metrics.hpp 
b5f2806cff99ee2ee46c4ac8a13174ef699969aa 
  src/master/allocator/mesos/metrics.cpp 
2f3012a5bd40a4100e790e1d373832015c80b993 
  src/tests/hierarchical_allocator_tests.cpp 
e9cfcfc0ad8b0b89bbac459b7db39183f6c189be 

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


Testing
---

make check (OS X)

I confirmed that this does not lead to general performance regressions in the 
allocator; this is partially expected since the added code only inserts metrics 
in the allocator while the actual work is perform asynchronously. These tests 
where performed with `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an 
optimized build under OS X using clang(trunk) as compiler.


Thanks,

Benjamin Bannier

Re: Review Request 45082: Implemented cleanup() method of "network/cni" isolator.

[Qian Zhang]

> On March 22, 2016, 11:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, line 151
> > 
> >
> > I guess if we are going to use `attach` for connecting to networks, 
> > lets use `detach` over here.

Agree.


> On March 22, 2016, 11:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 647-649
> > 
> >
> > This is a bit odd, __disconnect always returns an error ? The plugin 
> > can return error codes and error logs which we should be propagating 
> > upstream through failure semantics.

The reason that `__disconnect()` always returns a `Failure` is, it will be only 
called when the exit code of plugin is not 0 (i.e., plugin fails for a reason), 
in case of plugin success, we will return `Nothing()` in `_disconnect()`. So we 
only need `__disconnect()` in case of plugin failure to get the output (i.e., 
detailed failure reason) of plugin and propagate it upstream.


> On March 22, 2016, 11:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 653
> > 
> >
> > Why are we returning a Future over here? We should be 
> > returning a Future there is no list since you are deletnig the 
> > entire container directory in one shot. This seems a bit odd.

I tend to agree with you, however, I see CPU share isolator does the similar 
thing: 
https://github.com/apache/mesos/blob/0.28.0/src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp#L536:L547,
 maybe we should change it as well?


> On March 22, 2016, 11:37 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 644-645
> > 
> >
> > These two CHECKS don't make sense. What if the plugin got deleted or 
> > there was a bug in the plugin because of which it wasn't able to delete the 
> > interfaces or release the IP addresses. The Agent should not die because of 
> > an error in a 3rd party plugin.

If the plugin got deleted, then `Subprocess.isError()` will be true and we will 
return a `Failure`, if there was a bug in the plugin, then 
`Subprocess.status()` will be non-zero and we will read its output for the 
detailed error message. For either of these cases, agent will not die.


- Qian


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


On March 21, 2016, 12:30 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45082/
> ---
> 
> (Updated March 21, 2016, 12:30 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 cleanup() method of "network/cni" isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45082/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 45280: Test upload binary file.

[haosdent huang]

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

(Updated March 24, 2016, 11:08 a.m.)


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload binary file.


Diffs (updated)
-

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

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45280: Test upload binary file.

[haosdent huang]

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

(Updated March 24, 2016, 11:03 a.m.)


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload binary file.


Diffs
-

  3rdparty/pip-7.1.2.tar.gz 56ead414cfdbcb9224877c2c070251e9d8feaaa0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44846: Deprecated the plain text credential format.

[Jan Schlicht]

> On March 17, 2016, 6:21 p.m., Joerg Schad wrote:
> > src/master/flags.cpp, line 227
> > 
> >
> > We now mention deprecated twice here.
> > The 'Note that' also seems to be out of sync with the configuration.md.

The whole sentence will be removed, as per Adam's comments below the deprecated 
format won't be mentioned at all.


- Jan


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


On March 15, 2016, 3:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44846/
> ---
> 
> (Updated March 15, 2016, 3:01 p.m.)
> 
> 
> Review request for mesos, Adam B and Joerg Schad.
> 
> 
> Bugs: MESOS-2281
> https://issues.apache.org/jira/browse/MESOS-2281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated the plain text credential format.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
>   docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
>   docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
>   src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 
>   src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 
>   src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
> 
> Diff: https://reviews.apache.org/r/44846/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44255]

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:54 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 24, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4492
> https://issues.apache.org/jira/browse/MESOS-4492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.
> 
> 
> Diffs
> -
> 
>   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 45280: Test upload binary file.

[haosdent huang]

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

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


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload binary file.


Diffs (updated)
-

  3rdparty/pip-7.1.2.tar.gz 56ead414cfdbcb9224877c2c070251e9d8feaaa0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45280: Test upload binary file.

[haosdent huang]

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

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


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload binary file.


Diffs (updated)
-

  3rdparty/pip-7.1.2.tar.gz 56ead414cfdbcb9224877c2c070251e9d8feaaa0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45280: Test upload binary file.

[haosdent huang]

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

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


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload binary file.


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/45280/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 44448: Replaced EXIT(1) with EXIT(EXIT_FAILURE).

[Alexander Rukletsov]

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

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


Review request for mesos and Bernd Mathiske.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/examples/balloon_framework.cpp 16eea37ee327c6da1db96a6f4e3e596be5a7a6ad 
  src/examples/docker_no_executor_framework.cpp 
cf7b4e1b4a27a3a6d484b1e205b5ca1c3ec583e1 
  src/examples/load_generator_framework.cpp 
7d64b6617564f43ef383ee60d92da92b2c958c47 
  src/examples/long_lived_framework.cpp 
289a0b9dd3d1ce30f20dd9bb381126bff30c 
  src/examples/test_framework.cpp c9695c4aa20ab8e726ad47b2ebbd3f10777cf828 
  src/examples/test_http_executor.cpp 55a427fb240fdfa018ac70fa3af5a6654cb71979 
  src/examples/test_http_framework.cpp 64c509717b996085ff029c7bfbf0f1a914a29f80 
  src/executor/executor.cpp 48b00504cac83aa0f4b1fc0b81480d3010edebae 
  src/jvm/jvm.cpp 909d34a0112b219456dce76126029c603077e66d 
  src/launcher/fetcher.cpp 28dbb98e4610cb4e2e8d3b84ab1d5ee4d930e0f9 
  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/logging/logging.cpp 8d9e4e9b200a0df1c67d4e7cd57107b7780f9812 
  src/master/master.cpp 32914a2de324e85df4806d446554b3fbfd0824a9 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/scheduler/scheduler.cpp 6a834473ef35540eedac7e211b5204ab5f4eb7b2 
  src/slave/containerizer/mesos/launcher.cpp 
c4ae1c5c319f9def2f18bafbad102ddf25d3c9f4 
  src/slave/containerizer/mesos/provisioner/store.cpp 
8324269c786d2894a4c009428f1a6ec381c209e5 
  src/slave/slave.cpp 6954f86def6aefd7e1718634435722a0ad34de1b 
  src/tests/cluster.cpp 22167da70a855a39fd9c3ca980304372c70bd8d3 
  src/tests/containerizer/cgroups_tests.cpp 
acaed9b3f8a04964092cef413133834d0cf5a145 
  src/tests/environment.cpp ee59a09c44242a2d31ff539106edbcb316b120aa 

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


Testing
---

`make check` on Mac OS 10.10.4


Thanks,

Alexander Rukletsov

Re: Review Request 44447: Replaced EXIT(1) with EXIT(EXIT_FAILURE) in libprocess.

[Alexander Rukletsov]

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

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


Review request for mesos and Bernd Mathiske.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/include/process/gmock.hpp 
15b982f101994ce717ceeb29a1bd0028bd2ba940 

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


Testing
---

Chain tested in https://reviews.apache.org/r/8/


Thanks,

Alexander Rukletsov

Re: Review Request 44446: Replaced EXIT(1) with EXIT(EXIT_FAILURE) in stout.

[Alexander Rukletsov]

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

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


Review request for mesos and Bernd Mathiske.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/exit.hpp 
061e167c8ee036298d14aebfa5ffbe5601e54bb3 
  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
96ffc427007b3b1447d291bba0d029646118d4b3 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
8d8ed0b85c5e281e686b23acae202cfd2d1ffb33 

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


Testing
---

Chain tested in https://reviews.apache.org/r/8/


Thanks,

Alexander Rukletsov

Review Request 45280: Test upload binary file.

[haosdent huang]

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

Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload binary file.


Diffs
-

  3rdparty/pip-7.1.2.tar.gz 56ead414cfdbcb9224877c2c070251e9d8feaaa0 

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


Testing
---


Thanks,

haosdent huang

Review Request 45278: Replace remain zookeeper version number.

[haosdent huang]

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

Review request for mesos.


Repository: mesos


Description
---

Replace remain zookeeper version number.


Diffs
-

  3rdparty/Makefile.am 49aa55741d76aa88c8fbb526f18908312bb0c717 
  3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
  3rdparty/versions.am c2dae2fb521b12344b93bf771dd5497ba8d446c3 
  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/45278/diff/


Testing
---


Thanks,

haosdent huang

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

[haosdent huang]

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

Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Add zookeeper-3.4.8.tar.gz.


Diffs
-

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

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


Testing
---


Thanks,

haosdent huang

Review Request 45276: Remove zookeeper-3.4.5.tar.gz.

[haosdent huang]

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

Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Remove zookeeper-3.4.5.tar.gz.


Diffs
-

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

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45037: Regenerated agent endpoint documentation.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 24, 2016, 2:07 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45037/
> ---
> 
> (Updated March 24, 2016, 2:07 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reran the generate-endpoint-help.py scipt after adding the AUTHENTICATION 
> information to HELP.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/api/v1/executor.md 
> 7fa7cdaeac1e9940fb9d8d6b58a96e360ad81b40 
>   docs/endpoints/slave/flags.md 44cbf1f3f99e9ce7af16a57d1391d24ceff7de5e 
>   docs/endpoints/slave/health.md db907e05e213a3710b5d6d9cd3c7ba1c9d959ee1 
>   docs/endpoints/slave/state.json.md b0bd1ada735c0b807b9184a1dba72576394f0218 
>   docs/endpoints/slave/state.md 3bab316872afdd79a3af79cd81ea3b7817b9bb38 
> 
> Diff: https://reviews.apache.org/r/45037/diff/
> 
> 
> Testing
> ---
> 
> Viewed rendered page via docker website  container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45037: Regenerated agent endpoint documentation.

[Joerg Schad]

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

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


Review request for mesos, Adam B and Greg Mann.


Changes
---

Regenerated.


Repository: mesos


Description
---

Reran the generate-endpoint-help.py scipt after adding the AUTHENTICATION 
information to HELP.


Diffs (updated)
-

  docs/endpoints/slave/api/v1/executor.md 
7fa7cdaeac1e9940fb9d8d6b58a96e360ad81b40 
  docs/endpoints/slave/flags.md 44cbf1f3f99e9ce7af16a57d1391d24ceff7de5e 
  docs/endpoints/slave/health.md db907e05e213a3710b5d6d9cd3c7ba1c9d959ee1 
  docs/endpoints/slave/state.json.md b0bd1ada735c0b807b9184a1dba72576394f0218 
  docs/endpoints/slave/state.md 3bab316872afdd79a3af79cd81ea3b7817b9bb38 

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


Testing
---

Viewed rendered page via docker website  container.


Thanks,

Joerg Schad

Re: Review Request 45037: Regenerated agent endpoint documentation.

[Joerg Schad]

> On March 21, 2016, 10:30 a.m., Adam B wrote:
> > docs/endpoints/slave/health.md, lines 17-18
> > 
> >
> > Why the double-blank line here (and elsewhere)?

This is caused by the way the Description help is generated. We could change 
this, but I feel a) not an issue as it won't show up in the rendered help anb 
b) is not relevant to this review.
https://github.com/apache/mesos/commit/4209ad61c81a49f3e8f31689fccc20d0473878b6#diff-649de92701805e2ffac5604eef155befR59


- Joerg


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


On March 18, 2016, 4:53 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45037/
> ---
> 
> (Updated March 18, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reran the generate-endpoint-help.py scipt after adding the AUTHENTICATION 
> information to HELP.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/api/v1/scheduler.md 
> 7a75fd75eef458f26f9e5067b792329f310d5d24 
>   docs/endpoints/slave/api/v1/executor.md 
> 7fa7cdaeac1e9940fb9d8d6b58a96e360ad81b40 
>   docs/endpoints/slave/flags.md 44cbf1f3f99e9ce7af16a57d1391d24ceff7de5e 
>   docs/endpoints/slave/health.md db907e05e213a3710b5d6d9cd3c7ba1c9d959ee1 
>   docs/endpoints/slave/state.json.md b0bd1ada735c0b807b9184a1dba72576394f0218 
>   docs/endpoints/slave/state.md 3bab316872afdd79a3af79cd81ea3b7817b9bb38 
> 
> Diff: https://reviews.apache.org/r/45037/diff/
> 
> 
> Testing
> ---
> 
> Viewed rendered page via docker website  container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44376: Upgrade zookeeper to 3.4.8 to support PowerPC LE platform.

[Zhiwei Chen]

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

(Updated March 24, 2016, 4:58 p.m.)


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


Summary (updated)
-

Upgrade zookeeper to 3.4.8 to support PowerPC LE platform.


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


Repository: mesos


Description (updated)
---

Upgrade zookeeper to 3.4.8 to support PowerPC LE platform.


Diffs (updated)
-

  3rdparty/Makefile.am 49aa55741d76aa88c8fbb526f18908312bb0c717 
  3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
  3rdparty/versions.am c2dae2fb521b12344b93bf771dd5497ba8d446c3 
  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
  3rdparty/zookeeper-3.4.5.tar.gz 1a547fe17a6fad86012f855d3c4cc38fed4899fc 
  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/44376/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 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, 8:52 a.m.)


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload zookeeper tar.


Diffs (updated)
-

  3rdparty/Makefile.am 49aa557 
  3rdparty/cmake/Versions.cmake 24490d3 
  3rdparty/versions.am c2dae2f 
  3rdparty/zookeeper-3.4.5.patch 3ca180d 
  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, 8:46 a.m.)


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload zookeeper tar.


Diffs (updated)
-

  3rdparty/Makefile.am 49aa557 
  3rdparty/cmake/Versions.cmake 24490d3 
  3rdparty/versions.am c2dae2f 
  3rdparty/zookeeper-3.4.5.patch 3ca180d 
  3rdparty/zookeeper-3.4.5.tar.gz 1a547fe17a6fad86012f855d3c4cc38fed4899fc 
  src/examples/java/test-log.in 4c8547a 
  src/python/native_common/ext_modules.py.in c335bd8 
  src/tests/zookeeper_test_server.cpp 0dc041f 

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, 8:44 a.m.)


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload zookeeper tar.


Diffs (updated)
-

  3rdparty/Makefile.am 49aa557 
  3rdparty/cmake/Versions.cmake 24490d3 
  3rdparty/versions.am c2dae2f 
  3rdparty/zookeeper-3.4.5.patch 3ca180d 
  3rdparty/zookeeper-3.4.5.tar.gz 1a547fe17a6fad86012f855d3c4cc38fed4899fc 
  src/examples/java/test-log.in 4c8547a 
  src/python/native_common/ext_modules.py.in c335bd8 
  src/tests/zookeeper_test_server.cpp 0dc041f 

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, 8:40 a.m.)


Review request for mesos and Zhiwei Chen.


Repository: mesos


Description
---

Test upload zookeeper tar.


Diffs (updated)
-

  3rdparty/Makefile.am 49aa557 
  3rdparty/cmake/Versions.cmake 24490d3 
  3rdparty/versions.am c2dae2f 
  3rdparty/zookeeper-3.4.5.patch 3ca180d 
  3rdparty/zookeeper-3.4.5.tar.gz 1a547fe17a6fad86012f855d3c4cc38fed4899fc 
  src/examples/java/test-log.in 4c8547a 
  src/python/native_common/ext_modules.py.in c335bd8 
  src/tests/zookeeper_test_server.cpp 0dc041f 

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


Testing
---


Thanks,

haosdent huang

Re: 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/
---

(Updated March 24, 2016, 8:39 a.m.)


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 44511: Add registry tests for /weights endpoint.

[Yongqiao Wang]

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

(Updated March 24, 2016, 8:35 a.m.)


Review request for mesos and Adam B.


Changes
---

Adressed comments of Adam.


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


Repository: mesos


Description
---

Add registry tests for /weights endpoint.


Diffs (updated)
-

  src/tests/dynamic_weights_tests.cpp ee0c4b1cc20e76a35a8e4e445f6827a1fc33e6c6 
  src/tests/registrar_tests.cpp c330af2a190282e159d9ab477cdc36a7881842cd 

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


Testing
---

Add three sub tests for /weights endpoint registry in this patch:

1. Tests whether the weights replicated log is initialized with the `--weights` 
flag when bootstrapping the cluster.
2. Tests whether the weights replicated log can be updated with `/weights` 
endpoint.
3. Tests whether the `--weights` flag is ignored and use the registry value 
instead when Mesos master subsequently starts with `--weights` flag still 
specified.

./src/mesos-tests --gtest_filter=Strict/RegistrarTest.UpdateWeights/*
[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from Strict/RegistrarTest
[ RUN  ] Strict/RegistrarTest.UpdateWeights/0
[   OK ] Strict/RegistrarTest.UpdateWeights/0 (103 ms)
[ RUN  ] Strict/RegistrarTest.UpdateWeights/1
[   OK ] Strict/RegistrarTest.UpdateWeights/1 (67 ms)
[--] 2 tests from Strict/RegistrarTest (171 ms total)

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


make && make check.


Thanks,

Yongqiao Wang

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Klaus Ma]

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



ping @mcypark, would you help to review it again?

- Klaus Ma


On March 22, 2016, 11:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 45015: Windows: Fixed bug causing `os::exists` to report invalid paths exist.

[Alex Clemmer]

> On March 21, 2016, 9:13 p.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp, lines 
> > 146-166
> > 
> >
> > Which of these are setting up the test and therefore remain `ASSERT_*` 
> > and which are actually the test cases, which should be converted to 
> > `EXPECT_*`?

Changed commit to clarify. Thanks!


- Alex


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


On March 24, 2016, 7:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45015/
> ---
> 
> (Updated March 24, 2016, 7:58 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently on Windows, `os::exists` will return true if a component of a
> path does not exist. For example if you have `a/fancy/path`, and you ask
> `os::exists("a/fake/path")`, the result currently reports `true`. In
> other words, the Windows code path only checks for the error that a file
> does not exist, and ignores the error that says the path is not valid.
> 
> This commit will fix this, and also add a test that will verify we don't
> regress.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp 
> 9211851e4562e04045276421b359c3c78cdae7f1 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/45015/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45015: Windows: Fixed bug causing `os::exists` to report invalid paths exist.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

Currently on Windows, `os::exists` will return true if a component of a
path does not exist. For example if you have `a/fancy/path`, and you ask
`os::exists("a/fake/path")`, the result currently reports `true`. In
other words, the Windows code path only checks for the error that a file
does not exist, and ignores the error that says the path is not valid.

This commit will fix this, and also add a test that will verify we don't
regress.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp 
9211851e4562e04045276421b359c3c78cdae7f1 
  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 44511: Add registry tests for /weights endpoint.

[Adam B]

> On March 21, 2016, 1:35 a.m., Adam B wrote:
> > Need to get rid of the unnecessary stringify/atof translation, but I like 
> > the new low-level registry Operation test.
> 
> Yongqiao Wang wrote:
> Adam, cloud you help to review and commit this patch firstly, then I will 
> update the all tests in `dynamic_weights_tests.cpp` to checkout the updated 
> results with /weights endpoint rather than /roles endpoint.

Of course. Updating tests to GET /weights before /roles has to happen after we 
get the GET /weights endpoint committed anyway. This can go in asap.


- Adam


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


On March 22, 2016, 12:22 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44511/
> ---
> 
> (Updated March 22, 2016, 12:22 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4797
> https://issues.apache.org/jira/browse/MESOS-4797
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add registry tests for /weights endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/dynamic_weights_tests.cpp 
> ee0c4b1cc20e76a35a8e4e445f6827a1fc33e6c6 
>   src/tests/registrar_tests.cpp c330af2a190282e159d9ab477cdc36a7881842cd 
> 
> Diff: https://reviews.apache.org/r/44511/diff/
> 
> 
> Testing
> ---
> 
> Add three sub tests for /weights endpoint registry in this patch:
> 
> 1. Tests whether the weights replicated log is initialized with the 
> `--weights` flag when bootstrapping the cluster.
> 2. Tests whether the weights replicated log can be updated with `/weights` 
> endpoint.
> 3. Tests whether the `--weights` flag is ignored and use the registry value 
> instead when Mesos master subsequently starts with `--weights` flag still 
> specified.
> 
> ./src/mesos-tests --gtest_filter=Strict/RegistrarTest.UpdateWeights/*
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from Strict/RegistrarTest
> [ RUN  ] Strict/RegistrarTest.UpdateWeights/0
> [   OK ] Strict/RegistrarTest.UpdateWeights/0 (103 ms)
> [ RUN  ] Strict/RegistrarTest.UpdateWeights/1
> [   OK ] Strict/RegistrarTest.UpdateWeights/1 (67 ms)
> [--] 2 tests from Strict/RegistrarTest (171 ms total)
> 
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (178 ms total)
> [  PASSED  ] 2 tests.
> 
> 
> make && make check.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44511: Add registry tests for /weights endpoint.

[Adam B]

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


Fix it, then Ship it!




Looks great! I'd move the weightInfo setup right before it's used, but that's 
trivial enough I can do it before committing.


src/tests/registrar_tests.cpp (lines 912 - 915)


I would move this below the "prepare the registrar" step, just before the 
`registrar.apply()`



src/tests/registrar_tests.cpp (lines 929 - 933)


Ditto. Don't confuse me with new weights when you're about to check that 
the old weights are still there after recovery.


- Adam B


On March 22, 2016, 12:22 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44511/
> ---
> 
> (Updated March 22, 2016, 12:22 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4797
> https://issues.apache.org/jira/browse/MESOS-4797
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add registry tests for /weights endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/dynamic_weights_tests.cpp 
> ee0c4b1cc20e76a35a8e4e445f6827a1fc33e6c6 
>   src/tests/registrar_tests.cpp c330af2a190282e159d9ab477cdc36a7881842cd 
> 
> Diff: https://reviews.apache.org/r/44511/diff/
> 
> 
> Testing
> ---
> 
> Add three sub tests for /weights endpoint registry in this patch:
> 
> 1. Tests whether the weights replicated log is initialized with the 
> `--weights` flag when bootstrapping the cluster.
> 2. Tests whether the weights replicated log can be updated with `/weights` 
> endpoint.
> 3. Tests whether the `--weights` flag is ignored and use the registry value 
> instead when Mesos master subsequently starts with `--weights` flag still 
> specified.
> 
> ./src/mesos-tests --gtest_filter=Strict/RegistrarTest.UpdateWeights/*
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from Strict/RegistrarTest
> [ RUN  ] Strict/RegistrarTest.UpdateWeights/0
> [   OK ] Strict/RegistrarTest.UpdateWeights/0 (103 ms)
> [ RUN  ] Strict/RegistrarTest.UpdateWeights/1
> [   OK ] Strict/RegistrarTest.UpdateWeights/1 (67 ms)
> [--] 2 tests from Strict/RegistrarTest (171 ms total)
> 
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (178 ms total)
> [  PASSED  ] 2 tests.
> 
> 
> make && make check.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

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

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44372]

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

Error:
2016-03-24 07:16:51 URL:https://reviews.apache.org/r/44372/diff/raw/ 
[104639/104639] -> "44372.patch" [1]
Total errors found: 0
Checking 1 files
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
libprocess:
  3rdparty/libprocess/3rdparty/CMakeLists.txt
  3rdparty/libprocess/3rdparty/Makefile.am
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz
  3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch
  3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz
  3rdparty/libprocess/3rdparty/versions.am
  3rdparty/libprocess/Makefile.am
  3rdparty/libprocess/src/decoder.hpp
mesos:
  3rdparty/cmake/Versions.cmake
  LICENSE

Full log: https://builds.apache.org/job/mesos-reviewbot/12121/console

- Mesos ReviewBot


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 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, 7:04 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/dvd.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/dvd/dvd.cpp PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Review Request 45270: Added spec protobut for external mount.

[Guangya Liu]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added spec protobut for external mount.


Diffs
-

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

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


Testing
---


Thanks,

Guangya Liu

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, 7:04 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 (updated)
-

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

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


Testing
---


Thanks,

Guangya Liu

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