Re: Review Request 44475: Improve master slaves metrics.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44473, 44474, 44475]

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 7, 2016, 10:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44475/
> ---
> 
> (Updated March 7, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44475/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44441: Treated command as executable value and arguments in mesos-execute.

[Guangya Liu]

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

(Updated 三月 8, 2016, 5:17 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Treated command as executable value and arguments in mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

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


Testing
---

./src/mesos-execute --master=192.168.56.12:5050  --name=test_mesos 
--docker_image=busybox:latest --containerizer=mesos --no-shell  
--command="ls,/etc/passwd" 
I0307 15:48:09.834506 27450 sched.cpp:222] Version: 0.29.0
I0307 15:48:09.841404 27468 sched.cpp:326] New master detected at 
master@192.168.56.12:5050
I0307 15:48:09.843992 27468 sched.cpp:336] No credentials provided. Attempting 
to register without authentication
I0307 15:48:09.848901 27468 sched.cpp:703] Framework registered with 
a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
Framework registered with a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
task test_mesos submitted to slave 6fa2afa1-768b-4f2e-9c69-9f1017634e72-S2
Received status update TASK_RUNNING for task test_mesos
Received status update TASK_FINISHED for task test_mesos
I0307 15:48:10.284418 27466 sched.cpp:1903] Asked to stop the driver
I0307 15:48:10.284494 27466 sched.cpp:1143] Stopping framework 
'a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001'


Thanks,

Guangya Liu

Re: Review Request 44441: Treated command as executable value and arguments in mesos-execute.

[Guangya Liu]

> On 三月 7, 2016, 4:48 p.m., Gilbert Song wrote:
> > Hi Guangya, Thanks for taking this on. My main consideration about this 
> > patch is making distinction between shell command v.s. non-shell command. 
> > We have two choices:
> > 
> > 1. Following your current implementation, the description of the `command` 
> > flag should be updated, because people may not know they should use 
> > `--command=value,argv1,argv1` as non-shell command.
> > 2. Or we should make user specifying `value` and `argv` explicitly, by 
> > making them as flags.
> > 
> > We should find out which is better.

I think that option 1 is better as it does not introduce new parameter and also 
easy to be extended to support both shell and no shell command


- Guangya


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


On 三月 7, 2016, 8:05 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1/
> ---
> 
> (Updated 三月 7, 2016, 8:05 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4882
> https://issues.apache.org/jira/browse/MESOS-4882
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Treated command as executable value and arguments in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/1/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-execute --master=192.168.56.12:5050  --name=test_mesos 
> --docker_image=busybox:latest --containerizer=mesos --no-shell  
> --command="ls,/etc/passwd" 
> I0307 15:48:09.834506 27450 sched.cpp:222] Version: 0.29.0
> I0307 15:48:09.841404 27468 sched.cpp:326] New master detected at 
> master@192.168.56.12:5050
> I0307 15:48:09.843992 27468 sched.cpp:336] No credentials provided. 
> Attempting to register without authentication
> I0307 15:48:09.848901 27468 sched.cpp:703] Framework registered with 
> a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
> Framework registered with a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
> task test_mesos submitted to slave 6fa2afa1-768b-4f2e-9c69-9f1017634e72-S2
> Received status update TASK_RUNNING for task test_mesos
> Received status update TASK_FINISHED for task test_mesos
> I0307 15:48:10.284418 27466 sched.cpp:1903] Asked to stop the driver
> I0307 15:48:10.284494 27466 sched.cpp:1143] Stopping framework 
> 'a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001'
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44470: Implemented runtime isoaltor default entrypoint test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43956, 44467, 44468, 44469, 44470]

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 7, 2016, 9:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44470/
> ---
> 
> (Updated March 7, 2016, 9:47 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4889
> https://issues.apache.org/jira/browse/MESOS-4889
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented runtime isoaltor default entrypoint test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/runtime_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44470/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Guangya Liu]

> On 三月 5, 2016, 3:18 a.m., Guangya Liu wrote:
> > docs/container-image.md, lines 67-77
> > 
> >
> > Does it make sense to enable slave add those isolations based on 
> > containizer technology? If using `Docker`, then add `docker/runtime` 
> > dynamically if the `isolation` flag does not include `docker/runtime`? This 
> > can make the configuration simple.
> 
> Jie Yu wrote:
> I think that's a good idea. We need to do that for filesystem/linux as 
> well. Let's address that in a separate ticket. Here, I just want to document 
> the existing behavior.

Filed an issue here https://issues.apache.org/jira/browse/MESOS-4896


- Guangya


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


