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

[Jie Yu]

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




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


Should we return a Failure here  instead?



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


There's a problem with the current code. 'collect' (in isolate()) on 
'attach' will fail if any of the call to the plugin (ADD) fails. And we'll call 
'cleanup' right after that, it's likely that another call to the plugin (ADD) 
is still pending. I don't think calling DEL while an ADD is still pending is 
gonna work.

Therefore, I think we should use 'await' in isolate instead of 'collect' so 
that we can make sure all of them are in termimal state before we return the 
result.



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


You want to read 'stderr' as well.



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


Could you please add a comment about the fact that destroy cannot happen in 
the middle of attach and `_attach` because the containerizer will wait for 
isolate to finish before destroying the container.

Also, add a CHECK(infos.container(containerId));


- Jie Yu


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

Re: Review Request 45404: Cleaned up a test case in stout.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45403, 45404]

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

- Mesos ReviewBot


On March 28, 2016, 10:57 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45404/
> ---
> 
> (Updated March 28, 2016, 10:57 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up a test case in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/45404/diff/
> 
> 
> Testing
> ---
> 
> Compiled tests on CentOS 7 and OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

[Jie Yu]

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




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


maybe use getenv("PATH")?


- Jie Yu


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

Re: Review Request 44421: Added support for "overlay" keyword.

[Guangya Liu]

> On 三月 29, 2016, 4:42 a.m., Cong Wang wrote:
> > src/slave/containerizer/mesos/provisioner/backend.cpp, line 50
> > 
> >
> > Why do you need to test overlay fs support in create()? If kernel 
> > doesn't support this, the mount in provision() will simply fail.

I think that the original thinking is `fail fast`, if the agent was started 
with `overlay` as backend and the agent does not support `overlayfs`, the agent 
will be failed to start.

I was removing this logic in https://reviews.apache.org/r/44431/ because the 
`overlay` support logic was already checked when creating the backends creators.


- Guangya


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


On 三月 18, 2016, 4:22 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44421/
> ---
> 
> (Updated 三月 18, 2016, 4:22 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Shuai Lin.
> 
> 
> Bugs: MESOS-4874
> https://issues.apache.org/jira/browse/MESOS-4874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The "overlayfs" was renamed to "overlay" in kernel 4.2, for overlay
> support check function, it should check both "overlay" and "overlayfs"
> in "/proc/filesystems".
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
>   src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> 55652540e35f9c451ad85cfead575a788aa3eba1 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 5cc0f8b5a8cd4c945023f874056a8184113186c5 
>   src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
> 
> Diff: https://reviews.apache.org/r/44421/diff/
> 
> 
> Testing
> ---
> 
> root@mesos002:~/src/mesos/m1/mesos/build# uname -r
> 4.2.3-040203-generic
> root@mesos002:~/src/mesos/m1/mesos/build# lsmod  | grep over
> overlay45056  1 
> root@mesos002:~/src/mesos/m1/mesos/build# cat /proc/filesystems | grep over
> nodev overlay
> root@mesos002:~/src/
> 
> make
> make check
> ./bin/mesos-tests.sh 
> --gtest_filter="OverlayBackendTest.ROOT_OVERLAYFS_OverlayFSBackend" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45401: Fixed typo in subprocess doxygen comments.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 28, 2016, 9:42 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45401/
> ---
> 
> (Updated March 28, 2016, 9:42 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in subprocess doxygen comments.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> da806640a238e168bd24ea837cbb711041fe1d12 
> 
> Diff: https://reviews.apache.org/r/45401/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45400: Adapted port_mapping with missing subprocess parameter.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 28, 2016, 9:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45400/
> ---
> 
> (Updated March 28, 2016, 9:41 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adapted port_mapping with missing subprocess parameter.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 134b6c759b769cf335539e49eff817973c7f96a4 
>   src/tests/containerizer/port_mapping_tests.cpp 
> de4b6f99f3a994bcedafa801eed9c4a7b79bac23 
> 
> Diff: https://reviews.apache.org/r/45400/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on with port_isolator enabled.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45399: Fixed capitalization of Watchdog enum.

[Jie Yu]

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



Can you provide more context for this change? For instance, why change Watchdog 
and not Setsid?

- Jie Yu


On March 28, 2016, 9:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45399/
> ---
> 
> (Updated March 28, 2016, 9:41 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed capitalization of Watchdog enum.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> da806640a238e168bd24ea837cbb711041fe1d12 
>   3rdparty/libprocess/src/subprocess.cpp 
> be32962e94b605ee2e15d35010acd05d58c2b2d3 
> 
> Diff: https://reviews.apache.org/r/45399/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45404: Cleaned up a test case in stout.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 28, 2016, 10:57 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45404/
> ---
> 
> (Updated March 28, 2016, 10:57 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up a test case in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/45404/diff/
> 
> 
> Testing
> ---
> 
> Compiled tests on CentOS 7 and OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44964: Removed unused header includes.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 29, 2016, 12:25 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44964/
> ---
> 
> (Updated March 29, 2016, 12:25 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This mostly targets unnecessary inclusion of stout headers,
> which can be quite expensive to compile.
> 
> 
> Diffs
> -
> 
>   src/credentials/credentials.hpp 32492f2fa4126d26afe3ab0b523891023deb6a67 
>   src/examples/persistent_volume_framework.cpp 
> 3848e247994b511fb2b1dcde337f8f37bb7472da 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/slave/state.cpp 94b6959cbca7ba14dfc70caa3402fe7d72d4757a 
>   src/tests/fetcher_tests.cpp bd86c962b37e2c584e261ba95798ac74a07091c2 
>   src/tests/group_tests.cpp ed10f1fe8a5cd2a353175e13ec3c464da69956f4 
>   src/tests/master_contender_detector_tests.cpp 
> bbce379e5a0a0ca608579d0ab2b10970e9cd5ef1 
>   src/tests/master_quota_tests.cpp c2b46d23002481e63ff162e8628f9b974e3e8ef9 
>   src/tests/mesos.hpp f2a29d0ddce5ef2b9ef2f0155c428311a67dbab3 
>   src/tests/module_tests.cpp eddb0e5484964595797758e2742b10431e76c039 
>   src/tests/paths_tests.cpp 7952ae239470580f06df373c80f9115634b42640 
>   src/tests/persistent_volume_tests.cpp 
> aa097c3da1237e564dd4081c23034ab185be09bd 
>   src/tests/script.cpp 36ddf40fc003435f852ca138f66c4f33def1637b 
>   src/tests/state_tests.cpp 8fb2798e12fe77d6b93268e71e3a8261cb0842d5 
>   src/tests/status_update_manager_tests.cpp 
> 801767ba6b4a08a4f653a7735bf3a581a0f8f95f 
>   src/tests/zookeeper_test_server.hpp 
> e2d8218fd7000904acaee2221da648c665da8dfc 
>   src/zookeeper/group.cpp 7aa95a7b7eafe0a2c49dc077b3a4abd688e5d8e8 
> 
> Diff: https://reviews.apache.org/r/44964/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Timed the compilation of a few files to verify the compilation time 
> improvement -- e.g., removing an unused include of `stout/os.hpp` reduces 
> compile time by ~1 second on my laptop.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44963: Added a missing include to a stout header.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 17, 2016, 4:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44963/
> ---
> 
> (Updated March 17, 2016, 4:41 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout headers should typically include their own dependencies.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp 
> 295eac77d5aba4084bbc044d94f45d660410d725 
> 
> Diff: https://reviews.apache.org/r/44963/diff/
> 
> 
> Testing
> ---
> 
> `make`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45381: Pass containerizer to ResourceMonitor and ResourceMonitorProcess.

[Guangya Liu]

> On 三月 28, 2016, 3:52 p.m., Guangya Liu wrote:
> > src/slave/slave.hpp, lines 398-399
> > 
> >
> > A question here: What is the use of this API?
> 
> Jay Guo wrote:
> This line of code is leaked from another implementation. I intended to 
> retrieve a list container ids from slave. Anyhow, as we communicated with 
> Jie, they decided that the whole logic should be kept in slave process. So 
> this patch should be discarded. Thanks for reviewing!

I think that you do not need to discard this patch but continue update this 
patch should be good enough.


- Guangya


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


On 三月 28, 2016, 11:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45381/
> ---
> 
> (Updated 三月 28, 2016, 11:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used by ResourceMonitorProcess to collect ContainerStatus
> internally, to avoid dispatching this action back and forth between
> slave process and monitor process.
> 
> 
> Diffs
> -
> 
>   src/slave/monitor.hpp 70a7c88fefd4577f53f85e3eccdd6b69ab6f3acd 
>   src/slave/monitor.cpp 5c1dd354595e67e5eb603c5d49850f84b84b 
>   src/slave/slave.hpp 3ba335fcd31a92af9609023daae328b7f4bf5e59 
>   src/slave/slave.cpp f383605a52f31d7b805ad6153adc409dcb40f83a 
>   src/tests/containerizer.hpp efc1ca87c00b04d70efd1e3f6acf4e763132d6b0 
>   src/tests/containerizer.cpp 4c7f5a26d38222daf013bd3ca9a133e12d7bf338 
>   src/tests/monitor_tests.cpp 5dcb2481ff2f1a7caf54036bc3e60c78feb982b1 
> 
> Diff: https://reviews.apache.org/r/45381/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> https://issues.apache.org/jira/browse/MESOS-4891
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 45358: Improved overlay backend to make the rootfs writable.

[Guangya Liu]

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



The document here 
https://github.com/apache/mesos/blob/master/docs/container-image.md#overlay 
should also be updated, you can either handle it in this patch or another.

- Guangya Liu


On 三月 27, 2016, 2:41 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45358/
> ---
> 
> (Updated 三月 27, 2016, 2:41 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4944
> https://issues.apache.org/jira/browse/MESOS-4944
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved overlay backend to make the rootfs writable.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e7f7e7fd1304e14dbfaab8b53cea16efc0417911 
>   src/slave/containerizer/mesos/provisioner/backend.hpp 
> c6cca81e287bb9a62e0390f96e1773841887a206 
>   src/slave/containerizer/mesos/provisioner/backends/bind.hpp 
> 9eda944e14f2b05f28620c2e40594ed4e7bab69e 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 9b9f0b9e99a0fc0e9bad8fb2dad41acdc0ca1da1 
>   src/slave/containerizer/mesos/provisioner/backends/copy.hpp 
> b62507f9fc757349d39f39a6654ddd69053bf0e7 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> f353c89806816e85bb27875ef5fa68d5c0eaf9ca 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 
> 85cc737438c88b355f37611bfde50dc80efab017 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 5cc0f8b5a8cd4c945023f874056a8184113186c5 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 8a4938ebe4e00779e88e7c538445e9ffd51202e2 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 09742ff21513dc2570684d384b257868dd57a9ce 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 23a5b1059b4d9fde1e4a1aab5cd4fa6d05862332 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> d49204f220c6212f83c2adf5544d04b3386c8eb7 
> 
> Diff: https://reviews.apache.org/r/45358/diff/
> 
> 
> Testing
> ---
> 
> - make check
> - tested manually, with slave using overlay backend. Create a task with mesos 
> containerizer, ubuntu docker image, with the command `mkdir -p /abc && touch 
> /abc/def.txt`.
> - also tested manually with alpine image to verify overlay backend could 
> support 1-layer images.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 45358: Improved overlay backend to make the rootfs writable.

[Guangya Liu]

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




src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (lines 134 - 142)


Those .overlay folders will not be in rootfs, can you please also add some 
comments here to specifiy the behavaior?



src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (lines 148 - 150)


1 layer was also supportted for now, can you update the comments too?


- Guangya Liu


On 三月 27, 2016, 2:41 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45358/
> ---
> 
> (Updated 三月 27, 2016, 2:41 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4944
> https://issues.apache.org/jira/browse/MESOS-4944
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved overlay backend to make the rootfs writable.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e7f7e7fd1304e14dbfaab8b53cea16efc0417911 
>   src/slave/containerizer/mesos/provisioner/backend.hpp 
> c6cca81e287bb9a62e0390f96e1773841887a206 
>   src/slave/containerizer/mesos/provisioner/backends/bind.hpp 
> 9eda944e14f2b05f28620c2e40594ed4e7bab69e 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 9b9f0b9e99a0fc0e9bad8fb2dad41acdc0ca1da1 
>   src/slave/containerizer/mesos/provisioner/backends/copy.hpp 
> b62507f9fc757349d39f39a6654ddd69053bf0e7 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> f353c89806816e85bb27875ef5fa68d5c0eaf9ca 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 
> 85cc737438c88b355f37611bfde50dc80efab017 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 5cc0f8b5a8cd4c945023f874056a8184113186c5 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 8a4938ebe4e00779e88e7c538445e9ffd51202e2 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 09742ff21513dc2570684d384b257868dd57a9ce 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 23a5b1059b4d9fde1e4a1aab5cd4fa6d05862332 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> d49204f220c6212f83c2adf5544d04b3386c8eb7 
> 
> Diff: https://reviews.apache.org/r/45358/diff/
> 
> 
> Testing
> ---
> 
> - make check
> - tested manually, with slave using overlay backend. Create a task with mesos 
> containerizer, ubuntu docker image, with the command `mkdir -p /abc && touch 
> /abc/def.txt`.
> - also tested manually with alpine image to verify overlay backend could 
> support 1-layer images.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 42860: Add paths::overlapping to check whether paths are overlapping.

[Neil Conway]

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



Made a few quick comments. I didn't look at the `CheckOverlapTree` logic itself 
yet though.


3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 33)


Semicolon seems unnecessary.



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 53)


Semicolon seems unnecessary.



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 126)


Can we clarify what "overlapping" means here?



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 127)


What does "flat" mean, here? Also, is it worth adding a `CHECK` that all 
paths are absolute?



3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp (line 128)


Should probably be `const std::vector&`.


- Neil Conway


On March 29, 2016, 2:30 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42860/
> ---
> 
> (Updated March 29, 2016, 2:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Joris Van Remoortere, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-4521
> https://issues.apache.org/jira/browse/MESOS-4521
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add paths::overlapping to check whether paths are overlapping.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
> ef538045a8b7a1e3d8962c869317d86a85e0259f 
>   3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
> 6dff5e76e0e15098c5a262adc50bfcb65f933697 
> 
> Diff: https://reviews.apache.org/r/42860/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42861: Ensure two Mount Disk resources do not have the same root path.

[Neil Conway]

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



It would be good to have a unit test for the end-to-end behavior (i.e., we have 
a test for `path::overlapping` in the other RR, but we should also have a test 
for trying to create two volumes with overlapping paths).


src/slave/slave.cpp (line 504)


Comment should be updated if we're going to do this even when you're not 
"verifying against the mount table entries".



src/slave/slave.cpp (line 552)


Error message shouldn't end in punctuation.


- Neil Conway


On March 29, 2016, 2:30 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42861/
> ---
> 
> (Updated March 29, 2016, 2:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Joris Van Remoortere, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-4521
> https://issues.apache.org/jira/browse/MESOS-4521
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensure two Mount Disk resources do not have the same root path.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f383605a52f31d7b805ad6153adc409dcb40f83a 
> 
> Diff: https://reviews.apache.org/r/42861/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 44421: Added support for "overlay" keyword.

[Guangya Liu]

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



@jieyu, can you please help check if we can merge this, this can help a lot 
when using 4.2 or higher kernel.

- Guangya Liu


On 三月 18, 2016, 4:22 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44421/
> ---
> 
> (Updated 三月 18, 2016, 4:22 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Shuai Lin.
> 
> 
> Bugs: MESOS-4874
> https://issues.apache.org/jira/browse/MESOS-4874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The "overlayfs" was renamed to "overlay" in kernel 4.2, for overlay
> support check function, it should check both "overlay" and "overlayfs"
> in "/proc/filesystems".
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
>   src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> 55652540e35f9c451ad85cfead575a788aa3eba1 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 5cc0f8b5a8cd4c945023f874056a8184113186c5 
>   src/tests/environment.cpp ee1bbe6b4e3dda1e27b63d71a08ef0d1d254741a 
> 
> Diff: https://reviews.apache.org/r/44421/diff/
> 
> 
> Testing
> ---
> 
> root@mesos002:~/src/mesos/m1/mesos/build# uname -r
> 4.2.3-040203-generic
> root@mesos002:~/src/mesos/m1/mesos/build# lsmod  | grep over
> overlay45056  1 
> root@mesos002:~/src/mesos/m1/mesos/build# cat /proc/filesystems | grep over
> nodev overlay
> root@mesos002:~/src/
> 
> make
> make check
> ./bin/mesos-tests.sh 
> --gtest_filter="OverlayBackendTest.ROOT_OVERLAYFS_OverlayFSBackend" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45358: Improved overlay backend to make the rootfs writable.

[Guangya Liu]

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




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


two spaces



src/slave/containerizer/mesos/provisioner/backend.hpp (lines 47 - 50)


The comments shoud also be updated.



src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 154)


VLOG(1) to print out all of the mount options for overlay.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines 304 - 308)


return backends.get(backend).get()->provision(
ImageInfo.layers, rootfs, executorDirectory)
  .then([rootfs, ImageInfo]() -> Future {
return ProvisionInfo{rootfs, ImageInfo.dockerManifest};
  });


- Guangya Liu


On 三月 27, 2016, 2:41 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45358/
> ---
> 
> (Updated 三月 27, 2016, 2:41 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4944
> https://issues.apache.org/jira/browse/MESOS-4944
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved overlay backend to make the rootfs writable.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e7f7e7fd1304e14dbfaab8b53cea16efc0417911 
>   src/slave/containerizer/mesos/provisioner/backend.hpp 
> c6cca81e287bb9a62e0390f96e1773841887a206 
>   src/slave/containerizer/mesos/provisioner/backends/bind.hpp 
> 9eda944e14f2b05f28620c2e40594ed4e7bab69e 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 9b9f0b9e99a0fc0e9bad8fb2dad41acdc0ca1da1 
>   src/slave/containerizer/mesos/provisioner/backends/copy.hpp 
> b62507f9fc757349d39f39a6654ddd69053bf0e7 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> f353c89806816e85bb27875ef5fa68d5c0eaf9ca 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.hpp 
> 85cc737438c88b355f37611bfde50dc80efab017 
>   src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
> 5cc0f8b5a8cd4c945023f874056a8184113186c5 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 8a4938ebe4e00779e88e7c538445e9ffd51202e2 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 09742ff21513dc2570684d384b257868dd57a9ce 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 23a5b1059b4d9fde1e4a1aab5cd4fa6d05862332 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> d49204f220c6212f83c2adf5544d04b3386c8eb7 
> 
> Diff: https://reviews.apache.org/r/45358/diff/
> 
> 
> Testing
> ---
> 
> - make check
> - tested manually, with slave using overlay backend. Create a task with mesos 
> containerizer, ubuntu docker image, with the command `mkdir -p /abc && touch 
> /abc/def.txt`.
> - also tested manually with alpine image to verify overlay backend could 
> support 1-layer images.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 42860: Add paths::overlapping to check whether paths are overlapping.

[haosdent huang]

> On March 21, 2016, 5:29 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp, line 70
> > 
> >
> > `if (paths.empty()) { ... }` seems like a clearer way to write this.

@neic, @bbannier Thank you for your detail reviews. I rework the patches, could 
you help review them again? Thanks a lot.


- haosdent


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


On March 29, 2016, 2:30 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42860/
> ---
> 
> (Updated March 29, 2016, 2:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Joris Van Remoortere, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-4521
> https://issues.apache.org/jira/browse/MESOS-4521
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add paths::overlapping to check whether paths are overlapping.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
> ef538045a8b7a1e3d8962c869317d86a85e0259f 
>   3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
> 6dff5e76e0e15098c5a262adc50bfcb65f933697 
> 
> Diff: https://reviews.apache.org/r/42860/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42860: Add paths::overlapping to check whether paths are overlapping.

[haosdent huang]

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

(Updated March 29, 2016, 2:30 a.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, Joris Van Remoortere, and 
Neil Conway.


Changes
---

Address @comments.


Summary (updated)
-

Add paths::overlapping to check whether paths are overlapping.


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


Repository: mesos


Description (updated)
---

Add paths::overlapping to check whether paths are overlapping.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
ef538045a8b7a1e3d8962c869317d86a85e0259f 
  3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
6dff5e76e0e15098c5a262adc50bfcb65f933697 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42861: Ensure two Mount Disk resources do not have the same root path.

[haosdent huang]

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

(Updated March 29, 2016, 2:30 a.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, Joris Van Remoortere, and 
Neil Conway.


Changes
---

Address comments.


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


Repository: mesos


Description
---

Ensure two Mount Disk resources do not have the same root path.


Diffs (updated)
-

  src/slave/slave.cpp f383605a52f31d7b805ad6153adc409dcb40f83a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45381: Pass containerizer to ResourceMonitor and ResourceMonitorProcess.

[Jay Guo]

> On March 28, 2016, 3:52 p.m., Guangya Liu wrote:
> > src/slave/slave.hpp, lines 398-399
> > 
> >
> > A question here: What is the use of this API?

This line of code is leaked from another implementation. I intended to retrieve 
a list container ids from slave. Anyhow, as we communicated with Jie, they 
decided that the whole logic should be kept in slave process. So this patch 
should be discarded. Thanks for reviewing!


- Jay


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


On March 28, 2016, 11:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45381/
> ---
> 
> (Updated March 28, 2016, 11:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used by ResourceMonitorProcess to collect ContainerStatus
> internally, to avoid dispatching this action back and forth between
> slave process and monitor process.
> 
> 
> Diffs
> -
> 
>   src/slave/monitor.hpp 70a7c88fefd4577f53f85e3eccdd6b69ab6f3acd 
>   src/slave/monitor.cpp 5c1dd354595e67e5eb603c5d49850f84b84b 
>   src/slave/slave.hpp 3ba335fcd31a92af9609023daae328b7f4bf5e59 
>   src/slave/slave.cpp f383605a52f31d7b805ad6153adc409dcb40f83a 
>   src/tests/containerizer.hpp efc1ca87c00b04d70efd1e3f6acf4e763132d6b0 
>   src/tests/containerizer.cpp 4c7f5a26d38222daf013bd3ca9a133e12d7bf338 
>   src/tests/monitor_tests.cpp 5dcb2481ff2f1a7caf54036bc3e60c78feb982b1 
> 
> Diff: https://reviews.apache.org/r/45381/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> https://issues.apache.org/jira/browse/MESOS-4891
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 45342: Make the Action enum optional to support upgrades (MESOS-5031).

[Yong Tang]

> On March 28, 2016, 10:23 p.m., Adam B wrote:
> > Let's change the UNREACHABLE to an error+unauthorized.

Thanks for the review Adam. I updated the review request. Let me know if the 
change meets the requirements or not.


- Yong


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


On March 29, 2016, 2:04 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45342/
> ---
> 
> (Updated March 29, 2016, 2:04 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix tries to make the Action enum in Authorization optional
> to support upgrades. See MESOS-4997 for details.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 944a493e0979c7ffbd99f3a67785a10425fd9040 
>   src/authorizer/local/authorizer.cpp 
> 0f0d9276337858984f0b19a82ffca74ee84dc650 
> 
> Diff: https://reviews.apache.org/r/45342/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45342: Make the Action enum optional to support upgrades (MESOS-5031).

[Yong Tang]

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

(Updated March 29, 2016, 2:04 a.m.)


Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.


Changes
---

Update the review request and change UNREACHABLE to an error+unauthorized.


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


Repository: mesos


Description
---

This fix tries to make the Action enum in Authorization optional
to support upgrades. See MESOS-4997 for details.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
944a493e0979c7ffbd99f3a67785a10425fd9040 
  src/authorizer/local/authorizer.cpp 0f0d9276337858984f0b19a82ffca74ee84dc650 

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


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

Re: Review Request 44974: Added device support in cgroups abstraction.

[Ben Mahler]

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


Fix it, then Ship it!




Thanks! Kevin and I went over this and made some adjustments to the comments 
and error formatting. Will commit shortly.


src/linux/cgroups.hpp (line 625)


s/subsytem/subsystem/ and end with a period here



src/linux/cgroups.hpp (lines 628 - 629)


How about s/ListEntry/Entry/ ? I was a bit confused about whether 
"whitelist" in this context meant that this isn't used for the "deny" file, for 
example.



src/linux/cgroups.hpp (lines 634 - 664)


I'm not sure if we need some of these comments, for example I can tell 
pretty easily from 'Access' that it represents the access permissions and it is 
part of a whitelist entry because it is contained within the ListEntry class. 
Ditto for selector.


- Ben Mahler


On March 21, 2016, 6:06 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44974/
> ---
> 
> (Updated March 21, 2016, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and 
> Niklas Nielsen.
> 
> 
> Bugs: MESOS-3368
> https://issues.apache.org/jira/browse/MESOS-3368
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There are some helper methods added to support device cgroups in
> cgroups abstraction to aid isolators controlling access to devices.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 51ccefd1fd2d0989b9bd31342d3d1e3701f88ed2 
>   src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c 
> 
> Diff: https://reviews.apache.org/r/44974/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 44975: Updated cgroups test cases for cgroups device support.

[Ben Mahler]

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


Ship it!




Thanks! Kevin and I made some minor style adjustments and will get this 
committed shortly.

- Ben Mahler


On March 21, 2016, 6:14 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44975/
> ---
> 
> (Updated March 21, 2016, 6:14 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and 
> Niklas Nielsen.
> 
> 
> Bugs: MESOS-3368
> https://issues.apache.org/jira/browse/MESOS-3368
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroups test cases for cgroups device support: DevicesTest.Parse
> and CgroupsAnyHierarchyDevicesTest.ROOT_CGROUPS_Devices
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_tests.cpp 
> acaed9b3f8a04964092cef413133834d0cf5a145 
> 
> Diff: https://reviews.apache.org/r/44975/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 45371: Upgrade libev to 4.22 to support PowerPC LE platform [mesos].

[Zhiwei Chen]

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

(Updated March 29, 2016, 10 a.m.)


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


Repository: mesos


Description
---

Upgrade libev to 4.22 to support PowerPC LE platform [mesos].


Diffs
-

  3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
  LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
  src/python/native_common/ext_modules.py.in 
c335bd83024bc07b6243dd59d775e7f29adc7520 

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


Testing (updated)
---

../configure --enable-libevent --enable-ssl

sudo make dist check

sudo ./src/mesos-tests --benchmark

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


Thanks,

Zhiwei Chen

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

[Zhiwei Chen]

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

(Updated March 29, 2016, 10 a.m.)


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


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


Repository: mesos


Description
---

Upgrade libev to 4.22 to support PowerPC LE platform [libprocess].


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
6b07aefc58a1daa235b35e83832e47d1878e2e94 
  3rdparty/libprocess/3rdparty/libev-4.15.patch 
bbd83e6928e6caba3bc5a9739823d60923cfaa2a 
  3rdparty/libprocess/3rdparty/libev-4.15.tar.gz 
4c282b573aa9331fd16197ef286faf323b6515eb 
  3rdparty/libprocess/3rdparty/libev-4.22.patch PRE-CREATION 
  3rdparty/libprocess/3rdparty/libev-4.22.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
98195b8eb60b2673d610d8ab7ea31103f137debf 

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


Testing (updated)
---

../configure --enable-libevent --enable-ssl

sudo make dist check

sudo ./src/mesos-tests --benchmark

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


Thanks,

Zhiwei Chen

Re: Review Request 42516: Add support for user-defined networks.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42516]

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

- Mesos ReviewBot


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

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

[Vinod Kone]

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




src/examples/long_lived_framework.cpp (lines 39 - 40)


Add new constants for executor and use them, instead of resuing these.

CPUS_PER_EXECUTOR
MEM_PER_EXECUTOR



src/examples/long_lived_framework.cpp (line 74)


s/. Reject/. Reject/ <-- extraneous space



src/examples/long_lived_framework.cpp (lines 84 - 85)


So if resources are not enough we do not explicitly decline the offer?

How about:

```
// Reject offer if the offer belongs to a different slave than the one 
currently running the executor(s).
if (slaveID.isSome() && slaveID != offers[i].slave_id()) {
  // decline offer.
}

// Reject the offer if the resources are not enough.
if (!Resources(offers[i].resources()).flatten().contains(TASK_RESOURCES + 
EXECUTOR_RESOURCES)) {
  // decline offer.
}

// Launch the task.

```

Also, are you expecting the executor to be multi-task? If yes, the 
resources check above should only include task resources? 

Since you do not control whether the executor is single or multi-task, I 
would recommend to launch each task with an unique executor id.



src/examples/long_lived_framework.cpp (line 85)


TASK_RESOURCES + EXEcUTOR_RESOURCES



src/examples/long_lived_framework.cpp (line 128)


s/sid/slaveID/



src/examples/long_lived_framework.cpp (line 129)


I thought we could compare option directly with a value instead of doing a 
.get()?



src/examples/long_lived_framework.cpp (line 138)


seems weird that you are setting residency/slaveID to None() if one 
executor is lost. isn't it still possible for other executors to be running on 
that slave? do we want to launch new executors on a different slave in that 
case?

I would recommend to keep track of running executors with `set`.



src/examples/long_lived_framework.cpp (line 151)


why is this called "residency" instead of "slaveID" ?



src/examples/long_lived_framework.cpp (line 174)


why does the framework assume master lives on the same machine?



src/examples/long_lived_framework.cpp (line 184)


s/mesos// ?



src/examples/long_lived_framework.cpp (line 212)


no underscores please. you can just name it "resources" here.



src/examples/long_lived_framework.cpp (lines 213 - 214)


use "*_PER_EXECUTOR" here



src/examples/long_lived_framework.cpp (lines 222 - 242)


Can you rephrase the comments to use the flag based executor first? I think 
it is easier to follow.

```
if (flags.executor_command.iSome()) {

} else if (flags.build_dir.isSome()) {

} else {

}

```



src/examples/long_lived_framework.cpp (line 228)


s/executor_command/command/


- Vinod Kone


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

Review Request 45410: Documented when to invoke `send` using the scheduler library.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Trivial change to add a comment to invoke `send` only after
receiving the `connected` callback. If not, all calls would
be dropped while disconnected.


Diffs
-

  include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 45409: Added test for `reconnect` functionality.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds a trivial test for testing the `reconnect`
method on the scheduler library.


Diffs
-

  src/tests/scheduler_tests.cpp 917058f4dcf32ddaaeda8a3ff21898571f4829dd 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 45408: Introduced a `reconnect` method on the scheduler library.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds a `reconnect` method that allows schedulers
to force a reconnection with the master. This is useful when
the scheduler detects that there is a one-way partition with
the master (e.g., lack of `HEARTBEAT` events) and wants to
trigger a reconnection rather than relying on the `disconnected`
callback.


Diffs
-

  include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
  src/scheduler/scheduler.cpp 13972449363b633f21ddec7649b1b170703c773a 

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


Testing
---

make check (added a test later in the review chain)


Thanks,

Anand Mazumdar

Re: Review Request 45401: Fixed typo in subprocess doxygen comments.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On March 28, 2016, 9:42 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45401/
> ---
> 
> (Updated March 28, 2016, 9:42 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in subprocess doxygen comments.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> da806640a238e168bd24ea837cbb711041fe1d12 
> 
> Diff: https://reviews.apache.org/r/45401/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45399: Fixed capitalization of Watchdog enum.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On March 28, 2016, 9:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45399/
> ---
> 
> (Updated March 28, 2016, 9:41 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed capitalization of Watchdog enum.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> da806640a238e168bd24ea837cbb711041fe1d12 
>   3rdparty/libprocess/src/subprocess.cpp 
> be32962e94b605ee2e15d35010acd05d58c2b2d3 
> 
> Diff: https://reviews.apache.org/r/45399/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Greg Mann]

> On March 28, 2016, 9:10 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch, line 10
> > 
> >
> > Do you know why the non-ssl build didn't have a problem with this 
> > conflict?
> > 
> > Also, the real fix is for stout to namespace its flags (e.g., 
> > stout::flags) implementation instead of fixing the library. But I don't 
> > know how big of a change is that.
> 
> Greg Mann wrote:
> It's due to the order of includes. The name collision occurs when 
> `3rdparty/libprocess/src/process.cpp` includes `openssl.hpp` - this leads to 
> a chain of includes that produces the collision. If SSL is not enabled, 
> `3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp` doesn't 
> get included in the same scope as `http_parser.h`. If I add `#include 
> ` to `process.cpp`, the same collision occurs when compiling.
> 
> Yep, I agree that stout should namespace its flags. It looks like that 
> header is included in 43 files, so we would need to add appropriate `using` 
> declarations in those files; not too bad.
> 
> It seems like it might be a good idea to also namespace all of our 3rd 
> party libraries, i.e. that enum could become `3rdparty::http-parser::flags`.
> 
> Vinod Kone wrote:
> Since this is blocking the CI, lets commit this for now. Can you create a 
> ticket to namespace stout flags?

SGTM. Ticket for the flags namespacing here: 
https://issues.apache.org/jira/browse/MESOS-5054


- Greg


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


On March 28, 2016, 9:58 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45397/
> ---
> 
> (Updated March 28, 2016, 9:58 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4805
> https://issues.apache.org/jira/browse/MESOS-4805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed name of http-parser enum to 'flags_enum'.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 04a4cf4241b8188c093f4942d27b050fa7b20397 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 385302a491aba5a38ff74c0debfc2a423b0c5a8a 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45397/diff/
> 
> 
> Testing
> ---
> 
> `../configure --enable-libevent --enable-ssl && make check` was used to text 
> on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 45403: Fixed build breakage with CentOS 7.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 28, 2016, 10:57 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45403/
> ---
> 
> (Updated March 28, 2016, 10:57 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed build breakage with CentOS 7.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 46ac919d9e795b575a31b21dd8613f38f33591f2 
> 
> Diff: https://reviews.apache.org/r/45403/diff/
> 
> 
> Testing
> ---
> 
> Compiled tests successfully on CentOS 7 and OSX.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45381: Pass containerizer to ResourceMonitor and ResourceMonitorProcess.

[Jie Yu]

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



Chatted with BenM on this. I think we should just kill the ResourceMonitor and 
move all the logics to the agent. We still need to keep the 
/monitor/statistics.json and /monitor/statistics endpoints for backwards 
compatibility. But since slave properly sets up the delagates, this should just 
work.

- Jie Yu


On March 28, 2016, 11:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45381/
> ---
> 
> (Updated March 28, 2016, 11:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used by ResourceMonitorProcess to collect ContainerStatus
> internally, to avoid dispatching this action back and forth between
> slave process and monitor process.
> 
> 
> Diffs
> -
> 
>   src/slave/monitor.hpp 70a7c88fefd4577f53f85e3eccdd6b69ab6f3acd 
>   src/slave/monitor.cpp 5c1dd354595e67e5eb603c5d49850f84b84b 
>   src/slave/slave.hpp 3ba335fcd31a92af9609023daae328b7f4bf5e59 
>   src/slave/slave.cpp f383605a52f31d7b805ad6153adc409dcb40f83a 
>   src/tests/containerizer.hpp efc1ca87c00b04d70efd1e3f6acf4e763132d6b0 
>   src/tests/containerizer.cpp 4c7f5a26d38222daf013bd3ca9a133e12d7bf338 
>   src/tests/monitor_tests.cpp 5dcb2481ff2f1a7caf54036bc3e60c78feb982b1 
> 
> Diff: https://reviews.apache.org/r/45381/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> https://issues.apache.org/jira/browse/MESOS-4891
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Review Request 45403: Fixed build breakage with CentOS 7.

[Neil Conway]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed build breakage with CentOS 7.


Diffs
-

  src/tests/persistent_volume_tests.cpp 
46ac919d9e795b575a31b21dd8613f38f33591f2 

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


Testing
---

Compiled tests successfully on CentOS 7 and OSX.


Thanks,

Neil Conway

Review Request 45404: Cleaned up a test case in stout.

[Neil Conway]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Cleaned up a test case in stout.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 

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


Testing
---

Compiled tests on CentOS 7 and OSX.


Thanks,

Neil Conway

Re: Review Request 45008: Moved command scheduler to use the scheduler library.

[Anand Mazumdar]

> On March 28, 2016, 9:39 p.m., Vinod Kone wrote:
> > src/cli/execute.cpp, lines 245-266
> > 
> >
> > The semantics of stop() are a bit hard to intuit.
> > 
> > How about we have two methods 
> >  - `error()` that handles error cases (logs error and exits)
> >  - `stop()` that exits cleanly by calling terminate
> >  - 
> > 
> > Also not sure if TEARDOWN call is strictly necessary? Master is going 
> > to clean it up anyway once the socket breaks?

Thanks for the suggestion.

As per our offline discussion, removed `stop()` completely.


> On March 28, 2016, 9:39 p.m., Vinod Kone wrote:
> > src/cli/execute.cpp, line 418
> > 
> >
> > I think it's probably worth logging the FAILURE event.

The previous version did not use to log anything upon an executor/agent 
failure. https://github.com/apache/mesos/blob/master/src/cli/execute.cpp#L301

That said, I am happy to add it in a subsequent review if you feel strongly 
about it. I am dropping this for now. Feel free to reopen.


- Anand


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


On March 18, 2016, 3:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45008/
> ---
> 
> (Updated March 18, 2016, 3:27 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Vinod Kone.
> 
> 
> Bugs: MESOS-3559
> https://issues.apache.org/jira/browse/MESOS-3559
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change moves the command scheduler (`mesos-execute`) to use
> the scheduler library instead of the driver.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/45008/diff/
> 
> 
> Testing
> ---
> 
> make check, an example run:
> 
> ```
> build git:(mesos-3559) : ./src/mesos-execute --command="sleep 1" 
> --master=127.0.1.1:5050 --name=task1
> I0317 19:04:43.974733 21678 scheduler.cpp:172] Version: 0.29.0
> Subscribed with ID '7decbfe9-8b46-48f5-8573-0a5a3e465a1f-0001
> task task1 submitted to agent 5a54dc00-7fd8-477f-a666-1b1ab86a0033-S0
> Received status update TASK_RUNNING for task task1
> Received status update TASK_FINISHED for task task1
> E0317 19:04:45.148203 21680 scheduler.cpp:585] End-Of-File received from 
> master. The master closed the event stream
> 
> build git:(mesos-3559) : echo $?
> 0
> ```
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45008: Moved command scheduler to use the scheduler library.

[Anand Mazumdar]

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

(Updated March 28, 2016, 10:32 p.m.)


Review request for mesos, Guangya Liu and Vinod Kone.


Changes
---

Review comments from Vinod


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


Repository: mesos


Description
---

This change moves the command scheduler (`mesos-execute`) to use
the scheduler library instead of the driver.


Diffs (updated)
-

  src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 

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


Testing
---

make check, an example run:

```
build git:(mesos-3559) : ./src/mesos-execute --command="sleep 1" 
--master=127.0.1.1:5050 --name=task1
I0317 19:04:43.974733 21678 scheduler.cpp:172] Version: 0.29.0
Subscribed with ID '7decbfe9-8b46-48f5-8573-0a5a3e465a1f-0001
task task1 submitted to agent 5a54dc00-7fd8-477f-a666-1b1ab86a0033-S0
Received status update TASK_RUNNING for task task1
Received status update TASK_FINISHED for task task1
E0317 19:04:45.148203 21680 scheduler.cpp:585] End-Of-File received from 
master. The master closed the event stream

build git:(mesos-3559) : echo $?
0
```


Thanks,

Anand Mazumdar

Re: Review Request 45342: Make the Action enum optional to support upgrades (MESOS-5031).

[Adam B]

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



Let's change the UNREACHABLE to an error+unauthorized.


src/authorizer/local/authorizer.cpp (line 205)


On second thought, this case is NOT unreachable, and we should error and 
fail the request rather than aborting the process.


- Adam B


On March 25, 2016, 1:13 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45342/
> ---
> 
> (Updated March 25, 2016, 1:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix tries to make the Action enum in Authorization optional
> to support upgrades. See MESOS-4997 for details.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 944a493e0979c7ffbd99f3a67785a10425fd9040 
>   src/authorizer/local/authorizer.cpp 
> 0f0d9276337858984f0b19a82ffca74ee84dc650 
> 
> Diff: https://reviews.apache.org/r/45342/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45400: Adapted port_mapping with missing subprocess parameter.

[Cong Wang]

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


Ship it!




Ship It!

- Cong Wang


On March 28, 2016, 9:41 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45400/
> ---
> 
> (Updated March 28, 2016, 9:41 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adapted port_mapping with missing subprocess parameter.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 134b6c759b769cf335539e49eff817973c7f96a4 
>   src/tests/containerizer/port_mapping_tests.cpp 
> de4b6f99f3a994bcedafa801eed9c4a7b79bac23 
> 
> Diff: https://reviews.apache.org/r/45400/diff/
> 
> 
> Testing
> ---
> 
> sudo make check on with port_isolator enabled.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

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

[Vinod Kone]

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



Can you set the "depends on" field correctly for this and any other reviews?

- Vinod Kone


On March 28, 2016, 2:21 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44378/
> ---
> 
> (Updated March 28, 2016, 2:21 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4803
> https://issues.apache.org/jira/browse/MESOS-4803
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade libev to 4.22 to support PowerPC LE platform [libprocess].
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 6b07aefc58a1daa235b35e83832e47d1878e2e94 
>   3rdparty/libprocess/3rdparty/libev-4.15.patch 
> bbd83e6928e6caba3bc5a9739823d60923cfaa2a 
>   3rdparty/libprocess/3rdparty/libev-4.15.tar.gz 
> 4c282b573aa9331fd16197ef286faf323b6515eb 
>   3rdparty/libprocess/3rdparty/libev-4.22.patch PRE-CREATION 
>   3rdparty/libprocess/3rdparty/libev-4.22.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
> 
> Diff: https://reviews.apache.org/r/44378/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Greg Mann]

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

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


Review request for mesos and Vinod Kone.


Changes
---

attached bug id. -- @vinodkone


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


Repository: mesos


Description
---

Changed name of http-parser enum to 'flags_enum'.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
04a4cf4241b8188c093f4942d27b050fa7b20397 
  3rdparty/libprocess/3rdparty/Makefile.am 
385302a491aba5a38ff74c0debfc2a423b0c5a8a 
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 

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


Testing
---

`../configure --enable-libevent --enable-ssl && make check` was used to text on 
OSX.


Thanks,

Greg Mann

Re: Review Request 45230: Refactored subproces setup functions [1/7].

[Jörg Schad]

Only '../configure --with-network-isolator' which unfortunately wasn't the
case on neither of the system..
Sorry for the inconvenience, fix is just being tested.

On Mon, Mar 28, 2016 at 7:15 PM, Cong Wang  wrote:

> I am wondering how does it pass your test since you did `make check`
> on both Linux and OSX? It fails immediately for me on Linux...
>
>
> On Mon, Mar 28, 2016 at 10:09 AM, Joris Van Remoortere
>  wrote:
> > Joerg will fix these.
> > Thanks!
> >
> > On Mon, Mar 28, 2016 at 7:06 PM, Cong Wang 
> wrote:
> >
> >> This is an automatically generated e-mail. To reply, visit:
> >> https://reviews.apache.org/r/45230/
> >> 3rdparty/libprocess/include/process/subprocess.hpp
> >> <
> https://reviews.apache.org/r/45230/diff/2/?file=1316622#file1316622line311>
> (Diff
> >> revision 2)
> >>
> >> public:
> >>
> >> 301
> >>
> >> const Setsid setsid = NO_SETSID,
> >>
> >> This one breaks build even after all of your 7 patches are committed,
> because port_mapping.cpp passes flags but no setsid to subprocess(), this
> is not allowed by C++.
> >>
> >>
> >> 3rdparty/libprocess/include/process/subprocess.hpp
> >> <
> https://reviews.apache.org/r/45230/diff/2/?file=1316622#file1316622line356>
> (Diff
> >> revision 2)
> >>
> >> public:
> >>
> >> 342
> >>
> >> const Setsid setsid = NO_SETSID,
> >>
> >> Ditto
> >>
> >>
> >> - Cong Wang
> >>
> >> On March 28th, 2016, 4:51 p.m. UTC, Joerg Schad wrote:
> >> Review request for mesos and Joris Van Remoortere.
> >> By Joerg Schad.
> >>
> >> *Updated March 28, 2016, 4:51 p.m.*
> >> *Bugs: * MESOS-5049 
> >> *Repository: * mesos
> >> Description
> >>
> >> Executing arbitrary setup functions while creating new processes is
> >> dangerous as all functions called have to be async safe. As setup
> >> functions are used for only very few purposes (setsid, chdir, monitoring
> >> and killing a process (see upcoming review) it makes sense to support
> >> them safely via parameters to subprocess. Note this review by itself
> >> -without the following ones- removing the uses of the old interface will
> >> break the build.
> >>
> >> Testing
> >>
> >> tested entire chain (see https://reviews.apache.org/r/45236/).
> >>
> >> Diffs
> >>
> >>- 3rdparty/libprocess/include/process/ssl/gtest.hpp
> >>(2ca705524c8f9bba3c03eef296dc04a353dd236c)
> >>- 3rdparty/libprocess/include/process/subprocess.hpp
> >>(e0c306aa5cf5f393abb73768bbd287c45730f076)
> >>- 3rdparty/libprocess/src/subprocess.cpp
> >>(b99bad04f7251169df3bfcec5dee459977440997)
> >>
> >> View Diff 
> >>
>

Re: Review Request 45398: Added CHANGELOG for python module changes.

[Vinod Kone]

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



Can you also update the release guide 
(https://github.com/apache/mesos/blob/0af8bd6fe28e3cad82d7e05ae3e308c3b2febf29/docs/release-guide.md)
 regarding how to upload these new packages to pypi?


CHANGELOG (line 26)


How about

// This change also removes un-necessary 3rd party dependencies from 
`mesos.executor` and `mesos.scheduler`.



CHANGELOG (line 27)


s/man fewer/fewer/

s/.  /. / <- extraneous space here

s/cominding/combining/



CHANGELOG (line 28)


s/modules/modules,/


- Vinod Kone


On March 28, 2016, 9:13 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45398/
> ---
> 
> (Updated March 28, 2016, 9:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added CHANGELOG for python module changes.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 4c73a2638074ac9f11de1438a5dd3d70e8201229 
> 
> Diff: https://reviews.apache.org/r/45398/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Vinod Kone]

> On March 28, 2016, 9:10 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch, line 10
> > 
> >
> > Do you know why the non-ssl build didn't have a problem with this 
> > conflict?
> > 
> > Also, the real fix is for stout to namespace its flags (e.g., 
> > stout::flags) implementation instead of fixing the library. But I don't 
> > know how big of a change is that.
> 
> Greg Mann wrote:
> It's due to the order of includes. The name collision occurs when 
> `3rdparty/libprocess/src/process.cpp` includes `openssl.hpp` - this leads to 
> a chain of includes that produces the collision. If SSL is not enabled, 
> `3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp` doesn't 
> get included in the same scope as `http_parser.h`. If I add `#include 
> ` to `process.cpp`, the same collision occurs when compiling.
> 
> Yep, I agree that stout should namespace its flags. It looks like that 
> header is included in 43 files, so we would need to add appropriate `using` 
> declarations in those files; not too bad.
> 
> It seems like it might be a good idea to also namespace all of our 3rd 
> party libraries, i.e. that enum could become `3rdparty::http-parser::flags`.

Since this is blocking the CI, lets commit this for now. Can you create a 
ticket to namespace stout flags?


- Vinod


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


On March 28, 2016, 9:18 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45397/
> ---
> 
> (Updated March 28, 2016, 9:18 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed name of http-parser enum to 'flags_enum'.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 04a4cf4241b8188c093f4942d27b050fa7b20397 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 385302a491aba5a38ff74c0debfc2a423b0c5a8a 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45397/diff/
> 
> 
> Testing
> ---
> 
> `../configure --enable-libevent --enable-ssl && make check` was used to text 
> on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 45401: Fixed typo in subprocess doxygen comments.

[Joerg Schad]

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

Review request for mesos.


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


Repository: mesos


Description
---

Fixed typo in subprocess doxygen comments.


Diffs
-

  3rdparty/libprocess/include/process/subprocess.hpp 
da806640a238e168bd24ea837cbb711041fe1d12 

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


Testing
---


Thanks,

Joerg Schad

Review Request 45399: Fixed capitalization of Watchdog enum.

[Joerg Schad]

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

Review request for mesos.


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


Repository: mesos


Description
---

Fixed capitalization of Watchdog enum.


Diffs
-

  3rdparty/libprocess/include/process/subprocess.hpp 
da806640a238e168bd24ea837cbb711041fe1d12 
  3rdparty/libprocess/src/subprocess.cpp 
be32962e94b605ee2e15d35010acd05d58c2b2d3 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Review Request 45400: Adapted port_mapping with missing subprocess parameter.

[Joerg Schad]

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

Review request for mesos.


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


Repository: mesos


Description
---

Adapted port_mapping with missing subprocess parameter.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
134b6c759b769cf335539e49eff817973c7f96a4 
  src/tests/containerizer/port_mapping_tests.cpp 
de4b6f99f3a994bcedafa801eed9c4a7b79bac23 

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


Testing
---

sudo make check on with port_isolator enabled.


Thanks,

Joerg Schad

Re: Review Request 45230: Refactored subproces setup functions [1/7].

[Joerg Schad]

> On March 28, 2016, 5:06 p.m., Cong Wang wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 311
> > 
> >
> > This one breaks build even after all of your 7 patches are committed, 
> > because port_mapping.cpp passes flags but no setsid to subprocess(), this 
> > is not allowed by C++.
> 
> Joerg Schad wrote:
> Thx for catching: Fixed with https://reviews.apache.org/r/45400/

Actually: https://reviews.apache.org/r/45399/


- Joerg


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


On March 28, 2016, 4:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45230/
> ---
> 
> (Updated March 28, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executing arbitrary setup functions while creating new processes is
> dangerous as all functions called have to be async safe. As setup
> functions are used for only very few purposes (setsid, chdir, monitoring
> and killing a process (see upcoming review) it makes sense to support
> them safely via parameters to subprocess. Note this review by itself
> -without the following ones- removing the uses of the old interface will
> break the build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 2ca705524c8f9bba3c03eef296dc04a353dd236c 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> e0c306aa5cf5f393abb73768bbd287c45730f076 
>   3rdparty/libprocess/src/subprocess.cpp 
> b99bad04f7251169df3bfcec5dee459977440997 
> 
> Diff: https://reviews.apache.org/r/45230/diff/
> 
> 
> Testing
> ---
> 
> tested entire chain (see https://reviews.apache.org/r/45236/).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45008: Moved command scheduler to use the scheduler library.

[Vinod Kone]

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




src/cli/execute.cpp (lines 45 - 47)


reorder?



src/cli/execute.cpp (lines 242 - 263)


The semantics of stop() are a bit hard to intuit.

How about we have two methods 
 - `error()` that handles error cases (logs error and exits)
 - `stop()` that exits cleanly by calling terminate
 - 

Also not sure if TEARDOWN call is strictly necessary? Master is going to 
clean it up anyway once the socket breaks?



src/cli/execute.cpp (line 412)


I think it's probably worth logging the FAILURE event.


- Vinod Kone


On March 18, 2016, 3:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45008/
> ---
> 
> (Updated March 18, 2016, 3:27 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Vinod Kone.
> 
> 
> Bugs: MESOS-3559
> https://issues.apache.org/jira/browse/MESOS-3559
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change moves the command scheduler (`mesos-execute`) to use
> the scheduler library instead of the driver.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/45008/diff/
> 
> 
> Testing
> ---
> 
> make check, an example run:
> 
> ```
> build git:(mesos-3559) : ./src/mesos-execute --command="sleep 1" 
> --master=127.0.1.1:5050 --name=task1
> I0317 19:04:43.974733 21678 scheduler.cpp:172] Version: 0.29.0
> Subscribed with ID '7decbfe9-8b46-48f5-8573-0a5a3e465a1f-0001
> task task1 submitted to agent 5a54dc00-7fd8-477f-a666-1b1ab86a0033-S0
> Received status update TASK_RUNNING for task task1
> Received status update TASK_FINISHED for task task1
> E0317 19:04:45.148203 21680 scheduler.cpp:585] End-Of-File received from 
> master. The master closed the event stream
> 
> build git:(mesos-3559) : echo $?
> 0
> ```
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45232: Introduced subprocess watchdog option [3/7].

[Joerg Schad]

> On March 28, 2016, 5 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 46
> > 
> >
> > When did we start making 'enum' names be in caps? There is an enum 7 
> > lines above that is not in all caps.

Thx! Fixed that with this review: https://reviews.apache.org/r/45400/


- Joerg


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


On March 28, 2016, 4:52 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45232/
> ---
> 
> (Updated March 28, 2016, 4:52 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some newly created processes such as perf should be killed in case the
> parent dies. Currently this is achieved by forking a new process from the
> child process which serves as a 'watchdog' and kill the child if the parent
> dies. This review introduces this as a general behavior into subprocess
> (and hence removes the need for the custom setup function).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> e0c306aa5cf5f393abb73768bbd287c45730f076 
>   3rdparty/libprocess/src/subprocess.cpp 
> b99bad04f7251169df3bfcec5dee459977440997 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 09e46eb1ce10a6c38cc364077f0b5952579d46e7 
> 
> Diff: https://reviews.apache.org/r/45232/diff/
> 
> 
> Testing
> ---
> 
> Tested complete chain (see https://reviews.apache.org/r/45236/).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45230: Refactored subproces setup functions [1/7].

[Joerg Schad]

> On March 28, 2016, 5:06 p.m., Cong Wang wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 311
> > 
> >
> > This one breaks build even after all of your 7 patches are committed, 
> > because port_mapping.cpp passes flags but no setsid to subprocess(), this 
> > is not allowed by C++.

Thx for catching: Fixed with https://reviews.apache.org/r/45400/


- Joerg


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


On March 28, 2016, 4:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45230/
> ---
> 
> (Updated March 28, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executing arbitrary setup functions while creating new processes is
> dangerous as all functions called have to be async safe. As setup
> functions are used for only very few purposes (setsid, chdir, monitoring
> and killing a process (see upcoming review) it makes sense to support
> them safely via parameters to subprocess. Note this review by itself
> -without the following ones- removing the uses of the old interface will
> break the build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 2ca705524c8f9bba3c03eef296dc04a353dd236c 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> e0c306aa5cf5f393abb73768bbd287c45730f076 
>   3rdparty/libprocess/src/subprocess.cpp 
> b99bad04f7251169df3bfcec5dee459977440997 
> 
> Diff: https://reviews.apache.org/r/45230/diff/
> 
> 
> Testing
> ---
> 
> tested entire chain (see https://reviews.apache.org/r/45236/).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Greg Mann]

> On March 28, 2016, 9:10 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch, line 10
> > 
> >
> > Do you know why the non-ssl build didn't have a problem with this 
> > conflict?
> > 
> > Also, the real fix is for stout to namespace its flags (e.g., 
> > stout::flags) implementation instead of fixing the library. But I don't 
> > know how big of a change is that.

It's due to the order of includes. The name collision occurs when 
`3rdparty/libprocess/src/process.cpp` includes `openssl.hpp` - this leads to a 
chain of includes that produces the collision. If SSL is not enabled, 
`3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp` doesn't get 
included in the same scope as `http_parser.h`. If I add `#include 
` to `process.cpp`, the same collision occurs when compiling.

Yep, I agree that stout should namespace its flags. It looks like that header 
is included in 43 files, so we would need to add appropriate `using` 
declarations in those files; not too bad.

It seems like it might be a good idea to also namespace all of our 3rd party 
libraries, i.e. that enum could become `3rdparty::http-parser::flags`.


- Greg


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


On March 28, 2016, 9:18 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45397/
> ---
> 
> (Updated March 28, 2016, 9:18 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed name of http-parser enum to 'flags_enum'.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 04a4cf4241b8188c093f4942d27b050fa7b20397 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 385302a491aba5a38ff74c0debfc2a423b0c5a8a 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45397/diff/
> 
> 
> Testing
> ---
> 
> `../configure --enable-libevent --enable-ssl && make check` was used to text 
> on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44843: Replace NULL with nullptr.

[Tomasz Janiszewski]

> On March 26, 2016, 7:10 p.m., Michael Park wrote:
> > src/exec/exec.cpp, lines 637-638
> > 
> >
> > This is clearly beyond just a `s/NULL/nullptr`. I assume this is 
> > `clang-tidy` magic, right?

Exactly


- Tomasz


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


On March 28, 2016, 8:42 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44843/
> ---
> 
> (Updated March 28, 2016, 8:42 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3243
> https://issues.apache.org/jira/browse/MESOS-3243
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RUN: find . -name "*.[hc]pp" | xargs -P 4 sed -i 's/\bNULL\b/nullptr/g'
> 
> 
> Diffs
> -
> 
>   include/mesos/module.hpp 30a9674686221d25132368e6c10664daa9cd6dc0 
>   src/authentication/cram_md5/authenticatee.cpp 
> 7d4994333428e5d8aa9ce6b85e0a7f42a0ddc4c8 
>   src/authentication/cram_md5/authenticator.cpp 
> 027eba4f433fff328d68c41005bc59c41c7ae668 
>   src/authentication/cram_md5/auxprop.cpp 
> d82d2d2f7793859772d89cc91bee09240624c613 
>   src/examples/test_hook_module.cpp abd132b3f39265683542a9d1533d2a31bd81769a 
>   src/examples/test_isolator_module.cpp 
> a4a2103b1e449837b95948c8b5c25e05c5d13860 
>   src/exec/exec.cpp 8f672602daf090dec032d2b684e407e5d043af9c 
>   src/files/files.hpp 90acb3406c46c164108deb559af71fb109a5773b 
>   src/java/jni/construct.cpp 0bfe6291e94a69a787c965fa3a3a90d2ebae8d72 
>   src/java/jni/org_apache_mesos_Log.cpp 
> 140b950136417eed7cba363a89537ed2f33832ff 
>   src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp 
> 9a92ade5d54bd814353acb40170d7aa4d8fe4a77 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> c723e2d7bce98e6ac5ab587ce8c42fa9ba8b47e4 
>   src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
> 23a89c3e5c5309a8a9b45c168fb61b12d89db1ec 
>   src/jvm/jvm.hpp d5023e4050d109ca97cedb05f85e1c24202ac3b0 
>   src/jvm/jvm.cpp 779f8b987e2673ca5ab924caf00eea09a0a79af5 
>   src/jvm/org/apache/zookeeper.hpp 7e9c3aaa9f0eed44994004f9e32ce7ce3cf6d335 
>   src/launcher/executor.cpp 4e9b4d9820f7c2f4cb1b3e16e2f4c8c13500693f 
>   src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c 
>   src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
>   src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
>   src/linux/perf.cpp 749e676aaf2ce639dd976f2b23e323300c6114c5 
>   src/linux/routing/diagnosis/diagnosis.cpp 
> 8b86a8864c08b078425dcc242d323763d6ec15dd 
>   src/linux/routing/filter/internal.hpp 
> 8690bf56472c318e85afb53d35494790ae4da27b 
>   src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
>   src/linux/routing/link/internal.hpp 
> 8f05e5f513208c8f966bc324c9fe994a5807b051 
>   src/linux/routing/queueing/internal.hpp 
> 768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 
>   src/linux/routing/route.cpp 4b33350dc3761e19121c29c0d090cfdae544e4ef 
>   src/linux/systemd.cpp 9f6e06cfcf6f5b38971ff75eb85326d043140b4b 
>   src/local/local.hpp f4ae285edc30a0fb1c960d50dfb1a859b2eae166 
>   src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
>   src/log/leveldb.cpp ba2f62bc97e4a0eaa8a54824551686467db4b9e4 
>   src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
>   src/log/tool/benchmark.hpp 3051b6ff025252d2c770b0a70d6a66f4f34d7e69 
>   src/log/tool/initialize.hpp 99c80061442c6379978f907c80a7e9d54a015f04 
>   src/log/tool/read.hpp 937ae8f15f5e2227247ab43aa24c887c13c3cf2c 
>   src/log/tool/replica.hpp c25bb09abc4391614c7a35b9ca91d678a745ffdd 
>   src/logging/logging.cpp 20d2f6341bd39fc5d056f1046d258d006fc602e4 
>   src/master/allocator/mesos/hierarchical.hpp 
> e979fdf60da1409d1c2d08f0e9f03cef067506dd 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/http.cpp 03d4ebabfba1bf711fc0897801a989ac3c72f9f1 
>   src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
>   src/master/master.cpp 655b0b8d0156b44914578a1241beb1fb7c92ca23 
>   src/master/validation.cpp 9c9e42283baa6e49d86af2ce7222131ce53ccaff 
>   src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
>   src/module/manager.cpp 8c9aaf7cd00c904daba9994a99df9e1329831c01 
>   src/python/executor/src/mesos/executor/mesos_executor_driver_impl.cpp 
> 843771ae5b4f7e283d85c093e63c235dc753397d 
>   src/python/executor/src/mesos/executor/module.cpp 
> f8c63821db616475b95c3d1687893c251fa2daca 
>   src/python/executor/src/mesos/executor/proxy_executor.cpp 
> b9c8a2d131e5aadf6fa79af023bb34ae5a3cebba 
>   src/python/native_common/common.hpp 
> 166adb32978260750898537e8f45e4f3bbd19c79 
>   

Re: Review Request 45006: Added `devolve` overloads for `v1::TaskStatus`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 18, 2016, 3:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45006/
> ---
> 
> (Updated March 18, 2016, 3:27 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3559
> https://issues.apache.org/jira/browse/MESOS-3559
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp cf5aad45fe3a1ca17161ec27e68884a93934cc42 
>   src/internal/devolve.cpp 27918f1fc385b1770843697c16a29fd0d376f39d 
> 
> Diff: https://reviews.apache.org/r/45006/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Greg Mann]

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

(Updated March 28, 2016, 9:18 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Addressed comment.


Repository: mesos


Description
---

Changed name of http-parser enum to 'flags_enum'.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
04a4cf4241b8188c093f4942d27b050fa7b20397 
  3rdparty/libprocess/3rdparty/Makefile.am 
385302a491aba5a38ff74c0debfc2a423b0c5a8a 
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 

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


Testing
---

`../configure --enable-libevent --enable-ssl && make check` was used to text on 
OSX.


Thanks,

Greg Mann

Re: Review Request 45013: Removed un-needed methods from command scheduler.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 18, 2016, 3:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45013/
> ---
> 
> (Updated March 18, 2016, 3:27 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3559
> https://issues.apache.org/jira/browse/MESOS-3559
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change cleans up the unused scheduler driver methods
> that would not be needed after moving to the scheduler library.
> 
> This review mainly eases reviewing later in the chain and can't
> be committed separately.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/45013/diff/
> 
> 
> Testing
> ---
> 
> This change should not be committed with the review after it.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45007: Fixed `using` declaration order in `mesos-execute`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 18, 2016, 3:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45007/
> ---
> 
> (Updated March 18, 2016, 3:27 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3559
> https://issues.apache.org/jira/browse/MESOS-3559
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed `using` declaration order in `mesos-execute`.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/45007/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 45398: Added CHANGELOG for python module changes.

[Steve Niemitz]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Added CHANGELOG for python module changes.


Diffs
-

  CHANGELOG 4c73a2638074ac9f11de1438a5dd3d70e8201229 

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


Testing
---


Thanks,

Steve Niemitz

Re: Review Request 45005: Fixed minor spacing issue in the example test framework.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 18, 2016, 3:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45005/
> ---
> 
> (Updated March 18, 2016, 3:27 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/test_http_framework.cpp 
> 64c509717b996085ff029c7bfbf0f1a914a29f80 
> 
> Diff: https://reviews.apache.org/r/45005/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45004: Fixed a memory leak in the scheduler driver.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 18, 2016, 3:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45004/
> ---
> 
> (Updated March 18, 2016, 3:27 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We were not freeing up the heap allocated `Credentials` object.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
> 
> Diff: https://reviews.apache.org/r/45004/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Vinod Kone]

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




3rdparty/libprocess/3rdparty/Makefile.am (line 61)


Update the comment.



3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch (line 10)


Do you know why the non-ssl build didn't have a problem with this conflict?

Also, the real fix is for stout to namespace its flags (e.g., stout::flags) 
implementation instead of fixing the library. But I don't know how big of a 
change is that.


- Vinod Kone


On March 28, 2016, 9:08 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45397/
> ---
> 
> (Updated March 28, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed name of http-parser enum to 'flags_enum'.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 04a4cf4241b8188c093f4942d27b050fa7b20397 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 385302a491aba5a38ff74c0debfc2a423b0c5a8a 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45397/diff/
> 
> 
> Testing
> ---
> 
> `../configure --enable-libevent --enable-ssl && make check` was used to text 
> on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Greg Mann]

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

(Updated March 28, 2016, 9:08 p.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Changed name of http-parser enum to 'flags_enum'.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
04a4cf4241b8188c093f4942d27b050fa7b20397 
  3rdparty/libprocess/3rdparty/Makefile.am 
385302a491aba5a38ff74c0debfc2a423b0c5a8a 
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 

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


Testing
---

`../configure --enable-libevent --enable-ssl && make check` was used to text on 
OSX.


Thanks,

Greg Mann

Review Request 45397: Changed name of http-parser enum to 'flags_enum'.

[Greg Mann]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Changed name of http-parser enum to 'flags_enum'.


Diffs
-

  3rdparty/libprocess/3rdparty/Makefile.am 
385302a491aba5a38ff74c0debfc2a423b0c5a8a 
  3rdparty/libprocess/3rdparty/http-parser-2.6.1.patch PRE-CREATION 

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


Testing
---

`../configure --enable-libevent --enable-ssl && make check` was used to text on 
OSX.


Thanks,

Greg Mann

Re: Review Request 45313: Introduced `WindowsSocketError` and refactored out `WindowsErrorBase`.

[Michael Park]

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

(Updated March 28, 2016, 8:57 p.m.)


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


Changes
---

Addressed Daniel's comments.


Summary (updated)
-

Introduced `WindowsSocketError` and refactored out `WindowsErrorBase`.


Repository: mesos


Description (updated)
---

Introduced `WindowsSocketError` and refactored out `WindowsErrorBase`.


Diffs (updated)
-

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

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 44843: Replace NULL with nullptr.

[Tomasz Janiszewski]

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

(Updated March 28, 2016, 8:42 p.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description (updated)
---

RUN: find . -name "*.[hc]pp" | xargs -P 4 sed -i 's/\bNULL\b/nullptr/g'


Diffs (updated)
-

  include/mesos/module.hpp 30a9674686221d25132368e6c10664daa9cd6dc0 
  src/authentication/cram_md5/authenticatee.cpp 
7d4994333428e5d8aa9ce6b85e0a7f42a0ddc4c8 
  src/authentication/cram_md5/authenticator.cpp 
027eba4f433fff328d68c41005bc59c41c7ae668 
  src/authentication/cram_md5/auxprop.cpp 
d82d2d2f7793859772d89cc91bee09240624c613 
  src/examples/test_hook_module.cpp abd132b3f39265683542a9d1533d2a31bd81769a 
  src/examples/test_isolator_module.cpp 
a4a2103b1e449837b95948c8b5c25e05c5d13860 
  src/exec/exec.cpp 8f672602daf090dec032d2b684e407e5d043af9c 
  src/files/files.hpp 90acb3406c46c164108deb559af71fb109a5773b 
  src/java/jni/construct.cpp 0bfe6291e94a69a787c965fa3a3a90d2ebae8d72 
  src/java/jni/org_apache_mesos_Log.cpp 
140b950136417eed7cba363a89537ed2f33832ff 
  src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp 
9a92ade5d54bd814353acb40170d7aa4d8fe4a77 
  src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
c723e2d7bce98e6ac5ab587ce8c42fa9ba8b47e4 
  src/java/jni/org_apache_mesos_state_ZooKeeperState.cpp 
23a89c3e5c5309a8a9b45c168fb61b12d89db1ec 
  src/jvm/jvm.hpp d5023e4050d109ca97cedb05f85e1c24202ac3b0 
  src/jvm/jvm.cpp 779f8b987e2673ca5ab924caf00eea09a0a79af5 
  src/jvm/org/apache/zookeeper.hpp 7e9c3aaa9f0eed44994004f9e32ce7ce3cf6d335 
  src/launcher/executor.cpp 4e9b4d9820f7c2f4cb1b3e16e2f4c8c13500693f 
  src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c 
  src/linux/fs.hpp 4525a5d5566e2bc913894e993ac7350f1bbd9cc0 
  src/linux/fs.cpp dbf94759b6382bfafd3f3b8b4c2047af36a53ad5 
  src/linux/perf.cpp 749e676aaf2ce639dd976f2b23e323300c6114c5 
  src/linux/routing/diagnosis/diagnosis.cpp 
8b86a8864c08b078425dcc242d323763d6ec15dd 
  src/linux/routing/filter/internal.hpp 
8690bf56472c318e85afb53d35494790ae4da27b 
  src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
  src/linux/routing/link/internal.hpp 8f05e5f513208c8f966bc324c9fe994a5807b051 
  src/linux/routing/queueing/internal.hpp 
768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 
  src/linux/routing/route.cpp 4b33350dc3761e19121c29c0d090cfdae544e4ef 
  src/linux/systemd.cpp 9f6e06cfcf6f5b38971ff75eb85326d043140b4b 
  src/local/local.hpp f4ae285edc30a0fb1c960d50dfb1a859b2eae166 
  src/local/local.cpp e777ea2938a23db8b407676a0f7e635e63d032fa 
  src/log/leveldb.cpp ba2f62bc97e4a0eaa8a54824551686467db4b9e4 
  src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
  src/log/tool/benchmark.hpp 3051b6ff025252d2c770b0a70d6a66f4f34d7e69 
  src/log/tool/initialize.hpp 99c80061442c6379978f907c80a7e9d54a015f04 
  src/log/tool/read.hpp 937ae8f15f5e2227247ab43aa24c887c13c3cf2c 
  src/log/tool/replica.hpp c25bb09abc4391614c7a35b9ca91d678a745ffdd 
  src/logging/logging.cpp 20d2f6341bd39fc5d056f1046d258d006fc602e4 
  src/master/allocator/mesos/hierarchical.hpp 
e979fdf60da1409d1c2d08f0e9f03cef067506dd 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/http.cpp 03d4ebabfba1bf711fc0897801a989ac3c72f9f1 
  src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
  src/master/master.cpp 655b0b8d0156b44914578a1241beb1fb7c92ca23 
  src/master/validation.cpp 9c9e42283baa6e49d86af2ce7222131ce53ccaff 
  src/module/manager.hpp 9944af0daf6c9cb5a8ff338099401b1db88ee237 
  src/module/manager.cpp 8c9aaf7cd00c904daba9994a99df9e1329831c01 
  src/python/executor/src/mesos/executor/mesos_executor_driver_impl.cpp 
843771ae5b4f7e283d85c093e63c235dc753397d 
  src/python/executor/src/mesos/executor/module.cpp 
f8c63821db616475b95c3d1687893c251fa2daca 
  src/python/executor/src/mesos/executor/proxy_executor.cpp 
b9c8a2d131e5aadf6fa79af023bb34ae5a3cebba 
  src/python/native_common/common.hpp 166adb32978260750898537e8f45e4f3bbd19c79 
  src/python/scheduler/src/mesos/scheduler/mesos_scheduler_driver_impl.cpp 
78dc298ec19a61cd491b2b43b463db67528c5526 
  src/python/scheduler/src/mesos/scheduler/module.cpp 
62eaf3166c4ebef42665358e8da438b2e3afba20 
  src/python/scheduler/src/mesos/scheduler/proxy_scheduler.cpp 
3d8f3bfc9cbff761822580310432177d00450f51 
  src/sched/sched.cpp ade6907b299a327a7766f9a2159d61a709357d6e 
  src/scheduler/scheduler.cpp 13972449363b633f21ddec7649b1b170703c773a 
  src/slave/container_logger.cpp 85b8c1d3a42baba14741996d791926816b252742 
  src/slave/container_loggers/lib_logrotate.cpp 
1f228806da32832c9ca1ae4defcd1bdc154adc18 
  src/slave/containerizer/docker.cpp c5007a311ae9c1766dd4522ccbddbdb506d4ae4e 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Maged Michael]

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

(Updated March 28, 2016, 8:28 p.m.)


Review request for mesos, Guangya Liu, Klaus Ma, and Qian Zhang.


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


Repository: mesos


Description
---

Added env var to set upper bound on number of libprocess worker threads.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp feaffa4 
  docs/configuration.md 9ad0c2a 

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


Testing
---

Passed make check on x86_64 Ubuntu 14.04.

Invocations of mesos-master, mesos-slave, and mesos-tests with the 
LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
"-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
The last 3 tests generated warnings.


Thanks,

Maged Michael

Re: Review Request 45311: Added an additional template parameter to 'Try' for typeful error.

[Michael Park]

> On March 26, 2016, 8:07 p.m., Daniel Pravat wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp, line 33
> > 
> >
> > With this default parameter the code has to be changed on every place 
> > where we return or handle a WindowsError object. 
> > 
> > One sugestion is to use different defaults on Windows and Posix and 
> > handle the conversion from ErrnoError/Error to WindowsError inside 
> > WindowsError class.

> With this default parameter the code has to be changed on every place where 
> we return or handle a WindowsError object. 

This is not true. In places that return a `Try`, we only care about the 
error string, not the error code.
Therefore, this is backwards-compatible, non-intrusive, and 
no-worse-than-before, and still allows us to be more specific going forward.
> One sugestion is to use different defaults on Windows and Posix and handle 
> the conversion from ErrnoError/Error to WindowsError inside WindowsError 
> class.

We could take this suggestion within the `os` namespace, but not at this level.
An error code such as `errno` is only present and relevant when we interact 
with the C API, which is encapsulated within the `os` namespace.

Wihtin the `os` namespace we could do something like:

```cpp
namespace os {

  using OsError =
#ifdef __WINDOWS__
WindowsError;
#else
ErrnoError;
#endif

  template 
  using Try = ::Try;

} // namespace os {
```

But again, I think this is future work, and also out of the scope of this 
review.


- Michael


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


On March 24, 2016, 8:23 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45311/
> ---
> 
> (Updated March 24, 2016, 8:23 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
> c444c0118d39ee6a5da4618d7c62784464377280 
> 
> Diff: https://reviews.apache.org/r/45311/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

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

[Michael Park]

> On March 26, 2016, 7:43 p.m., Daniel Pravat wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp, line 56
> > 
> >
> > Without additional context the reviewer may think that this change will 
> > break the build. Maybe you can change the order between this review and the 
> > next or eliminate the constructors in the next commit.

This is ordered this way, because once the `Error` is templated, it might not 
be possible to have ctors that take `ErrnoError` and `WindowsError`.
Simply fixing a hack that was introduced before, to set the table for the 
subsequent changes.


- Michael


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


On March 24, 2016, 8:22 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45310/
> ---
> 
> (Updated March 24, 2016, 8:22 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Correctly constrained the templated constructor.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
> c444c0118d39ee6a5da4618d7c62784464377280 
> 
> Diff: https://reviews.apache.org/r/45310/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 41410: Export CFS metrics for docker containers

[Steve Niemitz]

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

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


Review request for mesos, Ian Downes and Jie Yu.


Changes
---

ping


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


Repository: mesos


Description
---

Export CFS metrics for docker containers


Diffs
-

  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 

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


Testing
---


Thanks,

Steve Niemitz

Re: Review Request 45342: Make the Action enum optional to support upgrades (MESOS-5031).

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 25, 2016, 8:13 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45342/
> ---
> 
> (Updated March 25, 2016, 8:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix tries to make the Action enum in Authorization optional
> to support upgrades. See MESOS-4997 for details.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 944a493e0979c7ffbd99f3a67785a10425fd9040 
>   src/authorizer/local/authorizer.cpp 
> 0f0d9276337858984f0b19a82ffca74ee84dc650 
> 
> Diff: https://reviews.apache.org/r/45342/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45369: Upgrade http-parser to 2.6.1 to support PowerPC LE platform [mesos].

[Vinod Kone]

Hmm. Look like this broke the SSL build

.

@Zhiweil for all these upgrades can you also test libevent based SSL builds
work (./configure --enable-libevent --enable-ssl) in addition to the libev
builds (./configure)?

On Mon, Mar 28, 2016 at 11:37 AM, Vinod Kone  wrote:

>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45369/#review125699
> ---
>
>
> Ship it!
>
>
>
>
> Ship It!
>
> - Vinod Kone
>
>
> On March 28, 2016, 6:36 p.m., Zhiwei Chen wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/45369/
> > ---
> >
> > (Updated March 28, 2016, 6:36 p.m.)
> >
> >
> > Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil
> Conway, and Vinod Kone.
> >
> >
> > Bugs: MESOS-4805
> > https://issues.apache.org/jira/browse/MESOS-4805
> >
> >
> > Repository: mesos
> >
> >
> > Description
> > ---
> >
> > Upgrade http-parser to 2.6.1 to support PowerPC LE platform [mesos].
> >
> >
> > Diffs
> > -
> >
> >   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7
> >   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39
> >
> > Diff: https://reviews.apache.org/r/45369/diff/
> >
> >
> > Testing
> > ---
> >
> > sudo make dist check
> >
> > sudo ./src/mesos-tests --benchmark
> >
> > sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> >
> >
> > Thanks,
> >
> > Zhiwei Chen
> >
> >
>
>

Re: Review Request 45367: Upgrade protobuf to 2.6.1 to support PowerPC LE platform [mesos].

[James Peach]

> On March 28, 2016, 7:13 p.m., James Peach wrote:
> > Do you also need to regenerate 
> > ``3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc``?

Oh, I see it is in [r44257](https://reviews.apache.org/r/44257).


- James


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


On March 28, 2016, 7:05 p.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45367/
> ---
> 
> (Updated March 28, 2016, 7:05 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, James Peach, Kapil Arya, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-4678
> https://issues.apache.org/jira/browse/MESOS-4678
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade protobuf to 2.6.1 to support PowerPC LE platform [mesos].
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
>   configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 
>   src/java/mesos.pom.in 7615d61eb6fedfa0ead785cd360946c56ccf80af 
>   src/python/interface/setup.py.in d73996734c3a3c70c3a6c0c697bb6733c241c091 
>   src/python/native_common/ext_modules.py.in 
> c335bd83024bc07b6243dd59d775e7f29adc7520 
>   src/python/protocol/setup.py.in 4c50fbbf1ce11c4c42c848364523225ee7ea5a3b 
> 
> Diff: https://reviews.apache.org/r/45367/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45367: Upgrade protobuf to 2.6.1 to support PowerPC LE platform [mesos].

[James Peach]

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



Do you also need to regenerate 
``3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc``?

- James Peach


On March 28, 2016, 7:05 p.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45367/
> ---
> 
> (Updated March 28, 2016, 7:05 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, James Peach, Kapil Arya, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-4678
> https://issues.apache.org/jira/browse/MESOS-4678
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade protobuf to 2.6.1 to support PowerPC LE platform [mesos].
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
>   configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 
>   src/java/mesos.pom.in 7615d61eb6fedfa0ead785cd360946c56ccf80af 
>   src/python/interface/setup.py.in d73996734c3a3c70c3a6c0c697bb6733c241c091 
>   src/python/native_common/ext_modules.py.in 
> c335bd83024bc07b6243dd59d775e7f29adc7520 
>   src/python/protocol/setup.py.in 4c50fbbf1ce11c4c42c848364523225ee7ea5a3b 
> 
> Diff: https://reviews.apache.org/r/45367/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45122: Updated tests for deletion of persistent volumes.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 28, 2016, 6:20 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45122/
> ---
> 
> (Updated March 28, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated tests for deletion of persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 26fff19daa8b175fdcc06fd9467224d5920a1967 
> 
> Diff: https://reviews.apache.org/r/45122/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45367: Upgrade protobuf to 2.6.1 to support PowerPC LE platform [mesos].

[Zhiwei Chen]

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

(Updated March 28, 2016, 6:55 p.m.)


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


Changes
---

Fixed "bugs" and "depends on" fields -- @vinodkone


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


Repository: mesos


Description
---

Upgrade protobuf to 2.6.1 to support PowerPC LE platform [mesos].


Diffs
-

  3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
  LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
  configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 
  src/java/mesos.pom.in 7615d61eb6fedfa0ead785cd360946c56ccf80af 
  src/python/interface/setup.py.in d73996734c3a3c70c3a6c0c697bb6733c241c091 
  src/python/native_common/ext_modules.py.in 
c335bd83024bc07b6243dd59d775e7f29adc7520 
  src/python/protocol/setup.py.in 4c50fbbf1ce11c4c42c848364523225ee7ea5a3b 

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


Testing
---

sudo make dist check

sudo ./src/mesos-tests --benchmark

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


Thanks,

Zhiwei Chen

Re: Review Request 45368: Upgrade protobuf to 2.6.1 to support PowerPC LE platform [libprocess].

[Zhiwei Chen]

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

(Updated March 28, 2016, 6:54 p.m.)


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


Changes
---

Fixed "bugs" and "depends on" fields -- @vinodkone


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


Repository: mesos


Description
---

Upgrade protobuf to 2.6.1 to support PowerPC LE platform [libprocess].


Diffs
-

  3rdparty/libprocess/3rdparty/protobuf-2.5.0.tar.gz 
e600ac57be4c88efb5f146e4b3ec226d8f685033 
  3rdparty/libprocess/3rdparty/protobuf-2.6.1.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 
98195b8eb60b2673d610d8ab7ea31103f137debf 

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


Testing
---

sudo make dist check

sudo ./src/mesos-tests --benchmark

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


Thanks,

Zhiwei Chen

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support PPC LE platform [libprocess].

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


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

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support PPC LE platform [libprocess].

[Vinod Kone]

> On March 4, 2016, 7:37 a.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/Makefile.am, line 68
> > 
> >
> > ditto.
> 
> Zhiwei Chen wrote:
> In the real file, there is a table bracket and backslash.
> 
> haosdent huang wrote:
> I see, but need make sure the lines you added should align.

I'll fix this while committing.


- Vinod


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


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

Re: Review Request 45369: Upgrade http-parser to 2.6.1 to support PowerPC LE platform [mesos].

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 28, 2016, 6:36 p.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45369/
> ---
> 
> (Updated March 28, 2016, 6:36 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4805
> https://issues.apache.org/jira/browse/MESOS-4805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade http-parser to 2.6.1 to support PowerPC LE platform [mesos].
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
> 
> Diff: https://reviews.apache.org/r/45369/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45369: Upgrade http-parser to 2.6.1 to support PowerPC LE platform [mesos].

[Zhiwei Chen]

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

(Updated March 28, 2016, 6:36 p.m.)


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


Changes
---

Fixing the "bugs" and "depends on" fields -- @vinodkone

@Zhiwei please make sure you link related reviews by setting "depends on" 
field. That way ReviewBot can test the whole chain.


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


Repository: mesos


Description
---

Upgrade http-parser to 2.6.1 to support PowerPC LE platform [mesos].


Diffs
-

  3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
  LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 

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


Testing
---

sudo make dist check

sudo ./src/mesos-tests --benchmark

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


Thanks,

Zhiwei Chen

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43935]

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

- Mesos ReviewBot


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

Re: Review Request 45122: Updated tests for deletion of persistent volumes.

[Neil Conway]

> On March 26, 2016, 12:53 a.m., Jie Yu wrote:
> > src/tests/persistent_volume_tests.cpp, line 810
> > 
> >
> > What the purpose of adding another file here?
> 
> Neil Conway wrote:
> I wanted to check that if files are directly written into the directory 
> that corresponds to the persistent volume (not via the task itself), they are 
> still cleaned up when the volume is destroyed.
> 
> Jie Yu wrote:
> hum, I still don't understand. What's the purpose of this test? to 
> simulate what situation?
> 
> Neil Conway wrote:
> The situation I wanted to test is that you have a directory `/foo` that 
> is being used by a Mesos task as a persistent volume. Another process on the 
> box writes a file `/foo/bar`, and then the Mesos task terminates and the 
> volume is destroyed. The test checks that `/foo/bar` is removed, even though 
> it wasn't created by the Mesos task.
> 
> Jie Yu wrote:
> I understand what it is doing. I don't understand is: is this a 
> legitimate use case? You mean some out of band process is writing the 
> persistent volume? This does not fit well with the Mesos model, isn't it? I 
> would suggest we don't add those.
> 
> Neil Conway wrote:
> Well, we're already testing whether an out-of-band process can read from 
> the persistent volume (for `filePath1`), so it seemed natural to test writing 
> to it as well -- happy to remove it if you'd prefer though.
> 
> Jie Yu wrote:
> I think the 'read' is to make sure the file written by the task is 
> actually persisted in the persistent volume.

Okay, removed.


- Neil


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


On March 28, 2016, 6:20 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45122/
> ---
> 
> (Updated March 28, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated tests for deletion of persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 26fff19daa8b175fdcc06fd9467224d5920a1967 
> 
> Diff: https://reviews.apache.org/r/45122/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45122: Updated tests for deletion of persistent volumes.

[Neil Conway]

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

(Updated March 28, 2016, 6:20 p.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


Changes
---

Remove writing a file to persistent volume.


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


Repository: mesos


Description
---

Updated tests for deletion of persistent volumes.


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
26fff19daa8b175fdcc06fd9467224d5920a1967 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 45122: Updated tests for deletion of persistent volumes.

[Jie Yu]

> On March 26, 2016, 12:53 a.m., Jie Yu wrote:
> > src/tests/persistent_volume_tests.cpp, line 810
> > 
> >
> > What the purpose of adding another file here?
> 
> Neil Conway wrote:
> I wanted to check that if files are directly written into the directory 
> that corresponds to the persistent volume (not via the task itself), they are 
> still cleaned up when the volume is destroyed.
> 
> Jie Yu wrote:
> hum, I still don't understand. What's the purpose of this test? to 
> simulate what situation?
> 
> Neil Conway wrote:
> The situation I wanted to test is that you have a directory `/foo` that 
> is being used by a Mesos task as a persistent volume. Another process on the 
> box writes a file `/foo/bar`, and then the Mesos task terminates and the 
> volume is destroyed. The test checks that `/foo/bar` is removed, even though 
> it wasn't created by the Mesos task.
> 
> Jie Yu wrote:
> I understand what it is doing. I don't understand is: is this a 
> legitimate use case? You mean some out of band process is writing the 
> persistent volume? This does not fit well with the Mesos model, isn't it? I 
> would suggest we don't add those.
> 
> Neil Conway wrote:
> Well, we're already testing whether an out-of-band process can read from 
> the persistent volume (for `filePath1`), so it seemed natural to test writing 
> to it as well -- happy to remove it if you'd prefer though.

I think the 'read' is to make sure the file written by the task is actually 
persisted in the persistent volume.


- Jie


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


On March 28, 2016, 4:07 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45122/
> ---
> 
> (Updated March 28, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated tests for deletion of persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 26fff19daa8b175fdcc06fd9467224d5920a1967 
> 
> Diff: https://reviews.apache.org/r/45122/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45122: Updated tests for deletion of persistent volumes.

[Neil Conway]

> On March 26, 2016, 12:53 a.m., Jie Yu wrote:
> > src/tests/persistent_volume_tests.cpp, line 810
> > 
> >
> > What the purpose of adding another file here?
> 
> Neil Conway wrote:
> I wanted to check that if files are directly written into the directory 
> that corresponds to the persistent volume (not via the task itself), they are 
> still cleaned up when the volume is destroyed.
> 
> Jie Yu wrote:
> hum, I still don't understand. What's the purpose of this test? to 
> simulate what situation?
> 
> Neil Conway wrote:
> The situation I wanted to test is that you have a directory `/foo` that 
> is being used by a Mesos task as a persistent volume. Another process on the 
> box writes a file `/foo/bar`, and then the Mesos task terminates and the 
> volume is destroyed. The test checks that `/foo/bar` is removed, even though 
> it wasn't created by the Mesos task.
> 
> Jie Yu wrote:
> I understand what it is doing. I don't understand is: is this a 
> legitimate use case? You mean some out of band process is writing the 
> persistent volume? This does not fit well with the Mesos model, isn't it? I 
> would suggest we don't add those.

Well, we're already testing whether an out-of-band process can read from the 
persistent volume (for `filePath1`), so it seemed natural to test writing to it 
as well -- happy to remove it if you'd prefer though.


- Neil


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


On March 28, 2016, 4:07 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45122/
> ---
> 
> (Updated March 28, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated tests for deletion of persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 26fff19daa8b175fdcc06fd9467224d5920a1967 
> 
> Diff: https://reviews.apache.org/r/45122/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

> On March 28, 2016, 3:43 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 554-572
> > 
> >
> > 2 spaces indent, you can take 
> > https://github.com/apache/mesos/blob/master/src/master/http.cpp#L482 as 
> > reference.
> > 
> > I think that we still need update `DockerInfo` by adding `network_name` 
> > and always using `network_name` from `DockerInfo` first; if no 
> > `network_name` in `DockerInfo`, use `name` from `NetworkInfo`
> 
> Ezra Silvera wrote:
> Why? I thought the point was to use NetworkInfo instead of network_name . 
> Filling the NetworkInfo in case of user-defined network should be mandatory.
> 
> Avinash sridharan wrote:
> I agree with Ezra on this. We shouldn't have multiple fields in the 
> protobuf trying to give the same information. The idea of introducing the 
> `name` field was to specify the user-defined network. Duplicating this field 
> in Docker doesn't help.
> 
> Guangya Liu wrote:
> You can take a look at @Jie Yu's comments: I think what I am proposing is 
> that: we add a NetworkInfo.name, and if DOckerInfo.network is not set and 
> NetworkInfo.name is set, the docker containerizer will do 
> --net=.
> 
> I think that keeping a network name in `DockerInfo` is more suitable for 
> Docker containerizer.
> 
> Ezra Silvera wrote:
> You can't use DockerInfo.network because it is an enum that always has a 
> value. 
> This is why we added the network_name and everything worked perfectly 
> E2E, we then removed it because you were pointing/asking to use the new 
> NetworkInfo.
> And if we have that we field why at all we need to check the NetworkInfo 
> for ?   Those are both new fields that are going to be filled by the 
> framework.

Just to clarify the previous comment - as Avinash sridharan said - we need one 
of these fields. Let's just agree which and push this patch :-)


- Ezra


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


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

> On March 28, 2016, 3:43 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 554-572
> > 
> >
> > 2 spaces indent, you can take 
> > https://github.com/apache/mesos/blob/master/src/master/http.cpp#L482 as 
> > reference.
> > 
> > I think that we still need update `DockerInfo` by adding `network_name` 
> > and always using `network_name` from `DockerInfo` first; if no 
> > `network_name` in `DockerInfo`, use `name` from `NetworkInfo`
> 
> Ezra Silvera wrote:
> Why? I thought the point was to use NetworkInfo instead of network_name . 
> Filling the NetworkInfo in case of user-defined network should be mandatory.
> 
> Avinash sridharan wrote:
> I agree with Ezra on this. We shouldn't have multiple fields in the 
> protobuf trying to give the same information. The idea of introducing the 
> `name` field was to specify the user-defined network. Duplicating this field 
> in Docker doesn't help.
> 
> Guangya Liu wrote:
> You can take a look at @Jie Yu's comments: I think what I am proposing is 
> that: we add a NetworkInfo.name, and if DOckerInfo.network is not set and 
> NetworkInfo.name is set, the docker containerizer will do 
> --net=.
> 
> I think that keeping a network name in `DockerInfo` is more suitable for 
> Docker containerizer.

You can't use DockerInfo.network because it is an enum that always has a value. 
This is why we added the network_name and everything worked perfectly E2E, we 
then removed it because you were pointing/asking to use the new NetworkInfo.
And if we have that we field why at all we need to check the NetworkInfo for ?  
 Those are both new fields that are going to be filled by the framework.


- Ezra


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


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 45122: Updated tests for deletion of persistent volumes.

[Jie Yu]

> On March 26, 2016, 12:53 a.m., Jie Yu wrote:
> > src/tests/persistent_volume_tests.cpp, line 810
> > 
> >
> > What the purpose of adding another file here?
> 
> Neil Conway wrote:
> I wanted to check that if files are directly written into the directory 
> that corresponds to the persistent volume (not via the task itself), they are 
> still cleaned up when the volume is destroyed.
> 
> Jie Yu wrote:
> hum, I still don't understand. What's the purpose of this test? to 
> simulate what situation?
> 
> Neil Conway wrote:
> The situation I wanted to test is that you have a directory `/foo` that 
> is being used by a Mesos task as a persistent volume. Another process on the 
> box writes a file `/foo/bar`, and then the Mesos task terminates and the 
> volume is destroyed. The test checks that `/foo/bar` is removed, even though 
> it wasn't created by the Mesos task.

I understand what it is doing. I don't understand is: is this a legitimate use 
case? You mean some out of band process is writing the persistent volume? This 
does not fit well with the Mesos model, isn't it? I would suggest we don't add 
those.


- Jie


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


On March 28, 2016, 4:07 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45122/
> ---
> 
> (Updated March 28, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated tests for deletion of persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 26fff19daa8b175fdcc06fd9467224d5920a1967 
> 
> Diff: https://reviews.apache.org/r/45122/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44947: Add tests for XFS project quota utilities.

[Jiang Yan Xu]

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



Minor comments. More thorough review once the tests reflect the proposed new 
APIs.


src/tests/containerizer/xfs_quota_tests.cpp (line 17)


Insert a blank line below.



src/tests/containerizer/xfs_quota_tests.cpp (line 51)


`mesos:internal::xfs` goes alphabetically before `process` and leave a 
blank line in between.



src/tests/containerizer/xfs_quota_tests.cpp (line 102)


End the sentence with '.'.



src/tests/containerizer/xfs_quota_tests.cpp (line 115)


In the Mesos codebase C++ style argument comment is preferred: `0, // 
Flags.`



src/tests/containerizer/xfs_quota_tests.cpp (lines 133 - 135)


The indentation here treats `subprocess(` as if it was a method argument.

Suggestion:

```
Try s = subprocess(
"losetup -d " + loopDevice.get(),
Subprocess::PATH("/dev/null"));
```



src/tests/containerizer/xfs_quota_tests.cpp (line 138)


We should give the wait a timeout. e.g., Seconds(15) as we don't want the 
tests to block forever under any circumstances.



src/tests/containerizer/xfs_quota_tests.cpp (line 150)


s/a/an/



src/tests/containerizer/xfs_quota_tests.cpp (line 167)


Doesn't look like we need `ftruncate` if we use `posix_fallocate`?

And you are not checking the exit status here.

IIUC, `os::ftruncate()` is not going to trigger ENOSPC but 
`::posix_fallocate` is. However the method is going to return `Nothing()`.



src/tests/containerizer/xfs_quota_tests.cpp (line 180)


s/ctlfd/fd/



src/tests/containerizer/xfs_quota_tests.cpp (line 189)


s/r//



src/tests/environment.cpp (line 467)


Looks like #if also works but it's more standard to use #ifdef 

http://www.faqs.org/docs/Linux-HOWTO/GCC-HOWTO.html


- Jiang Yan Xu


On March 26, 2016, 10:05 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44947/
> ---
> 
> (Updated March 26, 2016, 10:05 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
> https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add tests for XFS project quota utilities.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/tests/containerizer/xfs_quota_tests.cpp PRE-CREATION 
>   src/tests/environment.cpp 7617e43587cb81104786d06f753f08565a6c2d0a 
> 
> Diff: https://reviews.apache.org/r/44947/diff/
> 
> 
> Testing
> ---
> 
> Make check. Manual testing. These tests.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 45386: Fix container destroy provisioning race.

[Gilbert Song]

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

(Updated March 28, 2016, 10:16 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description (updated)
---

Fix container destroy provisioning race.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
e7f7e7fd1304e14dbfaab8b53cea16efc0417911 

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


Testing
---

make check

./bin/mesos-tests.sh --gtest_filter="*DestroyWhileProvisioning*" 
--gtest_repeat=-1 --gtest_break_on_failure


Thanks,

Gilbert Song

Re: Review Request 45230: Refactored subproces setup functions [1/7].

[Cong Wang]

I am wondering how does it pass your test since you did `make check`
on both Linux and OSX? It fails immediately for me on Linux...


On Mon, Mar 28, 2016 at 10:09 AM, Joris Van Remoortere
 wrote:
> Joerg will fix these.
> Thanks!
>
> On Mon, Mar 28, 2016 at 7:06 PM, Cong Wang  wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/45230/
>> 3rdparty/libprocess/include/process/subprocess.hpp
>>  
>> (Diff
>> revision 2)
>>
>> public:
>>
>> 301
>>
>> const Setsid setsid = NO_SETSID,
>>
>> This one breaks build even after all of your 7 patches are committed, 
>> because port_mapping.cpp passes flags but no setsid to subprocess(), this is 
>> not allowed by C++.
>>
>>
>> 3rdparty/libprocess/include/process/subprocess.hpp
>>  
>> (Diff
>> revision 2)
>>
>> public:
>>
>> 342
>>
>> const Setsid setsid = NO_SETSID,
>>
>> Ditto
>>
>>
>> - Cong Wang
>>
>> On March 28th, 2016, 4:51 p.m. UTC, Joerg Schad wrote:
>> Review request for mesos and Joris Van Remoortere.
>> By Joerg Schad.
>>
>> *Updated March 28, 2016, 4:51 p.m.*
>> *Bugs: * MESOS-5049 
>> *Repository: * mesos
>> Description
>>
>> Executing arbitrary setup functions while creating new processes is
>> dangerous as all functions called have to be async safe. As setup
>> functions are used for only very few purposes (setsid, chdir, monitoring
>> and killing a process (see upcoming review) it makes sense to support
>> them safely via parameters to subprocess. Note this review by itself
>> -without the following ones- removing the uses of the old interface will
>> break the build.
>>
>> Testing
>>
>> tested entire chain (see https://reviews.apache.org/r/45236/).
>>
>> Diffs
>>
>>- 3rdparty/libprocess/include/process/ssl/gtest.hpp
>>(2ca705524c8f9bba3c03eef296dc04a353dd236c)
>>- 3rdparty/libprocess/include/process/subprocess.hpp
>>(e0c306aa5cf5f393abb73768bbd287c45730f076)
>>- 3rdparty/libprocess/src/subprocess.cpp
>>(b99bad04f7251169df3bfcec5dee459977440997)
>>
>> View Diff 
>>

Re: Review Request 45230: Refactored subproces setup functions [1/7].

[Joris Van Remoortere]

Joerg will fix these.
Thanks!

On Mon, Mar 28, 2016 at 7:06 PM, Cong Wang  wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45230/
> 3rdparty/libprocess/include/process/subprocess.hpp
>  
> (Diff
> revision 2)
>
> public:
>
> 301
>
> const Setsid setsid = NO_SETSID,
>
> This one breaks build even after all of your 7 patches are committed, because 
> port_mapping.cpp passes flags but no setsid to subprocess(), this is not 
> allowed by C++.
>
>
> 3rdparty/libprocess/include/process/subprocess.hpp
>  
> (Diff
> revision 2)
>
> public:
>
> 342
>
> const Setsid setsid = NO_SETSID,
>
> Ditto
>
>
> - Cong Wang
>
> On March 28th, 2016, 4:51 p.m. UTC, Joerg Schad wrote:
> Review request for mesos and Joris Van Remoortere.
> By Joerg Schad.
>
> *Updated March 28, 2016, 4:51 p.m.*
> *Bugs: * MESOS-5049 
> *Repository: * mesos
> Description
>
> Executing arbitrary setup functions while creating new processes is
> dangerous as all functions called have to be async safe. As setup
> functions are used for only very few purposes (setsid, chdir, monitoring
> and killing a process (see upcoming review) it makes sense to support
> them safely via parameters to subprocess. Note this review by itself
> -without the following ones- removing the uses of the old interface will
> break the build.
>
> Testing
>
> tested entire chain (see https://reviews.apache.org/r/45236/).
>
> Diffs
>
>- 3rdparty/libprocess/include/process/ssl/gtest.hpp
>(2ca705524c8f9bba3c03eef296dc04a353dd236c)
>- 3rdparty/libprocess/include/process/subprocess.hpp
>(e0c306aa5cf5f393abb73768bbd287c45730f076)
>- 3rdparty/libprocess/src/subprocess.cpp
>(b99bad04f7251169df3bfcec5dee459977440997)
>
> View Diff 
>

Re: Review Request 45230: Refactored subproces setup functions [1/7].

[Cong Wang]

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




3rdparty/libprocess/include/process/subprocess.hpp (line 301)


This one breaks build even after all of your 7 patches are committed, 
because port_mapping.cpp passes flags but no setsid to subprocess(), this is 
not allowed by C++.



3rdparty/libprocess/include/process/subprocess.hpp (line 342)


Ditto


- Cong Wang


On March 28, 2016, 4:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45230/
> ---
> 
> (Updated March 28, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executing arbitrary setup functions while creating new processes is
> dangerous as all functions called have to be async safe. As setup
> functions are used for only very few purposes (setsid, chdir, monitoring
> and killing a process (see upcoming review) it makes sense to support
> them safely via parameters to subprocess. Note this review by itself
> -without the following ones- removing the uses of the old interface will
> break the build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp 
> 2ca705524c8f9bba3c03eef296dc04a353dd236c 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> e0c306aa5cf5f393abb73768bbd287c45730f076 
>   3rdparty/libprocess/src/subprocess.cpp 
> b99bad04f7251169df3bfcec5dee459977440997 
> 
> Diff: https://reviews.apache.org/r/45230/diff/
> 
> 
> Testing
> ---
> 
> tested entire chain (see https://reviews.apache.org/r/45236/).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45232: Introduced subprocess watchdog option [3/7].

[Benjamin Hindman]

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




3rdparty/libprocess/include/process/subprocess.hpp (line 46)


When did we start making 'enum' names be in caps? There is an enum 7 lines 
above that is not in all caps.


- Benjamin Hindman


On March 28, 2016, 4:52 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45232/
> ---
> 
> (Updated March 28, 2016, 4:52 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5049
> https://issues.apache.org/jira/browse/MESOS-5049
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some newly created processes such as perf should be killed in case the
> parent dies. Currently this is achieved by forking a new process from the
> child process which serves as a 'watchdog' and kill the child if the parent
> dies. This review introduces this as a general behavior into subprocess
> (and hence removes the need for the custom setup function).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> e0c306aa5cf5f393abb73768bbd287c45730f076 
>   3rdparty/libprocess/src/subprocess.cpp 
> b99bad04f7251169df3bfcec5dee459977440997 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 09e46eb1ce10a6c38cc364077f0b5952579d46e7 
> 
> Diff: https://reviews.apache.org/r/45232/diff/
> 
> 
> Testing
> ---
> 
> Tested complete chain (see https://reviews.apache.org/r/45236/).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45245: Added watchdog to 'du' disk isolator process [7/7].

[Joerg Schad]

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

(Updated March 28, 2016, 4:54 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

The disk isolator process should also be killed when
the parent process dies.


Diffs
-

  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
b38d83fbb29f46552ffbda7b17cbc85af15550e1 

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


Testing
---

sudo make check linux/OSX


Thanks,

Joerg Schad

Re: Review Request 45236: Refactored isolator tests to use parentHook [6/7].

[Joerg Schad]

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

(Updated March 28, 2016, 4:53 p.m.)


Review request for mesos and Joris Van Remoortere.


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


Repository: mesos


Description
---

The isolator tests parent process isolates the child while the child
is being blocked. This this the exact patter of a parentHook.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
df506fc793e480e825b476e43c683ef8bcf676b2 

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


Testing
---

sudo make check on Linux and OSX.


Thanks,

Joerg Schad