Re: Review Request 50523: Updated docker recovery to account for GPU resources.

> On Nov. 5, 2016, 12:56 p.m., Guangya Liu wrote: > > src/slave/containerizer/docker.cpp, lines 1054-1060 > > > > > > The `recoverDevices` will block here, what about using continuation as > >

Re: Review Request 50523: Updated docker recovery to account for GPU resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50523/ --- (Updated Dec. 20, 2016, 7:53 a.m.) Review request for mesos, Benjamin Mahler,

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 Dec. 20, 2016, 7:35 a.m.) Review request for mesos, Benjamin Mahler,

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 Dec. 20, 2016, 7:20 a.m.) Review request for mesos, Benjamin Mahler,

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

> On Dec. 12, 2016, 9:30 a.m., Jiang Yan Xu wrote: > > src/master/allocator/mesos/hierarchical.cpp, line 1307 > > > > > > s/a single slaveId/a single `slaveId`/ to be consistent. Changed the wording to describe

Re: Review Request 54212: Fixed overlay backend provisioning multi images symlink.

> On Dec. 15, 2016, 11:22 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/provisioner/backends/overlay.cpp, line 167 > > > > > > Do you need to create the directory first? backendDir/scratch/links > > might

Re: Review Request 54873: WIP: Added asynchronous libcurl support to Mesos.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54873/#review159679 --- Bad review! Reviews applied: [] Error: No reviewers specified.

Re: Review Request 54877: Windows: Stout: Removed dependency on Shell API.

> On Dec. 20, 2016, 12:29 a.m., Daniel Pravat wrote: > > 3rdparty/stout/include/stout/windows/os.hpp, line 748 > > > > > > I don't think the conversion to UTF-8 is appropiate here. > > Andrew Schwartzmeyer wrote: >

Review Request 54880: Added unit-test for dynamic addition/deletion of CNI config.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54880/ --- Review request for mesos, Jie Yu and Qian Zhang. Bugs: MESOS-6567

