Re: Review Request 56895: Allow agents to recover slave state post a reboot.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56895/#review177438 --- Patch looks great! Reviews applied: [56895] Passed command:

Re: Review Request 56895: Allow agents to recover slave state post a reboot.

> On May 23, 2017, 9:32 p.m., Vinod Kone wrote: > > No tests!? I have added a new test, so in total this change has two tests: one verifying that the state is recovered correctly and agentId is retained post the agent host reboot given the recovery finishes without errors and a second one to

Re: Review Request 56895: Allow agents to recover slave state post a reboot.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56895/ --- (Updated June 9, 2017, 4:27 a.m.) Review request for mesos, Neil Conway, Vinod

Re: Review Request 59874: Made `CheckerProcess` use a consistent description for log messages.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59874/#review177435 --- Bad patch! Reviews applied: [59874, 59902, 59873, 59872, 59871]

Re: Review Request 59874: Made `CheckerProcess` use a consistent description for log messages.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59874/ --- (Updated June 9, 2017, 3:43 a.m.) Review request for mesos, Alexander

Re: Review Request 59902: Improved health checks validations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59902/ --- (Updated June 9, 2017, 3:42 a.m.) Review request for mesos, Alexander

Re: Review Request 59873: Refactored `HealthChecker` to use `CheckerProcess`.

> On June 8, 2017, 4:33 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp > > Line 199 (original), 263-264 (patched) > > > > > > I think we can do it `CheckerProcess` Nope, because `CheckerProcess`

Re: Review Request 59873: Refactored `HealthChecker` to use `CheckerProcess`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59873/ --- (Updated June 9, 2017, 3:36 a.m.) Review request for mesos, Alexander

Re: Review Request 59464: Add Fetcher task total and failed fetch metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59464/#review177428 --- src/slave/containerizer/fetcher.hpp Lines 113-114 (patched)

Re: Review Request 59930: Support RO mode for bind mount volumes with filesystem/linux isolator

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59930/#review177420 --- Patch looks great! Reviews applied: [59930] Passed command:

Re: Review Request 59937: Added agent `--resource_provider_config_dir` flag.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59937/ --- (Updated June 8, 2017, 11:19 p.m.) Review request for mesos, Benjamin Bannier,

Review Request 59937: Added agent `--resource_provider_config_dir` flag.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59937/ --- Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Jan Schlicht.

Review Request 59936: Added streaming function for ResourceProviderInfo.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59936/ --- Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Jan Schlicht.

Review Request 59935: Introduced a streaming function for a vector.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59935/ --- Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Jan Schlicht.

Re: Review Request 59599: Added 'type' and 'name' fields to ResourceProviderInfo.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59599/ --- (Updated June 8, 2017, 11:17 p.m.) Review request for mesos, Benjamin Bannier,

Review Request 59932: Added devolve functions for resource provider Event/Call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59932/ --- Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Jan Schlicht.

Re: Review Request 59601: Removed an unused declaration in type_utils.hpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59601/ --- (Updated June 8, 2017, 11:10 p.m.) Review request for mesos, Benjamin Bannier,

Review Request 59930: Support RO mode for bind mount volumes with filesystem/linux isolator

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59930/ --- Review request for mesos, Jason Lai, Jie Yu, and Zhitao Li. Repository: mesos

Re: Review Request 59792: Cleaned up leveldb build.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59792/#review177414 --- Ship it! Ship It! - Joseph Wu On June 3, 2017, 12:21 p.m.,

Re: Review Request 59780: Fixed libevent CMake arguments.

> On June 8, 2017, 2:20 p.m., Joseph Wu wrote: > > 3rdparty/CMakeLists.txt > > Lines 173-178 (original), 172-177 (patched) > > > > > > We should be able to delete this line now that we have: > >

Re: Review Request 59776: Added `CMAKE_FORWARD_ARGS` to `3rdparty/CMakeLists.txt`.

> On June 8, 2017, 8:56 p.m., Joseph Wu wrote: > > 3rdparty/CMakeLists.txt > > Lines 67 (patched) > > > > > > Oh, I missed this in my first pass. > > > > This should be: > > ``` > >

Re: Review Request 59776: Added `CMAKE_FORWARD_ARGS` to `3rdparty/CMakeLists.txt`.

> On June 8, 2017, 8:56 p.m., Joseph Wu wrote: > > 3rdparty/CMakeLists.txt > > Lines 67 (patched) > > > > > > Oh, I missed this in my first pass. > > > > This should be: > > ``` > >

Re: Review Request 59791: Windows: Fixed path to protoc.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59791/#review177405 --- Ship it! Ship It! - Joseph Wu On June 3, 2017, 12:19 p.m.,

Re: Review Request 59778: Forwarded CMake arguments to protobuf.

> On June 8, 2017, 8:14 p.m., Joseph Wu wrote: > > 3rdparty/CMakeLists.txt > > Lines 360 (patched) > > > > > > Hm. Surprised we didn't notice this `src/..` part of the path to the > > configure script. I'll get

