Re: Review Request 53127: Added the test `ProvisionerDockerWhiteoutTest`.

[Jie Yu]

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




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


Hum, does this work? Do you need to have filters? Aufs and overlayfs is not 
always available.


- Jie Yu


On Nov. 1, 2016, 6:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53127/
> ---
> 
> (Updated Nov. 1, 2016, 6:58 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the test `ProvisionerDockerWhiteoutTest`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d8fb7abb55410cfd0835f59bfc0f35566aeeae37 
> 
> Diff: https://reviews.apache.org/r/53127/diff/
> 
> 
> Testing
> ---
> 
> [==] Running 3 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 3 tests from BackendFlag/ProvisionerDockerWhiteoutTest
> [ RUN  ] 
> BackendFlag/ProvisionerDockerWhiteoutTest.ROOT_INTERNET_CURL_Whiteout/0
> Executing pre-exec command 
> '{"arguments":["mesos-containerizer","mount","--help=false","--operation=make-rslave","--path=\/"],"shell":false,"value":"\/home\/stack\/workspace\/mesos\/build\/src\/mesos-containerizer"}'
> Executing pre-exec command 
> '{"arguments":["mount","-n","--rbind","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX\/slaves\/5ec98305-4a7d-4351-976e-7c7f4023329c-S0\/frameworks\/5ec98305-4a7d-4351-976e-7c7f4023329c-\/executors\/7bea80dc-b644-430f-abb0-cabe2af3ded8\/runs\/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX\/provisioner\/containers\/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1\/backends\/aufs\/rootfses\/8837b468-156f-4e04-96b2-ab201626ad3c\/mnt\/mesos\/sandbox"],"shell":false,"value":"mount"}'
> I1024 16:17:23.245787 13269 exec.cpp:162] Version: 1.2.0
> I1024 16:17:23.256351 13271 exec.cpp:237] Executor registered on agent 
> 5ec98305-4a7d-4351-976e-7c7f4023329c-S0
> Received SUBSCRIBED event
> Subscribed executor on workstation
> Received LAUNCH event
> Starting task 7bea80dc-b644-430f-abb0-cabe2af3ded8
> /home/stack/workspace/mesos/build/src/mesos-containerizer launch 
> --command="{"arguments":["test","!","-f","\/etc\/rc3.d\/S40-network"],"shell":false,"value":"\/usr\/bin\/test"}"
>  --help="false" 
> --rootfs="/tmp/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX/provisioner/containers/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1/backends/aufs/rootfses/8837b468-156f-4e04-96b2-ab201626ad3c"
>  --unshare_namespace_mnt="true" --user="root" 
> --working_directory="/mnt/mesos/sandbox"
> Forked command at 13278
> Changing root to 
> /tmp/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX/provisioner/containers/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1/backends/aufs/rootfses/8837b468-156f-4e04-96b2-ab201626ad3c
> Command exited with status 0 (pid: 13278)
> I1024 16:17:23.500792 13273 exec.cpp:414] Executor asked to shutdown
> Received SHUTDOWN event
> Shutting down
> [   OK ] 
> BackendFlag/ProvisionerDockerWhiteoutTest.ROOT_INTERNET_CURL_Whiteout/0 (1365 
> ms)
> [ RUN  ] 
> BackendFlag/ProvisionerDockerWhiteoutTest.ROOT_INTERNET_CURL_Whiteout/1
> Executing pre-exec command 
> '{"arguments":["mesos-containerizer","mount","--help=false","--operation=make-rslave","--path=\/"],"shell":false,"value":"\/home\/stack\/workspace\/mesos\/build\/src\/mesos-containerizer"}'
> Executing pre-exec command 
> '{"arguments":["mount","-n","--rbind","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_1_Zm1mK7\/slaves\/9d49d96c-189d-47e9-9370-72146356af7c-S0\/frameworks\/9d49d96c-189d-47e9-9370-72146356af7c-\/executors\/ae8fda3e-db36-49f7-8699-9a4768d4af27\/runs\/49f24d09-6474-4a39-9da6-8c8da0ad4238","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_1_Zm1mK7\/provisioner\/containers\/49f24d09-6474-4a39-9da6-8c8da0ad4238\/backends\/copy\/rootfses\/db73608a-614b-49be-bebe-cd9aa42e31f5\/mnt\/mesos\/sandbox"],"shell":false,"value":"mount"}'
> I1024 16:17:24.971269 13344 exec.cpp:162] Version: 1.2.0
> I1024 16:17:24.981901 13364 exec.cpp:237] Executor registered on agent 
> 9d49d96c-189d-47e9-9370-72146356af7c-S0
> Received SUBSCRIBED event
> Subscribed executor on workstation
> Received LAUNCH event
> Starting task ae8fda3e-db36-49f7-8699-9a4768d4af27
> /home/stack/workspace/mesos/build/src/mesos-containerizer launch 
> 

Review Request 53372: MESOS-5662: Fixed parent class SetUpTestCase calls.

[Manuwela Kanade]

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

MESOS-5662: Fixed parent class SetUpTestCase calls.


Diffs
-

  src/tests/containerizer/memory_pressure_tests.cpp 
42ae3232a5e0c508382ba3be97c4ee9787151236 
  src/tests/mesos.cpp 2aae160fb941ab3672a5665ae27f517ff40600e2 
  src/tests/module_tests.cpp 6e003e56582b58b4a6c1d14105159e1190c36fb9 

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


Testing
---

make check


Thanks,

Manuwela Kanade

Re: Review Request 53115: Implemented handling AUFS whiteouts for copy backend.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/backends/copy.cpp (lines 156 - 157)


This sounds pretty hacky to me. What if there are multiple 'rootfs' along 
the path?

Can you use 'layer' to calculate the relative path?


- Jie Yu