Re: Review Request 52071: Updated docs to handle resources with no size in agent flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52071/ --- (Updated Dec. 20, 2016, 2:05 a.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 51880: Added unit tests to determine disk size for MOUNT or PATH disks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51880/ --- (Updated Dec. 20, 2016, 2:04 a.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 51880: Added unit tests to determine disk size for MOUNT or PATH disks.

> On Dec. 17, 2016, 2:28 a.m., Jiang Yan Xu wrote: > > src/Makefile.am, line 2263 > > > > > > How about `constainerizer_tests.cpp` so it's easy to establish the > > mapping from to _tests.cpp. > > > >

Re: Review Request 54821: Refactored Docker::run() to make it only aware of docker cli options.

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

Re: Review Request 54877: Windows: Stout: Removed dependency on Shell API.

> On Dec. 20, 2016, 12:29 a.m., Daniel Pravat wrote: > > 3rdparty/stout/include/stout/windows/os.hpp, line 748 > > > > > > I don't think the conversion to UTF-8 is appropiate here. What would you convert it to?

Re: Review Request 53717: Added `Winsock` initialization in `docker-mesos-executor`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53717/#review159673 --- Ship it! I can tweak these before committing.

Re: Review Request 54877: Windows: Stout: Removed dependency on Shell API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54877/#review159672 --- 3rdparty/stout/include/stout/windows/os.hpp (line 743)

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

> On Dec. 19, 2016, 8:33 a.m., Jiang Yan Xu wrote: > > Will we still be able to get the symlink uninstalled? If so then LGTM. I removed the symlink in `make uninstall`, though there's a lot of other cruft left behind. - James --- This

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54613/ --- (Updated Dec. 20, 2016, 12:11 a.m.) Review request for mesos, Benjamin

Re: Review Request 54803: Fixed `SlaveTests` to pass when `HAS_AUTHENTICATION` is undefined.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54803/#review159647 --- I was able to catch one flaky test by running the agent tests in

Re: Review Request 53707: Added a Windows isolator. Removed `#ifdef`-ed block from Posix.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53707/#review159667 --- src/slave/containerizer/mesos/isolators/filesystem/posix.cpp

Re: Review Request 53712: Added `system` environement variables in ` execvpe.cpp`.

> On Dec. 19, 2016, 7:07 p.m., Joseph Wu wrote: > > src/slave/containerizer/mesos/launch.cpp, line 689 > > > > > > We should allow system environment variables to be overwritten if they > > are specified by the

Re: Review Request 53790: Move containerizer Rootfs support to a cpp file.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53790/ --- (Updated Dec. 19, 2016, 11:28 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 53791: Use the stout ELF parser to collect Linux rootfs files.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53791/ --- (Updated Dec. 19, 2016, 11:24 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 54712: Use the stout ELF parser to implement ldd.

> On Dec. 15, 2016, 2:47 p.m., Jiang Yan Xu wrote: > > src/linux/ldd.cpp, line 50 > > > > > > `s/dependencies/_dependencies/`? > > > > I would say `needed` is good variable name here but `_dependencies` and

Re: Review Request 54712: Use the stout ELF parser to implement ldd.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54712/ --- (Updated Dec. 19, 2016, 11:23 p.m.) Review request for mesos, Jie Yu, Kevin

Review Request 54878: Add some simple ldd() tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54878/ --- Review request for mesos and Jiang Yan Xu. Bugs: MESOS-6588

Review Request 54877: Windows: Stout: Removed dependency on Shell API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54877/ --- Review request for mesos, Daniel Pravat, Alex Clemmer, and Joseph Wu.

Re: Review Request 52190: Removed deprecated compiler warnings.

> On Nov. 23, 2016, 9:27 p.m., Joseph Wu wrote: > > cmake/CompilationConfigure.cmake, lines 97-98 > > > > > > Can you add some comments above these? > > > > I presume that the first definition silences some

Re: Review Request 54875: CMake: Fixed typo `-DHAS_AUTHENTICATION=0` instructions.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54875/#review159664 --- Ship it! Ship It! - Alex Clemmer On Dec. 19, 2016, 10:12

Re: Review Request 54875: CMake: Fixed typo `-DHAS_AUTHENTICATION=0` instructions.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54875/#review159661 --- Ship it! Nice catch :) - Joseph Wu On Dec. 19, 2016, 2:12

Re: Review Request 54828: Fixed flags::fetch() to support Windows file paths.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54828/#review159659 --- 3rdparty/stout/include/stout/flags/fetch.hpp (lines 56 - 62)

Review Request 54875: CMake: Fixed typo `-DHAS_AUTHENTICATION=0` instructions.

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

Re: Review Request 54717: Modified isolator for dynamic addition/deletion of CNI networks.

> On Dec. 17, 2016, 3:28 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 1159-1170 > > > > > > Just curious in which case `getNetwork` will succeeds but > >

Re: Review Request 54717: Modified isolator for dynamic addition/deletion of CNI networks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54717/ --- (Updated Dec. 19, 2016, 9:53 p.m.) Review request for mesos, Jie Yu and Qian

Re: Review Request 54832: Removed extra space in method definitions in src [2/2].

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54832/#review159655 --- Ship it! Ship It! - Joseph Wu On Dec. 17, 2016, 1:38 a.m.,

Re: Review Request 54831: Removed extra space in method definition in 3rdparty [1/2].

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54831/#review159653 --- Ship it! Ship It! - Joseph Wu On Dec. 17, 2016, 1:38 a.m.,

Re: Review Request 54833: Fixed the metrics display bug in agent.html.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54833/#review159652 --- Ship it! Ship It! - Joseph Wu On Dec. 17, 2016, 1:48 a.m.,

Re: Review Request 53239: Changed master to make use of "retired" agent IDs.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53239/ --- (Updated Dec. 19, 2016, 9:15 p.m.) Review request for mesos and Vinod Kone.

Re: Review Request 53237: Improved slave recovery tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53237/ --- (Updated Dec. 19, 2016, 9:15 p.m.) Review request for mesos and Vinod Kone.

Re: Review Request 53237: Improved slave recovery tests to check reconciliation, metrics.

> On Nov. 4, 2016, 11:53 p.m., Vinod Kone wrote: > > src/tests/slave_recovery_tests.cpp, line 2886 > > > > > > why? can you add more info for posterity? The reason is that the test terminated the slave but hadn't

Review Request 54873: WIP: Added asynchronous libcurl support to Mesos.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54873/ --- Review request for mesos. Bugs: MESOS-4853 and MESOS-6129

Re: Review Request 54807: Windows: Disable CHECK-failing on symlink creation.

> On Dec. 16, 2016, 10:22 a.m., Alex Clemmer wrote: > > While not ideal, in the event that we were wrong about whether this symlink > > is required, it seems likely that it would at least make visible, obvious > > errors, instead of subtle ones. But, can we please create an issue to track > >

Re: Review Request 54768: Libprocess: Reduced binary bloat due to 'mime'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54768/ --- (Updated Dec. 19, 2016, 8:07 p.m.) Review request for mesos and Michael Park.

Re: Review Request 54859: Shortened socket paths used in test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54859/#review159641 --- Ship it! Ship It! - Joseph Wu On Dec. 19, 2016, 3:16 a.m.,

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

> On Dec. 19, 2016, 8:01 a.m., Jiang Yan Xu wrote: > > src/examples/persistent_volume_framework.cpp, lines 244-248 > > > > > > Good find. However I think the fix would be cleaner if we just update > > the state

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 Dec. 19, 2016, 7:24 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 Dec. 19, 2016, 7:25 p.m.) Review request for mesos and Jiang Yan Xu.

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 Dec. 19, 2016, 7:24 p.m.) Review request for mesos and Jiang Yan Xu.

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

> On Dec. 19, 2016, 7:46 a.m., Jiang Yan Xu wrote: > > src/master/allocator/mesos/hierarchical.hpp, lines 477-478 > > > > > > Update the comment per the comment about the role of the method. > > > > ``` > >

Re: Review Request 53712: Added `system` environement variables in ` execvpe.cpp`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53712/#review159636 --- src/slave/containerizer/mesos/launch.cpp (line 689)

Re: Review Request 53713: Reversed recoursive path creation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53713/#review159634 --- 3rdparty/stout/include/stout/os/mkdir.hpp (lines 45 - 47)

Re: Review Request 54840: Used process::loop to avoid stack overflow due to recursion.

> On Dec. 19, 2016, 5:06 p.m., Jie Yu wrote: > > 3rdparty/libprocess/src/http.cpp, line 1399 > > > > > > shared_ptr so that you don't need the `onAny` in the end. NVM, i saw encoder still takes raw pointers:) -

Re: Review Request 54768: Libprocess: Reduced binary bloat due to 'mime'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54768/ --- (Updated Dec. 19, 2016, 5:39 p.m.) Review request for mesos and Michael Park.

Re: Review Request 54840: Used process::loop to avoid stack overflow due to recursion.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54840/#review159619 --- Ship it! 3rdparty/libprocess/src/http.cpp (line 1399)

Re: Review Request 54841: Used `loop` in implementation of io::read and io::write.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54841/#review159612 --- Patch looks great! Reviews applied: [54838, 54358, 54839, 54840,

Re: Review Request 54803: Fixed `SlaveTests` to pass when `HAS_AUTHENTICATION` is undefined.

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

Review Request 54859: Shortened socket paths used in test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54859/ --- Review request for mesos and Kevin Klues. Bugs: MESOS-6811

Re: Review Request 54034: Introduced common fixture to PosixRLimitsIsolatorTest.

> On Dec. 16, 2016, 11:11 p.m., Jie Yu wrote: > > src/tests/containerizer/posix_rlimits_isolator_tests.cpp, lines 53-93 > > > > > > I don't like this refactor. I like the test to be self contained. I am > > fine

Re: Review Request 54035: Extended test coverage of posix/rlimits isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54035/ --- (Updated Dec. 19, 2016, 12:07 p.m.) Review request for mesos, Jie Yu and Till

Re: Review Request 54613: Install a symlink rather than building mesos-slave twice.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54613/#review159591 --- Will we still be able to get the symlink uninstalled? If so then

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/#review159590 --- Fix it, then Ship it! Otherwise it looks good. I am just going