Re: Review Request 55954: Changed 'Environment.Variable.Value' from required to optional.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55954/#review163128 --- Ship it! - Jan Schlicht On Jan. 26, 2017, 3:36 a.m., Greg

Re: Review Request 55942: Added a test for validating streaming request/response headers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55942/#review163125 --- Patch looks great! Reviews applied: [55936, 55937, 55938, 55939,

Re: Review Request 55945: Added a `install` template method to support 8 arguments.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55945/ --- (Updated Jan. 26, 2017, 7:46 p.m.) Review request for mesos and Benjamin

Re: Review Request 55944: Modified agent capabilities struct to reflect protobuf changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55944/ --- (Updated Jan. 26, 2017, 7:46 p.m.) Review request for mesos and Benjamin

Re: Review Request 55943: Moved agent capabilities from SlaveInfo to (re-)registerSlaveMessage.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55943/ --- (Updated Jan. 26, 2017, 7:11 p.m.) Review request for mesos and Benjamin

Re: Review Request 53993: Updated quota doc to support quota update.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53993/#review163120 --- Bad patch! Reviews applied: [53993, 54135, 53991, 52103, 53691,

Re: Review Request 55946: Added agent capabilities as part of agent (re-)registration process.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55946/ --- (Updated Jan. 26, 2017, 7:46 p.m.) Review request for mesos and Benjamin

Re: Review Request 55944: Modified agent capabilities struct to reflect protobuf changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55944/#review163159 --- Ship it! src/master/master.hpp (lines 243 - 247)

Re: Review Request 53517: Added test case for cgroup namespace isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53517/#review163150 --- Patch looks great! Reviews applied: [53296, 53516, 54105, 53517]

Re: Review Request 55943: Moved agent capabilities from SlaveInfo to (re-)registerSlaveMessage.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55943/#review163156 --- Ship it! Ship It! - Benjamin Mahler On Jan. 26, 2017, 11:11

Re: Review Request 55946: Added agent capabilities as part of agent (re-)registration process.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55946/#review163160 --- Ship it! Ship It! - Benjamin Mahler On Jan. 26, 2017, 11:46

Re: Review Request 55945: Added a `install` template method to support 8 arguments.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55945/#review163157 --- Ship it! Ship It! - Benjamin Mahler On Jan. 26, 2017, 11:46

Re: Review Request 55973: Update the tests to handle MULTI_ROLE support.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55973/#review163225 --- Bad patch! Reviews applied: [55973, 56006, 56005, 56004, 55972,

Re: Review Request 55790: Support the full CNI DNS specification.

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

Re: Review Request 52534: Dispatch filter expiration twice.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52534/#review163233 --- Bad patch! Reviews applied: [52534, 51027] Failed command:

Re: Review Request 55910: Prevent unintended mutation in the allocator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55910/#review163235 --- Fix it, then Ship it! Ship It!

Re: Review Request 51027: Track allocation candidates to bound allocator.

> On Jan. 19, 2017, 2:45 a.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp, lines 1273-1280 > > > > > > This change introduces an additional trip through the allocator's queue > > after

Re: Review Request 51027: Track allocation candidates to bound allocator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51027/ --- (Updated Jan. 27, 2017, 2:33 a.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 52534: Dispatch filter expiration twice.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52534/ --- (Updated Jan. 27, 2017, 2:34 a.m.) Review request for mesos, Alexander

Re: Review Request 51028: Fix tests with rapidly triggered allocations.

> On Jan. 23, 2017, 11:08 p.m., Jiang Yan Xu wrote: > > src/tests/hierarchical_allocator_tests.cpp, line 3765 > > > > > > Is `s/performing/; performed/?` better? yea I like that better - Jacob

Re: Review Request 51028: Fix tests with rapidly triggered allocations.

> On Jan. 19, 2017, 3:15 a.m., Benjamin Mahler wrote: > > src/tests/hierarchical_allocator_tests.cpp, lines 2994-2998 > > > > > > This test is already paused, all of these should be run with a paused > > clock,

Re: Review Request 51028: Fix tests with rapidly triggered allocations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51028/ --- (Updated Jan. 27, 2017, 2:33 a.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 51027: Track allocation candidates to bound allocator.

> On Jan. 21, 2017, 1:24 a.m., Jiang Yan Xu wrote: > > src/master/allocator/mesos/hierarchical.cpp, line 1314 > > > > > > We can use the `|=` operator now that it's added. Added, was waiting for that commit =D

Re: Review Request 49571: Added a benchmark test for allocations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49571/#review163220 --- Patch looks great! Reviews applied: [53096, 45962, 55359, 49571]

Re: Review Request 56006: Added CHECK logging to the allocator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56006/#review163223 --- src/master/allocator/mesos/hierarchical.cpp (line 432)

Re: Review Request 56005: Added a safety CHECK when accessing activeRoles in the master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56005/#review163224 --- Ship it! Ship It! - Guangya Liu On 一月 27, 2017, 12:31 a.m.,

Re: Review Request 53516: Moved `namespaces/pid` associated test cases to a separate file.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53516/ --- (Updated Jan. 26, 2017, 8:18 a.m.) Review request for mesos, Jason Lai, Jie

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53296/ --- (Updated Jan. 26, 2017, 8:18 a.m.) Review request for mesos, Jason Lai, Jie

Re: Review Request 55600: CMake: Transitioned Stout to automatic source grouping.

> On Jan. 25, 2017, 12:27 a.m., Joseph Wu wrote: > > 3rdparty/stout/cmake/StoutConfigure.cmake, line 47 > > > > > > Why not use `*.hpp` instead? Generally, we also would like to include .h files generated by

Re: Review Request 55955: Added validation tests to ensure environment variable value is set.

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

Re: Review Request 53517: Added test case for cgroup namespace isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53517/ --- (Updated Jan. 26, 2017, 8:19 a.m.) Review request for mesos, Jason Lai, Jie

Re: Review Request 55549: Windows: Added health checker to build.

> On Jan. 24, 2017, 9:37 p.m., Joseph Wu wrote: > > src/health-check/tcp_connect.cpp, lines 33-38 > > > > > > Libprocess headers should go above stout headers. Oh, did this change recently, or have I just always

Re: Review Request 55549: Windows: Added health checker to build.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55549/ --- (Updated Jan. 26, 2017, 8:45 a.m.) Review request for mesos, Andrew

Re: Review Request 55893: Fixed OversubscriptionTest.RescindRevocableOfferWithIncreasedRevocable.

> On 一月 25, 2017, 9:55 a.m., Guangya Liu wrote: > > @Yan, I posted some comments at https://reviews.apache.org/r/51027/ for > > this issue with some comments as: > > > > ``` > > Jacob, regaring the test failure of > > OversubscriptionTest.RescindRevocableOfferWithIncreasedRevocable, I think

Re: Review Request 54987: Updated `docs/monitoring/md` for new agent event queue metrics.

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

Re: Review Request 55599: CMake: Added `GroupSource` function to automate IDE source grouping.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55599/ --- (Updated Jan. 26, 2017, 9:12 a.m.) Review request for mesos, Andrew

Re: Review Request 55607: CMake: Added configuration of test scripts in the bin/ directory.

> On Jan. 25, 2017, 1:07 a.m., Joseph Wu wrote: > > cmake/MesosConfigure.cmake, line 217 > > > > > > How about moving the file instead? And cleaning up the bin/tmp folder? Unless I'm missing something, `COPY` is

Re: Review Request 55607: CMake: Added configuration of test scripts in the bin/ directory.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55607/ --- (Updated Jan. 26, 2017, 9:20 a.m.) Review request for mesos, Andrew

Re: Review Request 55893: Fixed OversubscriptionTest.RescindRevocableOfferWithIncreasedRevocable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55893/#review163111 --- I think that your patch need depend on /r/51027 , also in

Re: Review Request 55600: CMake: Transitioned Stout to automatic source grouping.

> On Jan. 24, 2017, 4:27 p.m., Joseph Wu wrote: > > 3rdparty/stout/cmake/StoutConfigure.cmake, line 47 > > > > > > Why not use `*.hpp` instead? > > Alex Clemmer wrote: > Generally, we also would like to include

Re: Review Request 55549: Windows: Added health checker to build.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55549/#review163171 --- Ship it! Ship It! - Joseph Wu On Jan. 26, 2017, 12:45 a.m.,

Re: Review Request 55549: Windows: Added health checker to build.

> On Jan. 24, 2017, 1:37 p.m., Joseph Wu wrote: > > src/health-check/tcp_connect.cpp, lines 33-38 > > > > > > Libprocess headers should go above stout headers. > > Alex Clemmer wrote: > Oh, did this change

Review Request 55971: Updated the agent to be MULTI_ROLE capable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55971/ --- Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael

Re: Review Request 55600: CMake: Transitioned Stout to automatic source grouping.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55600/#review163216 --- Ship it! Ship It! - Joseph Wu On Jan. 16, 2017, 10:41 p.m.,

Re: Review Request 55600: CMake: Transitioned Stout to automatic source grouping.

> On Jan. 24, 2017, 4:27 p.m., Joseph Wu wrote: > > 3rdparty/stout/cmake/StoutConfigure.cmake, line 47 > > > > > > Why not use `*.hpp` instead? > > Alex Clemmer wrote: > Generally, we also would like to include

Re: Review Request 55790: Support the full CNI DNS specification.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55790/ --- (Updated Jan. 27, 2017, 1:27 a.m.) Review request for mesos, Avinash

Review Request 55995: Restored quota correctly during allocator recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55995/ --- Review request for mesos, Alexander Rukletsov and Benjamin Bannier. Bugs:

Re: Review Request 55790: Support the full CNI DNS specification.

> On Jan. 25, 2017, 6:15 a.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 1006 > > > > > > Can we keep the `LOG(INFO)` dumping the contents of `resolv.conf`? > >

Re: Review Request 56001: Fixed a few executor segfaults during cleanup.

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

Re: Review Request 55971: Updated the agent to be MULTI_ROLE capable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55971/ --- (Updated Jan. 27, 2017, 12:27 a.m.) Review request for mesos, Benjamin

Review Request 55972: Updated master to handle non-MULTI_ROLE agents.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55972/ --- Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael

Review Request 56005: Added a safety CHECK when accessing activeRoles in the master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56005/ --- Review request for mesos, Michael Park and Neil Conway. Repository: mesos

Review Request 56006: Added CHECK logging to the allocator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56006/ --- Review request for mesos and Michael Park. Repository: mesos Description

Review Request 56004: Fixed MULTI_ROLE related bugs when updating framework info.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56004/ --- Review request for mesos, Benjamin Bannier and Michael Park. Repository: mesos

Review Request 55973: Update the tests to handle MULTI_ROLE support.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55973/ --- Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael

Re: Review Request 55550: Windows: Enabled health checker tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/0/#review163172 --- Ship it! Just a few minor things that I can tweak before

Re: Review Request 45962: Updated a persistent volume test framework to include shared volumes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45962/ --- (Updated Jan. 26, 2017, 10:06 p.m.) Review request for mesos, Greg Mann, Jie

Re: Review Request 49571: Added a benchmark test for allocations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49571/ --- (Updated Jan. 26, 2017, 10:06 p.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 55599: CMake: Added `GroupSource` function to automate IDE source grouping.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55599/#review163184 --- Ship it! Ship It! - Joseph Wu On Jan. 26, 2017, 1:12 a.m.,

Re: Review Request 56001: Fixed a few executor segfaults during cleanup.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56001/#review163190 --- Nice catch! As per our offline discussion, can you modify an

Re: Review Request 55829: Updated resources quantity stripping to strip AllocationInfo.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55829/#review163193 --- Ship it! Ship It! - Michael Park On Jan. 22, 2017, 6:09

Re: Review Request 55790: Support the full CNI DNS specification.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55790/ --- (Updated Jan. 26, 2017, 11:19 p.m.) Review request for mesos, Avinash

Re: Review Request 55790: Support the full CNI DNS specification.

> On Jan. 25, 2017, 6:15 a.m., Avinash sridharan wrote: > > src/tests/containerizer/cni_isolator_tests.cpp, line 945 > > > > > > why do a `dns.Clear()`? Consistency. Each block is the same and it remove the risk of

Re: Review Request 56001: Fixed a few executor segfaults during cleanup.

> On Jan. 26, 2017, 2:47 p.m., Anand Mazumdar wrote: > > src/docker/executor.cpp, line 817 > > > > > > Not yours, but we should have a comment here too as to why we need an > > explicit `finalize()` here in a

Re: Review Request 56001: Fixed a few executor segfaults during cleanup.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56001/ --- (Updated Jan. 26, 2017, 4:01 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 55995: Restored quota correctly during allocator recovery.

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

Review Request 56001: Fixed a few executor segfaults during cleanup.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56001/ --- Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu. Bugs:

Re: Review Request 55790: Support the full CNI DNS specification.

> On Jan. 25, 2017, 6:15 a.m., Avinash sridharan wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 1006 > > > > > > Can we keep the `LOG(INFO)` dumping the contents of `resolv.conf`?

Re: Review Request 55359: Consolidate update of allocations in `updateAllocation()`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55359/ --- (Updated Jan. 26, 2017, 10:06 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 53096: Fix handling in shared count in total resources in the sorter.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53096/ --- (Updated Jan. 26, 2017, 10:06 p.m.) Review request for mesos and Jiang Yan Xu.