date:20180611


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



Patch looks great!

Reviews applied: [67533]

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

- Mesos Reviewbot


On June 11, 2018, 8:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67533/
> ---
> 
> (Updated June 11, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed superfluous guard around `signals.hpp`.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.cpp 8e03ac66ea27ce1cc3a3fb5f2489ca201e1f8e92 
> 
> 
> Diff: https://reviews.apache.org/r/67533/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67541: Made `NoopResourceEstimator` return a forever-pending future.


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



PASS: Mesos patch 67541 was successfully built and tested.

Reviews applied: `['67541']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67541

- Mesos Reviewbot Windows


On June 11, 2018, 11:51 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67541/
> ---
> 
> (Updated June 11, 2018, 11:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gastón Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8873
> https://issues.apache.org/jira/browse/MESOS-8873
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is no need for the noop resource estimator to return a ready
> future of empty resource since it is only activated when the user
> does not want to use an resource estimator.
> 
> 
> Diffs
> -
> 
>   src/slave/resource_estimators/noop.cpp 
> c3ddc0149b2545183a105eb2b9dd31488f40 
> 
> 
> Diff: https://reviews.apache.org/r/67541/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67532: Removed superfluous guard around `signals.hpp`.


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



Patch looks great!

Reviews applied: [67532]

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

- Mesos Reviewbot


On June 11, 2018, 8:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67532/
> ---
> 
> (Updated June 11, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a follow-up to commit e765f8fab where we made this header safe
> to include, but a no-op on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/main.cpp 
> db94db677a2f61c83766179e726b5a595f419caa 
> 
> 
> Diff: https://reviews.apache.org/r/67532/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67517: Added a test to verify `min_allocatable_resources` flag.


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



PASS: Mesos patch 67517 was successfully built and tested.

Reviews applied: `['67510', '67516', '67513', '67517']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67517

- Mesos Reviewbot Windows


On June 11, 2018, 2:41 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67517/
> ---
> 
> (Updated June 11, 2018, 2:41 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-8935
> https://issues.apache.org/jira/browse/MESOS-8935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that the allocator honors the
> `min_allocatable_resources` flag and only offers resources
> that are more than at least one of the specified resources quantity.
> 
> 
> Diffs
> -
> 
>   src/tests/master_allocator_tests.cpp 
> e1aef8a9625a805e7ad2dfad37bfeedee82f160d 
> 
> 
> Diff: https://reviews.apache.org/r/67517/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67534: CMake: Added libapr as interface dependency of libsvn.


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



PASS: Mesos patch 67534 was successfully built and tested.

Reviews applied: `['67534']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67534

- Mesos Reviewbot Windows


On June 11, 2018, 9:57 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67534/
> ---
> 
> (Updated June 11, 2018, 9:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Apache Portable Runtime library is used only because SVN requires
> it, so SVN should explicitly require it in its interface.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 3c1dfcc7885693cd76331b515ca8e361bd1976d0 
> 
> 
> Diff: https://reviews.apache.org/r/67534/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67535: CMake: Removed explicit linkage of libapr.


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



PASS: Mesos patch 67535 was successfully built and tested.

Reviews applied: `['67535']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67535

- Mesos Reviewbot Windows


On June 11, 2018, 9:57 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67535/
> ---
> 
> (Updated June 11, 2018, 9:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Because SVN now includes APR as an interface dependency, the explicit
> linkage to APR in the stout interface is not necessary. Instead, APR
> is included transitively when SVN is included.
> 
> This fixes the unexpected behavior on Windows were APR is built and
> linked against despite not being used at all since SVN isn't used.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt 9cbb6f2a13fe9201fdebb9e9994d7725e53af083 
> 
> 
> Diff: https://reviews.apache.org/r/67535/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67526: WIP: Added container-specific cgroup FS mounts.


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



Patch looks great!

Reviews applied: [67526]

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

- Mesos Reviewbot


On June 11, 2018, 8:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67526/
> ---
> 
> (Updated June 11, 2018, 8:58 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-8327
> https://issues.apache.org/jira/browse/MESOS-8327
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added container-specific cgroup FS mounts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 81c934318dcc2bcc9df594af0ee25f0334541a65 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 215e32461e851668247f9fae62aa656f5dd5e245 
> 
> 
> Diff: https://reviews.apache.org/r/67526/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 67541: Made `NoopResourceEstimator` return a forever-pending future.

2018-06-11 Thread Chun-Hung Hsiao


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

Review request for mesos, Benjamin Bannier, Gastón Kleiman, and Jie Yu.


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


Repository: mesos


Description
---

There is no need for the noop resource estimator to return a ready
future of empty resource since it is only activated when the user
does not want to use an resource estimator.


Diffs
-

  src/slave/resource_estimators/noop.cpp 
c3ddc0149b2545183a105eb2b9dd31488f40 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 67264: Unmounted any mount points in gc paths.

2018-06-11 Thread Zhitao Li


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

(Updated June 11, 2018, 4:04 p.m.)


Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.


Changes
---

Use adaptor::reverse


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


Repository: mesos


Description
---

In various corner cases, agent may not get chance to properly unmount
persistent volumes mounted inside an executor's sandbox. When GC later
gets to these sandbox directories, permanent data loss can happen (see
MESOS-8830).

Currently, the only mounts in the host mount namespace under the sandbox
directories are persistent volumes, so this diff added protection to
unmount any dangling mount points before calling `rmdir` on the
directory.

NOTE: this means agent will not garbage collect any path if it cannot
read its own `mountinfo` table.


Diffs (updated)
-

  src/local/local.cpp afff54653e8e659d947ddbee6dc38ba2715f2a78 
  src/slave/gc.hpp df40165bb8a23f065156bf6c5f354b143d88c088 
  src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106 
  src/slave/gc_process.hpp 20374ad91820341282fdf18ecade60a020e26cea 
  src/slave/main.cpp 646125344d590b28256d8ee684d7e51a90e82f23 
  src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48 
  src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126 
  src/tests/cluster.cpp 01eb0950e687227dac81b1cdb9eaba3379cf5dbb 
  src/tests/gc_tests.cpp 619ed22edd9b3909ea24cdcbf62c354420a8d031 
  src/tests/mesos.hpp 733344a2f07ebd9d841a55fb9bbfda2e3c1a1eb2 
  src/tests/mesos.cpp d3c87c295429481c59d5a49398e289a4b84e4496 
  src/tests/slave_tests.cpp 3d67511de5abd3466eeb5ad1daf318209bd69eed 


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

Changes: https://reviews.apache.org/r/67264/diff/6-7/


Testing
---

Added a unit test in following patch.

Tested with following procedures:
1. Start a test master and agent;
2. Created a persistent volume on agent through operator API;
3. Use `mesos-execute` to run a task;
4. Stop the agent;
5. Manually bind mount persistent volume path into a `volume` directory inside 
the executor sandbox (to simulate a dangling mount in MESOS-8830);
6. Restart agent with `--gc_disk_headroom=1.0 --gc_delay=1secs` to force it gc 
the path immediately.

With this fix, we observed that the dangling mount is automatically cleaned up, 
and agent produces log line:
```
W0523 06:00:04.001075 82745 gc.cpp:229] Unmounting dangling mount point 
'/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0/frameworks/b3eb3aff-d19d-45ff-8113-f0316462d3fa-/executors/test_id/runs/1cd3bd06-2632-4541-a708-80c7cd51c74b/volume'
 of persistent volume '/home/zhitao/mesos-workdir/volumes/roles/role/id1' 
inside garbage collected path 
'/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0'
```


Thanks,

Zhitao Li

Re: Review Request 67513: Added a master flag to configure minimum allocatable resources.


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



PASS: Mesos patch 67513 was successfully built and tested.

Reviews applied: `['67510', '67516', '67513']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67513

- Mesos Reviewbot Windows


On June 11, 2018, 9:37 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67513/
> ---
> 
> (Updated June 11, 2018, 9:37 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Greg Mann.
> 
> 
> Bugs: MESOS-8935
> https://issues.apache.org/jira/browse/MESOS-8935
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new master flag `min_allocatable_resources`.
> It specifies one or more resources quantities that define the
> minimum allocatable resources for the allocator. The allocator
> will only offer resources that are more than at least one of
> the specified resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 647869286d46920b98521e219ce92c3260058c35 
>   src/master/allocator/mesos/allocator.hpp 
> c453c015b234deff7efd00269da25dcec8cbf1ae 
>   src/master/allocator/mesos/hierarchical.hpp 
> e9d1742bb35004735e3cb357286b4e5b17436a5c 
>   src/master/allocator/mesos/hierarchical.cpp 
> b558228290e5ae6dbcc1b8a6e1fe69db9fd5874c 
>   src/master/flags.hpp 94b8ac2211180416190448b71ea9c81c6c0cd7fc 
>   src/master/flags.cpp cc3317ee5d740cb1d58b51ae00ceeb8d55754b9d 
>   src/master/master.cpp 5db5a8da85f02323a5654c93ac47ec4aa7e711d2 
>   src/tests/allocator.hpp 341efa665ad0ce897e087fb8d73ec50fd041d559 
>   src/tests/api_tests.cpp 15dbbc1b3efe8e139a2c6b1a4a7087743dbe950c 
>   src/tests/hierarchical_allocator_tests.cpp 
> c97b2ba0884a7ded867c2d80e4749de54c89b5e4 
>   src/tests/master_allocator_tests.cpp 
> e1aef8a9625a805e7ad2dfad37bfeedee82f160d 
>   src/tests/master_quota_tests.cpp 94d85c5a4b70ef2bc4e8689b896fe99f3acfd4b9 
>   src/tests/reservation_tests.cpp 7d121bf56b913c3217dec00c57f81663e9831351 
>   src/tests/resource_offers_tests.cpp 
> 54aafdb4258ad7713c5f1a59956e7f76f0e84d5b 
>   src/tests/slave_recovery_tests.cpp 2a92acc193b4db5001ae5bca53e7333ba7203210 
> 
> 
> Diff: https://reviews.apache.org/r/67513/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> Fixed all existing tests.
> Dedicate test added in a subsequent patch.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67264: Unmounted any mount points in gc paths.

2018-06-11 Thread Jason Lai


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


Ship it!




Ship It!


src/slave/gc.cpp
Lines 225-227 (patched)


This works, but if you wanna be consistent with the rest of how reverse 
iteration of the mount entries, the following could be considered:

```
  foreach (const MountTable::Entry& entry,
   adaptor::reverse(mountTable->entries)) {
```

This is used in `src/linux/fs.cpp` and a couple of other places.


- Jason Lai


On June 11, 2018, 8:58 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67264/
> ---
> 
> (Updated June 11, 2018, 8:58 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-8830
> https://issues.apache.org/jira/browse/MESOS-8830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In various corner cases, agent may not get chance to properly unmount
> persistent volumes mounted inside an executor's sandbox. When GC later
> gets to these sandbox directories, permanent data loss can happen (see
> MESOS-8830).
> 
> Currently, the only mounts in the host mount namespace under the sandbox
> directories are persistent volumes, so this diff added protection to
> unmount any dangling mount points before calling `rmdir` on the
> directory.
> 
> NOTE: this means agent will not garbage collect any path if it cannot
> read its own `mountinfo` table.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp afff54653e8e659d947ddbee6dc38ba2715f2a78 
>   src/slave/gc.hpp df40165bb8a23f065156bf6c5f354b143d88c088 
>   src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106 
>   src/slave/gc_process.hpp 20374ad91820341282fdf18ecade60a020e26cea 
>   src/slave/main.cpp 646125344d590b28256d8ee684d7e51a90e82f23 
>   src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48 
>   src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126 
>   src/tests/cluster.cpp 01eb0950e687227dac81b1cdb9eaba3379cf5dbb 
>   src/tests/gc_tests.cpp 619ed22edd9b3909ea24cdcbf62c354420a8d031 
>   src/tests/mesos.hpp 733344a2f07ebd9d841a55fb9bbfda2e3c1a1eb2 
>   src/tests/mesos.cpp d3c87c295429481c59d5a49398e289a4b84e4496 
>   src/tests/slave_tests.cpp 3d67511de5abd3466eeb5ad1daf318209bd69eed 
> 
> 
> Diff: https://reviews.apache.org/r/67264/diff/6/
> 
> 
> Testing
> ---
> 
> Added a unit test in following patch.
> 
> Tested with following procedures:
> 1. Start a test master and agent;
> 2. Created a persistent volume on agent through operator API;
> 3. Use `mesos-execute` to run a task;
> 4. Stop the agent;
> 5. Manually bind mount persistent volume path into a `volume` directory 
> inside the executor sandbox (to simulate a dangling mount in MESOS-8830);
> 6. Restart agent with `--gc_disk_headroom=1.0 --gc_delay=1secs` to force it 
> gc the path immediately.
> 
> With this fix, we observed that the dangling mount is automatically cleaned 
> up, and agent produces log line:
> ```
> W0523 06:00:04.001075 82745 gc.cpp:229] Unmounting dangling mount point 
> '/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0/frameworks/b3eb3aff-d19d-45ff-8113-f0316462d3fa-/executors/test_id/runs/1cd3bd06-2632-4541-a708-80c7cd51c74b/volume'
>  of persistent volume '/home/zhitao/mesos-workdir/volumes/roles/role/id1' 
> inside garbage collected path 
> '/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0'
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 67496: Ported `EnvTest.EraseEnv` to Windows, with added comments.


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

(Updated June 11, 2018, 3:29 p.m.)


Review request for mesos, James Peach and Radhika Jandhyala.


Repository: mesos


Description
---

The difference between `os::unsetenv` and `os::eraseenv` is important,
but subtle. Now the reason for the existence of `os::eraseenv` is
documented in comments.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/os.hpp 
f898f07515bcc663fea1eef52497feb518d4ca53 
  3rdparty/stout/include/stout/windows/os.hpp 
3a728f8cb699c1a4a0bbbf1ecb44f9d33b60e4e3 
  3rdparty/stout/tests/os/env_tests.cpp 
b5b124dc6316e661af6dd90335ade5283c26d9f2 


Diff: https://reviews.apache.org/r/67496/diff/2/

Changes: https://reviews.apache.org/r/67496/diff/1-2/


Testing
---

stout-tests on Windows


Thanks,

Andrew Schwartzmeyer

Re: Review Request 67496: Ported `EnvTest.EraseEnv` to Windows, with added comments.

2018-06-11 Thread James Peach


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


Fix it, then Ship it!





3rdparty/stout/tests/os/env_tests.cpp
Line 86 (original), 97 (patched)


Can you spell `ernvironment` while you are here?

:)


- James Peach


On June 7, 2018, 11:19 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67496/
> ---
> 
> (Updated June 7, 2018, 11:19 p.m.)
> 
> 
> Review request for mesos, James Peach and Radhika Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The difference between `os::unsetenv` and `os::eraseenv` is important,
> but subtle. Now the reason for the existence of `os::eraseenv` is
> documented in comments.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/posix/os.hpp 
> f898f07515bcc663fea1eef52497feb518d4ca53 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 3a728f8cb699c1a4a0bbbf1ecb44f9d33b60e4e3 
>   3rdparty/stout/tests/os/env_tests.cpp 
> b5b124dc6316e661af6dd90335ade5283c26d9f2 
> 
> 
> Diff: https://reviews.apache.org/r/67496/diff/1/
> 
> 
> Testing
> ---
> 
> stout-tests on Windows
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 67535: CMake: Removed explicit linkage of libapr.


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

Review request for mesos, Benjamin Bannier and Joseph Wu.


Repository: mesos


Description
---

Because SVN now includes APR as an interface dependency, the explicit
linkage to APR in the stout interface is not necessary. Instead, APR
is included transitively when SVN is included.

This fixes the unexpected behavior on Windows were APR is built and
linked against despite not being used at all since SVN isn't used.


Diffs
-

  3rdparty/stout/CMakeLists.txt 9cbb6f2a13fe9201fdebb9e9994d7725e53af083 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 67534: CMake: Added libapr as interface dependency of libsvn.


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

Review request for mesos, Benjamin Bannier and Joseph Wu.


Repository: mesos


Description
---

The Apache Portable Runtime library is used only because SVN requires
it, so SVN should explicitly require it in its interface.


Diffs
-

  3rdparty/CMakeLists.txt 3c1dfcc7885693cd76331b515ca8e361bd1976d0 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 67517: Added a test to verify `min_allocatable_resources` flag.


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

Review request for mesos, Gastón Kleiman and Greg Mann.


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


Repository: mesos


Description
---

This test verifies that the allocator honors the
`min_allocatable_resources` flag and only offers resources
that are more than at least one of the specified resources quantity.


Diffs
-

  src/tests/master_allocator_tests.cpp e1aef8a9625a805e7ad2dfad37bfeedee82f160d 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 67525: Removed static variable with non-constant initialization.


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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot


On June 11, 2018, 12:36 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> ---
> 
> (Updated June 11, 2018, 12:36 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The static variable `http::statuses` was initialized using a
> dynamic memory allocation. This is against our coding style,
> and makes it as easy as writing
> 
> static process::http::NotFound DEFAULT_RESPONSE;
> 
> for a user to introduce a potential segfault into his code.
> 
> Therefore, it was changed to use an array with constant
> initialization instead.
> 
> The pointer `statuses` was also removed from the public API, since
> there are no current users outside libprocess, providing a non-const
> pointer to an internal data structure seems inherently unsafe, and
> the intended use-cases seem to be covered by the existing `Status::string()`
> function.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 055447e13117c0a3ba79d0fc326ece657e8f064f 
>   3rdparty/libprocess/src/decoder.hpp 
> a0784358d63279022473fdf82a72b7ecc9056658 
>   3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 
> 
> 
> Diff: https://reviews.apache.org/r/67525/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 67533: Removed superfluous guard around `signals.hpp`.


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



PASS: Mesos patch 67533 was successfully built and tested.

Reviews applied: `['67533']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67533

- Mesos Reviewbot Windows


On June 11, 2018, 8:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67533/
> ---
> 
> (Updated June 11, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed superfluous guard around `signals.hpp`.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.cpp 8e03ac66ea27ce1cc3a3fb5f2489ca201e1f8e92 
> 
> 
> Diff: https://reviews.apache.org/r/67533/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 67513: Added a master flag to configure minimum allocatable resources.


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

Review request for mesos, Gastón Kleiman and Greg Mann.


Repository: mesos


Description
---

This patch adds a new master flag `min_allocatable_resources`.
It specifies one or more resources quantities that define the
minimum allocatable resources for the allocator. The allocator
will only offer resources that are more than at least one of
the specified resources.


Diffs
-

  include/mesos/allocator/allocator.hpp 
647869286d46920b98521e219ce92c3260058c35 
  src/master/allocator/mesos/allocator.hpp 
c453c015b234deff7efd00269da25dcec8cbf1ae 
  src/master/allocator/mesos/hierarchical.hpp 
e9d1742bb35004735e3cb357286b4e5b17436a5c 
  src/master/allocator/mesos/hierarchical.cpp 
b558228290e5ae6dbcc1b8a6e1fe69db9fd5874c 
  src/master/flags.hpp 94b8ac2211180416190448b71ea9c81c6c0cd7fc 
  src/master/flags.cpp cc3317ee5d740cb1d58b51ae00ceeb8d55754b9d 
  src/master/master.cpp 5db5a8da85f02323a5654c93ac47ec4aa7e711d2 
  src/tests/allocator.hpp 341efa665ad0ce897e087fb8d73ec50fd041d559 
  src/tests/api_tests.cpp 15dbbc1b3efe8e139a2c6b1a4a7087743dbe950c 
  src/tests/hierarchical_allocator_tests.cpp 
c97b2ba0884a7ded867c2d80e4749de54c89b5e4 
  src/tests/master_allocator_tests.cpp e1aef8a9625a805e7ad2dfad37bfeedee82f160d 
  src/tests/master_quota_tests.cpp 94d85c5a4b70ef2bc4e8689b896fe99f3acfd4b9 
  src/tests/reservation_tests.cpp 7d121bf56b913c3217dec00c57f81663e9831351 
  src/tests/resource_offers_tests.cpp 54aafdb4258ad7713c5f1a59956e7f76f0e84d5b 
  src/tests/slave_recovery_tests.cpp 2a92acc193b4db5001ae5bca53e7333ba7203210 


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


Testing
---

make check
Fixed all existing tests.
Dedicate test added in a subsequent patch.


Thanks,

Meng Zhu

Re: Review Request 67532: Removed superfluous guard around `signals.hpp`.


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



PASS: Mesos patch 67532 was successfully built and tested.

Reviews applied: `['67532']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67532

- Mesos Reviewbot Windows


On June 11, 2018, 8:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67532/
> ---
> 
> (Updated June 11, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a follow-up to commit e765f8fab where we made this header safe
> to include, but a no-op on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/main.cpp 
> db94db677a2f61c83766179e726b5a595f419caa 
> 
> 
> Diff: https://reviews.apache.org/r/67532/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67264: Unmounted any mount points in gc paths.

2018-06-11 Thread Zhitao Li

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

(Updated June 11, 2018, 1:58 p.m.)

Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.

Changes
---

Fail `rmdir` future if mount table cannot be loaded for agent on Linux.

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

Repository: mesos

Description
---

In various corner cases, agent may not get chance to properly unmount
persistent volumes mounted inside an executor's sandbox. When GC later
gets to these sandbox directories, permanent data loss can happen (see
MESOS-8830).

Currently, the only mounts in the host mount namespace under the sandbox
directories are persistent volumes, so this diff added protection to
unmount any dangling mount points before calling `rmdir` on the
directory.

NOTE: this means agent will not garbage collect any path if it cannot
read its own `mountinfo` table.

Diffs (updated)
-

src/local/local.cpp afff54653e8e659d947ddbee6dc38ba2715f2a78
src/slave/gc.hpp df40165bb8a23f065156bf6c5f354b143d88c088
src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106
src/slave/gc_process.hpp 20374ad91820341282fdf18ecade60a020e26cea
src/slave/main.cpp 646125344d590b28256d8ee684d7e51a90e82f23
src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48
src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126
src/tests/cluster.cpp 01eb0950e687227dac81b1cdb9eaba3379cf5dbb
src/tests/gc_tests.cpp 619ed22edd9b3909ea24cdcbf62c354420a8d031
src/tests/mesos.hpp 733344a2f07ebd9d841a55fb9bbfda2e3c1a1eb2
src/tests/mesos.cpp d3c87c295429481c59d5a49398e289a4b84e4496
src/tests/slave_tests.cpp 3d67511de5abd3466eeb5ad1daf318209bd69eed

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

Changes: https://reviews.apache.org/r/67264/diff/5-6/

Testing
---

Added a unit test in following patch.

Tested with following procedures:
1. Start a test master and agent;
2. Created a persistent volume on agent through operator API;
3. Use `mesos-execute` to run a task;
4. Stop the agent;
5. Manually bind mount persistent volume path into a `volume` directory inside
the executor sandbox (to simulate a dangling mount in MESOS-8830);
6. Restart agent with `--gc_disk_headroom=1.0 --gc_delay=1secs` to force it gc
the path immediately.

With this fix, we observed that the dangling mount is automatically cleaned up,
and agent produces log line:
```
W0523 06:00:04.001075 82745 gc.cpp:229] Unmounting dangling mount point
'/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0/frameworks/b3eb3aff-d19d-45ff-8113-f0316462d3fa-/executors/test_id/runs/1cd3bd06-2632-4541-a708-80c7cd51c74b/volume'
of persistent volume '/home/zhitao/mesos-workdir/volumes/roles/role/id1'
inside garbage collected path
'/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0'
```

Thanks,

Zhitao Li

Review Request 67532: Removed superfluous guard around `signals.hpp`.


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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

This is a follow-up to commit e765f8fab where we made this header safe
to include, but a no-op on Windows.


Diffs
-

  3rdparty/libprocess/src/tests/main.cpp 
db94db677a2f61c83766179e726b5a595f419caa 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 67533: Removed superfluous guard around `signals.hpp`.


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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Removed superfluous guard around `signals.hpp`.


Diffs
-

  src/logging/logging.cpp 8e03ac66ea27ce1cc3a3fb5f2489ca201e1f8e92 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 67516: Added a resource utility `isScalarQuantity`.


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



PASS: Mesos patch 67516 was successfully built and tested.

Reviews applied: `['67510', '67516']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67516

- Mesos Reviewbot Windows


On June 11, 2018, 5:34 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67516/
> ---
> 
> (Updated June 11, 2018, 5:34 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `isScalarQuantity()` checks if a `Resources` object
> is "pure" scalar quantity i.e. its `Resource`(s) only has
> name, type (set to scalar) and scalar fields set.
> 
> Also added tests.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 7afe0d8090a5075faed22bd408310ae1bb464bce 
>   include/mesos/v1/resources.hpp c7120575d0dbfb9f8c9bb6b0cf9a4cea3d9f6932 
>   src/common/resources.cpp b1392012702865e39d17b8715747582529cbccca 
>   src/tests/resources_tests.cpp 2da583615ac8a8fb238f4996fe661343b718bad1 
>   src/v1/resources.cpp 50793271ea9b3013aa5b4dcd95f329db81dff4d8 
> 
> 
> Diff: https://reviews.apache.org/r/67516/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67510: Fixed a bug in `createStrippedScalarQuantity()`.


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



PASS: Mesos patch 67510 was successfully built and tested.

Reviews applied: `['67510']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67510

- Mesos Reviewbot Windows


On June 11, 2018, 5:30 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67510/
> ---
> 
> (Updated June 11, 2018, 5:30 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes `createStrippedScalarQuantity()` by
> stripping the revocable field in resources.
> 
> Also added a test.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp b1392012702865e39d17b8715747582529cbccca 
>   src/tests/resources_tests.cpp 2da583615ac8a8fb238f4996fe661343b718bad1 
>   src/v1/resources.cpp 50793271ea9b3013aa5b4dcd95f329db81dff4d8 
> 
> 
> Diff: https://reviews.apache.org/r/67510/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 67516: Added a resource utility `isScalarQuantity`.


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

Review request for mesos, Gastón Kleiman and Greg Mann.


Repository: mesos


Description
---

`isScalarQuantity()` checks if a `Resources` object
is "pure" scalar quantity i.e. its `Resource`(s) only has
name, type (set to scalar) and scalar fields set.

Also added tests.


Diffs
-

  include/mesos/resources.hpp 7afe0d8090a5075faed22bd408310ae1bb464bce 
  include/mesos/v1/resources.hpp c7120575d0dbfb9f8c9bb6b0cf9a4cea3d9f6932 
  src/common/resources.cpp b1392012702865e39d17b8715747582529cbccca 
  src/tests/resources_tests.cpp 2da583615ac8a8fb238f4996fe661343b718bad1 
  src/v1/resources.cpp 50793271ea9b3013aa5b4dcd95f329db81dff4d8 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 67510: Fixed a bug in `createStrippedScalarQuantity()`.


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

Review request for mesos, Gastón Kleiman and Greg Mann.


Repository: mesos


Description
---

This patch fixes `createStrippedScalarQuantity()` by
stripping the revocable field in resources.

Also added a test.


Diffs
-

  src/common/resources.cpp b1392012702865e39d17b8715747582529cbccca 
  src/tests/resources_tests.cpp 2da583615ac8a8fb238f4996fe661343b718bad1 
  src/v1/resources.cpp 50793271ea9b3013aa5b4dcd95f329db81dff4d8 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 67526: WIP: Added container-specific cgroup FS mounts.


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



PASS: Mesos patch 67526 was successfully built and tested.

Reviews applied: `['67526']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67526

- Mesos Reviewbot Windows


On June 11, 2018, 3:58 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67526/
> ---
> 
> (Updated June 11, 2018, 3:58 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-8327
> https://issues.apache.org/jira/browse/MESOS-8327
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added container-specific cgroup FS mounts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 81c934318dcc2bcc9df594af0ee25f0334541a65 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 215e32461e851668247f9fae62aa656f5dd5e245 
> 
> 
> Diff: https://reviews.apache.org/r/67526/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67394: White list fds that child processes can inherit in mesos containerizer.



> On June 11, 2018, 9:52 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/containerizer/launcher.hpp
> > Lines 59-72 (original), 59-82 (patched)
> > 
> >
> > I'm not sure I follow; why do we need this entire change instead of 
> > just `s/10/9`, adding `whiteListFds`, and changing to `containerIO`?

Dropping because the answer is that Google Mock cannot mock a function with 
default arguments.


- Andrew


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


On May 31, 2018, 3:50 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67394/
> ---
> 
> (Updated May 31, 2018, 3:50 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jie Yu, and Li 
> Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> White list fds that child processes can inherit in mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
>   src/slave/containerizer/mesos/launcher.hpp 
> f69d934d2e1a129e10df8c7f5c78723e832adc7d 
>   src/slave/containerizer/mesos/launcher.cpp 
> 2fe47d368cb82a46328e1f636baa836272db244c 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 80e444501e429c1e1ae354abcd51f86430316ada 
>   src/tests/containerizer/launcher.hpp 
> a8e436f164b67d937ebcff35e084d3ca755c003c 
>   src/tests/containerizer/launcher.cpp 
> a92d9890f0931425d69ef8ce0896d081b8722079 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 01f2b38cfa67b144298c361e92170322864ac201 
> 
> 
> Diff: https://reviews.apache.org/r/67394/diff/1/
> 
> 
> Testing
> ---
> 
> All mesos tests on windows
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67394: White list fds that child processes can inherit in mesos containerizer.


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




src/slave/containerizer/mesos/launcher.hpp
Lines 73 (patched)


`= {}`



src/slave/containerizer/mesos/launcher.hpp
Lines 110 (patched)


`= {}`



src/slave/containerizer/mesos/linux_launcher.hpp
Lines 60 (patched)


`= {}`



src/slave/containerizer/mesos/linux_launcher.cpp
Lines 82 (patched)


Ditto.



src/tests/containerizer/launcher.hpp
Lines 59-72 (original), 59-82 (patched)


I'm not sure I follow; why do we need this entire change instead of just 
`s/10/9`, adding `whiteListFds`, and changing to `containerIO`?



src/tests/containerizer/mesos_containerizer_tests.cpp
Lines 1338-1344 (original), 1338-1343 (patched)


Does `containerIO` default construct with stdin/out/err set to valid file 
descriptors?


- Andrew Schwartzmeyer


On May 31, 2018, 3:50 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67394/
> ---
> 
> (Updated May 31, 2018, 3:50 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jie Yu, and Li 
> Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> White list fds that child processes can inherit in mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
>   src/slave/containerizer/mesos/launcher.hpp 
> f69d934d2e1a129e10df8c7f5c78723e832adc7d 
>   src/slave/containerizer/mesos/launcher.cpp 
> 2fe47d368cb82a46328e1f636baa836272db244c 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 80e444501e429c1e1ae354abcd51f86430316ada 
>   src/tests/containerizer/launcher.hpp 
> a8e436f164b67d937ebcff35e084d3ca755c003c 
>   src/tests/containerizer/launcher.cpp 
> a92d9890f0931425d69ef8ce0896d081b8722079 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 01f2b38cfa67b144298c361e92170322864ac201 
> 
> 
> Diff: https://reviews.apache.org/r/67394/diff/1/
> 
> 
> Testing
> ---
> 
> All mesos tests on windows
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67286: White list fds that child processes can inherit in stout.


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


Ship it!





3rdparty/stout/include/stout/internal/windows/inherit.hpp
Lines 55 (patched)


`DeleteProcThreadAttributeList` doesn't seem to care if it fails, as it 
returns nothing, so to make the logic easier, we just bundled it into the 
destructor.


- Andrew Schwartzmeyer


On May 24, 2018, 3:47 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67286/
> ---
> 
> (Updated May 24, 2018, 3:47 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
> Yu, Li Li, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8926
> https://issues.apache.org/jira/browse/MESOS-8926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> White list fds that child processes can inherit in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp 
> 7dbde820e775cbaeb8db4bc4559ab432903e75ea 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
> 
> 
> Diff: https://reviews.apache.org/r/67286/diff/2/
> 
> 
> Testing
> ---
> 
> All Mesos-tests
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67287: White list fds that child processes can inherit in libprocess.


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


Fix it, then Ship it!





3rdparty/libprocess/include/process/subprocess.hpp
Lines 383 (patched)


`= {}` to match the rest of the parameters



3rdparty/libprocess/include/process/subprocess.hpp
Lines 420 (patched)


Ditto



3rdparty/libprocess/src/subprocess_windows.hpp
Lines 55 (patched)


Ditto


- Andrew Schwartzmeyer


On May 24, 2018, 3:47 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67287/
> ---
> 
> (Updated May 24, 2018, 3:47 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
> Yu, Li Li, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8926
> https://issues.apache.org/jira/browse/MESOS-8926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> White list fds that child processes can inherit in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> 6a1262340c333b617402637e648c12769827ffc4 
>   3rdparty/libprocess/src/subprocess.cpp 
> d7a725363251f9c54072cd7551f5598696938308 
>   3rdparty/libprocess/src/subprocess_windows.hpp 
> c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 
> 
> 
> Diff: https://reviews.apache.org/r/67287/diff/2/
> 
> 
> Testing
> ---
> 
> All Mesos-tests
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Review Request 67526: WIP: Added container-specific cgroup FS mounts.

2018-06-11 Thread Qian Zhang


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

Review request for mesos, Gilbert Song and Jason Lai.


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


Repository: mesos


Description
---

Added container-specific cgroup FS mounts.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
81c934318dcc2bcc9df594af0ee25f0334541a65 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
215e32461e851668247f9fae62aa656f5dd5e245 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 67522: Moved some constant definitions into header file.


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



Patch looks great!

Reviews applied: [67522]

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

- Mesos Reviewbot


On June 11, 2018, 5:58 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67522/
> ---
> 
> (Updated June 11, 2018, 5:58 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since we are using C++11 and the declared constants are either POD or
> `constexpr` types, we can define them in the header as `constexpr`.
> This also removes the need to declare the constants as `extern` which
> should using them easier (e.g., simpler linkage requirements).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am d434001fbc49d337b6e29f6ac8c9c7475922a819 
>   3rdparty/libprocess/include/process/timeseries.hpp 
> 64b10a8d551ba33e252aa33987e3d5da8d56a1d6 
>   3rdparty/libprocess/src/CMakeLists.txt 
> cf443dffd0663ecf02b7efd6f7094175b94aae19 
>   3rdparty/libprocess/src/timeseries.cpp 
> 528adef504ea82eb4bec309d8d17e6347c652a29 
> 
> 
> Diff: https://reviews.apache.org/r/67522/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67525: Removed static variable with non-constant initialization.


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



PASS: Mesos patch 67525 was successfully built and tested.

Reviews applied: `['67525']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67525

- Mesos Reviewbot Windows


On June 11, 2018, 12:36 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> ---
> 
> (Updated June 11, 2018, 12:36 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The static variable `http::statuses` was initialized using a
> dynamic memory allocation. This is against our coding style,
> and makes it as easy as writing
> 
> static process::http::NotFound DEFAULT_RESPONSE;
> 
> for a user to introduce a potential segfault into his code.
> 
> Therefore, it was changed to use an array with constant
> initialization instead.
> 
> The pointer `statuses` was also removed from the public API, since
> there are no current users outside libprocess, providing a non-const
> pointer to an internal data structure seems inherently unsafe, and
> the intended use-cases seem to be covered by the existing `Status::string()`
> function.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 055447e13117c0a3ba79d0fc326ece657e8f064f 
>   3rdparty/libprocess/src/decoder.hpp 
> a0784358d63279022473fdf82a72b7ecc9056658 
>   3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 
> 
> 
> Diff: https://reviews.apache.org/r/67525/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 67525: Removed static variable with non-constant initialization.

2018-06-11 Thread Benno Evers


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




3rdparty/libprocess/include/process/http.hpp
Line 121 (original)


I put this in the review, but actually I'm not sure if this is ok regarding 
our rules concerning API and ABI stability.

If desired, I  can change to one of these alternatives:

1) Keep the hashmap in the public API, mark it as deprecated
2) Remove hashmap from the public API, but still define it in `http.cpp` to 
keep ABI compatibility;


- Benno Evers


On June 11, 2018, 12:36 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67525/
> ---
> 
> (Updated June 11, 2018, 12:36 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The static variable `http::statuses` was initialized using a
> dynamic memory allocation. This is against our coding style,
> and makes it as easy as writing
> 
> static process::http::NotFound DEFAULT_RESPONSE;
> 
> for a user to introduce a potential segfault into his code.
> 
> Therefore, it was changed to use an array with constant
> initialization instead.
> 
> The pointer `statuses` was also removed from the public API, since
> there are no current users outside libprocess, providing a non-const
> pointer to an internal data structure seems inherently unsafe, and
> the intended use-cases seem to be covered by the existing `Status::string()`
> function.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 055447e13117c0a3ba79d0fc326ece657e8f064f 
>   3rdparty/libprocess/src/decoder.hpp 
> a0784358d63279022473fdf82a72b7ecc9056658 
>   3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 
> 
> 
> Diff: https://reviews.apache.org/r/67525/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 67525: Removed static variable with non-constant initialization.

2018-06-11 Thread Benno Evers


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

Review request for mesos.


Repository: mesos


Description
---

The static variable `http::statuses` was initialized using a
dynamic memory allocation. This is against our coding style,
and makes it as easy as writing

static process::http::NotFound DEFAULT_RESPONSE;

for a user to introduce a potential segfault into his code.

Therefore, it was changed to use an array with constant
initialization instead.

The pointer `statuses` was also removed from the public API, since
there are no current users outside libprocess, providing a non-const
pointer to an internal data structure seems inherently unsafe, and
the intended use-cases seem to be covered by the existing `Status::string()`
function.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
055447e13117c0a3ba79d0fc326ece657e8f064f 
  3rdparty/libprocess/src/decoder.hpp a0784358d63279022473fdf82a72b7ecc9056658 
  3rdparty/libprocess/src/http.cpp 9fd5ea02ee57b4ad2d30c4b9ac4cbd299942d501 


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


Testing
---

`make check`


Thanks,

Benno Evers

Re: Review Request 67521: Made some headers standalone.


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



Patch looks great!

Reviews applied: [67521]

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

- Mesos Reviewbot


On June 11, 2018, 9:58 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67521/
> ---
> 
> (Updated June 11, 2018, 9:58 a.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds includes to two header files in stout so that the
> headers are standalone.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/copyfile.hpp 
> bb5ec71b9a083f1af0c1a729377ee56842d1b734 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 
> 8bb8e2c25e1677056b165fe8a7f61f57b4127522 
> 
> 
> Diff: https://reviews.apache.org/r/67521/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67522: Moved some constant definitions into header file.


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



PASS: Mesos patch 67522 was successfully built and tested.

Reviews applied: `['67522']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67522

- Mesos Reviewbot Windows


On June 11, 2018, 2:58 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67522/
> ---
> 
> (Updated June 11, 2018, 2:58 a.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since we are using C++11 and the declared constants are either POD or
> `constexpr` types, we can define them in the header as `constexpr`.
> This also removes the need to declare the constants as `extern` which
> should using them easier (e.g., simpler linkage requirements).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am d434001fbc49d337b6e29f6ac8c9c7475922a819 
>   3rdparty/libprocess/include/process/timeseries.hpp 
> 64b10a8d551ba33e252aa33987e3d5da8d56a1d6 
>   3rdparty/libprocess/src/CMakeLists.txt 
> cf443dffd0663ecf02b7efd6f7094175b94aae19 
>   3rdparty/libprocess/src/timeseries.cpp 
> 528adef504ea82eb4bec309d8d17e6347c652a29 
> 
> 
> Diff: https://reviews.apache.org/r/67522/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67521: Made some headers standalone.