Re: Review Request 59790: Windows: Fixed libprocess 3rdparty `LIB_DIR` variables.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59790/#review177403 --- Ship it! Ship It! - Joseph Wu On June 3, 2017, 12:19 p.m.,

Re: Review Request 59776: Added `CMAKE_FORWARD_ARGS` to `3rdparty/CMakeLists.txt`.

> On June 8, 2017, 7:25 p.m., Joseph Wu wrote: > > 3rdparty/CMakeLists.txt > > Lines 75-86 (patched) > > > > > > Hum. I'm going to take some liberties with this comment :) > > > > * Going to move this

Re: Review Request 59789: Windows: Fixed gmock and gtest `LIB_DIR`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59789/#review177400 --- Ship it! Ship It! - Joseph Wu On June 3, 2017, 12:19 p.m.,

Re: Review Request 59788: Fixed glog `EXTERNAL` setup.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59788/#review177399 --- Ship it! Ship It! - Joseph Wu On June 3, 2017, 12:16 p.m.,

Re: Review Request 59782: Windows: Fixed curl build.

> On June 8, 2017, 9:17 p.m., Joseph Wu wrote: > > cmake/CompilationConfigure.cmake > > Lines 225-227 (patched) > > > > > > This was guarded in the other file, but why not here? > > > > Also, `curl.h` is

Re: Review Request 59891: Windows: Use `cmd /C exit 1` instead of `false`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59891/#review177397 --- Patch looks great! Reviews applied: [59891] Passed command:

Re: Review Request 59782: Windows: Fixed curl build.

> On June 8, 2017, 2:17 p.m., Joseph Wu wrote: > > cmake/CompilationConfigure.cmake > > Lines 225-227 (patched) > > > > > > This was guarded in the other file, but why not here? > > > > Also, `curl.h` is

Re: Review Request 59779: Fixed the configuration-dependent protobuf linkage.

> On June 8, 2017, 9:20 p.m., Joseph Wu wrote: > > 3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake > > Lines 115-116 (patched) > > > > > > Is there any reason why this isn't done in one line? > > > >

Re: Review Request 59780: Fixed libevent CMake arguments.

> On June 8, 2017, 9:20 p.m., Joseph Wu wrote: > > 3rdparty/CMakeLists.txt > > Lines 136-141 (original), 135-138 (patched) > > > > > > Looks like we can get rid of this entirely, as there is also some SSL > > flag

Re: Review Request 59786: Set default library linkage based on `BUILD_SHARED_LIBS`.

> On June 8, 2017, 9:53 p.m., Joseph Wu wrote: > > cmake/MesosConfigure.cmake > > Line 60 (original), 60 (patched) > > > > > > The `${}` here is extraneous. Yes, and should be removed as it can cause unexpected?

Re: Review Request 59786: Set default library linkage based on `BUILD_SHARED_LIBS`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59786/#review177390 --- Ship it! Minor nit. cmake/MesosConfigure.cmake Line 60

Re: Review Request 59785: Forwarded CMake arguments to gtest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59785/#review177389 --- Ship it! Ship It! - Joseph Wu On June 3, 2017, 12:33 p.m.,

Re: Review Request 59784: Fixed the configuration-dependent zlib linkage.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59784/#review177388 --- Ship it! Could use a couple extra newlines here and there, but

Re: Review Request 59780: Fixed libevent CMake arguments.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59780/#review177372 --- Fix it, then Ship it! 3rdparty/CMakeLists.txt Lines 136-141

Re: Review Request 59779: Fixed the configuration-dependent protobuf linkage.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59779/#review177369 --- Ship it! LGTM.

Re: Review Request 59783: Forwarded CMake arguments to zlib.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59783/#review177384 --- Ship it! Ship It! - Joseph Wu On June 3, 2017, 12:33 p.m.,

Re: Review Request 59782: Windows: Fixed curl build.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59782/#review177381 --- cmake/CompilationConfigure.cmake Lines 225-227 (patched)

Re: Review Request 59781: Fixed glog CMake arguments.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59781/#review177378 --- Ship it! Ship It! - Joseph Wu On June 3, 2017, 12:33 p.m.,

Re: Review Request 59776: Added `CMAKE_FORWARD_ARGS` to `3rdparty/CMakeLists.txt`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59776/#review177373 --- 3rdparty/CMakeLists.txt Lines 67 (patched)

Re: Review Request 59858: Allow Gauge metrics to avoid process::defer().

> On June 6, 2017, 9:37 p.m., Zhitao Li wrote: > > 3rdparty/libprocess/include/process/metrics/gauge.hpp > > Line 34 (original), 35-39 (patched) > > > > > > I am somehow against this general guideline in the long

Re: Review Request 59858: Allow Gauge metrics to avoid process::defer().

> On June 6, 2017, 10:18 p.m., Benjamin Bannier wrote: > > 3rdparty/libprocess/include/process/metrics/gauge.hpp > > Line 34 (original), 35-39 (patched) > > > > > > I believe the gist of this change is to recognize

Re: Review Request 59778: Forwarded CMake arguments to protobuf.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59778/#review177252 --- Fix it, then Ship it! 3rdparty/CMakeLists.txt Lines 360