On Nov. 1, 2016, 6:44 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53115/
> ---
> 
> (Updated Nov. 1, 2016, 6:44 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented handling AUFS whiteouts for copy backend.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> 9c5354e5fea488618ebdecf0aef9dd2fce555d20 
> 
> Diff: https://reviews.apache.org/r/53115/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 53369: Agent cgroup assignment should precede agent initialization.

[Jie Yu]

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



The agent subsystems is a hack to me. I think we should consider support 
running systemd (or other init system) to manage agent process and put it under 
proper cgroup using the init system, rather than doing it ourself.

- Jie Yu


On Nov. 2, 2016, 4:43 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53369/
> ---
> 
> (Updated Nov. 2, 2016, 4:43 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6523
> https://issues.apache.org/jira/browse/MESOS-6523
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is to ensure that we do not accept traffic on the agent by opening
> up libprocess port only after cgroup assigment for the agent is done.
> 
> 
> Diffs
> -
> 
>   src/slave/main.cpp 8010f8e229e2d820649750c9db0456ecd1b854d3 
>   src/slave/slave.cpp d6c337345707993b0729e9eaf36b5a9ecc52dc72 
> 
> Diff: https://reviews.apache.org/r/53369/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

[Yubo Li]

> On 十一月 1, 2016, 3:04 p.m., Guangya Liu wrote:
> > src/slave/containerizer/docker.cpp, line 701
> > 
> >
> > You need to keep the comments from Kevin as open here 
> > https://reviews.apache.org/r/50599/#comment223322

OK, I re-opened Kevin's comment.


- Yubo


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


On 十一月 1, 2016, 9:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十一月 1, 2016, 9:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Review Request 53369: Agent cgroup assignment should precede agent initialization.

[Anindya Sinha]

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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

This is to ensure that we do not accept traffic on the agent by opening
up libprocess port only after cgroup assigment for the agent is done.


Diffs
-

  src/slave/main.cpp 8010f8e229e2d820649750c9db0456ecd1b854d3 
  src/slave/slave.cpp d6c337345707993b0729e9eaf36b5a9ecc52dc72 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 53253: Fixed the bug when search base hierarchy in `cgroups_tests.cpp`.

[haosdent huang]

> On Nov. 1, 2016, 1:20 a.m., Jiang Yan Xu wrote:
> > We unfortunately have to copy the cleanup code from 
> > `ContainerizerTest::SetUp()` to here. FWIW the 
> > test fixture originally was written with co-mounted subsystems in a single 
> > hierarchy (hence *Any*), later altered when Mesos started to use separate 
> > hierarchies for each subsystem, but not in a clean way IMO.
> > 
> > The `CgroupsAnyHierarchyTest` fixture and its children AFAICT still works 
> > with one hierarchy with multiple co-mounted subsystems. In fact Mesos does 
> > support co-mounted subsystems.
> > 
> > So IMO it wouldn't be a bad idea to have tests in cgroups_tests.cpp to only 
> > **create** one hierarchy for all the specified subsystems (most tests 
> > assume one). Of course it's fine if the subsystems are already mounted in 
> > separate hierarchies but the tests only manages (create/cleanup) a single 
> > hierachy.
> > 
> > This would make fixes for related problems (e.g., MESOS-6422) cleaner as 
> > well.
> > 
> > Thoughts?
> 
> haosdent huang wrote:
> To be honest, I prefer to follow what Linux do now which mount subsystems 
> in different folders. Suppose the Linux have already mounted 
> `/sys/fs/cgroups/cpu`, `/sys/fs/cgroups/memory`. And in our test case, we 
> create `/sys/fs/cgroups/unified-mount` and mount rest subsystems(`devices`, 
> `net_cls`, `perf_event`) on it. Then it looks wried. If we choose to mount 
> them separately in our test cases, e.g, `/sys/fs/cgroups/devices`, 
> `/sys/fs/cgroups/perf_event` and etc. This is more consistent with current 
> operate system behaviour.
> 
> Co-mounted all hierarchies simplfy work if there is not any exist 
> subsystems mounted in current machine. But it would become tricky if some 
> subystems have been mounted. Anything I miss here?
> 
> Jiang Yan Xu wrote:
> Let me clarify:
> 
> 1. Mesos works with co-mounted subsystems: 
> https://github.com/apache/mesos/commit/31337348cef29719890bffb59fbf8df8b18b39d0
> 2. At the abstraction level of Mesos agent and Mesos containerizer, it's 
> OK if we **require** separately mounted subsystems in tests since it's what 
> we **currently** prefer.
> 3. However, at the abstraction level of linux/cgroups.cpp and 
> cgroups_tests.cpp, it's **OK** for the tests to **create** a single hierarchy 
> (so it only needs to cleanup one) but **allow** existing hierarhies (e.g., 
> `/sys/fs/cgroup/*`) with single subsystems because neither the logic in 
> linux/cgroups.cpp nor cgroups_tests.cpp require separately mounted 
> subsystems; that's above its abstraction level; linux/cgroups.cpp is just a 
> utility and as such doesn't have preferences on this.
> 
> The benefit is the simplicity in cleanups in cgroups_tests.cpp: you only 
> need to recursively destroy `TEST_CGROUPS_ROOT` and you clean up 
> `TEST_CGROUPS_HIERARCHY` (because it's always a hierarchy and never a base 
> hierarchy) and you are done. If `CgroupsAnyHierarchyTest` and its descendants 
> ever use existing hierarhies (e.g., `/sys/fs/cgroup/*`), easy, just destroy 
> `TEST_CGROUPS_ROOT`. It would be cleaner than copying the code over from 
> mesos.cpp and MESOS-6422 is fixed as well.

Suppose we have mount `cpu` and `memory` subsystems, and we choose the single 
hierarchy way in our test cases.
Then we still need to find out the base hierarchy first and use it to create 
`TEST_CGROUPS_HIERARCHY` under that base hierarchy.
Because we don't support multiple cgroups base hierarchies now. Is this fine? 
If so, I would update the patch chain and fix MESOS-6422 together.


- haosdent


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


On Oct. 28, 2016, 5:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53253/
> ---
> 
> (Updated Oct. 28, 2016, 5:13 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the bug when search base hierarchy in `cgroups_tests.cpp`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0afaec6ae948cabf1472bf01103210d8f9809cb1 
> 
> Diff: https://reviews.apache.org/r/53253/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53361: Fixed memory leak in request/response decoders.

[Anand Mazumdar]

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

(Updated Nov. 2, 2016, 4:27 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Review comments, NNFR.


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


Repository: mesos


Description
---

The leak can happen in cases where a client disconnects while the
request/response is in progress.


Diffs (updated)
-

  3rdparty/libprocess/src/decoder.hpp c79296bc50e5bf5adf9f2c62114bef83bb183f79 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 53296: Added cgroup namespace support for unified container.

[haosdent huang]

> On Nov. 1, 2016, 4:43 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp, line 28
> > 
> >
> > Instead of creating a new namespace/cgroup isolator, I would suggest we 
> > add the support to cgroups isolator. It looks weird to me to have a 
> > namespace/cgroup isolator without using the cgroups isolator.

I think it still possible to use `namespaces/cgroup` isolator without `cgroups` 
isolation? If user only want to isolate the host cgroups environment from the 
container.


- haosdent


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


On Oct. 30, 2016, 4:45 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Oct. 30, 2016, 4:45 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroup namespace support for unified container.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 639f8678ba23c4d9a2ea0bf84fbc3d6fc9286ef3 
>   src/Makefile.am c2f9e442182110d0b450d4824600a4a791f8cf27 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53296/diff/
> 
> 
> Testing
> ---
> 
> The test case is on the way.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50128: Added helper function to 'Docker::Device'.

[Yubo Li]

> On 十月 26, 2016, 3:22 a.m., Kevin Klues wrote:
> > src/docker/docker.cpp, line 388
> > 
> >
> > Add an error here to verify correct set of permissions in the 
> > permissions field.
> > 
> > Once we have done all of this validation, we know it is safe to set the 
> > fields as we do below without error.
> 
> Guangya Liu wrote:
> This is only called after `docker inspect`, so I think that there will be 
> no error field in the permission field? But adding a check here will not 
> impact much.

I think we could put it to another patch.


- Yubo


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


On 十一月 1, 2016, 9:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50128/
> ---
> 
> (Updated 十一月 1, 2016, 9:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Wrapped helper function to stringify 'Docker::Device' structure.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
> 
> Diff: https://reviews.apache.org/r/50128/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50128: Added helper function to 'Docker::Device'.

[Yubo Li]

> On 十一月 1, 2016, 3:19 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 756
> > 
> >
> > I think that we actually need to check invalid permissions here, as it 
> > is possible that there are invalid permissions when create container. But 
> > agree that we can handle this in a separate patch.

Sure


- Yubo


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


On 十一月 1, 2016, 9:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50128/
> ---
> 
> (Updated 十一月 1, 2016, 9:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Wrapped helper function to stringify 'Docker::Device' structure.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
> 
> Diff: https://reviews.apache.org/r/50128/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 52645: Harden Mesos

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52647, 52886, 52754, 52645]

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

- Mesos ReviewBot


On Nov. 1, 2016, 7:37 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52645/
> ---
> 
> (Updated Nov. 1, 2016, 7:37 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> Mesos. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   configure.ac c8d48be 
>   m4/ax_check_compile_flag.m4 PRE-CREATION 
>   src/Makefile.am c2f9e44 
> 
> Diff: https://reviews.apache.org/r/52645/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 53361: Fixed memory leak in request/response decoders.

[Benjamin Mahler]

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


Fix it, then Ship it!





3rdparty/libprocess/src/decoder.hpp (line 278)


Seems we need to also delete these requests? Ditto for the other deques.


- Benjamin Mahler


On Nov. 2, 2016, 12:20 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53361/
> ---
> 
> (Updated Nov. 2, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6527
> https://issues.apache.org/jira/browse/MESOS-6527
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The leak can happen in cases where a client disconnects while the
> request/response is in progress.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> c79296bc50e5bf5adf9f2c62114bef83bb183f79 
> 
> Diff: https://reviews.apache.org/r/53361/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 53366: Introduced a streaming gzip::Decompressor.

[Benjamin Mahler]

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

This enables incremental decompression of compressed input.


Diffs
-

  3rdparty/stout/include/stout/gzip.hpp 
b78a8a31204ee585f8e4a88eaefe7346faa46b8d 
  3rdparty/stout/tests/gzip_tests.cpp 814fb99da193cf950ba48817824acdabda2c2dfc 

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


Testing
---

Wrote a test for decompressing a byte at a time. Also ran this test manually 
with random chunk sizes.


Thanks,

Benjamin Mahler

Review Request 53365: Fixed an issue in the gzip error handling.

[Benjamin Mahler]

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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

It turns out that zlib does not always set the `msg` field, the
calling code is expected to handle the case where `msg` is NULL.
I discovered this while I was playing with the library during
the implementation of a streaming decompressor.


Diffs
-

  3rdparty/stout/include/stout/gzip.hpp 
b78a8a31204ee585f8e4a88eaefe7346faa46b8d 

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


Testing
---

make check


Thanks,

Benjamin Mahler

Review Request 53361: Fixed memory leak in request/response decoders.

[Anand Mazumdar]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

The leak can happen in cases where a client disconnects while the
request/response is in progress.


Diffs
-

  3rdparty/libprocess/src/decoder.hpp c79296bc50e5bf5adf9f2c62114bef83bb183f79 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 53161: Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.

[Jie Yu]

> On Nov. 1, 2016, 5 p.m., Zhitao Li wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp, lines 341-352
> > 
> >
> > Do we need to worry about previously cached image layers in stores, if 
> > the `image_provisioner_backend` flag value is changed from something else 
> > to `overlay`?

Nope, because we use the backend name as the suffix (e.g., `rootfs.overlay`). 
The content in `rootfs` from the previou layers pulled from docker registry 
should be in aufs whiteout format.

So switching the flag should be safe. The invariant here is that anything under 
`rootfs.overlay` will be in overlay whiteout format, and anything under 
`rootfs` should be in aufs whiteout format.


- Jie


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


On Nov. 1, 2016, 6:42 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53161/
> ---
> 
> (Updated Nov. 1, 2016, 6:42 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am c2f9e442182110d0b450d4824600a4a791f8cf27 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> e192f86a1848b373f3aa73d29688a96375cac313 
>   src/slave/containerizer/mesos/provisioner/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/utils.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53161/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52587: Allow CREATE of shared volumes based on capability of framework.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52587]

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

- Mesos ReviewBot


On Nov. 1, 2016, 4:54 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52587/
> ---
> 
> (Updated Nov. 1, 2016, 4:54 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6316
> https://issues.apache.org/jira/browse/MESOS-6316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework issues a CREATE for a shared volume, we allow that
> operation only if the framework has opted in for the capability of
> SHARED_RESOURCES. However, for HTTP operator (/create-volumes), we
> do not check as the operator API is not related to a specific
> framework.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 013bb592ba47b785c552e199633e4784e8aa71b1 
>   src/master/validation.hpp 035f721c610ae566c89a1cf0e65ff0df11679f15 
>   src/master/validation.cpp f690a9eacd278b51a52f5588dbeea377df074435 
>   src/tests/master_validation_tests.cpp 
> a5d8610bd61822cdf55cbc8d7056e5cf8fecfa54 
>   src/tests/persistent_volume_tests.cpp 
> 6289009fe9ed0a57ba5eff46dbbe0633a75d2616 
> 
> Diff: https://reviews.apache.org/r/52587/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 53124: Add documentation on Windows support

[Joseph Wu]

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


Fix it, then Ship it!





docs/windows.md (lines 61 - 62)


Our markdown renderer doesn't transform this into a list unless there is a 
newline in between here.

I'll add it before committing.



docs/windows.md (line 68)


"Notes" are usually added like: `**NOTE**:`.  I can fix this.


- Joseph Wu


On Oct. 31, 2016, 2:49 p.m., Lior Zeno wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53124/
> ---
> 
> (Updated Oct. 31, 2016, 2:49 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Alex Clemmer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds documentation on Windows support in Mesos.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md 69e8da5 
>   docs/home.md f47f7f9 
>   docs/windows.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53124/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lior Zeno
> 
>

Re: Review Request 53324: Add a column for FrameworkID when displaying tasks in the WebUI

[Joseph Wu]

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


Fix it, then Ship it!




Checked out the tables on a local cluster, looks good.


src/webui/master/static/home.html (lines 167 - 169)


Hm... Seems like some tabs snuck into your HTML...  I'll remove them before 
committing.


- Joseph Wu


On Nov. 1, 2016, 2:47 p.m., Miguel Bernadin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53324/
> ---
> 
> (Updated Nov. 1, 2016, 2:47 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-6369
> https://issues.apache.org/jira/browse/MESOS-6369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a column for FrameworkID when displaying tasks in the WebUI
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html 07f862f69802bef8fda2d0a94e33c3b463ddcd4c 
> 
> Diff: https://reviews.apache.org/r/53324/diff/
> 
> 
> Testing
> ---
> 
> Tested on my AWS Mesos Cluster
> 
> 
> Thanks,
> 
> Miguel Bernadin
> 
>

Review Request 53354: Updated namespace isolators to customize based on 'ContainerClass'.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The namespace-related isolators now do different things depending on
whether they are launching a "normal" nested container or a "debug"
nested container. Normal nested containers clone a new mount namespace
as well as a new pid namespace. Debug nested cotnainers do not -- they
simply inherit these namespaces from their parent.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
67cc595278f124cdf518d2f4fcfb257439f067e2 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
af9f3736b487b595e8768e56ce60dc4823db28a1 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
df16b8fee6799a69c7d96f33a5049bd9787c48f5 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
a1283e5ee92c916baaf9fca8ce314d597e8421b3 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
e3756c920081f2944bf4b640edf0a83f42784586 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
0d9ec57d9aa83bcc6cc2e5a8d75f2e2251179b1b 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
939142e36b926d9e4201d35dedd25e32e9f8c63c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
48202fb5bf1ede71b80760844c6d8a36ca7c700c 
  src/slave/containerizer/mesos/isolators/volume/image.cpp 
210e67ad0d84f52135e77184f21e574c9e31628d 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
7b976d29226c3e0a4d52922e9d2f7e685de72297 
  src/slave/containerizer/mesos/linux_launcher.cpp 
0305d14c1f791c93edcd3b32786b483b15f40a2d 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
e6c690c411f57138207044f31b4816bd4090c1b7 

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


Testing
---

make -j check
(Some tests are still fialing though -- need to debug)


Thanks,

Kevin Klues

Review Request 53353: Introduced a new 'DEBUG' container class.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Introduced a new 'DEBUG' container class.


Diffs
-

  include/mesos/slave/containerizer.proto 
f4c4ad771b5dead4ea3ee7cd1b4383c4dc2359b4 

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


Testing
---

make -j check


Thanks,

Kevin Klues

Review Request 53351: Added 'ContainerClass' to help decide how best to launch a container.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

A 'ContainerClass' must now be passed to all *nested*
containerizer->launch() calls in order to specify the class of
container being launched. For now, we simply have the default nested
container class (called 'NESTED') in a subsequent commit we will
introduce the 'DEBUG' class to help classify debug-based nested
containers that should be launched slightly differently than default
nested containers.

The 'ContainerClass' specified is passed through to each isolator in
'ContainerConfig' so they have a chance to custiomize their isolation
policies based on the class as well.


Diffs
-

  include/mesos/slave/containerizer.proto 
f4c4ad771b5dead4ea3ee7cd1b4383c4dc2359b4 
  src/slave/containerizer/composing.hpp 
68c0523bb96e1078bd95c6094bce04c38c2fc9dd 
  src/slave/containerizer/composing.cpp 
0c5489f8dffaf75bce26a042000eaf7376f05ff4 
  src/slave/containerizer/containerizer.hpp 
7554446ac51a9063bd52b8b99845c73650740849 
  src/slave/containerizer/mesos/containerizer.hpp 
c4fea8b56c39d5a363f6ea80bd109fd2d3db52d9 
  src/slave/containerizer/mesos/containerizer.cpp 
67cc595278f124cdf518d2f4fcfb257439f067e2 
  src/slave/http.cpp a32aca437cffd52bc2bcde859eedddca2038e3f1 
  src/tests/api_tests.cpp f87130d7554f98265ff6da39ea1d366839193e85 
  src/tests/containerizer.hpp d418ecebee155f894187e24549f42497372d8a5f 
  src/tests/containerizer.cpp c2208660352e5cf62cec5c7b92ec67fea62aea49 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
e6c690c411f57138207044f31b4816bd4090c1b7 
  src/tests/containerizer/volume_image_isolator_tests.cpp 
f14a0bd1c44c069c2784638132a08072a2343309 
  src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
f873e288f72dbecc33b9c1c817688332c0eb3d31 

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


Testing
---

make -j check


Thanks,

Kevin Klues

Review Request 53352: Updated 'LinuxLauncher->fork()` with *enter* and *clone* namespaces.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Instead of only taking the namespaces to *clone* inside a newly forked
container, 'LinuxLauncher->fork()' now takes a list of namespaces to
enter inside a parent container before forking. When both an *enter*
and a *clone* for the same namespace are passed, we will first enter
the namespace of the parent, and then clone a new one.


Diffs
-

  include/mesos/slave/containerizer.proto 
f4c4ad771b5dead4ea3ee7cd1b4383c4dc2359b4 
  src/slave/containerizer/mesos/containerizer.cpp 
67cc595278f124cdf518d2f4fcfb257439f067e2 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
af9f3736b487b595e8768e56ce60dc4823db28a1 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
df16b8fee6799a69c7d96f33a5049bd9787c48f5 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
a1283e5ee92c916baaf9fca8ce314d597e8421b3 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
e3756c920081f2944bf4b640edf0a83f42784586 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
0d9ec57d9aa83bcc6cc2e5a8d75f2e2251179b1b 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
939142e36b926d9e4201d35dedd25e32e9f8c63c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
48202fb5bf1ede71b80760844c6d8a36ca7c700c 
  src/slave/containerizer/mesos/isolators/volume/image.cpp 
210e67ad0d84f52135e77184f21e574c9e31628d 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
7b976d29226c3e0a4d52922e9d2f7e685de72297 
  src/slave/containerizer/mesos/launcher.hpp 
6ceb02de5dc143e545e2fec43e2608916e46b898 
  src/slave/containerizer/mesos/launcher.cpp 
b45313fd717f9553ccb0cbe9e8ac095e2536944a 
  src/slave/containerizer/mesos/linux_launcher.hpp 
d2353055a838c872d5852982cfede8e38c6e8701 
  src/slave/containerizer/mesos/linux_launcher.cpp 
0305d14c1f791c93edcd3b32786b483b15f40a2d 
  src/tests/containerizer/isolator_tests.cpp 
8fefeef8c83ed2ab01f56a1ec572d3acb307143c 
  src/tests/containerizer/launcher.hpp 773b458f19e11b219c3f13a43f2b751a4bbe7b85 
  src/tests/containerizer/launcher.cpp a92d9890f0931425d69ef8ce0896d081b8722079 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
4df537747d84daa68c29e2d05b22fa386a4a16db 
  src/tests/containerizer/port_mapping_tests.cpp 
fbdc0db9238c85d2f6eaba7d13ee5ce23342b527 

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


Testing
---

make -j check


Thanks,

Kevin Klues

Re: Review Request 52695: Harden libprocess

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Nov. 1, 2016, 7:12 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 1, 2016, 7:12 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use a default set of flags to provide additional security and hardening to 
> libprocess. Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 7131989 
>   3rdparty/libprocess/configure.ac 1644035 
> 
> Diff: https://reviews.apache.org/r/52695/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 53305: Add new param user to logrotate's prepare function.

[Sivaram Kannan]

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




src/slave/container_loggers/lib_logrotate.cpp (line 167)


I think we can delete the rbr #23209 with this change in place right? 
Should I discard #23209 ?


- Sivaram Kannan


On Nov. 1, 2016, 2 p.m., Sivaram Kannan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53305/
> ---
> 
> (Updated Nov. 1, 2016, 2 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5856
> https://issues.apache.org/jira/browse/MESOS-5856
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add new param user to logrotate's prepare function.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/container_logger.hpp 
> 939974736f9eb744c83036e074718d2a1eba8b0a 
>   src/slave/container_loggers/lib_logrotate.hpp 
> 28fdf3bdcc66d473521b377f66ab0b48f6900f58 
>   src/slave/container_loggers/lib_logrotate.cpp 
> 53698d339f0f4d2dc916b53239ca0c36bbebcd42 
>   src/slave/container_loggers/logrotate.hpp 
> d1db69236f5a9b1dbb3113ad02218a512afdb46b 
>   src/slave/container_loggers/sandbox.hpp 
> e0aeb32a9ec83af049af8a10010b819c1d8b25d8 
>   src/slave/container_loggers/sandbox.cpp 
> cc263ebef7e0c3e778fabafa49faa6dd315adc45 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
> 
> Diff: https://reviews.apache.org/r/53305/diff/
> 
> 
> Testing
> ---
> 
> 1. Run the mesos-logrotate-logger with un-priviledged user and verify whether 
> the logs are getting rotated.
> 2. Run the mesos-logrotate-logger as root user and verify whether the logs 
> are getting rotated.
> 3. Run all the unit test with sudo - sudo bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Sivaram Kannan
> 
>

Re: Review Request 53341: Fix stout build option argument handling.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53136, 53137, 53138, 53341]

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

- Mesos ReviewBot


On Nov. 1, 2016, 4:26 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53341/
> ---
> 
> (Updated Nov. 1, 2016, 4:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kapil Arya.
> 
> 
> Bugs: MESOS-2537
> https://issues.apache.org/jira/browse/MESOS-2537
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix all the AC_ARG_ENABLED options to correctly handle --enable-foo
> and --disable-foo variants. Update all the invocations to have a
> consistent calling convention.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/configure.ac cbb0fdb0e1584ace7bbd6bf496f0f55b9846ce0b 
> 
> Diff: https://reviews.apache.org/r/53341/diff/
> 
> 
> Testing
> ---
> 
> "make distcheck"
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53253: Fixed the bug when search base hierarchy in `cgroups_tests.cpp`.

[Jiang Yan Xu]

> On Oct. 31, 2016, 6:20 p.m., Jiang Yan Xu wrote:
> > We unfortunately have to copy the cleanup code from 
> > `ContainerizerTest::SetUp()` to here. FWIW the 
> > test fixture originally was written with co-mounted subsystems in a single 
> > hierarchy (hence *Any*), later altered when Mesos started to use separate 
> > hierarchies for each subsystem, but not in a clean way IMO.
> > 
> > The `CgroupsAnyHierarchyTest` fixture and its children AFAICT still works 
> > with one hierarchy with multiple co-mounted subsystems. In fact Mesos does 
> > support co-mounted subsystems.
> > 
> > So IMO it wouldn't be a bad idea to have tests in cgroups_tests.cpp to only 
> > **create** one hierarchy for all the specified subsystems (most tests 
> > assume one). Of course it's fine if the subsystems are already mounted in 
> > separate hierarchies but the tests only manages (create/cleanup) a single 
> > hierachy.
> > 
> > This would make fixes for related problems (e.g., MESOS-6422) cleaner as 
> > well.
> > 
> > Thoughts?
> 
> haosdent huang wrote:
> To be honest, I prefer to follow what Linux do now which mount subsystems 
> in different folders. Suppose the Linux have already mounted 
> `/sys/fs/cgroups/cpu`, `/sys/fs/cgroups/memory`. And in our test case, we 
> create `/sys/fs/cgroups/unified-mount` and mount rest subsystems(`devices`, 
> `net_cls`, `perf_event`) on it. Then it looks wried. If we choose to mount 
> them separately in our test cases, e.g, `/sys/fs/cgroups/devices`, 
> `/sys/fs/cgroups/perf_event` and etc. This is more consistent with current 
> operate system behaviour.
> 
> Co-mounted all hierarchies simplfy work if there is not any exist 
> subsystems mounted in current machine. But it would become tricky if some 
> subystems have been mounted. Anything I miss here?

Let me clarify:

1. Mesos works with co-mounted subsystems: 
https://github.com/apache/mesos/commit/31337348cef29719890bffb59fbf8df8b18b39d0
2. At the abstraction level of Mesos agent and Mesos containerizer, it's OK if 
we **require** separately mounted subsystems in tests since it's what we 
**currently** prefer.
3. However, at the abstraction level of linux/cgroups.cpp and 
cgroups_tests.cpp, it's **OK** for the tests to **create** a single hierarchy 
(so it only needs to cleanup one) but **allow** existing hierarhies (e.g., 
`/sys/fs/cgroup/*`) with single subsystems because neither the logic in 
linux/cgroups.cpp nor cgroups_tests.cpp require separately mounted subsystems; 
that's above its abstraction level; linux/cgroups.cpp is just a utility and as 
such doesn't have preferences on this.

The benefit is the simplicity in cleanups in cgroups_tests.cpp: you only need 
to recursively destroy `TEST_CGROUPS_ROOT` and you clean up 
`TEST_CGROUPS_HIERARCHY` (because it's always a hierarchy and never a base 
hierarchy) and you are done. If `CgroupsAnyHierarchyTest` and its descendants 
ever use existing hierarhies (e.g., `/sys/fs/cgroup/*`), easy, just destroy 
`TEST_CGROUPS_ROOT`. It would be cleaner than copying the code over from 
mesos.cpp and MESOS-6422 is fixed as well.


- Jiang Yan


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


On Oct. 27, 2016, 10:13 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53253/
> ---
> 
> (Updated Oct. 27, 2016, 10:13 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the bug when search base hierarchy in `cgroups_tests.cpp`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0afaec6ae948cabf1472bf01103210d8f9809cb1 
> 
> Diff: https://reviews.apache.org/r/53253/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

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

(Updated Nov. 1, 2016, 7:37 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Missed the `ax_check_compile_flag.m4` macro that should have been included with 
the previous update.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
Mesos. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  configure.ac c8d48be 
  m4/ax_check_compile_flag.m4 PRE-CREATION 
  src/Makefile.am c2f9e44 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52886: Fix new sign comparison errors in stout produced by hardened flags

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Oct. 27, 2016, 7:32 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52886/
> ---
> 
> (Updated Oct. 27, 2016, 7:32 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in stout that 
> need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/cache_tests.cpp 0950c85 
>   3rdparty/stout/tests/flags_tests.cpp da4deb9 
>   3rdparty/stout/tests/hashmap_tests.cpp 2626d67 
>   3rdparty/stout/tests/hashset_tests.cpp 66e59db 
>   3rdparty/stout/tests/ip_tests.cpp b5a206f 
>   3rdparty/stout/tests/json_tests.cpp 2bc4c88 
>   3rdparty/stout/tests/linkedhashmap_tests.cpp 7a80769 
>   3rdparty/stout/tests/multimap_tests.cpp 488991b 
>   3rdparty/stout/tests/os/process_tests.cpp 4cb3b5f 
>   3rdparty/stout/tests/os/sendfile_tests.cpp e221689 
>   3rdparty/stout/tests/os/systems_tests.cpp 110ba5b 
>   3rdparty/stout/tests/os_tests.cpp 0b7ee07 
>   3rdparty/stout/tests/strings_tests.cpp 7dd3301 
> 
> Diff: https://reviews.apache.org/r/52886/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran make && make check && make bench.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Oct. 27, 2016, 4:51 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Oct. 27, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in libprocess 
> that need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp c79296b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp ab2b5a9 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 533104c 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 0dc1c62 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52645: Harden Mesos

[Aaron Wood]

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

(Updated Nov. 1, 2016, 7:06 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
Mesos. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  configure.ac c8d48be 
  src/Makefile.am c2f9e44 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52695: Harden libprocess

[Aaron Wood]

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

(Updated Nov. 1, 2016, 7:12 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Only use `-fstack-protector-strong` if it's available to us. Remove `-Werror` 
and tackle this in another discussion/JIRA.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
libprocess. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 7131989 
  3rdparty/libprocess/configure.ac 1644035 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52696: Harden stout

[Aaron Wood]

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

(Updated Nov. 1, 2016, 7:10 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Only use `-fstack-protector-strong` when it's available. Remove `-Werror` and 
tackle this in another discussion/JIRA.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
stout. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  3rdparty/stout/Makefile.am 4e10ae2 
  3rdparty/stout/configure.ac cbb0fdb 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52877]

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

- Mesos ReviewBot


On Nov. 1, 2016, 2:45 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52877/
> ---
> 
> (Updated Nov. 1, 2016, 2:45 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6349
> https://issues.apache.org/jira/browse/MESOS-6349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resolves an isse with the JSON serialization functions where floats
> would use the process active locale as a decimal separator instead
> of the JSON conformant period (`.`).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 9734561549eea867d57f14a0ab71febeb9dddc06 
>   3rdparty/stout/include/stout/jsonify.hpp 
> 8220d001001e8b9f247c05be58534f1174611aad 
> 
> Diff: https://reviews.apache.org/r/52877/diff/
> 
> 
> Testing
> ---
> 
> In order to test the different options the following benchmarking program was
> written using [Google benchmark](https://github.com/google/benchmark):
> 
> ```c++
> #include 
> #include 
> 
> #include 
> #include 
> #include 
> #include 
> 
> static void BM_Jsonify(benchmark::State& state) {
>   while (state.KeepRunning()) {
> state.PauseTiming();
> JSON::Object object;
> object.values["float00"] = 1234567890.12345;
> object.values["float01"] = 123456789.012345;
> object.values["float02"] = 12345678.9012345;
> object.values["float03"] = 1234567.89012345;
> object.values["float04"] = 123456.789012345;
> object.values["float05"] = 12345.6789012345;
> object.values["float06"] = 1234.56789012345;
> object.values["float07"] = 123.456789012345;
> object.values["float08"] = 12.3456789012345;
> object.values["float09"] = 1.23456789012345;
> object.values["float10"] = 1234567890.12345;
> object.values["float11"] = 123456789.012345;
> object.values["float12"] = 12345678.9012345;
> object.values["float13"] = 1234567.89012345;
> object.values["float14"] = 123456.789012345;
> object.values["float15"] = 12345.6789012345;
> object.values["float16"] = 1234.56789012345;
> object.values["float17"] = 123.456789012345;
> object.values["float18"] = 12.3456789012345;
> object.values["float19"] = 1.23456789012345;
> object.values["float20"] = 1234567890.12345;
> object.values["float21"] = 123456789.012345;
> object.values["float22"] = 12345678.9012345;
> object.values["float23"] = 1234567.89012345;
> object.values["float24"] = 123456.789012345;
> object.values["float25"] = 12345.6789012345;
> object.values["float26"] = 1234.56789012345;
> object.values["float27"] = 123.456789012345;
> object.values["float28"] = 12.3456789012345;
> object.values["float29"] = 1.23456789012345;
> object.values["float30"] = 1234567890.12345;
> object.values["float31"] = 123456789.012345;
> object.values["float32"] = 12345678.9012345;
> object.values["float33"] = 1234567.89012345;
> object.values["float34"] = 123456.789012345;
> object.values["float35"] = 12345.6789012345;
> object.values["float36"] = 1234.56789012345;
> object.values["float37"] = 123.456789012345;
> object.values["float38"] = 12.3456789012345;
> object.values["float39"] = 1.23456789012345;
> object.values["float40"] = 1234567890.12345;
> object.values["float41"] = 123456789.012345;
> object.values["float42"] = 12345678.9012345;
> object.values["float43"] = 1234567.89012345;
> object.values["float44"] = 123456.789012345;
> object.values["float45"] = 12345.6789012345;
> object.values["float46"] = 1234.56789012345;
> object.values["float47"] = 123.456789012345;
> object.values["float48"] = 12.3456789012345;
> object.values["float49"] = 1.23456789012345;
> 
> state.ResumeTiming();
> 
> benchmark::DoNotOptimize(stringify(object));
>   }
> }
> 
> BENCHMARK(BM_Jsonify);
> 
> BENCHMARK_MAIN();
> ```
> 
> The program creates a JSON object with 50 floats in it, and then it creates
> its string representation. Since the algorithms in `json.hpp` and 
> `jsonify.hpp`
> are similar but not the same, they were benchmarked as different. While the
> original solutions are non solutions since they produce erroneous results if
> localization is active, they constitute a baseline for comparison with the
> alternatives. These 

Re: Review Request 53344: Updated `/slaves.md` doc.

[Zhitao Li]

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

(Updated Nov. 1, 2016, 6:41 p.m.)


Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.


Repository: mesos


Description
---

Updated `/slaves.md` doc.


Diffs (updated)
-

  docs/endpoints/master/slaves.md 90d0fb8496866b70777e5dbc86449b14ae1cc910 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52765: Populated `recovered_slaves` in `/state` and `/slaves` endpoints.

[Zhitao Li]

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

(Updated Nov. 1, 2016, 6:40 p.m.)


Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.


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


Repository: mesos


Description
---

Populated `recovered_slaves` in `/state` and `/slaves` endpoints.


Diffs (updated)
-

  src/master/http.cpp 2f275f6c78b92e13bd7d38043b581b5a3555ee40 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53285: Show the leader information in WebUI.

[haosdent huang]

> On Oct. 31, 2016, 10:47 p.m., Jiang Yan Xu wrote:
> > src/webui/master/static/home.html, line 26
> > 
> >
> > Since `hostname` is deprecated, use `address.hostname`? If that's not 
> > available, fall back to the current behavior: ip:port?
> 
> haosdent huang wrote:
> Good patch! It is my bad use it in 
> https://github.com/apache/mesos/commit/34eb567c4e0ed605eeed02a20213f28d51a4e305
>  Do you think is it OK to update there to use `info.address`. It would looks 
> like:
> 
> ```
> static void json(JSON::ObjectWriter* writer, const MasterInfo& info)
> {
>   writer->field("id", info.id());
>   writer->field("pid", info.pid());
>   writer->field("port", info.address().port());
>   writer->field("hostname", info.address().hostname());
> }
> ```
> 
> Jiang Yan Xu wrote:
> Oh OK I see. Sure if you do that in a separate review it would be more in 
> line with the suggested usage.
>  
> And it looks like the `hostname` field in the json object should always 
> be there so we don't need a fall back behavior.

Got it, let me fix this.


- haosdent


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


On Oct. 29, 2016, 3:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53285/
> ---
> 
> (Updated Oct. 29, 2016, 3:05 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6448
> https://issues.apache.org/jira/browse/MESOS-6448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Show the leader information in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html 07f862f69802bef8fda2d0a94e33c3b463ddcd4c 
> 
> Diff: https://reviews.apache.org/r/53285/diff/
> 
> 
> Testing
> ---
> 
> ![exists](https://issues.apache.org/jira/secure/attachment/12835981/exists.png)
> ![no_exists](https://issues.apache.org/jira/secure/attachment/12835982/no_exist.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53253: Fixed the bug when search base hierarchy in `cgroups_tests.cpp`.

[haosdent huang]

> On Nov. 1, 2016, 1:20 a.m., Jiang Yan Xu wrote:
> > We unfortunately have to copy the cleanup code from 
> > `ContainerizerTest::SetUp()` to here. FWIW the 
> > test fixture originally was written with co-mounted subsystems in a single 
> > hierarchy (hence *Any*), later altered when Mesos started to use separate 
> > hierarchies for each subsystem, but not in a clean way IMO.
> > 
> > The `CgroupsAnyHierarchyTest` fixture and its children AFAICT still works 
> > with one hierarchy with multiple co-mounted subsystems. In fact Mesos does 
> > support co-mounted subsystems.
> > 
> > So IMO it wouldn't be a bad idea to have tests in cgroups_tests.cpp to only 
> > **create** one hierarchy for all the specified subsystems (most tests 
> > assume one). Of course it's fine if the subsystems are already mounted in 
> > separate hierarchies but the tests only manages (create/cleanup) a single 
> > hierachy.
> > 
> > This would make fixes for related problems (e.g., MESOS-6422) cleaner as 
> > well.
> > 
> > Thoughts?

To be honest, I prefer to follow what Linux do now which mount subsystems in 
different folders. Suppose the Linux have already mounted 
`/sys/fs/cgroups/cpu`, `/sys/fs/cgroups/memory`. And in our test case, we 
create `/sys/fs/cgroups/unified-mount` and mount rest subsystems(`devices`, 
`net_cls`, `perf_event`) on it. Then it looks wried. If we choose to mount them 
separately in our test cases, e.g, `/sys/fs/cgroups/devices`, 
`/sys/fs/cgroups/perf_event` and etc. This is more consistent with current 
operate system behaviour.

Co-mounted all hierarchies simplfy work if there is not any exist subsystems 
mounted in current machine. But it would become tricky if some subystems have 
been mounted. Anything I miss here?


- haosdent


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


On Oct. 28, 2016, 5:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53253/
> ---
> 
> (Updated Oct. 28, 2016, 5:13 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the bug when search base hierarchy in `cgroups_tests.cpp`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0afaec6ae948cabf1472bf01103210d8f9809cb1 
> 
> Diff: https://reviews.apache.org/r/53253/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53285: Show the leader information in WebUI.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On Oct. 29, 2016, 8:05 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53285/
> ---
> 
> (Updated Oct. 29, 2016, 8:05 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6448
> https://issues.apache.org/jira/browse/MESOS-6448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Show the leader information in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html 07f862f69802bef8fda2d0a94e33c3b463ddcd4c 
> 
> Diff: https://reviews.apache.org/r/53285/diff/
> 
> 
> Testing
> ---
> 
> ![exists](https://issues.apache.org/jira/secure/attachment/12835981/exists.png)
> ![no_exists](https://issues.apache.org/jira/secure/attachment/12835982/no_exist.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53285: Show the leader information in WebUI.

[Jiang Yan Xu]

> On Oct. 31, 2016, 3:47 p.m., Jiang Yan Xu wrote:
> > src/webui/master/static/home.html, line 26
> > 
> >
> > Since `hostname` is deprecated, use `address.hostname`? If that's not 
> > available, fall back to the current behavior: ip:port?
> 
> haosdent huang wrote:
> Good patch! It is my bad use it in 
> https://github.com/apache/mesos/commit/34eb567c4e0ed605eeed02a20213f28d51a4e305
>  Do you think is it OK to update there to use `info.address`. It would looks 
> like:
> 
> ```
> static void json(JSON::ObjectWriter* writer, const MasterInfo& info)
> {
>   writer->field("id", info.id());
>   writer->field("pid", info.pid());
>   writer->field("port", info.address().port());
>   writer->field("hostname", info.address().hostname());
> }
> ```

Oh OK I see. Sure if you do that in a separate review it would be more in line 
with the suggested usage.
 
And it looks like the `hostname` field in the json object should always be 
there so we don't need a fall back behavior.


- Jiang Yan


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


On Oct. 29, 2016, 8:05 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53285/
> ---
> 
> (Updated Oct. 29, 2016, 8:05 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6448
> https://issues.apache.org/jira/browse/MESOS-6448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Show the leader information in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html 07f862f69802bef8fda2d0a94e33c3b463ddcd4c 
> 
> Diff: https://reviews.apache.org/r/53285/diff/
> 
> 
> Testing
> ---
> 
> ![exists](https://issues.apache.org/jira/secure/attachment/12835981/exists.png)
> ![no_exists](https://issues.apache.org/jira/secure/attachment/12835982/no_exist.png)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52765: Populated `recovered_slaves` in `/state` and `/slaves` endpoints.

[Neil Conway]

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




src/master/http.cpp (line 2241)


"registered in this master"
"recovered from the registry"



src/master/http.cpp (line 2687)


Can we make this comment symmetric with the comment on 2680? ("Model all of 
the recovered slaves.")


- Neil Conway


On Nov. 1, 2016, 5:29 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52765/
> ---
> 
> (Updated Nov. 1, 2016, 5:29 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.
> 
> 
> Bugs: MESOS-6177
> https://issues.apache.org/jira/browse/MESOS-6177
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Populated `recovered_slaves` in `/state` and `/slaves` endpoints.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 2f275f6c78b92e13bd7d38043b581b5a3555ee40 
> 
> Diff: https://reviews.apache.org/r/52765/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 53344: Updated `/slaves.md` doc.

[Zhitao Li]

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

Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.


Repository: mesos


Description
---

Updated `/slaves.md` doc.


Diffs
-

  docs/endpoints/master/slaves.md 90d0fb8496866b70777e5dbc86449b14ae1cc910 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52639: Added test for `recovered` AgentID and `AGENT_ADDED` after reregister.

[Zhitao Li]

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

(Updated Nov. 1, 2016, 5:30 p.m.)


Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.


Changes
---

Added test for `/slaves` endpoint too.


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


Repository: mesos


Description
---

Added test for `recovered` AgentID and `AGENT_ADDED` after reregister.


Diffs (updated)
-

  src/tests/api_tests.cpp f87130d7554f98265ff6da39ea1d366839193e85 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53095: Make `slaves.recovered` in master a hashmap.

[Zhitao Li]

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

(Updated Nov. 1, 2016, 5:28 p.m.)


Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Kunal Thakar.


Changes
---

rebase.


Repository: mesos


Description (updated)
---

This information will be used to in both
`GetAgents` API call and `/state` endpoint
to return `AgentInfo` of recovered but unregistered
slave, to facilitate operator to better handle
such case.

The reason I want to return `AgentInfo` instead
of `AgentID` is that hostname and ip address
can be find in former, but not latter.


Diffs (updated)
-

  src/master/master.hpp 87186c6e733a686f96528b1722fda1c287e9c881 
  src/master/master.cpp 013bb592ba47b785c552e199633e4784e8aa71b1 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53127: Added the test `ProvisionerDockerWhiteoutTest`.

[Zhitao Li]

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




src/tests/containerizer/provisioner_docker_tests.cpp (lines 689 - 696)


Is it possible to paste the Dockerfile used to build this image in this 
comment, or provide its content through a symlink here?


- Zhitao Li


On Nov. 1, 2016, 6:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53127/
> ---
> 
> (Updated Nov. 1, 2016, 6:58 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the test `ProvisionerDockerWhiteoutTest`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d8fb7abb55410cfd0835f59bfc0f35566aeeae37 
> 
> Diff: https://reviews.apache.org/r/53127/diff/
> 
> 
> Testing
> ---
> 
> [==] Running 3 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 3 tests from BackendFlag/ProvisionerDockerWhiteoutTest
> [ RUN  ] 
> BackendFlag/ProvisionerDockerWhiteoutTest.ROOT_INTERNET_CURL_Whiteout/0
> Executing pre-exec command 
> '{"arguments":["mesos-containerizer","mount","--help=false","--operation=make-rslave","--path=\/"],"shell":false,"value":"\/home\/stack\/workspace\/mesos\/build\/src\/mesos-containerizer"}'
> Executing pre-exec command 
> '{"arguments":["mount","-n","--rbind","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX\/slaves\/5ec98305-4a7d-4351-976e-7c7f4023329c-S0\/frameworks\/5ec98305-4a7d-4351-976e-7c7f4023329c-\/executors\/7bea80dc-b644-430f-abb0-cabe2af3ded8\/runs\/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX\/provisioner\/containers\/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1\/backends\/aufs\/rootfses\/8837b468-156f-4e04-96b2-ab201626ad3c\/mnt\/mesos\/sandbox"],"shell":false,"value":"mount"}'
> I1024 16:17:23.245787 13269 exec.cpp:162] Version: 1.2.0
> I1024 16:17:23.256351 13271 exec.cpp:237] Executor registered on agent 
> 5ec98305-4a7d-4351-976e-7c7f4023329c-S0
> Received SUBSCRIBED event
> Subscribed executor on workstation
> Received LAUNCH event
> Starting task 7bea80dc-b644-430f-abb0-cabe2af3ded8
> /home/stack/workspace/mesos/build/src/mesos-containerizer launch 
> --command="{"arguments":["test","!","-f","\/etc\/rc3.d\/S40-network"],"shell":false,"value":"\/usr\/bin\/test"}"
>  --help="false" 
> --rootfs="/tmp/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX/provisioner/containers/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1/backends/aufs/rootfses/8837b468-156f-4e04-96b2-ab201626ad3c"
>  --unshare_namespace_mnt="true" --user="root" 
> --working_directory="/mnt/mesos/sandbox"
> Forked command at 13278
> Changing root to 
> /tmp/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX/provisioner/containers/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1/backends/aufs/rootfses/8837b468-156f-4e04-96b2-ab201626ad3c
> Command exited with status 0 (pid: 13278)
> I1024 16:17:23.500792 13273 exec.cpp:414] Executor asked to shutdown
> Received SHUTDOWN event
> Shutting down
> [   OK ] 
> BackendFlag/ProvisionerDockerWhiteoutTest.ROOT_INTERNET_CURL_Whiteout/0 (1365 
> ms)
> [ RUN  ] 
> BackendFlag/ProvisionerDockerWhiteoutTest.ROOT_INTERNET_CURL_Whiteout/1
> Executing pre-exec command 
> '{"arguments":["mesos-containerizer","mount","--help=false","--operation=make-rslave","--path=\/"],"shell":false,"value":"\/home\/stack\/workspace\/mesos\/build\/src\/mesos-containerizer"}'
> Executing pre-exec command 
> '{"arguments":["mount","-n","--rbind","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_1_Zm1mK7\/slaves\/9d49d96c-189d-47e9-9370-72146356af7c-S0\/frameworks\/9d49d96c-189d-47e9-9370-72146356af7c-\/executors\/ae8fda3e-db36-49f7-8699-9a4768d4af27\/runs\/49f24d09-6474-4a39-9da6-8c8da0ad4238","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_1_Zm1mK7\/provisioner\/containers\/49f24d09-6474-4a39-9da6-8c8da0ad4238\/backends\/copy\/rootfses\/db73608a-614b-49be-bebe-cd9aa42e31f5\/mnt\/mesos\/sandbox"],"shell":false,"value":"mount"}'
> I1024 16:17:24.971269 13344 exec.cpp:162] Version: 1.2.0
> I1024 16:17:24.981901 13364 exec.cpp:237] Executor registered on agent 
> 9d49d96c-189d-47e9-9370-72146356af7c-S0
> Received SUBSCRIBED event
> Subscribed executor on workstation
> Received LAUNCH event
> Starting task ae8fda3e-db36-49f7-8699-9a4768d4af27
> 

Re: Review Request 53161: Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.

[Zhitao Li]

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




src/slave/containerizer/mesos/provisioner/docker/store.cpp (lines 341 - 352)


Do we need to worry about previously cached image layers in stores, if the 
`image_provisioner_backend` flag value is changed from something else to 
`overlay`?


- Zhitao Li


On Nov. 1, 2016, 6:42 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53161/
> ---
> 
> (Updated Nov. 1, 2016, 6:42 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am c2f9e442182110d0b450d4824600a4a791f8cf27 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> e192f86a1848b373f3aa73d29688a96375cac313 
>   src/slave/containerizer/mesos/provisioner/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/utils.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53161/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52587: Allow CREATE of shared volumes based on capability of framework.

[Anindya Sinha]

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

(Updated Nov. 1, 2016, 4:54 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

When a framework issues a CREATE for a shared volume, we allow that
operation only if the framework has opted in for the capability of
SHARED_RESOURCES. However, for HTTP operator (/create-volumes), we
do not check as the operator API is not related to a specific
framework.


Diffs (updated)
-

  src/master/master.cpp 013bb592ba47b785c552e199633e4784e8aa71b1 
  src/master/validation.hpp 035f721c610ae566c89a1cf0e65ff0df11679f15 
  src/master/validation.cpp f690a9eacd278b51a52f5588dbeea377df074435 
  src/tests/master_validation_tests.cpp 
a5d8610bd61822cdf55cbc8d7056e5cf8fecfa54 
  src/tests/persistent_volume_tests.cpp 
6289009fe9ed0a57ba5eff46dbbe0633a75d2616 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 53138: Fix libprocess build option argument handling.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53136, 53137, 53138]

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

- Mesos ReviewBot


On Nov. 1, 2016, 4:24 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53138/
> ---
> 
> (Updated Nov. 1, 2016, 4:24 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-2537
> https://issues.apache.org/jira/browse/MESOS-2537
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix all the AC_ARG_ENABLED options to correctly handle --enable-foo
> and --disable-foo variants. Update all the invocations to have a
> consistent calling convention.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac 1644035d7fe1a45c798b13dbfb3c66f74466f2a2 
> 
> Diff: https://reviews.apache.org/r/53138/diff/
> 
> 
> Testing
> ---
> 
> Make check on OS X and Fedora 24.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 53341: Fix stout build option argument handling.

[James Peach]

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

Review request for mesos, Benjamin Bannier and Kapil Arya.


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


Repository: mesos


Description
---

Fix all the AC_ARG_ENABLED options to correctly handle --enable-foo
and --disable-foo variants. Update all the invocations to have a
consistent calling convention.


Diffs
-

  3rdparty/stout/configure.ac cbb0fdb0e1584ace7bbd6bf496f0f55b9846ce0b 

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


Testing
---

"make distcheck"


Thanks,

James Peach

Re: Review Request 53138: Fix libprocess build option argument handling.

[James Peach]

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

(Updated Nov. 1, 2016, 4:24 p.m.)


Review request for mesos and Kapil Arya.


Summary (updated)
-

Fix libprocess build option argument handling.


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


Repository: mesos


Description
---

Fix all the AC_ARG_ENABLED options to correctly handle --enable-foo
and --disable-foo variants. Update all the invocations to have a
consistent calling convention.


Diffs (updated)
-

  3rdparty/libprocess/configure.ac 1644035d7fe1a45c798b13dbfb3c66f74466f2a2 

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


Testing
---

Make check on OS X and Fedora 24.


Thanks,

James Peach

Re: Review Request 53137: Fix Mesos build option argument handling.

[James Peach]

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

(Updated Nov. 1, 2016, 4:24 p.m.)


Review request for mesos and Kapil Arya.


Summary (updated)
-

Fix Mesos build option argument handling.


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


Repository: mesos


Description
---

Fix all the AC_ARG_ENABLED options to correctly handle --enable-foo
and --disable-foo variants. Update all the invocations to have a
consistent calling convention.


Diffs (updated)
-

  configure.ac c8d48be41d526c45ee00b404d0ad2b67ea11587f 

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


Testing
---

Make check on OS X and Fedora 24.


Thanks,

James Peach

Re: Review Request 53136: Emit the build options at the end of configure.

[James Peach]

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

(Updated Nov. 1, 2016, 4:24 p.m.)


Review request for mesos and Kapil Arya.


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


Repository: mesos


Description
---

Emit the build options at the end of configure.


Diffs (updated)
-

  configure.ac c8d48be41d526c45ee00b404d0ad2b67ea11587f 

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


Testing
---

Make check on OS X and Fedora 24.


Thanks,

James Peach

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Guangya Liu]

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




src/tests/containerizer/docker_containerizer_tests.cpp (line 4093)


```
foreach(const Docker::Device& device, inspect->devices) {
```



src/tests/containerizer/docker_containerizer_tests.cpp (lines 4094 - 4095)


It is not good to use `count()` as a boolean value, what about following?

```
EXPECT_EQ(1u, devicePaths.count(device.hostPath));
EXPECT_EQ(1u, devicePaths.count(device.containerPath));
```


- Guangya Liu


On 十一月 1, 2016, 9:12 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50127/
> ---
> 
> (Updated 十一月 1, 2016, 9:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This added a testing case for end-to-end GPU support for docker
> containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 810488d1476cadbbd5a4a7dcecaeec55739ab71f 
> 
> Diff: https://reviews.apache.org/r/50127/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
> check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50128: Added helper function to 'Docker::Device'.

[Guangya Liu]

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




src/docker/docker.cpp (line 756)


I think that we actually need to check invalid permissions here, as it is 
possible that there are invalid permissions when create container. But agree 
that we can handle this in a separate patch.


- Guangya Liu


On 十一月 1, 2016, 9:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50128/
> ---
> 
> (Updated 十一月 1, 2016, 9:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Wrapped helper function to stringify 'Docker::Device' structure.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
> 
> Diff: https://reviews.apache.org/r/50128/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50125: Added mesos-docker-executor support for devices control.

[Guangya Liu]

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


Fix it, then Ship it!




Ship It!


src/docker/executor.cpp (line 792)


kill this


- Guangya Liu


On 十一月 1, 2016, 9:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50125/
> ---
> 
> (Updated 十一月 1, 2016, 9:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new flag '--devices' to mesos-docker-executor, and gave its
> feature to control devices exposition, isolation, and access permission.
> Also, passed GPUs assignment to mesos-docker-executor.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
>   src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

[Guangya Liu]

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


Fix it, then Ship it!




Ship It!


src/slave/containerizer/docker.hpp (line 509)


You also need to `include ` at the beginning of this file.



src/slave/containerizer/docker.cpp (line 701)


You need to keep the comments from Kevin as open here 
https://reviews.apache.org/r/50599/#comment223322



src/slave/containerizer/docker.cpp (lines 705 - 710)


What about update this a bit to make it less jagged?

```
  // NOTE: GPU devices permissions are required to be `rmw` by default,
  // that is because GPU tasks launched in the container may need to
  // read/write/mknod to GPU devices in their lifecycle.
  //
  // `container->devices` records Nvidia GPU devices to be attached to
  // the docker container.
```


- Guangya Liu


On 十一月 1, 2016, 9:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十一月 1, 2016, 9:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50128: Added helper function to 'Docker::Device'.

[Guangya Liu]

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




src/docker/docker.hpp (line 313)


You should also include `ostream` after #21

```
#include 
```



src/docker/docker.cpp (line 1491)


I'd prefer that you include  at the beginning and use the 
following definition here directly.

```
ostream& operator<<(ostream& stream, const Docker::Device& device)
```

You can take a look at 
https://github.com/apache/mesos/blob/master/src/authorizer/acls.cpp as a 
reference.


- Guangya Liu


On 十一月 1, 2016, 9:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50128/
> ---
> 
> (Updated 十一月 1, 2016, 9:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Wrapped helper function to stringify 'Docker::Device' structure.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
> 
> Diff: https://reviews.apache.org/r/50128/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50841: Added GPU scheduling logic to docker containerizer process.

[Guangya Liu]

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


Fix it, then Ship it!




Ship It!


src/slave/containerizer/docker.hpp (lines 20 - 22)


```
#include 
#include 
#include 
#include 
```


- Guangya Liu


On 十一月 1, 2016, 6:10 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50841/
> ---
> 
> (Updated 十一月 1, 2016, 6:10 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added control logic to allocate/deallocate GPUs to GPU-related task
> when the task is started/terminated.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

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

[Alexander Rojas]

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

(Updated Nov. 1, 2016, 3:45 p.m.)


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


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  3rdparty/stout/include/stout/json.hpp 
9734561549eea867d57f14a0ab71febeb9dddc06 
  3rdparty/stout/include/stout/jsonify.hpp 
8220d001001e8b9f247c05be58534f1174611aad 

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


Testing
---

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

```c++
#include 
#include 

#include 
#include 
#include 
#include 

static void BM_Jsonify(benchmark::State& state) {
  while (state.KeepRunning()) {
state.PauseTiming();
JSON::Object object;
object.values["float00"] = 1234567890.12345;
object.values["float01"] = 123456789.012345;
object.values["float02"] = 12345678.9012345;
object.values["float03"] = 1234567.89012345;
object.values["float04"] = 123456.789012345;
object.values["float05"] = 12345.6789012345;
object.values["float06"] = 1234.56789012345;
object.values["float07"] = 123.456789012345;
object.values["float08"] = 12.3456789012345;
object.values["float09"] = 1.23456789012345;
object.values["float10"] = 1234567890.12345;
object.values["float11"] = 123456789.012345;
object.values["float12"] = 12345678.9012345;
object.values["float13"] = 1234567.89012345;
object.values["float14"] = 123456.789012345;
object.values["float15"] = 12345.6789012345;
object.values["float16"] = 1234.56789012345;
object.values["float17"] = 123.456789012345;
object.values["float18"] = 12.3456789012345;
object.values["float19"] = 1.23456789012345;
object.values["float20"] = 1234567890.12345;
object.values["float21"] = 123456789.012345;
object.values["float22"] = 12345678.9012345;
object.values["float23"] = 1234567.89012345;
object.values["float24"] = 123456.789012345;
object.values["float25"] = 12345.6789012345;
object.values["float26"] = 1234.56789012345;
object.values["float27"] = 123.456789012345;
object.values["float28"] = 12.3456789012345;
object.values["float29"] = 1.23456789012345;
object.values["float30"] = 1234567890.12345;
object.values["float31"] = 123456789.012345;
object.values["float32"] = 12345678.9012345;
object.values["float33"] = 1234567.89012345;
object.values["float34"] = 123456.789012345;
object.values["float35"] = 12345.6789012345;
object.values["float36"] = 1234.56789012345;
object.values["float37"] = 123.456789012345;
object.values["float38"] = 12.3456789012345;
object.values["float39"] = 1.23456789012345;
object.values["float40"] = 1234567890.12345;
object.values["float41"] = 123456789.012345;
object.values["float42"] = 12345678.9012345;
object.values["float43"] = 1234567.89012345;
object.values["float44"] = 123456.789012345;
object.values["float45"] = 12345.6789012345;
object.values["float46"] = 1234.56789012345;
object.values["float47"] = 123.456789012345;
object.values["float48"] = 12.3456789012345;
object.values["float49"] = 1.23456789012345;

state.ResumeTiming();

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

BENCHMARK(BM_Jsonify);

BENCHMARK_MAIN();
```

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

1. `json.hpp` default:

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

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

   Perfomance

```
Run on (8 X 1000 MHz CPU s)
2016-11-01 11:08:49
BenchmarkTime(ns)CPU(ns) Iterations
---
BM_Jsonify  62934  62927  11653
```

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

```c++
char buffer[50] {};
const int size = snprintf(
buffer,
sizeof(buffer),

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

[Alexander Rojas]

> On Nov. 1, 2016, 2:43 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/json.hpp, lines 680-681
> > 
> >
> > I believe we can drop the `static` here as these variables should have 
> > internal linkage automatically.

Nope, `static` cannot be dropped, otherwise the following error is caused:

```
In file included from ../../../3rdparty/stout/tests/jsonify_tests.cpp:20:
../../../3rdparty/stout/include/stout/jsonify.hpp:154:9: error: '__thread' 
variables must have global storage
THREAD_LOCAL std::stringstream* ss = nullptr;
^
../../../3rdparty/stout/include/stout/thread_local.hpp:24:22: note: expanded 
from macro 'THREAD_LOCAL'
#define THREAD_LOCAL __thread
 ^
```


> On Nov. 1, 2016, 2:43 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/json.hpp, line 694
> > 
> >
> > Could you check if this temporary string is removed when compiling 
> > optimized? If not IMHO it should be possible to avoid it by directly 
> > manipulating the underlying buffer's state.

Manipulating the underlying buffer would end up calling the same methog but in 
the buffer [std::basic_stringbuf::str(std::basic_string 
s)](http://en.cppreference.com/w/cpp/io/basic_stringbuf/str). One can also 
attempt to modify the `seek` pointer position, but it may require write the 
tail with zeroes.


> On Nov. 1, 2016, 2:43 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/jsonify.hpp, lines 154-155
> > 
> >
> > Remove `static`.

see above.


> On Nov. 1, 2016, 2:43 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/jsonify.hpp, line 181
> > 
> >
> > Investigate whether this creates a string in optimized code.

see above.


- Alexander


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


On Nov. 1, 2016, 1:27 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52877/
> ---
> 
> (Updated Nov. 1, 2016, 1:27 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6349
> https://issues.apache.org/jira/browse/MESOS-6349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resolves an isse with the JSON serialization functions where floats
> would use the process active locale as a decimal separator instead
> of the JSON conformant period (`.`).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 9734561549eea867d57f14a0ab71febeb9dddc06 
>   3rdparty/stout/include/stout/jsonify.hpp 
> 8220d001001e8b9f247c05be58534f1174611aad 
> 
> Diff: https://reviews.apache.org/r/52877/diff/
> 
> 
> Testing
> ---
> 
> In order to test the different options the following benchmarking program was
> written using [Google benchmark](https://github.com/google/benchmark):
> 
> ```c++
> #include 
> #include 
> 
> #include 
> #include 
> #include 
> #include 
> 
> static void BM_Jsonify(benchmark::State& state) {
>   while (state.KeepRunning()) {
> state.PauseTiming();
> JSON::Object object;
> object.values["float00"] = 1234567890.12345;
> object.values["float01"] = 123456789.012345;
> object.values["float02"] = 12345678.9012345;
> object.values["float03"] = 1234567.89012345;
> object.values["float04"] = 123456.789012345;
> object.values["float05"] = 12345.6789012345;
> object.values["float06"] = 1234.56789012345;
> object.values["float07"] = 123.456789012345;
> object.values["float08"] = 12.3456789012345;
> object.values["float09"] = 1.23456789012345;
> object.values["float10"] = 1234567890.12345;
> object.values["float11"] = 123456789.012345;
> object.values["float12"] = 12345678.9012345;
> object.values["float13"] = 1234567.89012345;
> object.values["float14"] = 123456.789012345;
> object.values["float15"] = 12345.6789012345;
> object.values["float16"] = 1234.56789012345;
> object.values["float17"] = 123.456789012345;
> object.values["float18"] = 12.3456789012345;
> object.values["float19"] = 1.23456789012345;
> object.values["float20"] = 1234567890.12345;
> object.values["float21"] = 123456789.012345;
> object.values["float22"] = 12345678.9012345;
> object.values["float23"] = 1234567.89012345;
> object.values["float24"] = 123456.789012345;
> object.values["float25"] = 12345.6789012345;
>  

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52877]

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

- Mesos ReviewBot


On Nov. 1, 2016, 12:27 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52877/
> ---
> 
> (Updated Nov. 1, 2016, 12:27 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6349
> https://issues.apache.org/jira/browse/MESOS-6349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resolves an isse with the JSON serialization functions where floats
> would use the process active locale as a decimal separator instead
> of the JSON conformant period (`.`).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 9734561549eea867d57f14a0ab71febeb9dddc06 
>   3rdparty/stout/include/stout/jsonify.hpp 
> 8220d001001e8b9f247c05be58534f1174611aad 
> 
> Diff: https://reviews.apache.org/r/52877/diff/
> 
> 
> Testing
> ---
> 
> In order to test the different options the following benchmarking program was
> written using [Google benchmark](https://github.com/google/benchmark):
> 
> ```c++
> #include 
> #include 
> 
> #include 
> #include 
> #include 
> #include 
> 
> static void BM_Jsonify(benchmark::State& state) {
>   while (state.KeepRunning()) {
> state.PauseTiming();
> JSON::Object object;
> object.values["float00"] = 1234567890.12345;
> object.values["float01"] = 123456789.012345;
> object.values["float02"] = 12345678.9012345;
> object.values["float03"] = 1234567.89012345;
> object.values["float04"] = 123456.789012345;
> object.values["float05"] = 12345.6789012345;
> object.values["float06"] = 1234.56789012345;
> object.values["float07"] = 123.456789012345;
> object.values["float08"] = 12.3456789012345;
> object.values["float09"] = 1.23456789012345;
> object.values["float10"] = 1234567890.12345;
> object.values["float11"] = 123456789.012345;
> object.values["float12"] = 12345678.9012345;
> object.values["float13"] = 1234567.89012345;
> object.values["float14"] = 123456.789012345;
> object.values["float15"] = 12345.6789012345;
> object.values["float16"] = 1234.56789012345;
> object.values["float17"] = 123.456789012345;
> object.values["float18"] = 12.3456789012345;
> object.values["float19"] = 1.23456789012345;
> object.values["float20"] = 1234567890.12345;
> object.values["float21"] = 123456789.012345;
> object.values["float22"] = 12345678.9012345;
> object.values["float23"] = 1234567.89012345;
> object.values["float24"] = 123456.789012345;
> object.values["float25"] = 12345.6789012345;
> object.values["float26"] = 1234.56789012345;
> object.values["float27"] = 123.456789012345;
> object.values["float28"] = 12.3456789012345;
> object.values["float29"] = 1.23456789012345;
> object.values["float30"] = 1234567890.12345;
> object.values["float31"] = 123456789.012345;
> object.values["float32"] = 12345678.9012345;
> object.values["float33"] = 1234567.89012345;
> object.values["float34"] = 123456.789012345;
> object.values["float35"] = 12345.6789012345;
> object.values["float36"] = 1234.56789012345;
> object.values["float37"] = 123.456789012345;
> object.values["float38"] = 12.3456789012345;
> object.values["float39"] = 1.23456789012345;
> object.values["float40"] = 1234567890.12345;
> object.values["float41"] = 123456789.012345;
> object.values["float42"] = 12345678.9012345;
> object.values["float43"] = 1234567.89012345;
> object.values["float44"] = 123456.789012345;
> object.values["float45"] = 12345.6789012345;
> object.values["float46"] = 1234.56789012345;
> object.values["float47"] = 123.456789012345;
> object.values["float48"] = 12.3456789012345;
> object.values["float49"] = 1.23456789012345;
> 
> state.ResumeTiming();
> 
> benchmark::DoNotOptimize(stringify(object));
>   }
> }
> 
> BENCHMARK(BM_Jsonify);
> 
> BENCHMARK_MAIN();
> ```
> 
> The program creates a JSON object with 50 floats in it, and then it creates
> its string representation. Since the algorithms in `json.hpp` and 
> `jsonify.hpp`
> are similar but not the same, they were benchmarked as different. While the
> original solutions are non solutions since they produce erroneous results if
> localization is active, they constitute a baseline for comparison with the
> alternatives. 

Re: Review Request 52879: Cleaned up the way in which the executors load configuration options.

[Gastón Kleiman]

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

(Updated Nov. 1, 2016, 2:06 p.m.)


Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, 
Jie Yu, and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Updated the executors so that they use only `stout::flags` to load
config options.

They used to use a mix of `stout::flags` and `os::getenv`.


Diffs (updated)
-

  src/CMakeLists.txt 639f8678ba23c4d9a2ea0bf84fbc3d6fc9286ef3 
  src/Makefile.am c2f9e442182110d0b450d4824600a4a791f8cf27 
  src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/exec/exec.cpp 1dc20390907253a466b7272b7f8b33ea14afb236 
  src/exec/flags.hpp PRE-CREATION 
  src/exec/flags.cpp PRE-CREATION 
  src/executor/executor.cpp 1d47b52d89eedee59d160badd6313d34e3bdb6d2 
  src/executor/flags.hpp PRE-CREATION 
  src/executor/flags.cpp PRE-CREATION 
  src/launcher/default_executor.cpp 10ca94ca3e584431d0dd7bf3b8bae98caca7eba9 
  src/launcher/executor.hpp 217680d99d6e8c31130d7dc714f8cd730f360852 
  src/launcher/executor.cpp 0544121e679db503fe4eaf23a24e315eb188a520 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
  src/tests/containerizer.cpp c2208660352e5cf62cec5c7b92ec67fea62aea49 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

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

[Benjamin Bannier]

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




3rdparty/stout/include/stout/json.hpp (lines 680 - 681)


I believe we can drop the `static` here as these variables should have 
internal linkage automatically.



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


Let's `s/once_flag/initialized/` for consitency.



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


You only use `ss` in the lambda which is `thread_local`, so you do not need 
to capture anything here.



3rdparty/stout/include/stout/json.hpp (lines 686 - 687)


It should be possible to move the stream setup under the once.



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


Could you check if this temporary string is removed when compiling 
optimized? If not IMHO it should be possible to avoid it by directly 
manipulating the underlying buffer's state.



3rdparty/stout/include/stout/jsonify.hpp (lines 154 - 155)


Remove `static`.



3rdparty/stout/include/stout/jsonify.hpp (line 155)


`s/once_flag/initialized/`



3rdparty/stout/include/stout/jsonify.hpp (line 156)


Nothing to capture here.



3rdparty/stout/include/stout/jsonify.hpp (lines 161 - 162)


Move stream setup under once.



3rdparty/stout/include/stout/jsonify.hpp (line 170)


Investigate whether this creates a string in optimized code.


- Benjamin Bannier


On Nov. 1, 2016, 1:27 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52877/
> ---
> 
> (Updated Nov. 1, 2016, 1:27 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6349
> https://issues.apache.org/jira/browse/MESOS-6349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resolves an isse with the JSON serialization functions where floats
> would use the process active locale as a decimal separator instead
> of the JSON conformant period (`.`).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 9734561549eea867d57f14a0ab71febeb9dddc06 
>   3rdparty/stout/include/stout/jsonify.hpp 
> 8220d001001e8b9f247c05be58534f1174611aad 
> 
> Diff: https://reviews.apache.org/r/52877/diff/
> 
> 
> Testing
> ---
> 
> In order to test the different options the following benchmarking program was
> written using [Google benchmark](https://github.com/google/benchmark):
> 
> ```c++
> #include 
> #include 
> 
> #include 
> #include 
> #include 
> #include 
> 
> static void BM_Jsonify(benchmark::State& state) {
>   while (state.KeepRunning()) {
> state.PauseTiming();
> JSON::Object object;
> object.values["float00"] = 1234567890.12345;
> object.values["float01"] = 123456789.012345;
> object.values["float02"] = 12345678.9012345;
> object.values["float03"] = 1234567.89012345;
> object.values["float04"] = 123456.789012345;
> object.values["float05"] = 12345.6789012345;
> object.values["float06"] = 1234.56789012345;
> object.values["float07"] = 123.456789012345;
> object.values["float08"] = 12.3456789012345;
> object.values["float09"] = 1.23456789012345;
> object.values["float10"] = 1234567890.12345;
> object.values["float11"] = 123456789.012345;
> object.values["float12"] = 12345678.9012345;
> object.values["float13"] = 1234567.89012345;
> object.values["float14"] = 123456.789012345;
> object.values["float15"] = 12345.6789012345;
> object.values["float16"] = 1234.56789012345;
> object.values["float17"] = 123.456789012345;
> object.values["float18"] = 12.3456789012345;
> object.values["float19"] = 1.23456789012345;
> object.values["float20"] = 1234567890.12345;
> object.values["float21"] = 123456789.012345;
> object.values["float22"] = 12345678.9012345;
> object.values["float23"] = 1234567.89012345;
> object.values["float24"] = 123456.789012345;
> object.values["float25"] = 12345.6789012345;
> object.values["float26"] = 1234.56789012345;
> object.values["float27"] = 123.456789012345;
> object.values["float28"] = 12.3456789012345;
> object.values["float29"] = 1.23456789012345;
> 

Re: Review Request 53138: Fix build option argument handling.

[Benjamin Bannier]

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


Ship it!




- Benjamin Bannier


On Oct. 31, 2016, 11:27 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53138/
> ---
> 
> (Updated Oct. 31, 2016, 11:27 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-2537
> https://issues.apache.org/jira/browse/MESOS-2537
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix all the AC_ARG_ENABLED options to correctly handle --enable-foo
> and --disable-foo variants. Update all the invocations to have a
> consistent calling convention.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac 1644035d7fe1a45c798b13dbfb3c66f74466f2a2 
> 
> Diff: https://reviews.apache.org/r/53138/diff/
> 
> 
> Testing
> ---
> 
> Make check on OS X and Fedora 24.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53136: Emit the build options at the end of configure.

[Benjamin Bannier]

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


Ship it!





configure.ac (line 2215)


Would it make sense to also emit the config options here, e.g.,

CONFIG: $CONFIGURE_ARGS


- Benjamin Bannier


On Oct. 31, 2016, 11:27 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53136/
> ---
> 
> (Updated Oct. 31, 2016, 11:27 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-2537
> https://issues.apache.org/jira/browse/MESOS-2537
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Emit the build options at the end of configure.
> 
> 
> Diffs
> -
> 
>   configure.ac c8d48be41d526c45ee00b404d0ad2b67ea11587f 
> 
> Diff: https://reviews.apache.org/r/53136/diff/
> 
> 
> Testing
> ---
> 
> Make check on OS X and Fedora 24.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53137: Fix build option argument handling.

[Benjamin Bannier]

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


Fix it, then Ship it!




Thanks for this series, really looking forward to it.

Do you also have a patch for stout's `configure.ac`? It has similar issues.


configure.ac (line 208)


Since you nicely fixed this in the patch for libprocess' `configure.ac` 
please `s/google/Google/` here as well.


- Benjamin Bannier


On Oct. 31, 2016, 11:27 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53137/
> ---
> 
> (Updated Oct. 31, 2016, 11:27 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-2537
> https://issues.apache.org/jira/browse/MESOS-2537
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix all the AC_ARG_ENABLED options to correctly handle --enable-foo
> and --disable-foo variants. Update all the invocations to have a
> consistent calling convention.
> 
> 
> Diffs
> -
> 
>   configure.ac c8d48be41d526c45ee00b404d0ad2b67ea11587f 
> 
> Diff: https://reviews.apache.org/r/53137/diff/
> 
> 
> Testing
> ---
> 
> Make check on OS X and Fedora 24.
> 
> 
> Thanks,
> 
> James Peach
> 
>

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

[Alexander Rojas]

> On Oct. 15, 2016, 1:23 a.m., Joris Van Remoortere wrote:
> > I have a feeling that if you kept around a stringstream with the local set 
> > your benchmarks would look rather different.
> > I also suggest using callgrind to get the instruction count / # of library 
> > calls made.
> 
> Alexander Rojas wrote:
> That occurred to me, but then some issues appeared with the idea. The 
> version in `json.hpp` uses a free function, the way to keep the 
> `stringstream` is by making it global (either having a global variable or 
> marking it `static`), however that makes the function non thread safe and a 
> mutex will be needed. It probably will still perform better in a single 
> thread situation though.
> 
> The `jsonify.hpp` version has more or less the same concerns. I though 
> moving the `stringstream` to the constructor of the `Number` object, which 
> would fix the thread safety issue, but at the end one is just moving the 
> object construction penalty from serialization to construction time. But 
> given the usage of `jsonify`, it wouldn't have any performance issue.
> 
> Do you have any other idea on how to keep the `stringstream` alive?
> 
> Joris Van Remoortere wrote:
> Thread Local?
> 
> Alexander Rojas wrote:
> From the `stout/thread_local.hpp`
> 
> ```c++
> // We required that THREAD_LOCAL is only used with POD types as this
> // is a requirement of `__thread`.
> ```
> 
> `std::stringstream` is definitely not a POD.
> 
> Joris Van Remoortere wrote:
> `std::stringstream*` is

Done, however I had to use `std::call_once()` instead of our `process::Once()` 
because our code exists inside stout.


- Alexander


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


On Nov. 1, 2016, 1:27 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52877/
> ---
> 
> (Updated Nov. 1, 2016, 1:27 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6349
> https://issues.apache.org/jira/browse/MESOS-6349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resolves an isse with the JSON serialization functions where floats
> would use the process active locale as a decimal separator instead
> of the JSON conformant period (`.`).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 9734561549eea867d57f14a0ab71febeb9dddc06 
>   3rdparty/stout/include/stout/jsonify.hpp 
> 8220d001001e8b9f247c05be58534f1174611aad 
> 
> Diff: https://reviews.apache.org/r/52877/diff/
> 
> 
> Testing
> ---
> 
> In order to test the different options the following benchmarking program was
> written using [Google benchmark](https://github.com/google/benchmark):
> 
> ```c++
> #include 
> #include 
> 
> #include 
> #include 
> #include 
> #include 
> 
> static void BM_Jsonify(benchmark::State& state) {
>   while (state.KeepRunning()) {
> state.PauseTiming();
> JSON::Object object;
> object.values["float00"] = 1234567890.12345;
> object.values["float01"] = 123456789.012345;
> object.values["float02"] = 12345678.9012345;
> object.values["float03"] = 1234567.89012345;
> object.values["float04"] = 123456.789012345;
> object.values["float05"] = 12345.6789012345;
> object.values["float06"] = 1234.56789012345;
> object.values["float07"] = 123.456789012345;
> object.values["float08"] = 12.3456789012345;
> object.values["float09"] = 1.23456789012345;
> object.values["float10"] = 1234567890.12345;
> object.values["float11"] = 123456789.012345;
> object.values["float12"] = 12345678.9012345;
> object.values["float13"] = 1234567.89012345;
> object.values["float14"] = 123456.789012345;
> object.values["float15"] = 12345.6789012345;
> object.values["float16"] = 1234.56789012345;
> object.values["float17"] = 123.456789012345;
> object.values["float18"] = 12.3456789012345;
> object.values["float19"] = 1.23456789012345;
> object.values["float20"] = 1234567890.12345;
> object.values["float21"] = 123456789.012345;
> object.values["float22"] = 12345678.9012345;
> object.values["float23"] = 1234567.89012345;
> object.values["float24"] = 123456.789012345;
> object.values["float25"] = 12345.6789012345;
> object.values["float26"] = 1234.56789012345;
> object.values["float27"] = 123.456789012345;
> object.values["float28"] = 12.3456789012345;
> object.values["float29"] = 1.23456789012345;
> object.values["float30"] = 1234567890.12345;
> object.values["float31"] = 123456789.012345;
> 

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

[Alexander Rojas]

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

(Updated Nov. 1, 2016, 1:27 p.m.)


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


Changes
---

Add local thread variables.


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  3rdparty/stout/include/stout/json.hpp 
9734561549eea867d57f14a0ab71febeb9dddc06 
  3rdparty/stout/include/stout/jsonify.hpp 
8220d001001e8b9f247c05be58534f1174611aad 

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


Testing (updated)
---

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

```c++
#include 
#include 

#include 
#include 
#include 
#include 

static void BM_Jsonify(benchmark::State& state) {
  while (state.KeepRunning()) {
state.PauseTiming();
JSON::Object object;
object.values["float00"] = 1234567890.12345;
object.values["float01"] = 123456789.012345;
object.values["float02"] = 12345678.9012345;
object.values["float03"] = 1234567.89012345;
object.values["float04"] = 123456.789012345;
object.values["float05"] = 12345.6789012345;
object.values["float06"] = 1234.56789012345;
object.values["float07"] = 123.456789012345;
object.values["float08"] = 12.3456789012345;
object.values["float09"] = 1.23456789012345;
object.values["float10"] = 1234567890.12345;
object.values["float11"] = 123456789.012345;
object.values["float12"] = 12345678.9012345;
object.values["float13"] = 1234567.89012345;
object.values["float14"] = 123456.789012345;
object.values["float15"] = 12345.6789012345;
object.values["float16"] = 1234.56789012345;
object.values["float17"] = 123.456789012345;
object.values["float18"] = 12.3456789012345;
object.values["float19"] = 1.23456789012345;
object.values["float20"] = 1234567890.12345;
object.values["float21"] = 123456789.012345;
object.values["float22"] = 12345678.9012345;
object.values["float23"] = 1234567.89012345;
object.values["float24"] = 123456.789012345;
object.values["float25"] = 12345.6789012345;
object.values["float26"] = 1234.56789012345;
object.values["float27"] = 123.456789012345;
object.values["float28"] = 12.3456789012345;
object.values["float29"] = 1.23456789012345;
object.values["float30"] = 1234567890.12345;
object.values["float31"] = 123456789.012345;
object.values["float32"] = 12345678.9012345;
object.values["float33"] = 1234567.89012345;
object.values["float34"] = 123456.789012345;
object.values["float35"] = 12345.6789012345;
object.values["float36"] = 1234.56789012345;
object.values["float37"] = 123.456789012345;
object.values["float38"] = 12.3456789012345;
object.values["float39"] = 1.23456789012345;
object.values["float40"] = 1234567890.12345;
object.values["float41"] = 123456789.012345;
object.values["float42"] = 12345678.9012345;
object.values["float43"] = 1234567.89012345;
object.values["float44"] = 123456.789012345;
object.values["float45"] = 12345.6789012345;
object.values["float46"] = 1234.56789012345;
object.values["float47"] = 123.456789012345;
object.values["float48"] = 12.3456789012345;
object.values["float49"] = 1.23456789012345;

state.ResumeTiming();

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

BENCHMARK(BM_Jsonify);

BENCHMARK_MAIN();
```

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

1. `json.hpp` default:

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

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

   Perfomance

```
Run on (8 X 1000 MHz CPU s)
2016-11-01 11:08:49
BenchmarkTime(ns)CPU(ns) Iterations
---
BM_Jsonify  62934  62927  11653
```

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

```c++
char buffer[50] {};
const int 

Re: Review Request 52735: Removed TODO message for docker killing.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51425, 50123, 50841, 50128, 50599, 50125, 50947, 50127, 52735]

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

- Mesos ReviewBot


On Nov. 1, 2016, 9:12 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52735/
> ---
> 
> (Updated Nov. 1, 2016, 9:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed TODO message for docker killing.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/52735/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 53324: Add a column for FrameworkID when displaying tasks in the WebUI

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53324]

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

- Mesos ReviewBot


On Oct. 31, 2016, 11:41 p.m., Miguel Bernadin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53324/
> ---
> 
> (Updated Oct. 31, 2016, 11:41 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-6369
> https://issues.apache.org/jira/browse/MESOS-6369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a column for FrameworkID when displaying tasks in the WebUI
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/home.html 07f862f69802bef8fda2d0a94e33c3b463ddcd4c 
> 
> Diff: https://reviews.apache.org/r/53324/diff/
> 
> 
> Testing
> ---
> 
> Tested on my AWS Mesos Cluster
> 
> 
> Thanks,
> 
> Miguel Bernadin
> 
>

Re: Review Request 52735: Removed TODO message for docker killing.

[Yubo Li]

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

(Updated Nov. 1, 2016, 9:12 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


Repository: mesos


Description
---

Removed TODO message for docker killing.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---


Thanks,

Yubo Li

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Yubo Li]

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

(Updated Nov. 1, 2016, 9:12 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

This added a testing case for end-to-end GPU support for docker
containerizer.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
810488d1476cadbbd5a4a7dcecaeec55739ab71f 

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


Testing
---

GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

(Updated Nov. 1, 2016, 9:11 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50125: Added mesos-docker-executor support for devices control.

[Yubo Li]

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

(Updated Nov. 1, 2016, 9:11 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

Added a new flag '--devices' to mesos-docker-executor, and gave its
feature to control devices exposition, isolation, and access permission.
Also, passed GPUs assignment to mesos-docker-executor.


Diffs (updated)
-

  src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

[Yubo Li]

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

(Updated Nov. 1, 2016, 9:11 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50128: Added helper function to 'Docker::Device'.

[Yubo Li]

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

(Updated Nov. 1, 2016, 9:11 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

Wrapped helper function to stringify 'Docker::Device' structure.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 52997: Improve Socket::connect error message.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52997]

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

- Mesos ReviewBot


On Nov. 1, 2016, 12:22 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52997/
> ---
> 
> (Updated Nov. 1, 2016, 12:22 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Bugs: MESOS-6412
> https://issues.apache.org/jira/browse/MESOS-6412
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than returning a fixed generic error when connect(2) fails,
> report the actual socket error as well as the address we were trying
> to connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 
> 
> Diff: https://reviews.apache.org/r/52997/diff/
> 
> 
> Testing
> ---
> 
> `make check` on a host whose hostname doesn't match its IP address.
> 
> ```
> [jpeach@jpeach libprocess]$ ./libprocess-tests 
> --gtest_filter=HTTPTest.Endpoints
> Note: Google Test filter = HTTPTest.Endpoints
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPTest
> [ RUN  ] HTTPTest.Endpoints
> ../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
> (socket.connect(http.process->self().address)).failure(): Failed to connect 
> to 17.203.52.49:39241: Connection refused
> [  FAILED  ] HTTPTest.Endpoints (4 ms)
> [--] 1 test from HTTPTest (4 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (5 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] HTTPTest.Endpoints
> 
>  1 FAILED TEST
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 50125: Added mesos-docker-executor support for devices control.

[Guangya Liu]

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




src/docker/docker.hpp (lines 71 - 96)


Move this to /r/50599/ , otherwies, /r/50599/ will build failed



src/slave/containerizer/docker.cpp (line 234)


I think that you should not update here but use `Option` as before.



src/slave/containerizer/docker.cpp (line 252)


Here should be 

```
if (devices.isSome() && !devices->empty()) {
```



src/slave/containerizer/docker.cpp (line 366)


Use `None()` directly


- Guangya Liu


On 十一月 1, 2016, 6:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50125/
> ---
> 
> (Updated 十一月 1, 2016, 6:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new flag '--devices' to mesos-docker-executor, and gave its
> feature to control devices exposition, isolation, and access permission.
> Also, passed GPUs assignment to mesos-docker-executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
>   src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 52587: Allow CREATE of shared volumes based on capability of framework.

[Jiang Yan Xu]

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


Fix it, then Ship it!





src/master/validation.hpp (line 197)


s/shared volume is/shared volumes are/?

s/FrameworkInfo/FrameworkInfo (unless the operation is requested by the 
operator)/



src/master/validation.cpp (line 1520)


s/which has not opted in for SHARED_RESOURCES/with no SHARED_RESOURCES 
capability/ 

slightly shorter and more direct about the capability?


- Jiang Yan Xu


On Oct. 6, 2016, 1:16 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52587/
> ---
> 
> (Updated Oct. 6, 2016, 1:16 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6316
> https://issues.apache.org/jira/browse/MESOS-6316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework issues a CREATE for a shared volume, we allow that
> operation only if the framework has opted in for the capability of
> SHARED_RESOURCES. However, for HTTP operator (/create-volumes), we
> do not check as the operator API is not related to a specific
> framework.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 02a2fb29bdd8484fc90e5cb033ac29b49a141860 
>   src/master/validation.hpp 035f721c610ae566c89a1cf0e65ff0df11679f15 
>   src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
>   src/tests/master_validation_tests.cpp 
> 99e350e0587e73e9ee25ef20dd369cd146bd446a 
>   src/tests/persistent_volume_tests.cpp 
> e10a79e9662530e143ccaa2aa2506a4d25158364 
> 
> Diff: https://reviews.apache.org/r/52587/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 53127: Added the test `ProvisionerDockerWhiteoutTest`.

[Qian Zhang]

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

(Updated Nov. 1, 2016, 2:58 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Added the logic to verify the whiteout file can NOT apply to the entries in the 
same layer.


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


Repository: mesos


Description
---

Added the test `ProvisionerDockerWhiteoutTest`.


Diffs (updated)
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
d8fb7abb55410cfd0835f59bfc0f35566aeeae37 

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


Testing
---

[==] Running 3 tests from 1 test case.
[--] Global test environment set-up.
[--] 3 tests from BackendFlag/ProvisionerDockerWhiteoutTest
[ RUN  ] 
BackendFlag/ProvisionerDockerWhiteoutTest.ROOT_INTERNET_CURL_Whiteout/0
Executing pre-exec command 
'{"arguments":["mesos-containerizer","mount","--help=false","--operation=make-rslave","--path=\/"],"shell":false,"value":"\/home\/stack\/workspace\/mesos\/build\/src\/mesos-containerizer"}'
Executing pre-exec command 
'{"arguments":["mount","-n","--rbind","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX\/slaves\/5ec98305-4a7d-4351-976e-7c7f4023329c-S0\/frameworks\/5ec98305-4a7d-4351-976e-7c7f4023329c-\/executors\/7bea80dc-b644-430f-abb0-cabe2af3ded8\/runs\/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX\/provisioner\/containers\/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1\/backends\/aufs\/rootfses\/8837b468-156f-4e04-96b2-ab201626ad3c\/mnt\/mesos\/sandbox"],"shell":false,"value":"mount"}'
I1024 16:17:23.245787 13269 exec.cpp:162] Version: 1.2.0
I1024 16:17:23.256351 13271 exec.cpp:237] Executor registered on agent 
5ec98305-4a7d-4351-976e-7c7f4023329c-S0
Received SUBSCRIBED event
Subscribed executor on workstation
Received LAUNCH event
Starting task 7bea80dc-b644-430f-abb0-cabe2af3ded8
/home/stack/workspace/mesos/build/src/mesos-containerizer launch 
--command="{"arguments":["test","!","-f","\/etc\/rc3.d\/S40-network"],"shell":false,"value":"\/usr\/bin\/test"}"
 --help="false" 
--rootfs="/tmp/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX/provisioner/containers/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1/backends/aufs/rootfses/8837b468-156f-4e04-96b2-ab201626ad3c"
 --unshare_namespace_mnt="true" --user="root" 
--working_directory="/mnt/mesos/sandbox"
Forked command at 13278
Changing root to 
/tmp/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_0_42lvlX/provisioner/containers/2f4f0799-8fc1-4260-b1d3-422aa40f8ef1/backends/aufs/rootfses/8837b468-156f-4e04-96b2-ab201626ad3c
Command exited with status 0 (pid: 13278)
I1024 16:17:23.500792 13273 exec.cpp:414] Executor asked to shutdown
Received SHUTDOWN event
Shutting down
[   OK ] 
BackendFlag/ProvisionerDockerWhiteoutTest.ROOT_INTERNET_CURL_Whiteout/0 (1365 
ms)
[ RUN  ] 
BackendFlag/ProvisionerDockerWhiteoutTest.ROOT_INTERNET_CURL_Whiteout/1
Executing pre-exec command 
'{"arguments":["mesos-containerizer","mount","--help=false","--operation=make-rslave","--path=\/"],"shell":false,"value":"\/home\/stack\/workspace\/mesos\/build\/src\/mesos-containerizer"}'
Executing pre-exec command 
'{"arguments":["mount","-n","--rbind","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_1_Zm1mK7\/slaves\/9d49d96c-189d-47e9-9370-72146356af7c-S0\/frameworks\/9d49d96c-189d-47e9-9370-72146356af7c-\/executors\/ae8fda3e-db36-49f7-8699-9a4768d4af27\/runs\/49f24d09-6474-4a39-9da6-8c8da0ad4238","\/tmp\/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_1_Zm1mK7\/provisioner\/containers\/49f24d09-6474-4a39-9da6-8c8da0ad4238\/backends\/copy\/rootfses\/db73608a-614b-49be-bebe-cd9aa42e31f5\/mnt\/mesos\/sandbox"],"shell":false,"value":"mount"}'
I1024 16:17:24.971269 13344 exec.cpp:162] Version: 1.2.0
I1024 16:17:24.981901 13364 exec.cpp:237] Executor registered on agent 
9d49d96c-189d-47e9-9370-72146356af7c-S0
Received SUBSCRIBED event
Subscribed executor on workstation
Received LAUNCH event
Starting task ae8fda3e-db36-49f7-8699-9a4768d4af27
/home/stack/workspace/mesos/build/src/mesos-containerizer launch 
--command="{"arguments":["test","!","-f","\/etc\/rc3.d\/S40-network"],"shell":false,"value":"\/usr\/bin\/test"}"
 --help="false" 
--rootfs="/tmp/BackendFlag_ProvisionerDockerWhiteoutTest_ROOT_INTERNET_CURL_Whiteout_1_Zm1mK7/provisioner/containers/49f24d09-6474-4a39-9da6-8c8da0ad4238/backends/copy/rootfses/db73608a-614b-49be-bebe-cd9aa42e31f5"
 --unshare_namespace_mnt="true" --user="root" 
--working_directory="/mnt/mesos/sandbox"
Forked command at 13367
Changing root to 

Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

[Guangya Liu]

> On 十月 26, 2016, 3:22 a.m., Kevin Klues wrote:
> > src/slave/containerizer/docker.cpp, line 691
> > 
> >
> > This is OK since we do this the same in other functions for this class, 
> > but we should really wrap pointers like this in Owned<> or Shared<> 
> > depending on their semantics.

I think that you did not resolve this, please keep this open.


- Guangya


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


On 十一月 1, 2016, 6:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十一月 1, 2016, 6:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

[Guangya Liu]

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




src/slave/containerizer/docker.cpp (lines 705 - 707)


kill this, there is no constant variable now



src/slave/containerizer/docker.cpp (line 712)


Why not kill this and use "/dev/nvidia" directly in #729?


- Guangya Liu


On 十一月 1, 2016, 6:11 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50599/
> ---
> 
> (Updated 十一月 1, 2016, 6:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Assigned Nvidia GPU devices to docker container based on
> GPUs allocated by Nvidia GPU allocator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/50599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 53116: Removed `ProvisionerProcess::__provision()`.

[Qian Zhang]

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

(Updated Nov. 1, 2016, 2:45 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Removed `ProvisionerProcess::__provision()`.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
c5481c585f2a55bb926340290b939f98af3c2135 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
de2c12140652244bd3de9763ced203b144688ab2 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 53115: Implemented handling AUFS whiteouts for copy backend.

[Qian Zhang]

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

(Updated Nov. 1, 2016, 2:44 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

Implemented handling AUFS whiteouts for copy backend.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
9c5354e5fea488618ebdecf0aef9dd2fce555d20 

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


Testing
---

make check.


Thanks,

Qian Zhang

Re: Review Request 53115: Implemented handling AUFS whiteouts for copy backend.

[Qian Zhang]

> On Oct. 28, 2016, 10:03 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/provisioner/backends/copy.cpp, line 133
> > 
> >
> > I am not sure if we need to pass in 'image' to backend. If we added OCI 
> > support, do we need to add another check here?
> > 
> > I think we can simply assume the layer's whiteout is in aufs whiteout 
> > format, no matter what image type it is. I think APPC, we probably needs to 
> > do the same, right?
> > 
> > You probably add a NOTE there saying that we assume all image type uses 
> > aufs whiteout format.
> > 
> > In that way, no need to pass 'image' around to backend.
> 
> Qian Zhang wrote:
> The reason I passed in `image` and added this check is that previously we 
> have such check in `ProvisionerProcess::__provision()`:
> 
> https://github.com/apache/mesos/blob/1.0.1/src/slave/containerizer/mesos/provisioner/provisioner.cpp#L334
> 
> I think one of the purposes of this check is to ensure we will NOT go 
> through the logic to handle whiteout files for APPC. I think this is correct 
> because APPC seems have a totally different way for whiteout, in its image 
> manifest (https://github.com/appc/spec/blob/master/spec/aci.md ), there is a 
> field `pathWhitelist`:
> ```
> pathWhitelist (list of strings, optional): whitelist of absolute paths 
> that will exist in the app's rootfs after rendering. This must be a complete 
> and absolute set. An empty list is equivalent to an absent value and means 
> that all files in this image and any dependencies will be available in the 
> rootfs.
> ```
> And there is PR in APPC to translate Docker whiteout files to 
> `pathWhitelist`: https://github.com/appc/docker2aci/pull/36
> 
> So I think if we do not pass in `image`, then copy backend will try to 
> handle whiteout files for APPC image which is not necessary since there is no 
> whiteout files to handle, instead, we should support `pathWhitelist` which 
> seems missed in 
> https://github.com/apache/mesos/blob/1.0.1/include/mesos/appc/spec.proto , 
> that is another issue.
> 
> Jie Yu wrote:
> If you think about that in a different way: can we convert the APPC 
> whiteout to be the same as OCI and Docker? I don't want each backend to have 
> different ways to handle whiteouts depending on the type. At the end of the 
> day, one needs to convert whiteout to aufs or overlayfs format, right?
> 
> Qian Zhang wrote:
> So you are suggesting in APPC store, we can convert APPC whiteout (i.e., 
> `pathWhitelist`) to AUFS whiteout (if the backend is aufs/copy) or overlay 
> whiteout (if the backend is overlay), and then in each backend, they can just 
> do their work regardless the image type, right? If so, then I think we should 
> create another JIRA ticket for converting APPC whiteout to AUFS/overlay 
> whiteout in APPC store.
> 
> Jie Yu wrote:
> Yup. No matter what, we need to fix the APPC image store.

OK, I have created a ticket for fixing APPC image store:
https://issues.apache.org/jira/browse/MESOS-6510


- Qian


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


On Oct. 25, 2016, 11:47 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53115/
> ---
> 
> (Updated Oct. 25, 2016, 11:47 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented handling AUFS whiteouts for copy backend.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backend.hpp 
> 7257d3a962ecdf87fe9d52facbd6a2619311a018 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp 
> a3c924195ae5eecc1caca9cd6fc0f6dc0df0a741 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
> c92c6c7174421158b9190ed0bfb00c1c97ef0f7b 
>   src/slave/containerizer/mesos/provisioner/backends/bind.hpp 
> 54c88d940aa64d13114fc5d79ecbc1d474d169a6 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 94dac40a12b6fd2e7d9733444d84763c77785402 
>   src/slave/containerizer/mesos/provisioner/backends/copy.hpp 
> 131d75521ca38afae651e8d885ebedad77d86a3e 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> 9c5354e5fea488618ebdecf0aef9dd2fce555d20 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 
> 4d591c5f988d87e0e905f973df5ab15a3386d676 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> b71a31323aef376c9a28e1d52322a1802fb212ad 
>   

Re: Review Request 53161: Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.

[Qian Zhang]

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

(Updated Nov. 1, 2016, 2:42 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addresed Jie's comments.


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


Repository: mesos


Description
---

Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.


Diffs (updated)
-

  src/Makefile.am c2f9e442182110d0b450d4824600a4a791f8cf27 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
e192f86a1848b373f3aa73d29688a96375cac313 
  src/slave/containerizer/mesos/provisioner/utils.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/utils.cpp PRE-CREATION 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 52735: Removed TODO message for docker killing.

[Yubo Li]

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

(Updated Nov. 1, 2016, 6:13 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


Repository: mesos


Description
---

Removed TODO message for docker killing.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---


Thanks,

Yubo Li

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Yubo Li]

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

(Updated Nov. 1, 2016, 6:12 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

This added a testing case for end-to-end GPU support for docker
containerizer.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
810488d1476cadbbd5a4a7dcecaeec55739ab71f 

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


Testing
---

GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

(Updated Nov. 1, 2016, 6:12 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50125: Added mesos-docker-executor support for devices control.

[Yubo Li]

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

(Updated Nov. 1, 2016, 6:11 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

Added a new flag '--devices' to mesos-docker-executor, and gave its
feature to control devices exposition, isolation, and access permission.
Also, passed GPUs assignment to mesos-docker-executor.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50841: Added GPU scheduling logic to docker containerizer process.

[Yubo Li]

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

(Updated Nov. 1, 2016, 6:10 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Added control logic to allocate/deallocate GPUs to GPU-related task
when the task is started/terminated.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

[Yubo Li]

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

(Updated Nov. 1, 2016, 6:11 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50128: Added helper function to 'Docker::Device'.

[Yubo Li]

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

(Updated Nov. 1, 2016, 6:10 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


Summary (updated)
-

Added helper function to 'Docker::Device'.


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


Repository: mesos


Description (updated)
---

Wrapped helper function to stringify 'Docker::Device' structure.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50123: Added GPU scheduler for docker containerizer process.

[Yubo Li]

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

(Updated Nov. 1, 2016, 6:10 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.


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


Repository: mesos


Description
---

This added 'NvidiaComponents' to docker containerizer process so that
docker containerizer process is ready to use it to allocate GPUs to task
with 'gpus' resource.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
  src/tests/mock_docker.hpp 1bf09c8dba020b421526b650523c879fb87380f8 
  src/tests/mock_docker.cpp 6a0e613bde6889864a37ffd7ec0b454e5fe4df1c 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 53053: Divided utils.hpp to utils.hpp and utils.cpp.

[Qian Zhang]

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

(Updated Nov. 1, 2016, 2:08 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Divided utils.hpp to utils.hpp and utils.cpp.


Diffs (updated)
-

  src/Makefile.am c2f9e442182110d0b450d4824600a4a791f8cf27 
  src/slave/containerizer/mesos/utils.hpp 
178ebf3effac824e4788d7282795c18dc1cb5265 
  src/slave/containerizer/mesos/utils.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang