Re: Review Request 46822: Avoided slicing of flags in subprocess.

[Benjamin Bannier]

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

(Updated July 5, 2016, 7:46 a.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


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


Repository: mesos


Description
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.


Diffs (updated)
-

  src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
  src/launcher/posix/executor.cpp 6814b9f2333a9f040228c41e1d7101b93c760c01 
  src/linux/perf.cpp ea823b32edaa82a71cbfac9932aae543c0d7bef4 
  src/slave/container_loggers/lib_logrotate.cpp 
d53847bbca83367927725f06e8f962ce7011f468 
  src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
  src/slave/containerizer/mesos/containerizer.cpp 
f53b01b0eef8dd24db28d9dbd86bcbd40dc8d17f 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
842f2b5d4037e076cac4fd9c2eeb8f69786cffa7 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
79ee960aa70f8bd39f9bf639cc54f6395ff21ad5 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
3dfe7ad4477dd1a6c8585bf761eaf435fd0cb366 
  src/slave/containerizer/mesos/launcher.hpp 
bf435e3a9c150648336a1becf2f075fa183428bd 
  src/slave/containerizer/mesos/launcher.cpp 
9efe8474a2210957ce256fc08cb35694194213c3 
  src/slave/containerizer/mesos/linux_launcher.hpp 
c1852226c74bc611d045be721e284141e59adcd9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
95dee95c5e6e613e526c92d8729ae5583c8b58f1 
  src/tests/containerizer/isolator_tests.cpp 
488747347f71a6a1bb6bc01477143d077d4fd3eb 
  src/tests/containerizer/launch_tests.cpp 
ef0c87c96e6a8379d119246a8ad044248522e67e 
  src/tests/containerizer/launcher.hpp 49f8ecb47abb098bf76017f79b7d00380401efe4 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
57588cc1fb918924a163bdb40b195cc5f4231f6e 
  src/tests/containerizer/ns_tests.cpp cd668ebb3b9461bee00dc338c288e5df6eb8fe31 
  src/tests/containerizer/port_mapping_tests.cpp 
3675c02453160417b24c8b3b0d001208504515a5 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On July 4, 2016, 11:04 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49617/
> ---
> 
> (Updated July 4, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5780
> https://issues.apache.org/jira/browse/MESOS-5780
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - This benchmark measures latency to stability of
>   the allocator following disconnection and
>   reconnection of all frameworks.
> - In this scenario, frameworks are offered resources
>   and suppressed in batches.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49617/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49616: Add suppression benchmark.

[Guangya Liu]

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



I also did some test with patch https://reviews.apache.org/r/43666/ which 
removes the suppressed frameworks from sorter, and this can help improve the 
performance much if there are many frameworks in the cluster was suppressed. 
The time is decreasing with the number of suppressed frameworks increasing.

I will contact @Dario Rexin to see how we can have the patch merged.

round 0 allocate took 3.196943secs to make 199 offers
round 1 allocate took 3.30666secs to make 198 offers
round 2 allocate took 3.118484secs to make 197 offers
round 3 allocate took 3.097783secs to make 196 offers
round 4 allocate took 3.007793secs to make 195 offers
round 5 allocate took 3.070196secs to make 194 offers
round 6 allocate took 3.098805secs to make 193 offers
round 7 allocate took 2.930603secs to make 192 offers
round 8 allocate took 3.251535secs to make 191 offers
round 9 allocate took 3.199918secs to make 190 offers
round 10 allocate took 2.929129secs to make 189 offers
round 11 allocate took 3.039566secs to make 188 offers
round 12 allocate took 3.16028secs to make 187 offers
round 13 allocate took 3.143058secs to make 186 offers
round 14 allocate took 3.155646secs to make 185 offers
round 15 allocate took 3.457729secs to make 184 offers
round 16 allocate took 2.994027secs to make 183 offers
round 17 allocate took 2.997673secs to make 182 offers
round 18 allocate took 3.160014secs to make 181 offers
round 19 allocate took 2.862197secs to make 180 offers
round 20 allocate took 2.804424secs to make 179 offers
round 21 allocate took 2.764234secs to make 178 offers
round 22 allocate took 2.885941secs to make 177 offers
round 23 allocate took 2.864947secs to make 176 offers
round 24 allocate took 2.759227secs to make 175 offers
round 25 allocate took 2.797377secs to make 174 offers
round 26 allocate took 2.881332secs to make 173 offers
round 27 allocate took 2.697596secs to make 172 offers
round 28 allocate took 2.727784secs to make 171 offers
round 29 allocate took 2.744891secs to make 170 offers
round 30 allocate took 2.736105secs to make 169 offers
round 31 allocate took 2.779183secs to make 168 offers
round 32 allocate took 2.816875secs to make 167 offers
.

- Guangya Liu


On 七月 4, 2016, 11:18 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated 七月 4, 2016, 11:18 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49615: Implemented 'shouldInject()' in the 'NvidiaVolume' component.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49598, 49615]

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

- Mesos ReviewBot


On July 4, 2016, 10:09 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49615/
> ---
> 
> (Updated July 4, 2016, 10:09 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5401
> https://issues.apache.org/jira/browse/MESOS-5401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We use the the `com.nvidia.volumes.needed` label from
> nvidia-docker to decide if we should inject the volume or not:
> 
> https://github.com/NVIDIA/nvidia-docker/wiki/Image-inspection
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
> 9aac3cc8622c21014de87576f84180d30ae3fadd 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 474040c89d69b50c051122d873c11a102b33c538 
> 
> Diff: https://reviews.apache.org/r/49615/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j check && 
> GTEST_FILTER="*NVIDIA_GPU_VolumeShouldInject" bin/mesos-tests.sh`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49616: Add suppression benchmark.

[Guangya Liu]

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




src/tests/hierarchical_allocator_tests.cpp (line 3617)


I think that it is better move this test case above 
`HierarchicalAllocator_BENCHMARK_Test, Metrics`

Also what about rename this test case as 
`HierarchicalAllocator_BENCHMARK_Test, SuppressOffers`



src/tests/hierarchical_allocator_tests.cpp (line 3674)


kill this line



src/tests/hierarchical_allocator_tests.cpp (line 3711)


Not yours, but I think that it is better use `batch allocation` here 
instead of `background allocation`


- Guangya Liu


On 七月 4, 2016, 11:18 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated 七月 4, 2016, 11:18 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49612: Removed unnecessary `Clock::settle` calls from test cases.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49610, 49612]

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

- Mesos ReviewBot


On July 4, 2016, 8:40 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49612/
> ---
> 
> (Updated July 4, 2016, 8:40 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a test case calls `Clock::settle` and then immediately waits for a
> future to be completed, settling the clock is usually unnecessary.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49612/diff/
> 
> 
> Testing
> ---
> 
> `mesos-tests --gtest_filter=HierarchicalAllocatorTest.* --gtest_repeat=500 
> --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49604: Fixed incorrect clock time in log messages.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49602, 49604]

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

- Mesos ReviewBot


On July 4, 2016, 6:21 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49604/
> ---
> 
> (Updated July 4, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previous code included a pointer value in a log message, rather
> than the clock time that the pointer points at.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/clock.cpp 3fd616e91c9b8c2ac2d14a49714d340ba5a3a5da 
> 
> Diff: https://reviews.apache.org/r/49604/diff/
> 
> 
> Testing
> ---
> 
> Previous log output:
> 
> ```
> I0704 20:15:47.270100 1970868224 clock.cpp:329] Clock paused at 0x7ff72842d310
> ```
> 
> Revised log output:
> 
> ```
> I0704 20:17:59.394853 1970868224 clock.cpp:329] Clock paused at 2016-07-04 
> 18:17:59.394846976+00:00
> ```
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49610: Added test case for quota allocation with reserved resources.

[Guangya Liu]

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




src/tests/hierarchical_allocator_tests.cpp (line 3192)


Can you please move this case right before `HierarchicalAllocatorTest, 
DeactivateAndReactivateFramework` to group all `quota` related test case?



src/tests/hierarchical_allocator_tests.cpp (lines 3208 - 3218)


Does there are any special reason that you are using `dynamic reservation` 
here? Seems using `static reservation` may be more simple as your test is 
against allocation but not dynamic reservation.


- Guangya Liu


On 七月 4, 2016, 8:39 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49610/
> ---
> 
> (Updated 七月 4, 2016, 8:39 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test checks that when setting aside unallocated resources to ensure
> that a quota guarantee can be met, we don't use resources that have been
> reserved for a different role.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0498cd5e54b0e4b87a767585a77699653aa52179 
> 
> Diff: https://reviews.apache.org/r/49610/diff/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests 
> --gtest_filter="HierarchicalAllocatorTest.QuotaSetAsideReservedResources" 
> --gtest_repeat=100 --gtest_break_on_failure` passes. When you apply a patch 
> that breaks quota allocation behavior 
> (https://gist.github.com/neilconway/97eed34e9b0b312b8511f0f4f57fe061), 
> previously `make check` passes, but it fails with this new test added.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 49616: Add suppression benchmark.

[Jacob Janco]

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

Review request for mesos.


Repository: mesos


Description
---

- Useful for high framework clusters which carry
  many suppressed frameworks that are considered
  during allocation.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

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


Testing
---

make check


Thanks,

Jacob Janco

Re: Review Request 46823: Fully qualified addresses of Flag members in add calls in stout.

[Benjamin Bannier]

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

(Updated July 5, 2016, 1:14 a.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
3f3e21514bd5e2e388165eb64d540764097557ac 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46825: Fully-typed all FlagsBase::add overloads.

[Benjamin Bannier]

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

(Updated July 5, 2016, 1:13 a.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Fully-typed all FlagsBase::add overloads.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/flags.hpp 
dd9362772d1fbd32638fc7e70126fd49d4a03c68 
  3rdparty/stout/tests/flags_tests.cpp 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
  3rdparty/stout/tests/subcommand_tests.cpp 
9213d6b9faec30b5be320ab37ca29c2406c964ac 

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


Testing
---

make check (OS X clang-trunk w/o optimizations)

The mesos tests suite do not execute at all with an optimizing clang-trunk 
since copying of the `map` inside `FlagsBase` reference random memory leading 
to `SEGFAULTs`. With these patches all of `stout-tests` and `libprocess-tests` 
can be executed, but some remaining failures persist in `mesos-tests` (their 
number is reduced though).


Thanks,

Benjamin Bannier

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

[Benjamin Bannier]

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

(Updated July 5, 2016, 1:13 a.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


Changes
---

Fixed missed new uses under Linux.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  src/cli/execute.cpp 18c2e3449bf5e50bea0bbd0a92efa20fc8b9032b 
  src/cli/resolve.cpp ac6be05e64c5e66703081068d992ebb0d6ca6c70 
  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/examples/balloon_framework.cpp 5613ff0c61e2d2f84684a206debc97dcb8b2c0d3 
  src/examples/dynamic_reservation_framework.cpp 
850bb2a5b243dd5d5a8b6476570b4f943fde6185 
  src/examples/load_generator_framework.cpp 
5402e31b89b7ead1dc8fdc9065980b5b1c0d380c 
  src/examples/long_lived_framework.cpp 
7c57eb5e4a34208504475013690ae8e3cae74155 
  src/examples/no_executor_framework.cpp 
57425726aa5ca27c9579b0d8ecc0bb9eb9b26852 
  src/examples/persistent_volume_framework.cpp 
fe2837cfffb6dd251ccb9c93197f623d0c55fb36 
  src/examples/test_framework.cpp a83766a92617d50eadd92ec55113e049a7290d03 
  src/examples/test_http_framework.cpp cf6baed07c862ccade080618f33cc921fc09415d 
  src/launcher/executor.cpp 5a5f95f04a6ce096079b67397cb324575409f795 
  src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
  src/master/main.cpp 84f3b0723fd3df02386c8072ded3cb42272cd065 
  src/slave/container_loggers/lib_logrotate.hpp 
8c5602da3e5ff7bcf758da61723b7a0ea00a6a6e 
  src/slave/container_loggers/logrotate.hpp 
16d92322079a69d8e6bed7830623c62f345cd51c 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
79ee960aa70f8bd39f9bf639cc54f6395ff21ad5 
  src/slave/containerizer/mesos/launch.cpp 
09733abc9b0ecc599efb114af8231c31c7ec6ffc 
  src/slave/containerizer/mesos/mount.cpp 
dbd7853a43ee1402f2f91d933a657010efdd76b0 
  src/slave/main.cpp a7ed669a0b6861d6ce8546dfafac849044a77eec 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46821: Avoided slicing of flags in subprocess in libprocess and stout.

[Benjamin Bannier]

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

(Updated July 5, 2016, 1:13 a.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Fixed ssl use case.


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


Repository: mesos


Description
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
a929cc90fc9ee975f3635957518ced4eb70bdfeb 
  3rdparty/libprocess/include/process/subprocess_base.hpp 
e0c6b2d930fbd2cfe011e6faf44843b83ab1db27 
  3rdparty/libprocess/src/subprocess.cpp 
44073146118b6c6e9e730b8c901852594080a3eb 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
3f3e21514bd5e2e388165eb64d540764097557ac 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46822: Avoided slicing of flags in subprocess.

[Benjamin Bannier]

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

(Updated July 5, 2016, 1:13 a.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Fixed misses Linux flags uses.


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


Repository: mesos


Description
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.


Diffs (updated)
-

  src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
  src/linux/perf.cpp ea823b32edaa82a71cbfac9932aae543c0d7bef4 
  src/slave/container_loggers/lib_logrotate.cpp 
d53847bbca83367927725f06e8f962ce7011f468 
  src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
  src/slave/containerizer/mesos/containerizer.cpp 
f53b01b0eef8dd24db28d9dbd86bcbd40dc8d17f 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
842f2b5d4037e076cac4fd9c2eeb8f69786cffa7 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
79ee960aa70f8bd39f9bf639cc54f6395ff21ad5 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
3dfe7ad4477dd1a6c8585bf761eaf435fd0cb366 
  src/slave/containerizer/mesos/launcher.hpp 
bf435e3a9c150648336a1becf2f075fa183428bd 
  src/slave/containerizer/mesos/launcher.cpp 
9efe8474a2210957ce256fc08cb35694194213c3 
  src/slave/containerizer/mesos/linux_launcher.hpp 
c1852226c74bc611d045be721e284141e59adcd9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
95dee95c5e6e613e526c92d8729ae5583c8b58f1 
  src/tests/containerizer/isolator_tests.cpp 
488747347f71a6a1bb6bc01477143d077d4fd3eb 
  src/tests/containerizer/launch_tests.cpp 
ef0c87c96e6a8379d119246a8ad044248522e67e 
  src/tests/containerizer/launcher.hpp 49f8ecb47abb098bf76017f79b7d00380401efe4 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
57588cc1fb918924a163bdb40b195cc5f4231f6e 
  src/tests/containerizer/ns_tests.cpp cd668ebb3b9461bee00dc338c288e5df6eb8fe31 
  src/tests/containerizer/port_mapping_tests.cpp 
3675c02453160417b24c8b3b0d001208504515a5 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

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

(Updated July 4, 2016, 11:04 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

- This benchmark measures latency to stability of
  the allocator following disconnection and
  reconnection of all frameworks.
- In this scenario, frameworks are offered resources
  and suppressed in batches.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

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


Testing
---

make check


Thanks,

Jacob Janco

Review Request 49617: Add benchmark for failover of many frameworks.

[Jacob Janco]

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

Review request for mesos.


Repository: mesos


Description
---

- This benchmark measures latency to stability of
  the allocator following disconnection and
  reconnection of all frameworks.
- In this scenario, frameworks are offered resources
  and suppressed in batches.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

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


Testing
---

make check


Thanks,

Jacob Janco

Review Request 49615: Implemented 'shouldInject()' in the 'NvidiaVolume' component.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

We use the the `com.nvidia.volumes.needed` label from
nvidia-docker to decide if we should inject the volume or not:

https://github.com/NVIDIA/nvidia-docker/wiki/Image-inspection


Diffs
-

  src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
9aac3cc8622c21014de87576f84180d30ae3fadd 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
474040c89d69b50c051122d873c11a102b33c538 

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


Testing
---

`GTEST_FILTER="" make -j check && GTEST_FILTER="*NVIDIA_GPU_VolumeShouldInject" 
bin/mesos-tests.sh`


Thanks,

Kevin Klues

Re: Review Request 49223: Enhance value parsing.

[Guangya Liu]

> On 七月 4, 2016, 11:55 a.m., Guangya Liu wrote:
> > src/common/values.cpp, line 673
> > 
> >
> > Sorry, I should ask this question in previous patch. Same as above, can 
> > you please show more comments for what do you want to check here? It would 
> > be great if you can make the comment easy to understand.
> 
> Klaus Ma wrote:
> Only [0-9a-z./] is available for Text in document.

OK, then what about "Check `text resource` in the format of 
`[a-zA-Z0-9_/.-]`.", ditto for above.


- Guangya


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


On 七月 4, 2016, 10:33 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated 七月 4, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49600: Added authz to /files/debug endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49600]

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

- Mesos ReviewBot


On July 4, 2016, 5:41 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49600/
> ---
> 
> (Updated July 4, 2016, 5:41 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Jan Schlicht.
> 
> 
> Bugs: MESOS-5369
> https://issues.apache.org/jira/browse/MESOS-5369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authz to /files/debug endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp fffa24cb07f73dd2bc8edc59fc276fdca5b86380 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
>   src/master/main.cpp 84f3b0723fd3df02386c8072ded3cb42272cd065 
>   src/slave/main.cpp a7ed669a0b6861d6ce8546dfafac849044a77eec 
>   src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 
> 
> Diff: https://reviews.apache.org/r/49600/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="FilesTest.DebugTest" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Review Request 49612: Removed unnecessary `Clock::settle` calls from test cases.

[Neil Conway]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

If a test case calls `Clock::settle` and then immediately waits for a
future to be completed, settling the clock is usually unnecessary.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

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


Testing
---

`mesos-tests --gtest_filter=HierarchicalAllocatorTest.* --gtest_repeat=500 
--gtest_break_on_failure`


Thanks,

Neil Conway

Review Request 49610: Added test case for quota allocation with reserved resources.

[Neil Conway]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

This test checks that when setting aside unallocated resources to ensure
that a quota guarantee can be met, we don't use resources that have been
reserved for a different role.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
0498cd5e54b0e4b87a767585a77699653aa52179 

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


Testing
---

`./src/mesos-tests 
--gtest_filter="HierarchicalAllocatorTest.QuotaSetAsideReservedResources" 
--gtest_repeat=100 --gtest_break_on_failure` passes. When you apply a patch 
that breaks quota allocation behavior 
(https://gist.github.com/neilconway/97eed34e9b0b312b8511f0f4f57fe061), 
previously `make check` passes, but it fails with this new test added.


Thanks,

Neil Conway

Re: Review Request 49412: Updated SSL.md with 'SSL_VERIFY_IPADD'.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [49412, 49411, 49402, 49401, 49400]

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

Error:
2016-07-04 19:46:23 URL:https://reviews.apache.org/r/49400/diff/raw/ 
[5866/5866] -> "49400.patch" [1]
error: patch failed: 3rdparty/libprocess/include/process/ssl/gtest.hpp:192
error: 3rdparty/libprocess/include/process/ssl/gtest.hpp: patch does not apply
error: patch failed: 3rdparty/libprocess/include/process/ssl/utilities.hpp:18
error: 3rdparty/libprocess/include/process/ssl/utilities.hpp: patch does not 
apply
error: patch failed: 3rdparty/libprocess/src/ssl/utilities.cpp:88
error: 3rdparty/libprocess/src/ssl/utilities.cpp: patch does not apply

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

- Mesos ReviewBot


On July 4, 2016, 4:31 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49412/
> ---
> 
> (Updated July 4, 2016, 4:31 p.m.)
> 
> 
> Review request for mesos, Adam B, Albert Strasheim, Artem Harutyunyan, Joris 
> Van Remoortere, and Lukas Loesche.
> 
> 
> Bugs: MESOS-5724
> https://issues.apache.org/jira/browse/MESOS-5724
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md 5eef9fd5afbe2b4984f8ba352438f8ea95676355 
> 
> Diff: https://reviews.apache.org/r/49412/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 49603: Fixed line in '3rdParty/Makefile.am' that had potential to 'rm -rf /'.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 4, 2016, 6:09 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49603/
> ---
> 
> (Updated July 4, 2016, 6:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed line in '3rdParty/Makefile.am' that had potential to 'rm -rf /'.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 97c2f9230058628502f375ba91510b8373216251 
> 
> Diff: https://reviews.apache.org/r/49603/diff/
> 
> 
> Testing
> ---
> 
> ```
> ../configure --prefix=$HOME/mesos-install --enable-install-module-dependencies
> make -j install
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49186: Added test for file volume from host sandbox mountpoint.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 4, 2016, 5:41 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49186/
> ---
> 
> (Updated July 4, 2016, 5:41 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Timothy Chen, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-5697
> https://issues.apache.org/jira/browse/MESOS-5697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for file volume from host sandbox mountpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> fd23519379755245ff7ebd4dab190d70e4a9f309 
> 
> Diff: https://reviews.apache.org/r/49186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49185: Added test for file volume from host.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 4, 2016, 5:40 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49185/
> ---
> 
> (Updated July 4, 2016, 5:40 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Timothy Chen, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-5697
> https://issues.apache.org/jira/browse/MESOS-5697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for file volume from host.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> fd23519379755245ff7ebd4dab190d70e4a9f309 
> 
> Diff: https://reviews.apache.org/r/49185/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49184: Implemented file volume support in mesos containerizer.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 4, 2016, 5:40 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49184/
> ---
> 
> (Updated July 4, 2016, 5:40 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Timothy Chen, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-5697
> https://issues.apache.org/jira/browse/MESOS-5697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented file volume support in mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> adacde191132a7a92c1fdf27ef8a4a41d2afa003 
> 
> Diff: https://reviews.apache.org/r/49184/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49223: Enhance value parsing.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49223]

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

- Mesos ReviewBot


On July 4, 2016, 10:33 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 49605: Renamed 'commands' to 'pre_exec_commands' in ContainerLaunchInfo.

[Gilbert Song]

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

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


Repository: mesos


Description
---

Renamed 'commands' to 'pre_exec_commands' in ContainerLaunchInfo.


Diffs
-

  include/mesos/slave/isolator.proto 22be7c0914a87b16342735bcf455b489da15c52f 
  src/slave/containerizer/mesos/containerizer.cpp 
856dd8238db7c3b98cb53e628e267f64238a2cb7 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
c72d3603ddf7dc698ecddd88b9f7b9e9a2c7aa0b 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
adacde191132a7a92c1fdf27ef8a4a41d2afa003 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
1846806831a46bb72d4ed5d2e136861301d8dd6a 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
bfc469894aa0a960a1f295df5f85134e8de6d50b 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
25d2a57d7a814d735db78bda8c6956199de5c390 
  src/tests/containerizer/isolator_tests.cpp 
7b4d47bd9e99b71269093d7c11559f3b74a3e22b 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
7ffec9ec9788d0e15c2c0ccc7d10b8b64ad40057 
  src/tests/containerizer/port_mapping_tests.cpp 
eeb893823ba9e6574232dd71c61c76836089ca06 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 49603: Fixed line in '3rdParty/Makefile.am' that had potential to 'rm -rf /'.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 4, 2016, 11:09 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49603/
> ---
> 
> (Updated July 4, 2016, 11:09 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed line in '3rdParty/Makefile.am' that had potential to 'rm -rf /'.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 97c2f9230058628502f375ba91510b8373216251 
> 
> Diff: https://reviews.apache.org/r/49603/diff/
> 
> 
> Testing
> ---
> 
> ```
> ../configure --prefix=$HOME/mesos-install --enable-install-module-dependencies
> make -j install
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 49604: Fixed incorrect clock time in log messages.

[Neil Conway]

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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

Previous code included a pointer value in a log message, rather
than the clock time that the pointer points at.


Diffs
-

  3rdparty/libprocess/src/clock.cpp 3fd616e91c9b8c2ac2d14a49714d340ba5a3a5da 

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


Testing
---

Previous log output:

```
I0704 20:15:47.270100 1970868224 clock.cpp:329] Clock paused at 0x7ff72842d310
```

Revised log output:

```
I0704 20:17:59.394853 1970868224 clock.cpp:329] Clock paused at 2016-07-04 
18:17:59.394846976+00:00
```


Thanks,

Neil Conway

Re: Review Request 49003: Added a Contributing to Mesos blog post.

[Michael Park]

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

(Updated July 4, 2016, 6:16 p.m.)


Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


Repository: mesos


Description
---

Added a Contributing to Mesos blog post.


Diffs (updated)
-

  site/source/assets/img/blog/contributing_to_mesos_commit_diversity.png 
PRE-CREATION 
  site/source/assets/img/blog/contributing_to_mesos_community_growing.png 
PRE-CREATION 
  site/source/assets/img/blog/contributing_to_mesos_contributor_diversity.png 
PRE-CREATION 
  site/source/blog/2016-06-20-contributing-to-mesos.md PRE-CREATION 

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


Testing
---


Thanks,

Michael Park

Review Request 49603: Fixed line in '3rdParty/Makefile.am' that had potential to 'rm -rf /'.

[Kevin Klues]

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

Review request for mesos, Jie Yu and Kapil Arya.


Repository: mesos


Description
---

Fixed line in '3rdParty/Makefile.am' that had potential to 'rm -rf /'.


Diffs
-

  3rdparty/Makefile.am 97c2f9230058628502f375ba91510b8373216251 

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


Testing
---

```
../configure --prefix=$HOME/mesos-install --enable-install-module-dependencies
make -j install
```


Thanks,

Kevin Klues

Review Request 49602: Fixed log message to avoid spanning multiple lines.

[Neil Conway]

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

Review request for mesos, Anand Mazumdar and Greg Mann.


Repository: mesos


Description
---

Fixed log message to avoid spanning multiple lines.


Diffs
-

  src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
  src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 

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


Testing
---

Previous output:

```
I0704 19:52:23.530931 1970868224 resources.cpp:572] Parsing resources as JSON 
failed: cpus:4;mem:512;disk:0
Trying semicolon-delimited string format instead
I0704 19:52:23.530920 528384 process.cpp:2676] Resuming (1)@192.168.0.104:61317 
at 2016-07-04 17:52:23.530142976+00:00
```

New output:

```
I0704 19:58:00.678043 1970868224 resources.cpp:572] Parsing resources as JSON 
failed: 'cpus:4;mem:512;disk:0'; trying semicolon-delimited string format 
instead
I0704 19:58:00.678032 2138112 process.cpp:2676] Resuming 
(1)@192.168.0.104:61374 at 2016-07-04 17:58:00.675815168+00:00
```

Note that I'm not convinced this is a super useful thing to be logging, even at 
`VLOG(1)`, so I'd also be fine with just removing the log statement altogether.


Thanks,

Neil Conway

Re: Review Request 49598: Added ability to parse docker v1 'ImageManifest' from 'docker inspect'.

[Kevin Klues]

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

(Updated July 4, 2016, 5:57 p.m.)


Review request for mesos, Benjamin Mahler and Yubo Li.


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


Repository: mesos


Description
---

The `docker::spec::v1::ImageManifest` protobuf implements the
official v1 image manifest specification found at:

https://github.com/docker/docker/blob/master/image/spec/v1.md

The field names in this spec are all written in snake_case as are the
field names of the JSON representing the image manifest when reading
it from disk (for example after performing a `docker save`). As such,
the protobuf for ImageManifest also provides these fields in
snake_case. Unfortunately, the `docker inspect` command also provides
a method of retrieving the JSON for an image manifest, with one major
caveat -- it represents all of its top level keys in CamelCase.

To allow both representations to be parsed in the same way, we
intercept the incoming JSON from either source (disk or `docker
inspect`) and convert it to a canonical snake_case representation.


Diffs
-

  src/docker/spec.cpp 2711578626dd1847f73cbf7bd3771f36e6755a99 
  src/tests/containerizer/docker_tests.cpp 
49bd7c252c342c8cb29ea916ad3f1f71638d2017 

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


Testing
---

`sudo GTEST_FILTER="*ROOT_DOCKER_CompareImageManifests" bin/mesos-tests.sh`


Thanks,

Kevin Klues

Review Request 49598: Added ability to parse docker v1 'ImageManifest' from 'docker inspect'.

[Kevin Klues]

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

Review request for mesos, Benjamin Mahler and Yubo Li.


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


Repository: mesos


Description
---

The `docker::spec::v1::ImageManifest` protobuf implements the
official v1 image manifest specification found at:

https://github.com/docker/docker/blob/master/image/spec/v1.md

The field names in this spec are all written in snake_case as are the
field names of the JSON representing the image manifest when reading
it from disk (for example after performing a `docker save`). As such,
the protobuf for ImageManifest also provides these fields in
snake_case. Unfortunately, the `docker inspect` command also provides
a method of retrieving the JSON for an image manifest, with one major
caveat -- it represents all of its top level keys in CamelCase.

To allow both representations to be parsed in the same way, we
intercept the incoming JSON from either source (disk or `docker
inspect`) and convert it to a canonical snake_case representation.


Diffs
-

  src/docker/spec.cpp 2711578626dd1847f73cbf7bd3771f36e6755a99 
  src/tests/containerizer/docker_tests.cpp 
49bd7c252c342c8cb29ea916ad3f1f71638d2017 

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


Testing
---

`sudo GTEST_FILTER="*ROOT_DOCKER_CompareImageManifests" bin/mesos-tests.sh`


Thanks,

Kevin Klues

Review Request 49600: Added authz to /files/debug endpoint.

[Abhishek Dasgupta]

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

Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and Jan 
Schlicht.


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


Repository: mesos


Description
---

Added authz to /files/debug endpoint.


Diffs
-

  src/common/http.cpp fffa24cb07f73dd2bc8edc59fc276fdca5b86380 
  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
  src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
  src/master/main.cpp 84f3b0723fd3df02386c8072ded3cb42272cd065 
  src/slave/main.cpp a7ed669a0b6861d6ce8546dfafac849044a77eec 
  src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 

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


Testing
---

On Ubuntu 16.04:

sudo GTEST_FILTER="FilesTest.DebugTest" make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49186: Added test for file volume from host sandbox mountpoint.

[Gilbert Song]

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

(Updated July 4, 2016, 10:41 a.m.)


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


Changes
---

rebase


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


Repository: mesos


Description
---

Added test for file volume from host sandbox mountpoint.


Diffs (updated)
-

  src/tests/containerizer/filesystem_isolator_tests.cpp 
fd23519379755245ff7ebd4dab190d70e4a9f309 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 49185: Added test for file volume from host.

[Gilbert Song]

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

(Updated July 4, 2016, 10:40 a.m.)


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


Changes
---

rebase


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


Repository: mesos


Description
---

Added test for file volume from host.


Diffs (updated)
-

  src/tests/containerizer/filesystem_isolator_tests.cpp 
fd23519379755245ff7ebd4dab190d70e4a9f309 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 49184: Implemented file volume support in mesos containerizer.

[Gilbert Song]

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

(Updated July 4, 2016, 10:40 a.m.)


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


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


Repository: mesos


Description
---

Implemented file volume support in mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
adacde191132a7a92c1fdf27ef8a4a41d2afa003 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 49184: Implemented file volume support in mesos containerizer.

[Gilbert Song]

> On June 25, 2016, 8:42 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp, line 429
> > 
> >
> > Just nit: What about adding a comment here, someone may be confused as 
> > when they saw that you only handled `file volume` for container with rootfs 
> > but not for containers without rootfs.
> > 
> > // The container path can be either a directory or a
> > // file. If the conainer path is a file here, the file
> > // will be mounted as a volume to the container as well.

Thanks! I will address it.


- Gilbert


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


On June 24, 2016, 12:19 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49184/
> ---
> 
> (Updated June 24, 2016, 12:19 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Timothy Chen, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-5697
> https://issues.apache.org/jira/browse/MESOS-5697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented file volume support in mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> adacde191132a7a92c1fdf27ef8a4a41d2afa003 
> 
> Diff: https://reviews.apache.org/r/49184/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49273: Helper binary for executors to chroot tasks.

[Jie Yu]

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



Please see the test section of https://reviews.apache.org/r/49569/

Let me know if that's ok or not. Thanks!

- Jie Yu


On June 27, 2016, 5:09 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49273/
> ---
> 
> (Updated June 27, 2016, 5:09 p.m.)
> 
> 
> Review request for mesos, Joshua Cohen and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Uses the same code as the agent uses for chroot'ing on Linux, i.e., 
> pivot_root and setting up /dev etc. Intention is that executors (like 
> Aurora's Thermos) can use it to chroot tasks.
> 
> Currently, the root path is specified as a flag and the remaining arguments 
> are exec'ed. Joshua has also requested that the root path could be specified 
> as the first arg. @Jie, thoughts?
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 86c39fdf379ada470c9b1f86be263ef71dc47c41 
>   src/slave/containerizer/mesos-chroot.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49273/diff/
> 
> 
> Testing
> ---
> 
> Manual.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 49003: Added a Contributing to Mesos blog post.

[Michael Park]

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

(Updated July 4, 2016, 5:10 p.m.)


Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


Repository: mesos


Description
---

Added a Contributing to Mesos blog post.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
a929cc90fc9ee975f3635957518ced4eb70bdfeb 
  3rdparty/libprocess/include/process/ssl/utilities.hpp 
c2f64a91a505675d568ddf5aa081125e4e32fe17 
  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
4d376d8b7c1b29105de69bed2e4077f8c94fed0b 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
2e7f33241b5291593cac4ea4c8f0351c19f7f0c2 
  3rdparty/libprocess/src/openssl.hpp 68f88970610293107b8349c216c34a32d5df9105 
  3rdparty/libprocess/src/openssl.cpp 63916ff66b4daa29120b7e6b12b329b68f746694 
  3rdparty/libprocess/src/ssl/utilities.cpp 
8aec613312eee3dd11d9df8c3828a5185407e073 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
72432ec8f91eb4d628df73e19edbaceb8a2c0fb9 
  CHANGELOG cacfb5bf029e711a83b706d789c454620dc85cad 
  docs/ssl.md 64f63b7c23ff97c027c24fad10ab7fa46ccc8117 
  site/source/assets/img/blog/contributing_to_mesos_commit_diversity.png 
PRE-CREATION 
  site/source/assets/img/blog/contributing_to_mesos_community_growing.png 
PRE-CREATION 
  site/source/assets/img/blog/contributing_to_mesos_contributor_diversity.png 
PRE-CREATION 
  site/source/blog/2016-06-20-contributing-to-mesos.md PRE-CREATION 
  src/master/allocator/mesos/hierarchical.cpp 
c1e00039606164599e25ff5f76245e4d35ec3112 
  src/master/allocator/sorter/drf/sorter.hpp 
e29feebd70277c79f7c3f6fb233e7a36501cf220 
  src/master/allocator/sorter/drf/sorter.cpp 
7df4dd641b21ea0705368861bf4679fed1ef078d 
  src/master/allocator/sorter/sorter.hpp 
f5f0b086cb95eb2ab70b3f67e5b20814925bf702 
  src/tests/persistent_volume_endpoints_tests.cpp 
2a22f3b0da817e650a25e5e2c951adb7462718b4 
  src/tests/sorter_tests.cpp 0a921347586808863e615ca3dcc23fae92b629f5 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 49587: Added "fair_sharing_excluded_resource_names" to configuration.md.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49587]

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

- Mesos ReviewBot


On July 4, 2016, 9:59 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49587/
> ---
> 
> (Updated July 4, 2016, 9:59 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5758
> https://issues.apache.org/jira/browse/MESOS-5758
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added "fair_sharing_excluded_resource_names" to configuration.md.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 8c8678c7e2251923298b90b7216a4e584faf6b26 
> 
> Diff: https://reviews.apache.org/r/49587/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> https://github.com/gyliu513/mesos/blob/master/docs/configuration.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 49568: Simplified a flag in the launch helper binary.

[Jie Yu]

> On July 4, 2016, 8:09 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 1179
> > 
> >
> > Actually I really dont like `commands` in isolator.proto 
> > `ContainerLaunchInfo`. Good to consider a deprecation cycle to change it to 
> > `pre_exec_commands`.

This is internal to Mesos so I don't think we need a deprecation cycle (we can 
just change the name without changing the tag number).


- Jie


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


On July 3, 2016, 5:50 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49568/
> ---
> 
> (Updated July 3, 2016, 5:50 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Ian Downes.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Using JSON::Object is not necessary. Also, the current name is quite
> confusing as we have another flag called '--command'. This patch
> simplified this flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a96b382f22886362a1159e1166dfe041072985ba 
>   src/slave/containerizer/mesos/launch.hpp 
> c716e0396736d1f2f60ec31540f12f4f7597d081 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49568/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49549: Used the launch helper binary to launch user task in command executor.

[Jie Yu]

> On July 4, 2016, 8:03 a.m., Gilbert Song wrote:
> > This patch is great!
> > 
> > I am still thinking it may be a little risky for 1.0 using command task 
> > with container image, because this is what we are currently doing:
> > 
> > mesos containerizer fork a subprocess -> MesosContainerizerLaunch 
> > subcommand -> execvp mesos-executor binary -> executor fork a subprocess -> 
> > containerizer binary to call MesosContainerizerLaunch subcommand.

I don't think it's too risky. It also addressed some of the long standing 
TODO's in our code (for instance, to make command executor async signal safe 
after fork). I think this is the right way forward.


- Jie


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


On July 2, 2016, 1:02 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49549/
> ---
> 
> (Updated July 2, 2016, 1:02 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, command executor and 'mesos-containerizer launch' share a
> lot of the logic. Command executor should in fact, just use
> `mesos-containerizer launch` to launch the user task. Potentially,
> 'mesos-containerizer launch' can be also used by custom executor to
> launch user tasks.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
>   src/launcher/posix/executor.hpp e5f3c0a8e2d3687a5330adbf2f667db9106970c7 
>   src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49549/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49541: Renamed healthCheckDir to launcherDir in command executor.

[Jie Yu]

> On July 4, 2016, 6:08 a.m., Gilbert Song wrote:
> > src/launcher/executor.cpp, lines 664-701
> > 
> >
> > This method was removed. Could you rebase?

yep. I'll rebase.


> On July 4, 2016, 6:08 a.m., Gilbert Song wrote:
> > src/launcher/executor.cpp, line 767
> > 
> >
> > My bad. I should have remove this var on health check patches. Could 
> > you please just remove `healthCheckDir` and update the commit msg in this 
> > patch? Thanks.

It was used later in the patch chain.


- Jie


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


On July 1, 2016, 11:30 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49541/
> ---
> 
> (Updated July 1, 2016, 11:30 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Joris Van Remoortere, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed healthCheckDir to launcherDir in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp bb88a4570d183d22dff65f6e220d566c0924001a 
> 
> Diff: https://reviews.apache.org/r/49541/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49540: Used the argv version for command that launches the command executor.

[Jie Yu]

> On July 4, 2016, 5:59 a.m., Gilbert Song wrote:
> > src/slave/slave.cpp, line 3964
> > 
> >
> > Hmm.. seems like we dont have `mesos-executor` as the first argv for a 
> > while. Just curious that is not supposed to work correctly, right?

Because we used shell=true version previously.


- Jie


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


On July 1, 2016, 11:24 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49540/
> ---
> 
> (Updated July 1, 2016, 11:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used the argv version for command that launches the command executor.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 
>   src/tests/slave_tests.cpp d800cb8c10b5730a711c8354802bdff6aaca8c01 
> 
> Diff: https://reviews.apache.org/r/49540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49412: Updated SSL.md with 'SSL_VERIFY_IPADD'.

[Till Toenshoff]

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

(Updated July 4, 2016, 4:31 p.m.)


Review request for mesos, Adam B, Albert Strasheim, Artem Harutyunyan, Joris 
Van Remoortere, and Lukas Loesche.


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


Repository: mesos


Description
---

see summary.


Diffs (updated)
-

  docs/ssl.md 5eef9fd5afbe2b4984f8ba352438f8ea95676355 

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


Testing
---


Thanks,

Till Toenshoff

Re: Review Request 49402: Added tests for IP based certificate validation.

[Till Toenshoff]

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

(Updated July 4, 2016, 4:30 p.m.)


Review request for mesos, Adam B, Albert Strasheim, Artem Harutyunyan, Joris 
Van Remoortere, and Lukas Loesche.


Changes
---

Fixed omission from manual rebase.


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


Repository: mesos


Description
---

see summary.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
5435ddda1fd7dfcff1a0b28f2abe35feb707ceeb 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
6fc90380fcc1d6c645d639edddba4b641f0fd624 

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


Testing
---

make check on OSX and various linux distros.


Thanks,

Till Toenshoff

Re: Review Request 49473: Made control pipe to mesos-containerizer launch optional.

[Jie Yu]

> On July 2, 2016, 8:09 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/launch.cpp, lines 374-375
> > 
> >
> > Also not yours. Could we fix the style here? Seems like from 
> > https://github.com/apache/mesos/commit/81e893d5cdb8a5873dd0c61ad7eceaa323aff466

This was fixed in the following patch.


- Jie


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


On June 30, 2016, 9:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49473/
> ---
> 
> (Updated June 30, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the first step allowing the command executor to directly use
> `mesos-containerizer launch` to launch user task. The control pipe is
> used to synchronize with the parent process. In the command executor,
> this synchronization is not needed. Therefore, we make it optional.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49473/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49425: Used os::raw::Argv in command executor.

[Jie Yu]

> On July 4, 2016, 9:50 a.m., Guangya Liu wrote:
> > src/launcher/windows/executor.cpp, lines 70-71
> > 
> >
> > remove this

I think this TODO is still valid. I think os::stringify_args should just take a 
vector (or an iterable).


- Jie


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


On June 30, 2016, 4:18 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49425/
> ---
> 
> (Updated June 30, 2016, 4:18 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used os::raw::Argv in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
>   src/launcher/windows/executor.cpp f6da398eeddaf68b8eaf510648c55964d9f5b7c7 
> 
> Diff: https://reviews.apache.org/r/49425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49319: Fixed incorrect comment on ACCESS_SANDBOX in authorizer.proto.

[Till Toenshoff]

> On June 28, 2016, 2:50 p.m., Alexander Rojas wrote:
> > include/mesos/authorizer/authorizer.proto, lines 90-91
> > 
> >
> > This comment is rather incomplete:
> > 
> > 1. Either they are both set or none.
> > 2. When none are set it is because the task has not yet created the 
> > directory (because is scheduled) or it is finished and there was a lag 
> > between access and destruction of the enpoint.
> > 
> > All in all, we should mention that having none set is an exceptional 
> > case, and could be treated by returning an error.
> 
> Joerg Schad wrote:
> 1. Are you sure from the setting code that 1. is true?
> 2. I am not sure this needs to mentioned here as this is just describes 
> the semantic, and a module writer has to deal with that no matter how 
> frequent it is.
> 
> We can surely mention this is an exceptional case, but I would answer 
> similarly as for 2.
> 
> Alexander Rojas wrote:
> 1. Yes, It is true!
> 2. if there is no object to authorize too, how do a module writer has to 
> react to it? This is an exceptional case and needs to be mention that it does 
> occur
> 
> Joerg Schad wrote:
> How about:
> // This action will have an object which might either have both a 
> `FrameworkInfo`,
> // and `ExecutorInfo`, or in exceptional cases nothing set.
> 
> Joerg Schad wrote:
> Btw Maybe we should later add a check which verfies this invariant...

Are you saying we should check for either "both" vs. "none" to make sure we 
never run into "some"; as in `FrameworkInfo` being set but `ExecutorInfo` not 
being set?


- Till


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


On June 28, 2016, 2:35 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49319/
> ---
> 
> (Updated June 28, 2016, 2:35 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5730
> https://issues.apache.org/jira/browse/MESOS-5730
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current semantic is that these fields might not be set.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> fc76796022a6fa3d36a1447c476980868d42c2d0 
> 
> Diff: https://reviews.apache.org/r/49319/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49424: Added an abstraction os::raw::Argv in stout.

[Jie Yu]

> On July 2, 2016, 5:11 p.m., Joris Van Remoortere wrote:
> > 3rdparty/stout/include/stout/os/raw/argv.hpp, line 40
> > 
> >
> > It seems like this extra vector and the subsequent copy into the argv 
> > array is only necessary because you're supporting structures with an 
> > unknown size.
> > 
> > Would this be simpler if you restricted to inputs that support 
> > `.size()`?

Yes, it would be simpler, but I still have to deal with issues like size() of 
protobuf repeated field returns an int while other std containers return size_t 
(unsigned). This is just pointer copies so it won't be too bad.


- Jie


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


On June 30, 2016, 4:18 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49424/
> ---
> 
> (Updated June 30, 2016, 4:18 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an abstraction os::raw::Argv in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am f10c836c1ac008cc4055741648b5e7dd697e4c1e 
>   3rdparty/stout/include/stout/os.hpp 
> 53b00932693fba7cf6514da6a519269a904de345 
>   3rdparty/stout/include/stout/os/raw/argv.hpp PRE-CREATION 
>   3rdparty/stout/tests/os_tests.cpp 786da14addf62be5f5270f156fb1e011d3f403e3 
> 
> Diff: https://reviews.apache.org/r/49424/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49400: Extended utilities to render certificate extension for IP.

[Till Toenshoff]

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

(Updated July 4, 2016, 4:17 p.m.)


Review request for mesos, Adam B, Albert Strasheim, Artem Harutyunyan, Joris 
Van Remoortere, and Lukas Loesche.


Changes
---

Removed pointless "struct" reference and fixed a possible leak.


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


Repository: mesos


Description
---

Adds the ability to render a subject alternative name based on a given
IP address within a X509 certificate extension. Additionally the
libprocess test suite makes use of this feature.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
5435ddda1fd7dfcff1a0b28f2abe35feb707ceeb 
  3rdparty/libprocess/include/process/ssl/utilities.hpp 
ad9ec5df9d3c3e46c775ac470bce92c5cb983b8f 
  3rdparty/libprocess/src/ssl/utilities.cpp 
02bfd1798c6037c88229bb0da33a614d4c598550 

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


Testing
---

make check on OSX and various linux distros.

Functional testing by validating a rendered certificate;

```
openssl x509 -text -noout -in "temp_cert_file_name"
```


Thanks,

Till Toenshoff

Re: Review Request 49415: Removed the argv parameter in command executor helper.

[Jie Yu]

> On July 1, 2016, 9:33 p.m., Gilbert Song wrote:
> > Do you miss `windows/executor.hpp`?
> 
> Gilbert Song wrote:
> Fix it, Ship it! :)

Thanks! Fixed it.


- Jie


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


On June 30, 2016, 12:54 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49415/
> ---
> 
> (Updated June 30, 2016, 12:54 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It's confusing to pass in 'argv' because it's already contained in
> 'command'. This patch removed this parameter.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp bb88a4570d183d22dff65f6e220d566c0924001a 
>   src/launcher/posix/executor.hpp e5f3c0a8e2d3687a5330adbf2f667db9106970c7 
>   src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
>   src/launcher/windows/executor.cpp f6da398eeddaf68b8eaf510648c55964d9f5b7c7 
> 
> Diff: https://reviews.apache.org/r/49415/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49401: Updated certificate validation to check 'IP Address' SAN.

[Till Toenshoff]

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

(Updated July 4, 2016, 4:09 p.m.)


Review request for mesos, Adam B, Albert Strasheim, Artem Harutyunyan, Joris 
Van Remoortere, and Lukas Loesche.


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


Repository: mesos


Description (updated)
---

Allows the verification of X509 certificates based on an IP address
instead of a hostname. Introduces a new environment variable;
\`SSL_VERIFY_IPADD\` which, when set to \`true\` will enable the
peer certificate verification to additionally rely on the IP
address of a connection.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
1dbdaa80d0726bcaa1cefc89c57bb7609b64 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
19d9ae59f1b474accaec924c4069c6b1ce995b46 
  3rdparty/libprocess/src/openssl.hpp 7d5502545ec5f8c495bd1d3f58a0f4b71bcb3386 
  3rdparty/libprocess/src/openssl.cpp 0f62aa6ade1c95e506fd06aa4806557ba6583433 

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


Testing
---

make check on OSX and various linux distros.


Thanks,

Till Toenshoff

Re: Review Request 49575: Fixed `delete` usage in authorization_test.cpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49369, 49370, 49574, 49575]

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

- Mesos ReviewBot


On July 4, 2016, 9:22 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49575/
> ---
> 
> (Updated July 4, 2016, 9:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Additionally this review fixes some minor
> formatting and typo issues.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 
> 
> Diff: https://reviews.apache.org/r/49575/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49246: Enhanced startsWith/endsWith's performance.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On July 4, 2016, 1:57 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49246/
> ---
> 
> (Updated July 4, 2016, 1:57 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-5715
> https://issues.apache.org/jira/browse/MESOS-5715
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced startsWith/endsWith's performance.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/strings.hpp 
> 162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
> 
> Diff: https://reviews.apache.org/r/49246/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49262: Added '--enable-netlink' into configure.ac.

[Qian Zhang]

> On June 28, 2016, 4:41 a.m., Jie Yu wrote:
> > configure.ac, lines 2087-2088
> > 
> >
> > Instead of copying it, can you place this check first and the 
> > `with_network_isolator` check can be simplified to check `enable_netlink` 
> > plus some additional functions that's needed by network port mapping 
> > isolator?
> > 
> > I think `nl_has_capability` was introduced in 3.2.25 or something, and 
> > only network port mapping isolator has this restriction. For getting stats, 
> > I don't think we need libnl 3.2.26 or higher.
> > 
> > Ideally, enable_netlink only requires the default libnl3 on 
> > ubuntu/centos (which is 3.2.21 I believe).

> Instead of copying it, can you place this check first and the 
> with_network_isolator check can be simplified to check enable_netlink plus 
> some additional functions that's needed by network port mapping isolator?

I had the same concern when I copied it. However I think `--enable-netlink` and 
`--with-network-isolator` are two separate options, that means user is free to 
specifiy both of them or either of them, so what about user only specifies 
`--with-network-isolator` but not `--enable-netlink`? If we want to avoid the 
copying, maybe we should make `--with-network-isolator` depend on 
`--enable-netlink`, i.e., `--with-network-isolator` only work when 
`--enable-netlink` is also specified, just like the relationship between 
`--enable-ssl` and `--enable-libevent`: if user only specifes `--enable-ssl` 
but not `--enable-libevent`, the `configure` script will fail with an error 
message `"SSL is currently only supported with libevent"`.


- Qian


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


On June 27, 2016, 9:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49262/
> ---
> 
> (Updated June 27, 2016, 9:03 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5646
> https://issues.apache.org/jira/browse/MESOS-5646
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '--enable-netlink' into configure.ac.
> 
> 
> Diffs
> -
> 
>   configure.ac 321436beb8ad87bb5727932eb2943986fe558237 
> 
> Diff: https://reviews.apache.org/r/49262/diff/
> 
> 
> Testing
> ---
> 
> After running `bootstrap` script, the newly introduced flag 
> `"--enable-netlink"` will appear in the `configure` script:
> 
> ```
> $ ../configure --help 
> `configure' configures mesos 1.0.0 to adapt to many kinds of systems.
>   ...
>   --enable-netlinkenable netlink for transferring miscellaneous
>   networking information between the kernel space and
>   userspace processes default: no
>   ...
> ```
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 49246: Enhanced startsWith/endsWith's performance.

[Klaus Ma]

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

(Updated July 4, 2016, 9:57 p.m.)


Review request for mesos and Michael Park.


Changes
---

Update format.


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


Repository: mesos


Description
---

Enhanced startsWith/endsWith's performance.


Diffs (updated)
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49377: Fixed allocator to update total resources in quota sorter.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On July 4, 2016, 8:53 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49377/
> ---
> 
> (Updated July 4, 2016, 8:53 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Each DRFSorter tracks the total resources in the cluster. This means
> that each sorter must be updated when the resources in the cluster have
> changed, e.g., due to the creation of a dynamic reservation or a
> persistent volume. In the previous implementation, the quota role sorter
> was not updated for non-quota roles when a reservation or persistent
> volume was created by a framework. This resulted in inconsistency
> between the total resources in the allocator and the quota role sorter.
> 
> This could cause several problems. First, removing a slave from the
> cluster would leak resources in the quota role sorter. Second, certain
> interleavings of slave removals and reserve/unreserve operations by
> frameworks and via HTTP endpoints could lead to CHECK failures.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> eca949e07abb00423a2f35a56eedc5d4287d81f3 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 971a7b38d26b26c8543815e467f30cbb83ee412c 
> 
> Diff: https://reviews.apache.org/r/49377/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX and recent Arch Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49323: Added tests that combine the two ways of creating volumes.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On July 4, 2016, 8:32 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49323/
> ---
> 
> (Updated July 4, 2016, 8:32 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests check that dynamic reservations and persistent volumes can
> be created via the offer API and then removed via the HTTP endpoints,
> and vice versa.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 971a7b38d26b26c8543815e467f30cbb83ee412c 
> 
> Diff: https://reviews.apache.org/r/49323/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes without any Mesos changes. i.e., it doesn't reproduce the 
> bug by itself, but having more test coverage for this scenario seems wise.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49377: Fixed allocator to update total resources in quota sorter.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49323, 49375, 49376, 49377]

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

- Mesos ReviewBot


On July 4, 2016, 8:53 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49377/
> ---
> 
> (Updated July 4, 2016, 8:53 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Each DRFSorter tracks the total resources in the cluster. This means
> that each sorter must be updated when the resources in the cluster have
> changed, e.g., due to the creation of a dynamic reservation or a
> persistent volume. In the previous implementation, the quota role sorter
> was not updated for non-quota roles when a reservation or persistent
> volume was created by a framework. This resulted in inconsistency
> between the total resources in the allocator and the quota role sorter.
> 
> This could cause several problems. First, removing a slave from the
> cluster would leak resources in the quota role sorter. Second, certain
> interleavings of slave removals and reserve/unreserve operations by
> frameworks and via HTTP endpoints could lead to CHECK failures.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> eca949e07abb00423a2f35a56eedc5d4287d81f3 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 971a7b38d26b26c8543815e467f30cbb83ee412c 
> 
> Diff: https://reviews.apache.org/r/49377/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX and recent Arch Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

> On July 4, 2016, 7:55 p.m., Guangya Liu wrote:
> > src/common/values.cpp, line 673
> > 
> >
> > Sorry, I should ask this question in previous patch. Same as above, can 
> > you please show more comments for what do you want to check here? It would 
> > be great if you can make the comment easy to understand.

Only [0-9a-z./] is available for Text in document.


- Klaus


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


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49586: Used startsWith char version for role validation.

[Klaus Ma]

> On July 4, 2016, 6:51 p.m., Michael Park wrote:
> > This is covered in https://reviews.apache.org/r/49582/

Yes, you're right :). Discard this patch.


- Klaus


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


On July 4, 2016, 4:05 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49586/
> ---
> 
> (Updated July 4, 2016, 4:05 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used startsWith char version for role validation.
> 
> 
> Diffs
> -
> 
>   src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
> 
> Diff: https://reviews.apache.org/r/49586/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

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




src/common/values.cpp (line 665)


Only `[0-9a-z./]` is available for Text in document.


- Klaus Ma


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46823: Fully qualified addresses of Flag members in add calls in stout.

[Benjamin Bannier]

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

(Updated July 4, 2016, 2:41 p.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
3f3e21514bd5e2e388165eb64d540764097557ac 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46822: Avoided slicing of flags in subprocess.

[Benjamin Bannier]

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

(Updated July 4, 2016, 2:41 p.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Pass `Flags` by raw ptr.


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


Repository: mesos


Description (updated)
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.


Diffs (updated)
-

  src/docker/docker.cpp aad5380f5848f1b49d5524032f3b5b18c2aa1c56 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
  src/slave/container_loggers/lib_logrotate.cpp 
d53847bbca83367927725f06e8f962ce7011f468 
  src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
  src/slave/containerizer/mesos/containerizer.cpp 
a523799b3bd8a69c3dcd5a47ed9d81eace906686 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
842f2b5d4037e076cac4fd9c2eeb8f69786cffa7 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
25d2a57d7a814d735db78bda8c6956199de5c390 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
3dfe7ad4477dd1a6c8585bf761eaf435fd0cb366 
  src/slave/containerizer/mesos/launcher.hpp 
bf435e3a9c150648336a1becf2f075fa183428bd 
  src/slave/containerizer/mesos/launcher.cpp 
9efe8474a2210957ce256fc08cb35694194213c3 
  src/slave/containerizer/mesos/linux_launcher.hpp 
c1852226c74bc611d045be721e284141e59adcd9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
95dee95c5e6e613e526c92d8729ae5583c8b58f1 
  src/tests/containerizer/isolator_tests.cpp 
7b4d47bd9e99b71269093d7c11559f3b74a3e22b 
  src/tests/containerizer/launch_tests.cpp 
3e36f2f7ab89b98de2c1a971e4ecca58c13ad642 
  src/tests/containerizer/launcher.hpp 49f8ecb47abb098bf76017f79b7d00380401efe4 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
7ffec9ec9788d0e15c2c0ccc7d10b8b64ad40057 
  src/tests/containerizer/port_mapping_tests.cpp 
1c50cf545392488cf1e2d51d8e03887bebef5e75 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

[Benjamin Bannier]

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

(Updated July 4, 2016, 2:41 p.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  src/cli/execute.cpp 18c2e3449bf5e50bea0bbd0a92efa20fc8b9032b 
  src/cli/resolve.cpp ac6be05e64c5e66703081068d992ebb0d6ca6c70 
  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/examples/balloon_framework.cpp 5613ff0c61e2d2f84684a206debc97dcb8b2c0d3 
  src/examples/dynamic_reservation_framework.cpp 
850bb2a5b243dd5d5a8b6476570b4f943fde6185 
  src/examples/load_generator_framework.cpp 
5402e31b89b7ead1dc8fdc9065980b5b1c0d380c 
  src/examples/long_lived_framework.cpp 
7c57eb5e4a34208504475013690ae8e3cae74155 
  src/examples/no_executor_framework.cpp 
57425726aa5ca27c9579b0d8ecc0bb9eb9b26852 
  src/examples/persistent_volume_framework.cpp 
fe2837cfffb6dd251ccb9c93197f623d0c55fb36 
  src/examples/test_framework.cpp a83766a92617d50eadd92ec55113e049a7290d03 
  src/examples/test_http_framework.cpp cf6baed07c862ccade080618f33cc921fc09415d 
  src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
  src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
  src/master/main.cpp 84f3b0723fd3df02386c8072ded3cb42272cd065 
  src/slave/container_loggers/lib_logrotate.hpp 
8c5602da3e5ff7bcf758da61723b7a0ea00a6a6e 
  src/slave/container_loggers/logrotate.hpp 
16d92322079a69d8e6bed7830623c62f345cd51c 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
25d2a57d7a814d735db78bda8c6956199de5c390 
  src/slave/containerizer/mesos/launch.cpp 
83f4d7f28c066a605aa84862eca9fde900ec96c6 
  src/slave/containerizer/mesos/mount.cpp 
dbd7853a43ee1402f2f91d933a657010efdd76b0 
  src/slave/main.cpp a7ed669a0b6861d6ce8546dfafac849044a77eec 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 46821: Avoided slicing of flags in subprocess in libprocess and stout.

[Benjamin Bannier]

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

(Updated July 4, 2016, 2:41 p.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Michael 
Park, and Vinod Kone.


Changes
---

Pass `Flags` by raw ptr.


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


Repository: mesos


Description (updated)
---

While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).

Since `Option` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.

Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess_base.hpp 
e0c6b2d930fbd2cfe011e6faf44843b83ab1db27 
  3rdparty/libprocess/src/subprocess.cpp 
44073146118b6c6e9e730b8c901852594080a3eb 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
3f3e21514bd5e2e388165eb64d540764097557ac 

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


Testing
---

Tested on various platforms in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 49223: Enhance value parsing.

[Guangya Liu]

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




src/common/values.cpp (line 648)


Can you please show more detail for what do you want check here in the 
comment? Do you mean you want to validate the `token` here?



src/common/values.cpp (line 665)


Sorry, I should ask this question in previous patch. Same as above, can you 
please show more comments for what do you want to check here? It would be great 
if you can make the comment easy to understand.


- Guangya Liu


On 七月 4, 2016, 10:33 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated 七月 4, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46825: Fully-typed all FlagsBase::add overloads.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46821, 46822, 46823, 46824, 46825]

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

- Mesos ReviewBot


On July 4, 2016, 8:17 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46825/
> ---
> 
> (Updated July 4, 2016, 8:17 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fully-typed all FlagsBase::add overloads.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> dd9362772d1fbd32638fc7e70126fd49d4a03c68 
>   3rdparty/stout/tests/flags_tests.cpp 
> 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
>   3rdparty/stout/tests/subcommand_tests.cpp 
> 9213d6b9faec30b5be320ab37ca29c2406c964ac 
> 
> Diff: https://reviews.apache.org/r/46825/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X clang-trunk w/o optimizations)
> 
> The mesos tests suite do not execute at all with an optimizing clang-trunk 
> since copying of the `map` inside `FlagsBase` reference random memory leading 
> to `SEGFAULTs`. With these patches all of `stout-tests` and 
> `libprocess-tests` can be executed, but some remaining failures persist in 
> `mesos-tests` (their number is reduced though).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 49586: Used startsWith char version for role validation.

[Michael Park]

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



This is covered in https://reviews.apache.org/r/49582/

- Michael Park


On July 4, 2016, 8:05 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49586/
> ---
> 
> (Updated July 4, 2016, 8:05 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used startsWith char version for role validation.
> 
> 
> Diffs
> -
> 
>   src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
> 
> Diff: https://reviews.apache.org/r/49586/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

> On July 1, 2016, 1:27 p.m., Guangya Liu wrote:
> > src/tests/values_tests.cpp, line 204
> > 
> >
> > Would it make sense to add some negative case here to test against the 
> > code for error handling?

Will handle in https://issues.apache.org/jira/browse/MESOS-5603 .


- Klaus


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


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

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

(Updated July 4, 2016, 6:33 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Address Guangya's comments


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


Repository: mesos


Description (updated)
---

Enhance value parsing.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49586: Used startsWith char version for role validation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49586]

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

- Mesos ReviewBot


On July 4, 2016, 8:05 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49586/
> ---
> 
> (Updated July 4, 2016, 8:05 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used startsWith char version for role validation.
> 
> 
> Diffs
> -
> 
>   src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
> 
> Diff: https://reviews.apache.org/r/49586/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 49587: Added "fair_sharing_excluded_resource_names" to configuration.md.

[Guangya Liu]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Added "fair_sharing_excluded_resource_names" to configuration.md.


Diffs
-

  docs/configuration.md 8c8678c7e2251923298b90b7216a4e584faf6b26 

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


Testing
---

make
make check

https://github.com/gyliu513/mesos/blob/master/docs/configuration.md


Thanks,

Guangya Liu

Re: Review Request 49425: Used os::raw::Argv in command executor.

[Guangya Liu]

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




src/launcher/windows/executor.cpp (lines 70 - 71)


remove this


- Guangya Liu


On 六月 30, 2016, 4:18 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49425/
> ---
> 
> (Updated 六月 30, 2016, 4:18 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used os::raw::Argv in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
>   src/launcher/windows/executor.cpp f6da398eeddaf68b8eaf510648c55964d9f5b7c7 
> 
> Diff: https://reviews.apache.org/r/49425/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49575: Fixed `delete` usage in authorization_test.cpp.

[Joerg Schad]

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

(Updated July 4, 2016, 9:22 a.m.)


Review request for mesos, Alexander Rojas and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Additionally this review fixes some minor
formatting and typo issues.


Diffs (updated)
-

  src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 49574: Refactored /role and getRoles endpoint code.

[Joerg Schad]

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

(Updated July 4, 2016, 9:21 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This patch factors out common functionality of
getting filtered/authorized roles which was
previously duplicated between both methods.


Diffs (updated)
-

  src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
  src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 

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


Testing
---

make (sudo) make check


Thanks,

Joerg Schad

Re: Review Request 49370: Updateted documentation for roles endpoint filtering.

[Joerg Schad]

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

(Updated July 4, 2016, 9:21 a.m.)


Review request for mesos, Adam B and Vinod Kone.


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


Repository: mesos


Description
---

Updateted documentation for roles endpoint filtering.


Diffs (updated)
-

  CHANGELOG 47a9dcbf5a45441e15d3e6b4a574696975efcaa9 
  docs/authorization.md fb56cdba6f142171b3d49e2582b7921f211907e3 
  src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 

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


Testing
---

Viewed docs via docker website container.


Thanks,

Joerg Schad

Re: Review Request 49369: Introduced authorization based filtering for /roles.

[Joerg Schad]

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

(Updated July 4, 2016, 9:12 a.m.)


Review request for mesos, Adam B and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Previously only /weights was filtered but /roles actually
contains the same information and hence both endpoints
should be filtered in the same way.
As part of this work we renamed the GetWeight action to
ViewRole (and same for the acls).


Diffs (updated)
-

  include/mesos/authorizer/acls.proto 31d5c144f92749d685dbf75b65362bd633c54ea5 
  include/mesos/authorizer/authorizer.proto 
e22d3a491e0c5e7c384af8635a7f555e2194b939 
  src/authorizer/local/authorizer.cpp e5d4addd0b6d0323144c9b28c0fb46331f5d4dc1 
  src/common/http.hpp c410d039d6e832e0f838591b06f5d6a5d1f4197d 
  src/common/http.cpp fffa24cb07f73dd2bc8edc59fc276fdca5b86380 
  src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
  src/master/weights_handler.cpp 5fc69c6a1c2663bad1774d9376b0aa9f90fa7275 
  src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 
  src/tests/dynamic_weights_tests.cpp c67ed75a050b9db5575ac2bb6100bcf01cfc04ff 
  src/tests/master_authorization_tests.cpp 
0807469763b14f39b594769341d5eb9678d26946 

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


Testing
---

sudo make check


Thanks,

Joerg Schad

Re: Review Request 49375: Simplified DRFSorter to not track per-slave total resources.

[Neil Conway]

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

(Updated July 4, 2016, 9:01 a.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
Michael Park.


Changes
---

Tweak comment per review.


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


Repository: mesos


Description
---

The DRFSorter previously kept the total resources at each slave, along
with the total quantity of resources in the cluster. The latter figure
is what most of the clients of the sorter care about. In the few places
where the previous coding was using the per-slave total resources, it is
relatively easy to adjust the code to remove this dependency.

As part of this change, remove `total()` and `update(const SlaveID&
slaveId, const Resources& resources)` from the Sorter interface. The
former was unused; for the latter, code that used it can instead be
rewritten to adjust the total resources in the cluster by first removing
the previous resources at a slave and then adding the new resources.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
eca949e07abb00423a2f35a56eedc5d4287d81f3 
  src/master/allocator/sorter/drf/sorter.hpp 
0aa1a71da4501a3b469d07538a043b4c1d74d688 
  src/master/allocator/sorter/drf/sorter.cpp 
967290d4d1100208900b4b724422c3218abc23cb 
  src/master/allocator/sorter/sorter.hpp 
5ce6bb82b0127257d97daf0cea6d1d0db405bf83 
  src/tests/sorter_tests.cpp 6fc58829892dc0223140f1b47593a3e5853cace5 

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


Testing
---

make check on OSX and recent Arch Linux.


Thanks,

Neil Conway

Re: Review Request 49375: Simplified DRFSorter to not track per-slave total resources.

[Neil Conway]

> On July 4, 2016, 7:51 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 716-723
> > 
> >
> > Just nit: I think that here the `role sorters` should include both 
> > `roleSorter` and `quotaRoleSorter`, so what about grouping the `sorters` 
> > into one code block as following?
> > 
> > // Now, update the total resources in the role sorters by removing
> > // the previous resources at this slave and adding the new resources.
> > roleSorter->remove(slaveId, oldTotal);
> > roleSorter->add(slaveId, updatedTotal.get());
> > quotaRoleSorter->remove(slaveId, oldTotal.nonRevocable());
> > quotaRoleSorter->add(slaveId, updatedTotal.get().nonRevocable());

Personally I think this is a bit easier to read with some whitespace. The 
comment about non-revocable resources in the quota role sorter is also worth 
keeping.


- Neil


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


On July 1, 2016, 9:47 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49375/
> ---
> 
> (Updated July 1, 2016, 9:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The DRFSorter previously kept the total resources at each slave, along
> with the total quantity of resources in the cluster. The latter figure
> is what most of the clients of the sorter care about. In the few places
> where the previous coding was using the per-slave total resources, it is
> relatively easy to adjust the code to remove this dependency.
> 
> As part of this change, remove `total()` and `update(const SlaveID&
> slaveId, const Resources& resources)` from the Sorter interface. The
> former was unused; for the latter, code that used it can instead be
> rewritten to adjust the total resources in the cluster by first removing
> the previous resources at a slave and then adding the new resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 38381237fa6ceb3f21fd0d4b07d7c3787f0129df 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 0aa1a71da4501a3b469d07538a043b4c1d74d688 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
>   src/master/allocator/sorter/sorter.hpp 
> 5ce6bb82b0127257d97daf0cea6d1d0db405bf83 
>   src/tests/sorter_tests.cpp 6fc58829892dc0223140f1b47593a3e5853cace5 
> 
> Diff: https://reviews.apache.org/r/49375/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX and recent Arch Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49377: Fixed allocator to update total resources in quota sorter.

[Neil Conway]

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

(Updated July 4, 2016, 8:53 a.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
Michael Park.


Changes
---

Remove comment per review.


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


Repository: mesos


Description
---

Each DRFSorter tracks the total resources in the cluster. This means
that each sorter must be updated when the resources in the cluster have
changed, e.g., due to the creation of a dynamic reservation or a
persistent volume. In the previous implementation, the quota role sorter
was not updated for non-quota roles when a reservation or persistent
volume was created by a framework. This resulted in inconsistency
between the total resources in the allocator and the quota role sorter.

This could cause several problems. First, removing a slave from the
cluster would leak resources in the quota role sorter. Second, certain
interleavings of slave removals and reserve/unreserve operations by
frameworks and via HTTP endpoints could lead to CHECK failures.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
eca949e07abb00423a2f35a56eedc5d4287d81f3 
  src/master/allocator/sorter/drf/sorter.cpp 
967290d4d1100208900b4b724422c3218abc23cb 
  src/tests/persistent_volume_endpoints_tests.cpp 
971a7b38d26b26c8543815e467f30cbb83ee412c 

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


Testing
---

make check on OSX and recent Arch Linux.


Thanks,

Neil Conway

Re: Review Request 49323: Added tests that combine the two ways of creating volumes.

[Neil Conway]

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

(Updated July 4, 2016, 8:32 a.m.)


Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
Michael Park.


Changes
---

Address review comment.


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


Repository: mesos


Description
---

These tests check that dynamic reservations and persistent volumes can
be created via the offer API and then removed via the HTTP endpoints,
and vice versa.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
971a7b38d26b26c8543815e467f30cbb83ee412c 

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


Testing
---

make check

This test passes without any Mesos changes. i.e., it doesn't reproduce the bug 
by itself, but having more test coverage for this scenario seems wise.


Thanks,

Neil Conway

Re: Review Request 49376: Added assertions to DRFSorter.

[Guangya Liu]

> On 七月 4, 2016, 8:01 a.m., Guangya Liu wrote:
> > src/master/allocator/sorter/drf/sorter.cpp, lines 272-275
> > 
> >
> > How about adjust the order a bit as following?
> > 
> > CHECK(allocations[name].resources[slaveId].contains(resources));
> > 
> > const Resources resourcesQuantity = 
> > resources.createStrippedScalarQuantity();
> > CHECK(allocations[name].scalarQuantities.contains(resourcesQuantity));
> 
> Neil Conway wrote:
> Personally, I prefer grouping the two `CHECKs` together because they are 
> semantically related.

I'm ok either way, the only advantage of updating 
`CHECK(allocations[name].resources[slaveId].contains(resources));` to be 
checked first can avoid the case of `const Resources resourcesQuantity = 
resources.createStrippedScalarQuantity();` was calculated but 
`CHECK(allocations[name].resources[slaveId].contains(resources));` failed.


- Guangya


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


On 七月 3, 2016, 8:36 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49376/
> ---
> 
> (Updated 七月 3, 2016, 8:36 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added assertions to DRFSorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
> 
> Diff: https://reviews.apache.org/r/49376/diff/
> 
> 
> Testing
> ---
> 
> These assertions DO NOT PASS. They are conceptually correct, however -- after 
> r/49377 they pass on `make check`.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49376: Added assertions to DRFSorter.

[Neil Conway]

> On July 4, 2016, 8:01 a.m., Guangya Liu wrote:
> > src/master/allocator/sorter/drf/sorter.cpp, lines 272-275
> > 
> >
> > How about adjust the order a bit as following?
> > 
> > CHECK(allocations[name].resources[slaveId].contains(resources));
> > 
> > const Resources resourcesQuantity = 
> > resources.createStrippedScalarQuantity();
> > CHECK(allocations[name].scalarQuantities.contains(resourcesQuantity));

Personally, I prefer grouping the two `CHECKs` together because they are 
semantically related.


- Neil


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


On July 3, 2016, 8:36 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49376/
> ---
> 
> (Updated July 3, 2016, 8:36 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added assertions to DRFSorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
> 
> Diff: https://reviews.apache.org/r/49376/diff/
> 
> 
> Testing
> ---
> 
> These assertions DO NOT PASS. They are conceptually correct, however -- after 
> r/49377 they pass on `make check`.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49575: Fixed `delete` usage in authorization_test.cpp.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On July 3, 2016, 7:51 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49575/
> ---
> 
> (Updated July 3, 2016, 7:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Additionally this review fixes some minor
> formatting and typo issues.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp ddd1f6f8caff7b78746f7344c83b89305e94e203 
> 
> Diff: https://reviews.apache.org/r/49575/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 46825: Fully-typed all FlagsBase::add overloads.

[Benjamin Bannier]

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

(Updated July 4, 2016, 10:17 a.m.)


Review request for mesos, Alexander Rukletsov and Vinod Kone.


Changes
---

Fixed a couple of instances where test flags were suddenly wrapped in Flags<>.


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


Repository: mesos


Description
---

Fully-typed all FlagsBase::add overloads.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/flags.hpp 
dd9362772d1fbd32638fc7e70126fd49d4a03c68 
  3rdparty/stout/tests/flags_tests.cpp 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
  3rdparty/stout/tests/subcommand_tests.cpp 
9213d6b9faec30b5be320ab37ca29c2406c964ac 

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


Testing
---

make check (OS X clang-trunk w/o optimizations)

The mesos tests suite do not execute at all with an optimizing clang-trunk 
since copying of the `map` inside `FlagsBase` reference random memory leading 
to `SEGFAULTs`. With these patches all of `stout-tests` and `libprocess-tests` 
can be executed, but some remaining failures persist in `mesos-tests` (their 
number is reduced though).


Thanks,

Benjamin Bannier

Re: Review Request 49569: Added an option to the launch helper binary to unshare mount namespace.

[Gilbert Song]

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


Ship it!




LGTM! I will do some real container image command task tests tomorrow.

- Gilbert Song


On July 2, 2016, 10:51 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49569/
> ---
> 
> (Updated July 2, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, Ian Downes, and 
> Joshua Cohen.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows a custom executor to use this command to launch a command in
> a new root filesystem without worrying about creating a new mount
> namespace first. For example, the following command can be used to
> launch a command (`ls -al /`) using a root filesystem (`/tmp/alpine`).
> 
> `mesos-containerizer launch \
> --unshare_namespace_mnt \
> --rootfs=/tmp/alpine\
> --command='{"shell":true,"value":"ls -al /"}'`
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.hpp 
> c716e0396736d1f2f60ec31540f12f4f7597d081 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49569/diff/
> 
> 
> Testing
> ---
> 
> Manually tested the command on CentOS7:
> ```
> [root@core-dev ~]# 
> /home/jie/workspace/dist/mesos/build/src/mesos-containerizer launch 
> --rootfs=/home/jie/alpine --unshare_namespace_mnt 
> --command='{"shell":true,"value":"ls -al /"}' --user=jie
> Changing root to /home/jie/alpine
> total 24
> drwxrwxr-x   17 1001 1002  4096 Jul  3 05:11 .
> drwxrwxr-x   17 1001 1002  4096 Jul  3 05:11 ..
> -rwxr-xr-x1 root root 0 Jul  3 05:09 .dockerenv
> drwxr-xr-x2 root root  4096 Apr  1 18:56 bin
> drwxr-xr-x4 root root   300 Jul  3 05:42 dev
> drwxr-xr-x   13 root root  4096 Jul  3 05:09 etc
> drwxr-xr-x2 root root 6 Apr  1 18:56 home
> drwxr-xr-x5 root root  4096 Apr  1 18:56 lib
> lrwxrwxrwx1 root root12 Apr  1 18:56 linuxrc -> 
> /bin/busybox
> drwxr-xr-x5 root root41 Apr  1 18:56 media
> drwxr-xr-x2 root root 6 Apr  1 18:56 mnt
> dr-xr-xr-x  685 root root 0 Jun 18 02:22 proc
> drwx--2 root root26 Jul  3 05:14 root
> drwxr-xr-x2 root root 6 Apr  1 18:56 run
> drwxr-xr-x2 root root  4096 Apr  1 18:56 sbin
> dr-xr-xr-x   13 root root 0 Jun 18 02:23 sys
> drwxrwxrwt2 root root 6 Jul  3 05:13 tmp
> drwxr-xr-x7 root root61 Apr  1 18:56 usr
> drwxr-xr-x   10 root root93 Apr  1 18:56 var
> ```
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49246: Enhanced startsWith/endsWith's performance.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49140, 49246]

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

- Mesos ReviewBot


On July 4, 2016, 6:49 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49246/
> ---
> 
> (Updated July 4, 2016, 6:49 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-5715
> https://issues.apache.org/jira/browse/MESOS-5715
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced startsWith/endsWith's performance.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/strings.hpp 
> 162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
> 
> Diff: https://reviews.apache.org/r/49246/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49568: Simplified a flag in the launch helper binary.

[Gilbert Song]

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


Ship it!




LGTM!


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


Actually I really dont like `commands` in isolator.proto 
`ContainerLaunchInfo`. Good to consider a deprecation cycle to change it to 
`pre_exec_commands`.


- Gilbert Song


On July 2, 2016, 10:50 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49568/
> ---
> 
> (Updated July 2, 2016, 10:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Ian Downes.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Using JSON::Object is not necessary. Also, the current name is quite
> confusing as we have another flag called '--command'. This patch
> simplified this flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a96b382f22886362a1159e1166dfe041072985ba 
>   src/slave/containerizer/mesos/launch.hpp 
> c716e0396736d1f2f60ec31540f12f4f7597d081 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49568/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 49586: Used startsWith char version for role validation.

[Klaus Ma]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Used startsWith char version for role validation.


Diffs
-

  src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49549: Used the launch helper binary to launch user task in command executor.

[Gilbert Song]

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


Ship it!




This patch is great!

I am still thinking it may be a little risky for 1.0 using command task with 
container image, because this is what we are currently doing:

mesos containerizer fork a subprocess -> MesosContainerizerLaunch subcommand -> 
execvp mesos-executor binary -> executor fork a subprocess -> containerizer 
binary to call MesosContainerizerLaunch subcommand.

- Gilbert Song


On July 1, 2016, 6:02 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49549/
> ---
> 
> (Updated July 1, 2016, 6:02 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, command executor and 'mesos-containerizer launch' share a
> lot of the logic. Command executor should in fact, just use
> `mesos-containerizer launch` to launch the user task. Potentially,
> 'mesos-containerizer launch' can be also used by custom executor to
> launch user tasks.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
>   src/launcher/posix/executor.hpp e5f3c0a8e2d3687a5330adbf2f667db9106970c7 
>   src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49549/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49376: Added assertions to DRFSorter.

[Guangya Liu]

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




src/master/allocator/sorter/drf/sorter.cpp (lines 272 - 275)


How about adjust the order a bit as following?

CHECK(allocations[name].resources[slaveId].contains(resources));

const Resources resourcesQuantity = 
resources.createStrippedScalarQuantity();
CHECK(allocations[name].scalarQuantities.contains(resourcesQuantity));


- Guangya Liu


On 七月 3, 2016, 8:36 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49376/
> ---
> 
> (Updated 七月 3, 2016, 8:36 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added assertions to DRFSorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
> 
> Diff: https://reviews.apache.org/r/49376/diff/
> 
> 
> Testing
> ---
> 
> These assertions DO NOT PASS. They are conceptually correct, however -- after 
> r/49377 they pass on `make check`.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49375: Simplified DRFSorter to not track per-slave total resources.

[Guangya Liu]

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




src/master/allocator/mesos/hierarchical.cpp (line 543)


s/roleSorter/`roleSorter`



src/master/allocator/mesos/hierarchical.cpp (lines 716 - 723)


Just nit: I think that here the `role sorters` should include both 
`roleSorter` and `quotaRoleSorter`, so what about grouping the `sorters` into 
one code block as following?

// Now, update the total resources in the role sorters by removing
// the previous resources at this slave and adding the new resources.
roleSorter->remove(slaveId, oldTotal);
roleSorter->add(slaveId, updatedTotal.get());
quotaRoleSorter->remove(slaveId, oldTotal.nonRevocable());
quotaRoleSorter->add(slaveId, updatedTotal.get().nonRevocable());


- Guangya Liu


On 七月 1, 2016, 9:47 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49375/
> ---
> 
> (Updated 七月 1, 2016, 9:47 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5698
> https://issues.apache.org/jira/browse/MESOS-5698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The DRFSorter previously kept the total resources at each slave, along
> with the total quantity of resources in the cluster. The latter figure
> is what most of the clients of the sorter care about. In the few places
> where the previous coding was using the per-slave total resources, it is
> relatively easy to adjust the code to remove this dependency.
> 
> As part of this change, remove `total()` and `update(const SlaveID&
> slaveId, const Resources& resources)` from the Sorter interface. The
> former was unused; for the latter, code that used it can instead be
> rewritten to adjust the total resources in the cluster by first removing
> the previous resources at a slave and then adding the new resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 38381237fa6ceb3f21fd0d4b07d7c3787f0129df 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 0aa1a71da4501a3b469d07538a043b4c1d74d688 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 967290d4d1100208900b4b724422c3218abc23cb 
>   src/master/allocator/sorter/sorter.hpp 
> 5ce6bb82b0127257d97daf0cea6d1d0db405bf83 
>   src/tests/sorter_tests.cpp 6fc58829892dc0223140f1b47593a3e5853cace5 
> 
> Diff: https://reviews.apache.org/r/49375/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX and recent Arch Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 49548: Added devolve function for CommandInfo.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 1, 2016, 6:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49548/
> ---
> 
> (Updated July 1, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added devolve function for CommandInfo.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 7c242d3d9235235c954f232af84b52d62373afeb 
>   src/internal/devolve.cpp 04fac996a0463bdb3218a90bcfdb68b79d08dd7b 
> 
> Diff: https://reviews.apache.org/r/49548/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49542: Explicitly passed in launcher dir to command executor.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 1, 2016, 4:53 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49542/
> ---
> 
> (Updated July 1, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Joris Van Remoortere, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Relying on dirname is not quite reliable, especially in tests where
> launchable scripts are under 'build/src', but actual binaries are
> under 'build/src/.libs'. This patch passed in launcher_dir explicitly
> through executor flags.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp bb88a4570d183d22dff65f6e220d566c0924001a 
>   src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 
>   src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 
>   src/tests/slave_tests.cpp d800cb8c10b5730a711c8354802bdff6aaca8c01 
> 
> Diff: https://reviews.apache.org/r/49542/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49540: Used the argv version for command that launches the command executor.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 1, 2016, 4:24 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49540/
> ---
> 
> (Updated July 1, 2016, 4:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used the argv version for command that launches the command executor.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 
>   src/tests/slave_tests.cpp d800cb8c10b5730a711c8354802bdff6aaca8c01 
> 
> Diff: https://reviews.apache.org/r/49540/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>