Re: Review Request 59777: Forwarded CMake arguments to ZooKeeper.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59777/#review177362 --- Ship it! LGTM. - Joseph Wu On June 3, 2017, 12:32 p.m.,

Re: Review Request 59858: Allow Gauge metrics to avoid process::defer().

> On June 7, 2017, 12:18 a.m., Benjamin Bannier wrote: > > 3rdparty/libprocess/include/process/metrics/gauge.hpp > > Line 34 (original), 35-39 (patched) > > > > > > I believe the gist of this change is to recognize

Re: Review Request 59776: Added `CMAKE_FORWARD_ARGS` to `3rdparty/CMakeLists.txt`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59776/#review177353 --- Ship it! 3rdparty/CMakeLists.txt Lines 69-71 (patched)

Re: Review Request 59625: Fixed interference of unbundled dependency include paths with Boost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59625/#review177345 --- Fix it, then Ship it! Thanks Benjamin! configure.ac Line 717

Re: Review Request 59860: Prevent allocating non-capable agents' resources to hierarchical roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59860/#review177341 --- Don't we also need to prevent the creation of volumes under

Re: Review Request 59859: Added `HIERARCHICAL_ROLE` agent capability.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59859/#review177339 --- include/mesos/mesos.proto Lines 809 (patched)

Re: Review Request 58510: Added two tests for hierarchical reservation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58510/#review177346 --- src/tests/hierarchical_allocator_tests.cpp Lines 4615 (patched)

Re: Review Request 59625: Fixed interference of unbundled dependency include paths with Boost.

> On June 8, 2017, 2:20 p.m., Benjamin Bannier wrote: > > I did not explicitly call out that this change leads to suppression of > > diagnostics from all headers found via the modified includes; AFAICT this > > changed behavior is OK. > > > > An alternative approach could be to e.g., surround

Re: Review Request 59921: Added agent domain to Offer message.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59921/#review177330 --- Bad patch! Reviews applied: [59921, 59766, 59764, 59763, 59762,

Review Request 59921: Added agent domain to Offer message.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59921/ --- Review request for mesos and Vinod Kone. Bugs: MESOS-7644

Re: Review Request 59873: Refactored `HealthChecker` to use `CheckerProcess`.

> On June 8, 2017, 4:33 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp > > Lines 298-300 (original), 343-345 (patched) > > > > > > Here we should probably print `consecutiveFailures`, because a

Re: Review Request 59873: Refactored `HealthChecker` to use `CheckerProcess`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59873/#review177328 --- src/checks/health_checker.cpp Lines 112 (patched)

Re: Review Request 59873: Refactored `HealthChecker` to use `CheckerProcess`.

> On June 8, 2017, 4:33 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp > > Lines 298-300 (original), 343-345 (patched) > > > > > > Here we should probably print `consecutiveFailures`, because a

Re: Review Request 59873: Refactored `HealthChecker` to use `CheckerProcess`.

> On June 7, 2017, 4:35 p.m., Alexander Rukletsov wrote: > > src/checks/health_checker.cpp > > Lines 255-256 (patched) > > > > > > Why do not want to leave it as it is now? > > ``` > > create =

Re: Review Request 59902: Improved health checks validations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59902/#review177320 --- src/checks/health_checker.cpp Lines 388-391 (original), 388-391

Re: Review Request 59874: Made `CheckerProcess` use a consistent description for log messages.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59874/#review177300 --- src/checks/checker.cpp Lines 176 (patched)

Re: Review Request 59873: Refactored `HealthChecker` to use `CheckerProcess`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59873/#review177306 --- src/checks/health_checker.hpp Line 39 (original), 32-33

Re: Review Request 59899: Test confirming the Docker containerizer stdout/stderr ownership.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59899/#review177319 --- Fix it, then Ship it! LGTM module minor nits.

Re: Review Request 59898: Fixed Docker containerizer stdout/stderr ownership.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59898/#review177318 --- Fix it, then Ship it! LGTM, modulo the nit below.

Re: Review Request 59898: Fixed Docker containerizer stdout/stderr ownership.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59898/#review177317 --- Ship it! LGTM. src/slave/containerizer/docker.cpp Line 423

Re: Review Request 59625: Fixed interference of unbundled dependency include paths with Boost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59625/#review177307 --- I did not explicitly call out that this change leads to

Re: Review Request 59625: Fixed interference of unbundled dependency include paths with Boost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59625/#review177305 --- Patch looks great! Reviews applied: [59625] Passed command:

Re: Review Request 59871: Fixed a typo in mesos.proto.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59871/#review177302 --- Ship it! Ship It! - Alexander Rukletsov On June 7, 2017,

Re: Review Request 59902: Improved health checks validations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59902/ --- (Updated June 8, 2017, 1:23 p.m.) Review request for mesos, Alexander

Re: Review Request 59625: Fixed interference of unbundled dependency include paths with Boost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59625/ --- (Updated June 8, 2017, 3:14 p.m.) Review request for mesos, Michael Park and