On 三月 5, 2016, 2:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated 三月 5, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44408: Remove setting up of ACLs (unneeded) in PersistentVolumeTests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44408]

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 7, 2016, 8:55 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44408/
> ---
> 
> (Updated March 7, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Adam B and Joseph Wu.
> 
> 
> Bugs: MESOS-4868
> https://issues.apache.org/jira/browse/MESOS-4868
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix removes the setting up of ACLs in PersistentVolumeTests
> as it is no longer needed any more with implicit roles (MESOS-4868).
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
> 
> Diff: https://reviews.apache.org/r/44408/diff/
> 
> 
> Testing
> ---
> 
> make check (in Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44299: Added unit test for file URI fetcher.

[Jie Yu]

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




src/tests/containerizer/provisioner_appc_tests.cpp (lines 587 - 588)


Looking at the original code, why do we need this id? Should that always be 
the same? Why do we need to pass it in?



src/tests/containerizer/provisioner_appc_tests.cpp (lines 591 - 592)


I don't like this. Sounds like to try very hard to reuse the same fixture, 
but that causes a lot of confusion. Let's chat about this tomorrow.



src/tests/containerizer/provisioner_appc_tests.cpp (line 665)


It looks weired to me that FileFetch test will use a 'Server'. It's very 
confusing. I understand that you want to re-use the same helper. Can we rename 
the helper and change the semantics to just create an image tarball? Let's chat 
about this tomorrow.


- Jie Yu


On March 3, 2016, 11:37 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44299/
> ---
> 
> (Updated March 3, 2016, 11:37 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit test for file URI fetcher.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 6c8087e17aa8b7139ba12337d5be472b7099e77f 
> 
> Diff: https://reviews.apache.org/r/44299/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44379: Correctly parse perf stat format for 3.10 kernel.

[haosdent huang]

> On March 8, 2016, 1:35 a.m., Ben Mahler wrote:
> > src/linux/perf.cpp, lines 430-431
> > 
> >
> > Couple of thoughts here:
> > 
> > (1) This comment is no longer accurate since there is a special case 
> > for 3.10. Would be better to re-structure the last block of this else flow 
> > to better reflect the new logic.
> > 
> > (2) What about 3.11? Is it the same as 3.10? It would be surprising if 
> > 3.11 and < 3.10 are the same, but 3.10 is different.

There is a exist patch may related to this. https://reviews.apache.org/r/43283/


- haosdent


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


On March 4, 2016, 6:28 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44379/
> ---
> 
> (Updated March 4, 2016, 6:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-4705
> https://issues.apache.org/jira/browse/MESOS-4705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Correctly parse perf stat format for 3.10 kernel.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 
> 
> Diff: https://reviews.apache.org/r/44379/diff/
> 
> 
> Testing
> ---
> 
> {Found and Test} with Serenity, ema filter could get perf event statistics 
> correctly as expected.
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 44379: Correctly parse perf stat format for 3.10 kernel.

[Ben Mahler]

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




src/linux/perf.cpp (lines 430 - 431)


Couple of thoughts here:

(1) This comment is no longer accurate since there is a special case for 
3.10. Would be better to re-structure the last block of this else flow to 
better reflect the new logic.

(2) What about 3.11? Is it the same as 3.10? It would be surprising if 3.11 
and < 3.10 are the same, but 3.10 is different.


- Ben Mahler


On March 4, 2016, 6:28 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44379/
> ---
> 
> (Updated March 4, 2016, 6:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-4705
> https://issues.apache.org/jira/browse/MESOS-4705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Correctly parse perf stat format for 3.10 kernel.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 
> 
> Diff: https://reviews.apache.org/r/44379/diff/
> 
> 
> Testing
> ---
> 
> {Found and Test} with Serenity, ema filter could get perf event statistics 
> correctly as expected.
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 44468: Fixed the logic for default docker cmd case.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 7, 2016, 9:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44468/
> ---
> 
> (Updated March 7, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4888
> https://issues.apache.org/jira/browse/MESOS-4888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the logic for default docker cmd case.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> 4241f4c127e89ac02d1b7838041836edbeb75672 
> 
> Diff: https://reviews.apache.org/r/44468/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43802: Wrapped TASK_LOST with `` in authorization.md.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On Feb. 20, 2016, 5:05 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43802/
> ---
> 
> (Updated Feb. 20, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Wrapped TASK_LOST with `` in authorization.md
> 
> 
> Diffs
> -
> 
>   docs/authorization.md bbb4f2a 
> 
> Diff: https://reviews.apache.org/r/43802/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 44454: Returned real path for PORT_MAPPING_BIND_MOUNT_ROOT.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/port_mapping.hpp (lines 66 - 72)


This is not the reliable solution. What if /var/run points to some other 
place?

I think we should make this a field member (instead of a constant), and 
call realpath when creating the isolator.


- Jie Yu


On March 7, 2016, 3:42 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44454/
> ---
> 
> (Updated March 7, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4662
> https://issues.apache.org/jira/browse/MESOS-4662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Returned real path for PORT_MAPPING_BIND_MOUNT_ROOT.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0fe2f486eb733acf738c1c61fc44f820d7401afc 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 134b6c759b769cf335539e49eff817973c7f96a4 
> 
> Diff: https://reviews.apache.org/r/44454/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Timothy Chen]

> On March 7, 2016, 5:36 p.m., Timothy Chen wrote:
> > Are you still be able to work on this? We like to get this merged, so if 
> > you can't or don't reply we will create a new patch based on this.
> 
> Travis Hegner wrote:
> Hi Timothy,
> 
> I've been stalling this for https://reviews.apache.org/r/42516, as that 
> patch will change the way this patch should be written. It only works now out 
> of pure luck in the way docker intereprets multiple "--net" parameters, since 
> mesos doesn't yet officially support user defined networks.
> 
> The linked patch will make this patch support it properly, as well as 
> make this patch easier to write, as the network name will not have to be 
> queried at all from json; it will be available as a variable.
> 
> Timothy Chen wrote:
> I don't quite understand the pure luck part, IIUC your patch is updating 
> how we parse the Docker inspect output for Network IPAddress, which shouldn't 
> matter how we actually launched the container right? How is the linked patch 
> related?

At this moment this is actually like becomes a blocker for 0.28 as we're not 
returning the right IP address with the latest Docker daemon, so I'd like to 
get this in ASAP. For now I'll create a new patch based on your changes, and 
once the dependend patch goes in will love if you can continue the work.


- Timothy


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


On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 17, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 44200: Add agent flags for specifying CNI plugin and config directories.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On March 4, 2016, 8:14 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44200/
> ---
> 
> (Updated March 4, 2016, 8:14 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4761
> https://issues.apache.org/jira/browse/MESOS-4761
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add agent flags for specifying CNI plugin and config directories.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 305ba2c801c2060db6dcb4ef83c1043aaa7d520c 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
> 
> Diff: https://reviews.apache.org/r/44200/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44004: Add 'name' field into NetworkInfo.

[Jie Yu]

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


Fix it, then Ship it!





include/mesos/mesos.proto (lines 1585 - 1586)


I wouldn't mention CNI here because if another network isolator is used, it 
can have different meaning. I would say it's up to the network isolator to 
decide how to interpret this field.



include/mesos/v1/mesos.proto (lines 1582 - 1583)


Ditto here.


- Jie Yu


On March 1, 2016, 7:28 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44004/
> ---
> 
> (Updated March 1, 2016, 7:28 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4758
> https://issues.apache.org/jira/browse/MESOS-4758
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'name' field into NetworkInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 
>   include/mesos/v1/mesos.proto 31960a52061f70d80528fb8326522ae1d6f75b2c 
>   src/common/http.cpp be8538fc222d83d802611392f6c94e80a1e56fa8 
> 
> Diff: https://reviews.apache.org/r/44004/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On March 4, 2016, 6:20 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 4, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Jie Yu]

> On March 8, 2016, 12:53 a.m., Timothy Chen wrote:
> > docs/container-image.md, line 210
> > 
> >
> > How about adding a subsection ### Current limitations?
> > 
> > It seems a bit out of place without a proper heading.

I'll change the subtitle to: docker support and current limitations


- Jie


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


On March 5, 2016, 2:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 5, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Jie Yu]

> On March 7, 2016, 6:40 p.m., Gilbert Song wrote:
> > docs/container-image.md, lines 54-60
> > 
> >
> > Should we explain why do we allow multiple types? because users may ask 
> > `why don't I specify all possible image providers if I want to use unified 
> > containerizer`.

I don't follow. I think the reason we need this flag is to allow operators to 
control if they want to allow this feature or not.


> On March 7, 2016, 6:40 p.m., Gilbert Song wrote:
> > docs/container-image.md, line 64
> > 
> >
> > s/needs/may need/g
> > 
> > to make distinction between appc runtime vs docker runtime in the 
> > future.

I think I already mentioned that?


> On March 7, 2016, 6:40 p.m., Gilbert Song wrote:
> > docs/container-image.md, lines 82-85
> > 
> >
> > Should we add `--launcher=linux`?

I think we'll use linux launcher by default on linux. I don't want to confuse 
readers.


> On March 7, 2016, 6:40 p.m., Gilbert Song wrote:
> > docs/container-image.md, line 150
> > 
> >
> > sbin -> /bin

It's sbin after make install.


> On March 7, 2016, 6:40 p.m., Gilbert Song wrote:
> > docs/container-image.md, line 185
> > 
> >
> > should we memtion `~/.curlrc`?

Nope.


- Jie


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


On March 5, 2016, 2:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 5, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Timothy Chen]

> On March 7, 2016, 5:36 p.m., Timothy Chen wrote:
> > Are you still be able to work on this? We like to get this merged, so if 
> > you can't or don't reply we will create a new patch based on this.
> 
> Travis Hegner wrote:
> Hi Timothy,
> 
> I've been stalling this for https://reviews.apache.org/r/42516, as that 
> patch will change the way this patch should be written. It only works now out 
> of pure luck in the way docker intereprets multiple "--net" parameters, since 
> mesos doesn't yet officially support user defined networks.
> 
> The linked patch will make this patch support it properly, as well as 
> make this patch easier to write, as the network name will not have to be 
> queried at all from json; it will be available as a variable.

I don't quite understand the pure luck part, IIUC your patch is updating how we 
parse the Docker inspect output for Network IPAddress, which shouldn't matter 
how we actually launched the container right? How is the linked patch related?


- Timothy


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


On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 17, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Timothy Chen]

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


Fix it, then Ship it!




Ship It!


docs/container-image.md (line 58)


s/multiple allowed/multiple



docs/container-image.md (line 210)


How about adding a subsection ### Current limitations?

It seems a bit out of place without a proper heading.


- Timothy Chen


On March 5, 2016, 2:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 5, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Jie Yu]

> On March 6, 2016, 4:35 a.m., Guangya Liu wrote:
> > docs/container-image.md, lines 146-177
> > 
> >
> > What about move this to `## Docker Support`

I would rather not. I want to have a complete flow in the 'Getting started' 
section. We just use docker as an example.


> On March 6, 2016, 4:35 a.m., Guangya Liu wrote:
> > docs/container-image.md, lines 216-220
> > 
> >
> > Suggest add the default value here: https://registry-1.docker.io
> > 
> > s/The default URL/The URL

It's the default URL. If the user specify a registry in Docker.name, we won't 
use this value (that's how we planned at least).


> On March 6, 2016, 4:35 a.m., Guangya Liu wrote:
> > docs/container-image.md, lines 228-237
> > 
> >
> > Can you please add some command line as Docker for how to test this out?

I'll do that later.


> On March 6, 2016, 4:35 a.m., Guangya Liu wrote:
> > docs/container-image.md, line 253
> > 
> >
> > move this word above?

I wrap all text using 70 width.


> On March 6, 2016, 4:35 a.m., Guangya Liu wrote:
> > docs/container-image.md, lines 273-279
> > 
> >
> > I filed an issue for this read-only mount point 
> > https://issues.apache.org/jira/browse/MESOS-4876 , so with a `bind` 
> > backend, the end user need to make sure that the `sandbox_directory` must 
> > already exist for the rootfs. By defult, the docker images will not include 
> > a default sandbox as `/mnt/mesos/sandbox`, the task will always be failed. 
> > Shall we highlight that it is better to specify an existing sandbox when 
> > start agent?

or the image writer can make sure --sandbox_directory exists in the image.


- Jie


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


On March 5, 2016, 2:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 5, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Jie Yu]

> On March 6, 2016, 2:06 a.m., Klaus Ma wrote:
> > docs/container-image.md, line 6
> > 
> >
> > The `Mesos Containerizer` seems ambiguous: Mesos Containerizer (vs. 
> > Docker Containerizer) and Mesos's feature.

OK, I'll add a link to the Mesos containerizer doc so it looks more clear.


> On March 6, 2016, 2:06 a.m., Klaus Ma wrote:
> > docs/container-image.md, lines 67-77
> > 
> >
> > +1

Let's do that in a separate patch.


- Jie


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


On March 5, 2016, 2:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 5, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Jie Yu]

> On March 5, 2016, 3:18 a.m., Guangya Liu wrote:
> > docs/container-image.md, line 11
> > 
> >
> > s/containerizers/__containerizers__

I think this is fine? Why do you want to make it bold?


> On March 5, 2016, 3:18 a.m., Guangya Liu wrote:
> > docs/container-image.md, lines 44-46
> > 
> >
> > What about highlight Docker, AppC, OCF here?
> > s/Docker/__Docker__
> > s/AppC/__AppC__
> > s/OCF/__OCF__

Ditto. I think this is fine to me.


> On March 5, 2016, 3:18 a.m., Guangya Liu wrote:
> > docs/container-image.md, lines 67-77
> > 
> >
> > Does it make sense to enable slave add those isolations based on 
> > containizer technology? If using `Docker`, then add `docker/runtime` 
> > dynamically if the `isolation` flag does not include `docker/runtime`? This 
> > can make the configuration simple.

I think that's a good idea. We need to do that for filesystem/linux as well. 
Let's address that in a separate ticket. Here, I just want to document the 
existing behavior.


> On March 5, 2016, 3:18 a.m., Guangya Liu wrote:
> > docs/container-image.md, lines 124-130
> > 
> >
> > Does it make sense to guide end user/framework developer refer to 
> > https://github.com/apache/mesos/blob/master/src/cli/execute.cpp for an 
> > example? I see that there are someone asking this question in dev list, it 
> > would be great if we can add the reference code here.

The CLI is too generic. I think we should have an example framework. I'll 
create a ticket.


> On March 5, 2016, 3:18 a.m., Guangya Liu wrote:
> > docs/container-image.md, line 152
> > 
> >
> > s/start the Mesos agent/start the Mesos agent with docker as image 
> > provider

It's pretty obvious in the following command that we are allowing docker images.


> On March 5, 2016, 3:18 a.m., Guangya Liu wrote:
> > docs/container-image.md, line 283
> > 
> >
> > I uploaded a patch here: https://reviews.apache.org/r/44391/
> > 
> > Will rebase this patch as mesos-provisioner.md is now deleted.

Yeah, I'll merge that later.


- Jie


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


On March 5, 2016, 2:20 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 5, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44476: Revised comments about `link` semantics in libprocess.

[Vinod Kone]

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


Ship it!




- Vinod Kone


On March 8, 2016, 12:13 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44476/
> ---
> 
> (Updated March 8, 2016, 12:13 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised comments about `link` semantics in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/process.hpp 
> c9ef4e86a4735c1c0342793b6d5144d80fc853a9 
> 
> Diff: https://reviews.apache.org/r/44476/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44475: Improve master slaves metrics.

[Ian Downes]

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



Please see my review for https://reviews.apache.org/r/44474/ which describes 
why I think this is an unexpected fundamental change in how the metrics are 
counted.

- Ian Downes


On March 7, 2016, 2:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44475/
> ---
> 
> (Updated March 7, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44475/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44474: Improve master tasks metrics.

[Ian Downes]

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



Please see the first comment... I think this unexpectedly introduces a 
fundamental change in how the metrics are counted.


src/master/master.cpp (line 6461)


I'm not familiar with this code but it appears to be changing the behavior 
substantially. 

The original code sums across all slaves that are registered at the time of 
the metric call. The new code tracks counters for the various states when a 
task on a registered slave changes. IIUC, this is a fundamental change in how 
the metrics are determined because the slave registration state is considered 
at different times. For example, a task may change state on a registered slave 
and it gets counted, then the slave becomes unregistered, then the metric 
endpoint is queried. Old and new code will give different numbers?

If my understanding is correct, then perhaps (somewhat) more performant 
code could be achieved by maintaining counters at the slave level, and then 
aggregating the counters from *registered* slaves?



src/master/metrics.cpp (lines 38 - 46)


Please keep the original order which more closely matches the progression 
of states, i.e., staging before starting.



src/master/metrics.cpp (line 193)


foreach?


- Ian Downes


On March 7, 2016, 2:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44474/
> ---
> 
> (Updated March 7, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44474/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44479: Revised HA framework guide documentation.

[Neil Conway]

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

(Updated March 8, 2016, 12:14 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Fixed review comment.


Repository: mesos


Description
---

Revised HA framework guide documentation.


Diffs (updated)
-

  docs/high-availability-framework-guide.md 
111d7b9094a7f36fef8cc95232f87c134630c031 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 44476: Revised comments about `link` semantics in libprocess.

[Neil Conway]

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

(Updated March 8, 2016, 12:13 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Addressed code review comment.


Repository: mesos


Description
---

Revised comments about `link` semantics in libprocess.


Diffs (updated)
-

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

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 44424: Updated http_command_executor.cpp to use v1 API.

[Anand Mazumdar]

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



Thanks for working on this. Looks good, just one major issue around handling 
`TaskHealthStatus` messages + minor suggested cleanups.

Would love to take a more detailed look after the cleanups.


include/mesos/v1/mesos.proto (lines 1796 - 1813)


hmmm .. Did you test if the health check workflow works?

IIUC, the `mesos-health-check` binary sends a `TaskHealthStatus` message 
back to the executor and that message is not of type `v1::TaskHealthStatus`. If 
we try to deserialize, it should fail at that point. 

For now, it seems to me that the best course of action is to preserve/keep 
using the unversioned health check binary/message. In future, we might want to 
either modify the existing `mesos-health-check` binary to emit 
`v1::TaskHealthStatus` messages in addition to the unversioned ones or create a 
new binary for versioned health checks. I would recommend filing a JIRA and a 
TODO in the code mentioning this. Makes sense?



src/launcher/http_command_executor.cpp (line 29)


Newline before.



src/launcher/http_command_executor.cpp (line 70)


Move this before `std::vector`



src/launcher/http_command_executor.cpp (lines 72 - 76)


Can we do a sweep and eliminate all existing occurences of types that still 
use the v1 namespace in this .cpp file even after the `using` declaration e.g., 
`v1::FrameworkId` etc. 

Also, can you add more using directives like `TaskState/TaskID`?

I am assuming after this cleanup the `.cpp` file would be free of any 
`v1::` directives.



src/launcher/http_command_executor.cpp (lines 75 - 76)


Move these  before `v1::executor::Call` i.e. above L72.



src/launcher/http_command_executor.cpp (line 77)


Nit: Can we also include all the things from `process` namespace that we 
use to be consistent like `Future/Clock/Owned` etc?



src/launcher/http_command_executor.cpp (line 99)


Why did you move this? Let's have the ordering of member variables the same 
as command executor.

Also thanks for resolving the ambiguity name in the name of `override` 
variable. I would love it that you make a similar change to the command 
executor code too.



src/launcher/http_command_executor.cpp (line 109)


Let's have the ordering of member variables the same as command executor.



src/launcher/http_command_executor.cpp (line 112)


Let's use the C++11 way of doing this:

```cpp
virtual ~HttpCommandExecutor() = default;
```



src/launcher/http_command_executor.cpp (line 113)


Let's scope all the functions after this to the `protected` namespace. 

I know that you had an initial look into the example code that has them in 
the `public` namespace. But, most of them are generally meant to be used as 
simple walkthrough code-samples.



src/launcher/http_command_executor.cpp (line 130)


Nit: Newline before.



src/launcher/http_command_executor.cpp (line 158)


Can we have the default value as `None()` here?

Would avoid passing `None()` explicitly for cases where you don't have the 
`message` argument.



src/launcher/http_command_executor.cpp (line 165)


Nit: newline before.



src/launcher/http_command_executor.cpp (line 168)


Newline before.



src/launcher/http_command_executor.cpp (line 171)


Newline before.



src/launcher/http_command_executor.cpp (line 199)


Newline before. We typically have a new-line after a statement having 
multiple lines.



src/launcher/http_command_executor.cpp (line 205)


We typically follow a procedural programming style. However, we also hate 
nested code as it's hard to read if it spans multiple lines. Let's try to 
create a function called `launch` here. Also, let's create relevant helper 
functions for all the switch cases except `ACKNOWDLEGED`/`SUBSCRIBED` since 
they seem trivial with 2-3 lines of code.

Hence, the ne

Re: Review Request 44479: Revised HA framework guide documentation.

[Vinod Kone]

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


Fix it, then Ship it!





docs/high-availability-framework-guide.md (lines 260 - 261)


s/more promptly/promptly/ ?


- Vinod Kone


On March 7, 2016, 10:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44479/
> ---
> 
> (Updated March 7, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised HA framework guide documentation.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 111d7b9094a7f36fef8cc95232f87c134630c031 
> 
> Diff: https://reviews.apache.org/r/44479/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44478: Revised slave recovery documentation.

[Vinod Kone]

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


Ship it!




Thanks for the cleanup!

- Vinod Kone


On March 7, 2016, 10:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44478/
> ---
> 
> (Updated March 7, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised slave recovery documentation.
> 
> 
> Diffs
> -
> 
>   docs/home.md 821026a12f422385e347e0037d6527efa9ffa2e1 
>   docs/slave-recovery.md 5c148e5a39121f9b5cb7b5a84429551996c1116d 
> 
> Diff: https://reviews.apache.org/r/44478/diff/
> 
> 
> Testing
> ---
> 
> Previewed using site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44477: Fixed typo in slave's `--help` output.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 7, 2016, 10:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44477/
> ---
> 
> (Updated March 7, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in slave's `--help` output.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
> 
> Diff: https://reviews.apache.org/r/44477/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44476: Revised comments about `link` semantics in libprocess.

[Vinod Kone]

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




3rdparty/libprocess/include/process/process.hpp (lines 112 - 114)


How about

"For remote linked processes, this indicates that the TCP socket between 
the linker and linkee has failed (e.g., linkee process died, network error). 
"

?


- Vinod Kone


On March 7, 2016, 10:54 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44476/
> ---
> 
> (Updated March 7, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised comments about `link` semantics in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/process.hpp 
> c9ef4e86a4735c1c0342793b6d5144d80fc853a9 
> 
> Diff: https://reviews.apache.org/r/44476/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44463: Made `timer.hpp` header self-contained.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44463]

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 7, 2016, 7:38 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44463/
> ---
> 
> (Updated March 7, 2016, 7:38 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `timer.hpp` was missing an include for `pid.hpp` required for accessing 
> `process::UPID`
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/timer.hpp 
> 6bf7216ab3994044faaf92c865b5324b89330106 
> 
> Diff: https://reviews.apache.org/r/44463/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 44474: Improve master tasks metrics.

[Neil Conway]

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




src/master/master.cpp (line 6461)


Seems like we use post-increment elsewhere in this RR?



src/master/metrics.cpp (line 327)


Why not use an iterator / foreach loop over `tasks_stats` instead?


- Neil Conway


On March 7, 2016, 10:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44474/
> ---
> 
> (Updated March 7, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44474/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 44476: Revised comments about `link` semantics in libprocess.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Revised comments about `link` semantics in libprocess.


Diffs
-

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

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


Testing
---


Thanks,

Neil Conway

Review Request 44479: Revised HA framework guide documentation.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Revised HA framework guide documentation.


Diffs
-

  docs/high-availability-framework-guide.md 
111d7b9094a7f36fef8cc95232f87c134630c031 

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


Testing
---


Thanks,

Neil Conway

Review Request 44477: Fixed typo in slave's `--help` output.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Fixed typo in slave's `--help` output.


Diffs
-

  src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 44478: Revised slave recovery documentation.

[Neil Conway]

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

(Updated March 7, 2016, 10:54 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Revised slave recovery documentation.


Diffs
-

  docs/home.md 821026a12f422385e347e0037d6527efa9ffa2e1 
  docs/slave-recovery.md 5c148e5a39121f9b5cb7b5a84429551996c1116d 

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


Testing (updated)
---

Previewed using site-docker.


Thanks,

Neil Conway

Review Request 44478: Revised slave recovery documentation.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Revised slave recovery documentation.


Diffs
-

  docs/home.md 821026a12f422385e347e0037d6527efa9ffa2e1 
  docs/slave-recovery.md 5c148e5a39121f9b5cb7b5a84429551996c1116d 

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


Testing
---


Thanks,

Neil Conway

Review Request 44473: Added decrement operator to Counter.

[Cong Wang]

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

Review request for mesos, Ian Downes and Vinod Kone.


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


Repository: mesos


Description
---

Added decrement operator to Counter.


Diffs
-

  3rdparty/libprocess/include/process/metrics/counter.hpp 
a13cc7e18c8b23eae83c326d63874d9d2aaedc0d 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
b84dc8d858f58bc9f52b218b7153510417cf34c2 

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


Testing
---

make check


Thanks,

Cong Wang

Review Request 44475: Improve master slaves metrics.

[Cong Wang]

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

Review request for mesos, Ian Downes and Vinod Kone.


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


Repository: mesos


Description
---

Avoid iterate the list of slaves, instead just maintain some counters.


Diffs
-

  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 

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


Testing
---

Manual check


Thanks,

Cong Wang

Review Request 44474: Improve master tasks metrics.

[Cong Wang]

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

Review request for mesos, Ian Downes and Vinod Kone.


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


Repository: mesos


Description
---

Avoid iterate the list of slaves, instead just maintain some counters.


Diffs
-

  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 

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


Testing
---

Manual check


Thanks,

Cong Wang

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

[Jie Yu]

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




src/master/master.cpp (line 2846)


I looked weird to me that we increase the metrics for reserve resources in 
'authorizeXXX' function. Can you do that in the callers of authorizeXXX 
function instead?


- Jie Yu


On March 3, 2016, 5:40 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 3, 2016, 5:40 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/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:
> 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 44342: XFS disk resource isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44342]

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 7, 2016, 6:38 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44342/
> ---
> 
> (Updated March 7, 2016, 6:38 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   configure.ac a20382e8d425eb297492a6e6c2c75ea59be097c2 
>   docs/configuration.md 305ba2c801c2060db6dcb4ef83c1043aaa7d520c 
>   docs/mesos-containerizer.md 15fb5bdbe74e059614b8948108f32cd04b623305 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/containerizer.cpp 
> af3ff5750649497d8852b4761c78d4cae5455a02 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
> 
> Diff: https://reviews.apache.org/r/44342/diff/
> 
> 
> Testing
> ---
> 
> Manual testing on Fedora 23 w/ XFS. Make check on Fedora and OS X.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Gilbert Song]

> On March 5, 2016, 8:35 p.m., Guangya Liu wrote:
> > docs/container-image.md, lines 222-223
> > 
> >
> > Suggest add the default value here: /tmp/mesos/store/docker

+1.


- Gilbert


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


On March 4, 2016, 6:20 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 4, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Gilbert Song]

> On March 4, 2016, 7:18 p.m., Guangya Liu wrote:
> > docs/container-image.md, line 83
> > 
> >
> > default value is `mesos`, do we need to specify it again?

It might be more safe if specified explicitly.


- Gilbert


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


On March 4, 2016, 6:20 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 4, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 44470: Implemented runtime isoaltor default entrypoint test.

[Gilbert Song]

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

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


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


Repository: mesos


Description
---

Implemented runtime isoaltor default entrypoint test.


Diffs
-

  src/tests/containerizer/runtime_isolator_tests.cpp PRE-CREATION 

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


Testing
---

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

Review Request 44468: Fixed the logic for default docker cmd case.

[Gilbert Song]

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

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


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


Repository: mesos


Description
---

Fixed the logic for default docker cmd case.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
4241f4c127e89ac02d1b7838041836edbeb75672 

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


Testing
---

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

Review Request 44467: Implemented local puller shell command test.

[Gilbert Song]

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

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


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


Repository: mesos


Description
---

Implemented local puller shell command test.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 

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


Testing
---

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

Review Request 44469: Implemented runtime isolator default cmd test.

[Gilbert Song]

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

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


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


Repository: mesos


Description
---

Implemented runtime isolator default cmd test.


Diffs
-

  src/tests/containerizer/runtime_isolator_tests.cpp PRE-CREATION 

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


Testing
---

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

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

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44376]

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

Error:
2016-03-07 21:33:02 URL:https://reviews.apache.org/r/44376/diff/raw/ 
[9353/9353] -> "44376.patch" [1]
error: missing binary patch data for '3rdparty/zookeeper-3.4.8.tar.gz'
error: binary patch does not apply to '3rdparty/zookeeper-3.4.5.tar.gz'
error: 3rdparty/zookeeper-3.4.5.tar.gz: patch does not apply

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

- Mesos ReviewBot


On March 4, 2016, 9:22 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44376/
> ---
> 
> (Updated March 4, 2016, 9:22 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4612
> https://issues.apache.org/jira/browse/MESOS-4612
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade zookeeper to 3.4.8 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 49aa55741d76aa88c8fbb526f18908312bb0c717 
>   3rdparty/cmake/Versions.cmake 6eac4dc0f7189e209e7d7232419e4de4bc0875c0 
>   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/ext_modules.py.in 
> eb93864733713dddad66141c6b8b6cd895f41484 
>   src/tests/zookeeper_test_server.cpp 
> 0dc041fef8973d35114b9f76a6a4002853884670 
> 
> Diff: https://reviews.apache.org/r/44376/diff/
> 
> 
> Testing
> ---
> 
> Added a new patch that solved 
> [ZOOKEEPER-1643](https://issues.apache.org/jira/browse/ZOOKEEPER-1643)
> 
> The original patch: 
> 
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Joseph Wu]

> On March 3, 2016, 2:15 p.m., Joseph Wu wrote:
> > src/master/main.cpp, line 61
> > 
> >
> > What about this header?
> 
> Anurag Singh wrote:
> Do you mean why is this header not ""? It's 
> different from  - it includes the 
> StandaloneMasterDetector and ZooperZooKeeperMasterDetector class definitions. 
> Although, after adding the contender and detect directories, these includes 
> will need to change.
> 
> Joseph Wu wrote:
> After the module change, shouldn't all internal inclusions of 
> "detector.hpp" and "contender.hpp" headers disappear?  I'm just wondering why 
> you kept it here.  (And if you still need them here, there's probably more 
> refactoring that needs to be done in the file than just a re-namespacing).
> 
> Anurag Singh wrote:
> Once I am done with moving Standalone and Zookeeper contender/detector 
> class definitions out, the detector.hpp and contender.hpp headers will go 
> away. However, main.cpp is still has a reference to StandaloneMasterDetector 
> (it calls appoint() on an object of this type) so it will be including 
> something like master/detectors/standalone.hpp. Are you fine with that?

Hmm, that seems unavoidable unless we add the `appoint` function (or something 
with similar) to the detector interface.  Should be fine.


- Joseph


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


On March 3, 2016, 9:30 a.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> ---
> 
> (Updated March 3, 2016, 9:30 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp 
> f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
>   src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
>   src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
>   src/tests/scheduler_event_call_tests.cpp 
> 8c02ceeb3ec1783cb2f63f100700508e70f586e4 
>   src/tests/scheduler_http_api_tests.cpp 
> 428e12646d80b45daec30cfe607b97f36170fdf5 
>   src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 
>   src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
>   src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 
> 
> Diff: https://reviews.apache.org/r/44288/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Anurag Singh]

> On March 3, 2016, 10:15 p.m., Joseph Wu wrote:
> > src/master/main.cpp, line 61
> > 
> >
> > What about this header?
> 
> Anurag Singh wrote:
> Do you mean why is this header not ""? It's 
> different from  - it includes the 
> StandaloneMasterDetector and ZooperZooKeeperMasterDetector class definitions. 
> Although, after adding the contender and detect directories, these includes 
> will need to change.
> 
> Joseph Wu wrote:
> After the module change, shouldn't all internal inclusions of 
> "detector.hpp" and "contender.hpp" headers disappear?  I'm just wondering why 
> you kept it here.  (And if you still need them here, there's probably more 
> refactoring that needs to be done in the file than just a re-namespacing).

Once I am done with moving Standalone and Zookeeper contender/detector class 
definitions out, the detector.hpp and contender.hpp headers will go away. 
However, main.cpp is still has a reference to StandaloneMasterDetector (it 
calls appoint() on an object of this type) so it will be including something 
like master/detectors/standalone.hpp. Are you fine with that?


- Anurag


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


On March 3, 2016, 5:30 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> ---
> 
> (Updated March 3, 2016, 5:30 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp 
> f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
>   src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
>   src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
>   src/tests/scheduler_event_call_tests.cpp 
> 8c02ceeb3ec1783cb2f63f100700508e70f586e4 
>   src/tests/scheduler_http_api_tests.cpp 
> 428e12646d80b45daec30cfe607b97f36170fdf5 
>   src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 
>   src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
>   src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 
> 
> Diff: https://reviews.apache.org/r/44288/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44239, 44298, 43969, 44299, 44455, 44456]

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 7, 2016, 3:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44456/
> ---
> 
> (Updated March 7, 2016, 3:47 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4818
> https://issues.apache.org/jira/browse/MESOS-4818
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test provides simple integartion test for a single layered linux image.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 6c8087e17aa8b7139ba12337d5be472b7099e77f 
> 
> Diff: https://reviews.apache.org/r/44456/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44408: Remove setting up of ACLs (unneeded) in PersistentVolumeTests.

[Yong Tang]

> On March 7, 2016, 7:39 p.m., Joseph Wu wrote:
> > src/tests/persistent_volume_tests.cpp, lines 295-299
> > 
> >
> > Nit: An arrangement like this is preferable:
> > ```
> >   FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
> >   frameworkInfo.set_role("role1");
> >   
> >   MockScheduler sched;
> >   MesosSchedulerDriver driver(
> >   &sched, frameworkInfo, master.get(), DEFAULT_CREDENTIAL);
> > ```

Thanks for the review Joseph. Just updated the review request, and also add 
Adam to the reviewer list. Let me know if there are any other issues.


- Yong


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


On March 7, 2016, 8:55 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44408/
> ---
> 
> (Updated March 7, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Adam B and Joseph Wu.
> 
> 
> Bugs: MESOS-4868
> https://issues.apache.org/jira/browse/MESOS-4868
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix removes the setting up of ACLs in PersistentVolumeTests
> as it is no longer needed any more with implicit roles (MESOS-4868).
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
> 
> Diff: https://reviews.apache.org/r/44408/diff/
> 
> 
> Testing
> ---
> 
> make check (in Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44408: Remove setting up of ACLs (unneeded) in PersistentVolumeTests.

[Yong Tang]

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

(Updated March 7, 2016, 8:52 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Move the placement of FrameworkInfo as suggested by Joseph.


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


Repository: mesos


Description
---

This fix removes the setting up of ACLs in PersistentVolumeTests
as it is no longer needed any more with implicit roles (MESOS-4868).


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 

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


Testing
---

make check (in Ubuntu 14.04)


Thanks,

Yong Tang

Re: Review Request 43819: Added Scheduler-Driver API to app-framework-development-guide.md.

[Joerg Schad]

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

(Updated March 7, 2016, 8:14 p.m.)


Review request for mesos, Adam B and Neil Conway.


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


Repository: mesos


Description
---

Previously the app-framework-development-guide only explained the scheduler 
callback interface. Equally important when developing frameworks is the 
knowledge if potential actions a scheduler can trigger via the SchedulerDriver.


Diffs
-

  docs/app-framework-development-guide.md 
55f09c7cf2262dda98dcc8b63b1e36ab78296141 

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


Testing
---

Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via 
docker website container.


Thanks,

Joerg Schad

Re: Review Request 43819: Added Scheduler-Driver API to app-framework-development-guide.md.

[Joerg Schad]

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

(Updated March 7, 2016, 8:13 p.m.)


Review request for mesos, Adam B and Neil Conway.


Changes
---

Improved description of SchedulerDriver.


Repository: mesos


Description
---

Previously the app-framework-development-guide only explained the scheduler 
callback interface. Equally important when developing frameworks is the 
knowledge if potential actions a scheduler can trigger via the SchedulerDriver.


Diffs (updated)
-

  docs/app-framework-development-guide.md 
55f09c7cf2262dda98dcc8b63b1e36ab78296141 

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


Testing
---

Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via 
docker website container.


Thanks,

Joerg Schad

Re: Review Request 43819: Added Scheduler-Driver API to app-framework-development-guide.md.

[Joerg Schad]

> On Feb. 24, 2016, 7:57 a.m., Adam B wrote:
> > docs/app-framework-development-guide.md, lines 137-139
> > 
> >
> > Let's be very explicit that they'll need to instantiate a 
> > MesosSchedulerDriver and call these methods on it. Probably also want to 
> > document the MesosSchedulerDriver constructors so they know how to 
> > instantiate one.

Refered to the test_framework.cpp instead for proper use of the the 
schedulerDriver...


- Joerg


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


On March 7, 2016, 8:05 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> ---
> 
> (Updated March 7, 2016, 8:05 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the app-framework-development-guide only explained the scheduler 
> callback interface. Equally important when developing frameworks is the 
> knowledge if potential actions a scheduler can trigger via the 
> SchedulerDriver.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> 55f09c7cf2262dda98dcc8b63b1e36ab78296141 
> 
> Diff: https://reviews.apache.org/r/43819/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and 
> via docker website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43819: Added Scheduler-Driver API to app-framework-development-guide.md.

[Joerg Schad]

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

(Updated March 7, 2016, 8:05 p.m.)


Review request for mesos, Adam B and Neil Conway.


Changes
---

addressed reviews.


Repository: mesos


Description (updated)
---

Previously the app-framework-development-guide only explained the scheduler 
callback interface. Equally important when developing frameworks is the 
knowledge if potential actions a scheduler can trigger via the SchedulerDriver.


Diffs (updated)
-

  docs/app-framework-development-guide.md 
55f09c7cf2262dda98dcc8b63b1e36ab78296141 

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


Testing
---

Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and via 
docker website container.


Thanks,

Joerg Schad

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Joseph Wu]

> On March 3, 2016, 2:15 p.m., Joseph Wu wrote:
> > src/master/contender.hpp, lines 17-18
> > 
> >
> > This whole file seems like a pretty substantial change.  I'd recommend 
> > pulling it out into a separate review (rather than hiding it in this 
> > review).
> > 
> > Also, you'll want to consider making folders "contenders" and 
> > "detectors".  Then renaming this file "standalone.hpp".
> 
> Anurag Singh wrote:
> Ok. Although then zookeeper's classes should also go into their own files.
> 
> Anurag Singh wrote:
> Looking at this again, the changes in this file are really just namespace 
> changes (the only thing substantial is the removal of the MasterContender 
> class definition) - I can't put them into a different commit without breaking 
> the build (I'm trying to make sure individual commits don't break builds, 
> which I think is a sensible goal). However, I can leave this commit as is but 
> create a separate commit to create the contenders/detectors directories. Is 
> that acceptable to you?
> 
> Anurag Singh wrote:
> Ping ... will the suggestion I made work for you?

Don't worry about having atomic commits (especially for verbose changes).

There are a couple of changes to consider here, each of which might deserve its 
own review:
- Pulling out the interface deletions.  (You also consider pulling it into the 
previous review, as you're really moving the code from private to public 
headers.)
- Adding a folder for each module you are adding ("contender" and "detector").
- Breaking apart the Standalone vs Zookeeper logic.


> On March 3, 2016, 2:15 p.m., Joseph Wu wrote:
> > src/master/main.cpp, line 61
> > 
> >
> > What about this header?
> 
> Anurag Singh wrote:
> Do you mean why is this header not ""? It's 
> different from  - it includes the 
> StandaloneMasterDetector and ZooperZooKeeperMasterDetector class definitions. 
> Although, after adding the contender and detect directories, these includes 
> will need to change.

After the module change, shouldn't all internal inclusions of "detector.hpp" 
and "contender.hpp" headers disappear?  I'm just wondering why you kept it 
here.  (And if you still need them here, there's probably more refactoring that 
needs to be done in the file than just a re-namespacing).


- Joseph


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


On March 3, 2016, 9:30 a.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> ---
> 
> (Updated March 3, 2016, 9:30 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp 
> f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073f

Re: Review Request 43819: Added Scheduler-Driver API to app-framework-development-guide.md.

[Joerg Schad]

> On Feb. 24, 2016, 7:57 a.m., Adam B wrote:
> > docs/app-framework-development-guide.md, line 144
> > 
> >
> > Get rid of the `virtual` and `= 0` since that's not a part of the API 
> > that their scheduler will call. They're not implementing a new 
> > SchedulerDriver, they're just instantiating one and calling its methods.

I actually just wanted it to be consistent with the below part. Will remove `= 
0` from both parts


- Joerg


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


On Feb. 22, 2016, 3 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> ---
> 
> (Updated Feb. 22, 2016, 3 a.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Scheduler-Driver API to app-framework-development-guide.md.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
> 
> Diff: https://reviews.apache.org/r/43819/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and 
> via docker website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 41049: New python lib with only the executor driver.

[Steve Niemitz]

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

(Updated March 7, 2016, 7:40 p.m.)


Review request for mesos, Till Toenshoff and Vinod Kone.


Changes
---

Rebased to ToT.


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


Repository: mesos


Description
---

New python lib with only the executor driver.

This patch produces a new python egg, mesos.executor, which contains only the 
code needed to create a MesosExecutorDriver.  By doing so, the linker can 
remove unused code in libmesos_no_3rdparty.a, and therefor not include any 
external dependencies in the resulting _mesos.so.


Diffs (updated)
-

  configure.ac b30a8d30076f3068fd7d5fc8ccea1982639e998a 
  src/Makefile.am 0409491b92c9720d60ad76fdbc2edff554fb4965 
  src/python/executor/setup.py.in PRE-CREATION 
  src/python/executor/src/mesos/__init__.py PRE-CREATION 
  src/python/executor/src/mesos/executor/__init__.py PRE-CREATION 
  src/python/executor/src/mesos/executor/module.cpp PRE-CREATION 
  src/python/native/ext_modules.py.in 4682e5eed0f7be23fb48ef628e1bebc7741431d7 
  src/python/native/setup.py.in 49ed61293281f65d6597470ce3697326ac769032 
  src/python/native/src/mesos/native/__init__.py 
226f94357cd81b700706edd68c94de461647fa1b 
  src/python/native/src/mesos/native/mesos_executor_driver_impl.hpp  
  src/python/native/src/mesos/native/mesos_executor_driver_impl.cpp 
7838a07ac12034a5eed2f459bd11e5e0a07e94de 
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp  
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp 
f8be49bf5feb6675c3c8e6a1df75a5e079d09fcc 
  src/python/native/src/mesos/native/module.hpp 
2cf7b57077d10c071bd4b8f36e2cb43041fc168d 
  src/python/native/src/mesos/native/module.cpp 
6e672a21edf0e9df1d95688620ea9cc6a1b2 
  src/python/native/src/mesos/native/proxy_executor.hpp  
  src/python/native/src/mesos/native/proxy_executor.cpp 
706f417b547bb9878346902b2a2f98b1afade5ad 
  src/python/native/src/mesos/native/proxy_scheduler.hpp  
  src/python/native/src/mesos/native/proxy_scheduler.cpp 
8afb3380d99b4dc9c4f7b3d926f4a5b1d6e94c20 
  src/python/native_common/ext_modules.py.in PRE-CREATION 
  src/python/scheduler/setup.py.in PRE-CREATION 
  src/python/scheduler/src/mesos/__init__.py PRE-CREATION 
  src/python/scheduler/src/mesos/scheduler/__init__.py PRE-CREATION 

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


Testing
---

On CentOS 7, GCC 4.8.5:
- make distcheck
- Tested with aurora executor
- Made sure mesos.scheduler still worked
- Made sure mesos.native still worked


Thanks,

Steve Niemitz

Re: Review Request 44408: Remove setting up of ACLs (unneeded) in PersistentVolumeTests.

[Joseph Wu]

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


Fix it, then Ship it!




LGTM.

You should probably ask Adam to shepherd since he committed the other implicit 
roles patches.


src/tests/persistent_volume_tests.cpp (lines 271 - 275)


Nit: An arrangement like this is preferable:
```
  FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
  frameworkInfo.set_role("role1");
  
  MockScheduler sched;
  MesosSchedulerDriver driver(
  &sched, frameworkInfo, master.get(), DEFAULT_CREDENTIAL);
```


- Joseph Wu


On March 4, 2016, 8:17 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44408/
> ---
> 
> (Updated March 4, 2016, 8:17 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4868
> https://issues.apache.org/jira/browse/MESOS-4868
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix removes the setting up of ACLs in PersistentVolumeTests
> as it is no longer needed any more with implicit roles (MESOS-4868).
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
> 
> Diff: https://reviews.apache.org/r/44408/diff/
> 
> 
> Testing
> ---
> 
> make check (in Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Review Request 44463: Made `timer.hpp` header self-contained.

[Anand Mazumdar]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

The `timer.hpp` was missing an include for `pid.hpp` required for accessing 
`process::UPID`


Diffs
-

  3rdparty/libprocess/include/process/timer.hpp 
6bf7216ab3994044faaf92c865b5324b89330106 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 44454: Returned real path for PORT_MAPPING_BIND_MOUNT_ROOT.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44454]

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 7, 2016, 3:42 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44454/
> ---
> 
> (Updated March 7, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4662
> https://issues.apache.org/jira/browse/MESOS-4662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Returned real path for PORT_MAPPING_BIND_MOUNT_ROOT.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0fe2f486eb733acf738c1c61fc44f820d7401afc 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 134b6c759b769cf335539e49eff817973c7f96a4 
> 
> Diff: https://reviews.apache.org/r/44454/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44433: Added empty line for list in maintenance doc.

[Joseph Wu]

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


Ship it!




Nice catch :)

- Joseph Wu


On March 6, 2016, 7:52 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44433/
> ---
> 
> (Updated March 6, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added empty line for list in maintenance doc.
> 
> 
> Diffs
> -
> 
>   docs/maintenance.md 365c920719dbd0c5e61efe1975547a2848647bce 
> 
> Diff: https://reviews.apache.org/r/44433/diff/
> 
> 
> Testing
> ---
> 
> Document update.
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 44433: Added empty line for list in maintenance doc.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On March 6, 2016, 3:52 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44433/
> ---
> 
> (Updated March 6, 2016, 3:52 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added empty line for list in maintenance doc.
> 
> 
> Diffs
> -
> 
>   docs/maintenance.md 365c920719dbd0c5e61efe1975547a2848647bce 
> 
> Diff: https://reviews.apache.org/r/44433/diff/
> 
> 
> Testing
> ---
> 
> Document update.
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 44414: Added documentation about container image support.

[Gilbert Song]

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




docs/container-image.md (line 46)


Do you mean `OCI` here?



docs/container-image.md (lines 54 - 60)


Should we explain why do we allow multiple types? because users may ask 
`why don't I specify all possible image providers if I want to use unified 
containerizer`.



docs/container-image.md (line 64)


s/needs/may need/g

to make distinction between appc runtime vs docker runtime in the future.



docs/container-image.md (lines 82 - 85)


Should we add `--launcher=linux`?



docs/container-image.md (line 150)


sbin -> /bin



docs/container-image.md (line 154)


sbin -> /bin



docs/container-image.md (line 166)


make consensus with the agent cli commands.



docs/container-image.md (line 185)


should we memtion `~/.curlrc`?



docs/container-image.md (line 194)


We have updated this. Should remove `file://`.



docs/container-image.md (line 200)


ditto.


- Gilbert Song


On March 4, 2016, 6:20 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44414/
> ---
> 
> (Updated March 4, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, Neil Conway, Timothy 
> Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-4873
> https://issues.apache.org/jira/browse/MESOS-4873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation about container image support.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md PRE-CREATION 
>   docs/mesos-provisioner.md 1b19406cb93bbc3f5330eaf9d29b1be98a674136 
> 
> Diff: https://reviews.apache.org/r/44414/diff/
> 
> 
> Testing
> ---
> 
> Tested the formatting in Mou
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 44342: XFS disk resource isolator.

[James Peach]

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

(Updated March 7, 2016, 6:38 p.m.)


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


Changes
---

Rebased. Addressed review comments. Added documentation.


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


Repository: mesos


Description
---

Track sandbox directory usage by dynamically assigning XFS project
quotas. We track a range of XFS project IDs, assigning a project ID
and a project quota to each sandbox as it is created. When the task
reaches the quota, writes will fail with EDQUOT, and the task will have
an opportunity to handle that.

Quotas are not applied to volume resources since the isolator interface
has no insight into the volume lifecycle. Thus it is not currently
possible to accurately assign and reclaim project IDs.

If LOW is the lower bound of the project ID range and HIGH is the upper
bound, you can show the currently allocated project quotas using the
xfs_quota command:

  $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"

To show the project ID assigned to the file PATH, use the xfs_io command:

  $ xfs_io -r -c stat PATH


Diffs (updated)
-

  configure.ac a20382e8d425eb297492a6e6c2c75ea59be097c2 
  docs/configuration.md 305ba2c801c2060db6dcb4ef83c1043aaa7d520c 
  docs/mesos-containerizer.md 15fb5bdbe74e059614b8948108f32cd04b623305 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/slave/containerizer/mesos/containerizer.cpp 
af3ff5750649497d8852b4761c78d4cae5455a02 
  src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 

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


Testing
---

Manual testing on Fedora 23 w/ XFS. Make check on Fedora and OS X.


Thanks,

James Peach

Re: Review Request 44342: XFS disk resource isolator.

[James Peach]

> On March 4, 2016, 1:08 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/disk/xfs.hpp, line 84
> > 
> >
> > This should be optional because some directories are not isolated.

On recovery, it is possible for some directories to not be isolated, but in 
that case, we never create a ``XfsDiskIsolatorProcess::Info`` record. So the 
``projid`` is never and is (effectively) not optional.


- James


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


On March 3, 2016, 6:06 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44342/
> ---
> 
> (Updated March 3, 2016, 6:06 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
> ---
> 
> Track sandbox directory usage by dynamically assigning XFS project
> quotas. We track a range of XFS project IDs, assigning a project ID
> and a project quota to each sandbox as it is created. When the task
> reaches the quota, writes will fail with EDQUOT, and the task will have
> an opportunity to handle that.
> 
> Quotas are not applied to volume resources since the isolator interface
> has no insight into the volume lifecycle. Thus it is not currently
> possible to accurately assign and reclaim project IDs.
> 
> If LOW is the lower bound of the project ID range and HIGH is the upper
> bound, you can show the currently allocated project quotas using the
> xfs_quota command:
> 
>   $ xfs_quota -x -c "report -a -n -L LOW -U HIGH"
> 
> To show the project ID assigned to the file PATH, use the xfs_io command:
> 
>   $ xfs_io -r -c stat PATH
> 
> 
> Diffs
> -
> 
>   configure.ac b045d3c68a2d440bed4d1b3e6ab21a1bbe063517 
>   src/Makefile.am b30cc25f61856d6417437547baaa0bb338a30d63 
>   src/slave/containerizer/mesos/containerizer.cpp 
> af3ff5750649497d8852b4761c78d4cae5455a02 
>   src/slave/containerizer/mesos/isolators/disk/xfs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/disk/xfs.cpp PRE-CREATION 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
> 
> Diff: https://reviews.apache.org/r/44342/diff/
> 
> 
> Testing
> ---
> 
> Manual testing on Fedora 23 w/ XFS. Make check on Fedora and OS X.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 43824: Addressed comments of 41672.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43824]

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 7, 2016, 1:55 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43824/
> ---
> 
> (Updated March 7, 2016, 1:55 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addressed comments of 41672.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43824/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

> On March 7, 2016, 5:36 p.m., Timothy Chen wrote:
> > Are you still be able to work on this? We like to get this merged, so if 
> > you can't or don't reply we will create a new patch based on this.

Hi Timothy,

I've been stalling this for https://reviews.apache.org/r/42516, as that patch 
will change the way this patch should be written. It only works now out of pure 
luck in the way docker intereprets multiple "--net" parameters, since mesos 
doesn't yet officially support user defined networks.

The linked patch will make this patch support it properly, as well as make this 
patch easier to write, as the network name will not have to be queried at all 
from json; it will be available as a variable.


- Travis


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


On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 17, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

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

[haosdent huang]

> On March 4, 2016, 1:45 a.m., haosdent huang wrote:
> > Ship It!

Sorry, I found you binary file in this patch is incomplete when you submit it. 
Could you apply this https://reviews.reviewboard.org/r/7571/diff/1#index_header 
to you rbtools and upload the patch again? Same as your patch for MESOS-4612


- haosdent


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


On March 4, 2016, 1:38 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44257/
> ---
> 
> (Updated March 4, 2016, 1:38 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4678
> https://issues.apache.org/jira/browse/MESOS-4678
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade protobuf to 2.6.1 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 932f2f66b04e5ca3d2ed04da1e7019d2ff7488e4 
>   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 c3aaa437af10533132698df3348114195d338965 
>   configure.ac b045d3c68a2d440bed4d1b3e6ab21a1bbe063517 
>   src/java/mesos.pom.in 7615d61eb6fedfa0ead785cd360946c56ccf80af 
>   src/python/interface/setup.py.in d73996734c3a3c70c3a6c0c697bb6733c241c091 
>   src/python/native/ext_modules.py.in 
> 4682e5eed0f7be23fb48ef628e1bebc7741431d7 
>   src/python/protocol/setup.py.in 4c50fbbf1ce11c4c42c848364523225ee7ea5a3b 
> 
> Diff: https://reviews.apache.org/r/44257/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

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

[haosdent huang]

> On March 4, 2016, 2:55 a.m., haosdent huang wrote:
> > Ship It!

Sorry, I found you binary file in this patch is incomplete when you submit it. 
Could you apply this https://reviews.reviewboard.org/r/7571/diff/1#index_header 
to you rbtools and upload the patch again?


- haosdent


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


On March 4, 2016, 9:22 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44376/
> ---
> 
> (Updated March 4, 2016, 9:22 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4612
> https://issues.apache.org/jira/browse/MESOS-4612
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade zookeeper to 3.4.8 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 49aa55741d76aa88c8fbb526f18908312bb0c717 
>   3rdparty/cmake/Versions.cmake 6eac4dc0f7189e209e7d7232419e4de4bc0875c0 
>   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/ext_modules.py.in 
> eb93864733713dddad66141c6b8b6cd895f41484 
>   src/tests/zookeeper_test_server.cpp 
> 0dc041fef8973d35114b9f76a6a4002853884670 
> 
> Diff: https://reviews.apache.org/r/44376/diff/
> 
> 
> Testing
> ---
> 
> Added a new patch that solved 
> [ZOOKEEPER-1643](https://issues.apache.org/jira/browse/ZOOKEEPER-1643)
> 
> The original patch: 
> 
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Timothy Chen]

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



Are you still be able to work on this? We like to get this merged, so if you 
can't or don't reply we will create a new patch based on this.

- Timothy Chen


On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 17, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Anurag Singh]

> On March 3, 2016, 10:15 p.m., Joseph Wu wrote:
> > src/master/contender.hpp, lines 17-18
> > 
> >
> > This whole file seems like a pretty substantial change.  I'd recommend 
> > pulling it out into a separate review (rather than hiding it in this 
> > review).
> > 
> > Also, you'll want to consider making folders "contenders" and 
> > "detectors".  Then renaming this file "standalone.hpp".
> 
> Anurag Singh wrote:
> Ok. Although then zookeeper's classes should also go into their own files.
> 
> Anurag Singh wrote:
> Looking at this again, the changes in this file are really just namespace 
> changes (the only thing substantial is the removal of the MasterContender 
> class definition) - I can't put them into a different commit without breaking 
> the build (I'm trying to make sure individual commits don't break builds, 
> which I think is a sensible goal). However, I can leave this commit as is but 
> create a separate commit to create the contenders/detectors directories. Is 
> that acceptable to you?

Ping ... will the suggestion I made work for you?


- Anurag


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


On March 3, 2016, 5:30 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> ---
> 
> (Updated March 3, 2016, 5:30 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp 
> f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
>   src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
>   src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
>   src/tests/scheduler_event_call_tests.cpp 
> 8c02ceeb3ec1783cb2f63f100700508e70f586e4 
>   src/tests/scheduler_http_api_tests.cpp 
> 428e12646d80b45daec30cfe607b97f36170fdf5 
>   src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 
>   src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
>   src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 
> 
> Diff: https://reviews.apache.org/r/44288/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44450: Rescind all outstanding offers to satisfy weights update.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [41681, 44450]

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 7, 2016, 1:46 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44450/
> ---
> 
> (Updated March 7, 2016, 1:46 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4881
> https://issues.apache.org/jira/browse/MESOS-4881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rescind all outstanding offers to satisfy weights update.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/weights_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44450/diff/
> 
> 
> Testing
> ---
> 
> Make && Make check.
> 
> Manual test steps:
> 
> - Start Mesos master:
> $ ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master
> 
> - Start Mesos slave:
> $ ./bin/mesos-slave.sh --master=127.0.0.1:5050
> 
> - Register a framwork with `curl`
> $ curl -v http://127.0.0.1:5050/api/v1/scheduler -H "Content-type: 
> application/json" -X POST -d @subscribe.json
> $  cat subscribe.json
> {
>"type": "SUBSCRIBE",
> 
>"subscribe" : {
>   "framework_info": {
>   "user" :  "root",
>   "name" :  "comsumer c1 HTTP Framework",
> "role" :  "mesos",
> "principal":"wyq"
>   },
> 
>   "force" : true
>   }
> }
> 
> And this framework will receive an offer:
> {"subscribed":{"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"}},"type":"SUBSCRIBED"}20
> {"type":"HEARTBEAT"}680
> {"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
> {"type":"HEARTBEAT"}20
> 
> - Update the weight of role `mesos`
> $ curl --data "[{\"weight\":1.8,\"role\":\"mesos\"}]" -X PUT 
> http://127.0.0.1:5050/weights
> 
> Receive an rescind offer, and a new offer received:
> {"rescind":{"offer_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"}},"type":"RESCIND"}680
> {"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O1"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
> {"type":"HEARTBEAT"}20
> 
> - Update the weight of role `mesos1`
> $ curl --data "[{\"weight\":2.8,\"role\":\"mesos1\"}]" -X PUT 
> http://127.0.0.1:5050/weights
> 
> - The outstanding offer will not be rescinded.
> 
> 
> (TODO) I will add couple of tests for this patch in another JIRA.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44441: Treated command as executable value and arguments in mesos-execute.

[Gilbert Song]

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



Hi Guangya, Thanks for taking this on. My main consideration about this patch 
is making distinction between shell command v.s. non-shell command. We have two 
choices:

1. Following your current implementation, the description of the `command` flag 
should be updated, because people may not know they should use 
`--command=value,argv1,argv1` as non-shell command.
2. Or we should make user specifying `value` and `argv` explicitly, by making 
them as flags.

We should find out which is better.

- Gilbert Song


On March 7, 2016, 12:05 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1/
> ---
> 
> (Updated March 7, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4882
> https://issues.apache.org/jira/browse/MESOS-4882
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Treated command as executable value and arguments in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/1/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-execute --master=192.168.56.12:5050  --name=test_mesos 
> --docker_image=busybox:latest --containerizer=mesos --no-shell  
> --command="ls,/etc/passwd" 
> I0307 15:48:09.834506 27450 sched.cpp:222] Version: 0.29.0
> I0307 15:48:09.841404 27468 sched.cpp:326] New master detected at 
> master@192.168.56.12:5050
> I0307 15:48:09.843992 27468 sched.cpp:336] No credentials provided. 
> Attempting to register without authentication
> I0307 15:48:09.848901 27468 sched.cpp:703] Framework registered with 
> a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
> Framework registered with a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
> task test_mesos submitted to slave 6fa2afa1-768b-4f2e-9c69-9f1017634e72-S2
> Received status update TASK_RUNNING for task test_mesos
> Received status update TASK_FINISHED for task test_mesos
> I0307 15:48:10.284418 27466 sched.cpp:1903] Asked to stop the driver
> I0307 15:48:10.284494 27466 sched.cpp:1143] Stopping framework 
> 'a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001'
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 44456: Added Appc provisioner integration test.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The test provides simple integartion test for a single layered linux image.


Diffs
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
6c8087e17aa8b7139ba12337d5be472b7099e77f 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 44455: Fixed AppcImageFetcherTest for manifest formatting.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed AppcImageFetcherTest for manifest formatting.


Diffs
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
6c8087e17aa8b7139ba12337d5be472b7099e77f 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 44454: Returned real path for PORT_MAPPING_BIND_MOUNT_ROOT.

[Guangya Liu]

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

Review request for mesos.


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


Repository: mesos


Description
---

Returned real path for PORT_MAPPING_BIND_MOUNT_ROOT.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
0fe2f486eb733acf738c1c61fc44f820d7401afc 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
134b6c759b769cf335539e49eff817973c7f96a4 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

> On March 4, 2016, 11:30 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 134
> > 
> >
> > I don't understand this comment. We just made sure the plugin does not 
> > exist? So what does the comment imply "it can
> > // still be valid as long as operator puts the CNI plugin binary
> > // that it uses under '--network_cni_plugins_dir'." ?
> > 
> > I think at this point we should return an error. If can't find an 
> > executable for a named network, the behavior will become undefined. We 
> > should bail at this point.
> 
> Qian Zhang wrote:
> My point is, if we can not find a plugin for a named network during 
> initilization, log a warning message to let operator know this issue, and 
> afterward operator can put the plugin in the plugin directory without 
> restarting agent, then the named network can still work.
> 
> Avinash sridharan wrote:
> Lets not rely on the operator heeding WARNING messages and fixing the 
> problem. My concern is that this is a `FATAL` error since before the operator 
> can rectify the error if containers are launched the behavior becomes 
> undefined.
> 
> Qian Zhang wrote:
> Agree, let's return an error :-)
> 
> Qian Zhang wrote:
> After more thinking, I think in this case, it makes more sense to log a 
> warning message and ignore the network config file rather than bail at this 
> point, because there can be other valid network config files. If in the end 
> there is no any valid network config files, we should definitely bail.
> 
> Avinash sridharan wrote:
> I think we should not allow any errors in the configs/plugins passed by 
> the operator . Reason being that frameworks are going to learn about networks 
> out-of-band, and if there are config/plugin errors we will have to throw 
> errors during task launch. Why should we allow the system to proceed knowing 
> that this is going to lead to erroneous situation? The only way the operator 
> can fix this error is by restarting the slave (and fixing the config), so 
> might as well bail out sooner rather than later.
> 
> Qian Zhang wrote:
> Can you please let me know how this can lead to erroneous situation? If a 
> network config file is invalid for whatever reason, "network/isolator" will 
> NOT load it and just ignore it, so how can framework launches a task to join 
> an invalid network which is not loaded by the isolator? I do not think 
> framework user has such knowledge, or you think framework user will know all 
> the network config files (valid or invalid) under "--network_cni_config_dir" 
> in some way?
> 
> Avinash sridharan wrote:
> Frameworks would know only the network name. Its the responsibility of 
> the operator to install the right config for the given `name`. Hence the 
> erroneous case. The fact that config was not loaded for valid network name 
> cause inconsistency between the frameworks view of what is available and the 
> isolators view of what is configurable.
> 
> Qian Zhang wrote:
> What about framework specifies a wrong network name by mistake? Even in 
> `create()` method we ensure every network config file is valid and agent is 
> started successfuly, there is still a chance for framework to specify a wrong 
> network name (which is actually out of our control), right? So my point is, 
> we have to handle this erroneous in launching task case anyway.
> 
> And I do not quite understand what is `the frameworks view of what is 
> available`, can you please elaborate how framework can know what is 
> available? My thinking is, in future we may expose the available CNI networks 
> to framework via an HTTP endpoint or even via an offer as shared resources, 
> but the networks exposed in this way must be the ones which are valid, the 
> invalids will not be loaded by isolator, hense will not be exposed.
> 
> Avinash sridharan wrote:
> Let me try explaining my perspective differently. If the operator was to 
> provide erroneous parameter to the isolator, the isolator would  bail out. My 
> perspective is that an erroneous network config is an erroneous parameter 
> being provided to the isolator and we should treat it so. I believe this 
> would simplify any error handling that needs to be done when frameworks 
> launch tasks on misconfigured networks.

Can you please elaborate how this would simplify any error handling that needs 
to be done when frameworks launch tasks on misconfigured networks? How can 
framework launches task on a misconfigured network which will actually not be 
loaded by isolator?


- Qian


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


On March 7, 2016, 12:03 a.m., Qian Zhang wrote:
> 

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [6, 7, 8]

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 7, 2016, 1:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8/
> ---
> 
> (Updated March 7, 2016, 1:09 p.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   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 
> c4c3aa68dc3e6e001f9a746ea5151b8ad958856f 
>   src/examples/test_framework.cpp c9695c4aa20ab8e726ad47b2ebbd3f10777cf828 
>   src/examples/test_http_executor.cpp 
> 55a427fb240fdfa018ac70fa3af5a6654cb71979 
>   src/examples/test_http_framework.cpp 
> 0f8f0b75d68ad9f5da8bd96ca670fad29933a399 
>   src/executor/executor.cpp c3e95ea7e4edf78f2a65ddc15e213aba66e69db2 
>   src/jvm/jvm.cpp 909d34a0112b219456dce76126029c603077e66d 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/logging/logging.cpp 8d9e4e9b200a0df1c67d4e7cd57107b7780f9812 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp b010a819132fb80810e7f8ce96778109f2e8b35e 
>   src/slave/containerizer/mesos/launcher.cpp 
> c4ae1c5c319f9def2f18bafbad102ddf25d3c9f4 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> 8324269c786d2894a4c009428f1a6ec381c209e5 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/containerizer/cgroups_tests.cpp 
> acaed9b3f8a04964092cef413133834d0cf5a145 
>   src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
> 
> Diff: https://reviews.apache.org/r/8/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

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

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On March 7, 2016, 5:09 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8/
> ---
> 
> (Updated March 7, 2016, 5:09 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   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 
> c4c3aa68dc3e6e001f9a746ea5151b8ad958856f 
>   src/examples/test_framework.cpp c9695c4aa20ab8e726ad47b2ebbd3f10777cf828 
>   src/examples/test_http_executor.cpp 
> 55a427fb240fdfa018ac70fa3af5a6654cb71979 
>   src/examples/test_http_framework.cpp 
> 0f8f0b75d68ad9f5da8bd96ca670fad29933a399 
>   src/executor/executor.cpp c3e95ea7e4edf78f2a65ddc15e213aba66e69db2 
>   src/jvm/jvm.cpp 909d34a0112b219456dce76126029c603077e66d 
>   src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/logging/logging.cpp 8d9e4e9b200a0df1c67d4e7cd57107b7780f9812 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp b010a819132fb80810e7f8ce96778109f2e8b35e 
>   src/slave/containerizer/mesos/launcher.cpp 
> c4ae1c5c319f9def2f18bafbad102ddf25d3c9f4 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> 8324269c786d2894a4c009428f1a6ec381c209e5 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/containerizer/cgroups_tests.cpp 
> acaed9b3f8a04964092cef413133834d0cf5a145 
>   src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
> 
> Diff: https://reviews.apache.org/r/8/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42719: Add doc for weights.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42719]

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 7, 2016, 11:35 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42719/
> ---
> 
> (Updated March 7, 2016, 11:35 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3945
> https://issues.apache.org/jira/browse/MESOS-3945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add doc for weights.
> 
> 
> Diffs
> -
> 
>   docs/home.md 821026a12f422385e347e0037d6527efa9ffa2e1 
>   docs/weights.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42719/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 43824: Addressed comments of 41672.

[Yongqiao Wang]

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

(Updated March 7, 2016, 1:55 p.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Addressed comments of Alex.


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


Repository: mesos


Description
---

Addressed comments of 41672.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 

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


Testing
---

make && make check successfully.


Thanks,

Yongqiao Wang

Review Request 44450: Rescind all outstanding offers to satisfy weights update.

[Yongqiao Wang]

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Rescind all outstanding offers to satisfy weights update.


Diffs
-

  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/weights_handler.cpp PRE-CREATION 

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


Testing
---

Make && Make check.

Manual test steps:

- Start Mesos master:
$ ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master

- Start Mesos slave:
$ ./bin/mesos-slave.sh --master=127.0.0.1:5050

- Register a framwork with `curl`
$ curl -v http://127.0.0.1:5050/api/v1/scheduler -H "Content-type: 
application/json" -X POST -d @subscribe.json
$  cat subscribe.json
{
   "type": "SUBSCRIBE",

   "subscribe" : {
  "framework_info": {
  "user" :  "root",
  "name" :  "comsumer c1 HTTP Framework",
  "role" :  "mesos",
  "principal":"wyq"
  },

  "force" : true
  }
}

And this framework will receive an offer:
{"subscribed":{"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"}},"type":"SUBSCRIBED"}20
{"type":"HEARTBEAT"}680
{"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
{"type":"HEARTBEAT"}20

- Update the weight of role `mesos`
$ curl --data "[{\"weight\":1.8,\"role\":\"mesos\"}]" -X PUT 
http://127.0.0.1:5050/weights

Receive an rescind offer, and a new offer received:
{"rescind":{"offer_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"}},"type":"RESCIND"}680
{"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O1"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
{"type":"HEARTBEAT"}20

- Update the weight of role `mesos1`
$ curl --data "[{\"weight\":2.8,\"role\":\"mesos1\"}]" -X PUT 
http://127.0.0.1:5050/weights

- The outstanding offer will not be rescinded.


(TODO) I will add couple of tests for this patch in another JIRA.


Thanks,

Yongqiao Wang

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

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On March 7, 2016, 5:09 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7/
> ---
> 
> (Updated March 7, 2016, 5:09 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   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.

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On March 7, 2016, 5:08 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6/
> ---
> 
> (Updated March 7, 2016, 5:08 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   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
> 
>

Re: Review Request 43824: Addressed comments of 41672.

[Alexander Rukletsov]

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




src/tests/hierarchical_allocator_tests.cpp (line 2445)


Backtick variable names, please. Here and in other newly added comments.


- Alexander Rukletsov


On March 7, 2016, 11:31 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43824/
> ---
> 
> (Updated March 7, 2016, 11:31 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addressed comments of 41672.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43824/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

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

Review request for mesos and Bernd Mathiske.


Repository: mesos


Description
---

See summary.


Diffs
-

  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

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

Review request for mesos and Bernd Mathiske.


Repository: mesos


Description
---

See summary.


Diffs
-

  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 
c4c3aa68dc3e6e001f9a746ea5151b8ad958856f 
  src/examples/test_framework.cpp c9695c4aa20ab8e726ad47b2ebbd3f10777cf828 
  src/examples/test_http_executor.cpp 55a427fb240fdfa018ac70fa3af5a6654cb71979 
  src/examples/test_http_framework.cpp 0f8f0b75d68ad9f5da8bd96ca670fad29933a399 
  src/executor/executor.cpp c3e95ea7e4edf78f2a65ddc15e213aba66e69db2 
  src/jvm/jvm.cpp 909d34a0112b219456dce76126029c603077e66d 
  src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/logging/logging.cpp 8d9e4e9b200a0df1c67d4e7cd57107b7780f9812 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/scheduler/scheduler.cpp b010a819132fb80810e7f8ce96778109f2e8b35e 
  src/slave/containerizer/mesos/launcher.cpp 
c4ae1c5c319f9def2f18bafbad102ddf25d3c9f4 
  src/slave/containerizer/mesos/provisioner/store.cpp 
8324269c786d2894a4c009428f1a6ec381c209e5 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
  src/tests/containerizer/cgroups_tests.cpp 
acaed9b3f8a04964092cef413133834d0cf5a145 
  src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 

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


Testing
---

`make check` on Mac OS 10.10.4


Thanks,

Alexander Rukletsov

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

Review request for mesos and Bernd Mathiske.


Repository: mesos


Description
---

See summary.


Diffs
-

  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

Re: Review Request 43824: Addressed comments of 41672.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43824]

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 7, 2016, 11:31 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43824/
> ---
> 
> (Updated March 7, 2016, 11:31 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addressed comments of 41672.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43824/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

[Yongqiao Wang]

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

(Updated March 7, 2016, 12:19 p.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Introduce HTTP endpoint /weights for updating weight.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
5ee3c7afadd131802c93febbb6b4dbad069c2d81 
  include/mesos/authorizer/authorizer.proto 
84d2cb3fbff3fbc7c3854d6eec5a3a55ad5760f8 
  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/authorizer/local/authorizer.hpp c7321c276d566eca6a91f45c546468bea1b0da15 
  src/authorizer/local/authorizer.cpp a1486bd042e1d59e5ac99c2619fb3228c37b9788 
  src/master/http.cpp 8276baa538eb4d2aaf54cc1aa516bffaadacc4dd 
  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
  src/master/weights_handler.cpp PRE-CREATION 
  src/tests/mesos.hpp d36840f6e23e5823332de53061bf6852330bdf0b 
  src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 

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


Testing
---

Make & Make check successfully!

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
--weights="role1=4.2,role2=3.1" --authenticate_http 
--credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
$ curl http://localhost:5050/roles | python -mjson.tool
{
"roles": [
{
"frameworks": [ ], 
"name": "*", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 1
}, 
{
"frameworks": [ ], 
"name": "role1", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 4.2
}, 
{
"frameworks": [ ], 
"name": "role2", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 3.1
}
]
}

Test update:
$ curl --user framework1:secret_string1 --data 
"[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
 -X PUT http://127.0.0.1:5050/weights
$ curl http://localhost:5050/roles | python -mjson.tool
{
"roles": [
{
"frameworks": [],
"name": "*",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"name": "role1",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.8
},
{
"frameworks": [],
"name": "role2",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"name": "role3",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 3.4
}
]
}

Test recovuery:
$ ps -ef | grep mesos-master
501 56292 1   0  6:18PM ttys0010:00.31 
/Users/yqwyq/Desktop/mesos/build/src/.libs/mesos-master --ip=127.0.0.1 
--work_dir=/tmp/mesos-master --weights=role1=4.2,role2=3.1 --authenticate_http 
--credentials=/opt/credentials.json
$ kill -9 56292

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
--weights="role1=4.2,role2=3.1,role6=9.0" --authenticate_http 
--credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
$ curl http://localhost:5050/roles | python -mjson.tool
{
"roles": [
{
"frameworks": [],
"name": "*",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"name": "role1",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.8
},
{
"frameworks": [],
"name": "role2",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"

Re: Review Request 42719: Add doc for weights.

[Yongqiao Wang]

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

(Updated March 7, 2016, 11:35 a.m.)


Review request for mesos and Adam B.


Changes
---

Updated the released version of dynamic weights to 0.29.


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


Repository: mesos


Description
---

Add doc for weights.


Diffs (updated)
-

  docs/home.md 821026a12f422385e347e0037d6527efa9ffa2e1 
  docs/weights.md PRE-CREATION 

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


Testing
---


Thanks,

Yongqiao Wang

Re: Review Request 43824: Addressed comments of 41672.

[Yongqiao Wang]

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

(Updated March 7, 2016, 11:31 a.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Addressed comments of Alex.


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


Repository: mesos


Description
---

Addressed comments of 41672.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 

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


Testing
---

make && make check successfully.


Thanks,

Yongqiao Wang

Re: Review Request 44286: Unified/Added Future checks for http methods in tests.

[Joerg Schad]

> On March 7, 2016, 9:28 a.m., Adam B wrote:
> > src/tests/fault_tolerance_tests.cpp, lines 230-233
> > 
> >
> > Should we also `AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, 
> > "Content-Type", masterState);`?
> > Or is the `ASSERT_SOME(parse)` good enough to indicate that we received 
> > json?

IMO the ASSERT_SOME(parse) is sufficient, but I will be happy to open an 
additional ticket to check where we would need to verify this. I believe the 
introduction of the AWAIT is more important as other wise the .get() could 
potentially block forever... (being bad for CI systems and tests in general


- Joerg


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


On March 2, 2016, 8:53 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44286/
> ---
> 
> (Updated March 2, 2016, 8:53 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified/Added Future checks for http methods in tests.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> 982468f851cd9d95eb6cde7c57f2d737d46a827c 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
> 
> Diff: https://reviews.apache.org/r/44286/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>