Re: Review Request 68810: Removed call to 'setsid()' in command executor if TTY attached.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68810/#review208915 --- PASS: Mesos patch 68810 was successfully built and tested.

Re: Review Request 68806: Fixed outdated comments for mocking the secret generator.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68806/#review208914 --- PASS: Mesos patch 68806 was successfully built and tested.

Re: Review Request 68807: Used `Future::discard` to abort an outdated resuorce provider launches.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68807/#review208913 --- PASS: Mesos patch 68807 was successfully built and tested.

Re: Review Request 68809: Fixed a typo in `slave.cpp`.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68809/#review208912 --- PASS: Mesos patch 68809 was successfully built and tested.

Re: Review Request 68732: Cached weights in the sorters nodes.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68732/#review208911 --- FAIL: Some of the unit tests failed. Please check the relevant

Review Request 68810: Removed call to 'setsid()' in command executor if TTY attached.

2018-09-21 Thread Kevin Klues
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68810/ --- Review request for mesos, Gilbert Song and Jie Yu. Bugs: MESOS-8978

Re: Review Request 68724: Added the ability to launch tasks with a TTY attached to mesos-execute.

2018-09-21 Thread Kevin Klues
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68724/ --- (Updated Sept. 22, 2018, 12:35 vorm.) Review request for mesos, Gilbert Song

Re: Review Request 68807: Used `Future::discard` to abort an outdated resuorce provider launches.

2018-09-21 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68807/ --- (Updated Sept. 22, 2018, 12:19 a.m.) Review request for mesos, Benjamin

Re: Review Request 68763: Cleaned up residual containers when removing resource provider configs.

2018-09-21 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68763/ --- (Updated Sept. 22, 2018, 12:17 a.m.) Review request for mesos, Benjamin

Re: Review Request 68138: Added tests to ensure correct quota accounting.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68138/#review208910 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 68731: Added a ScalarResourceQuantities type to improve sorters performance.

2018-09-21 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68731/#review208909 --- Ship it! Ship It! - Meng Zhu On Sept. 21, 2018, 4:24 p.m.,

Re: Review Request 68731: Added a ScalarResourceQuantities type to improve sorters performance.

2018-09-21 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68731/#review208908 --- src/master/allocator/sorter/sorter.hpp Lines 177-180 (patched)

Re: Review Request 68763: Cleaned up residual containers when removing resource provider configs.

2018-09-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68763/#review208891 --- Fix it, then Ship it! src/resource_provider/daemon.cpp Lines

Re: Review Request 68731: Added a ScalarResourceQuantities type to improve sorters performance.

2018-09-21 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68731/ --- (Updated Sept. 21, 2018, 11:24 p.m.) Review request for mesos, Gastón Kleiman

Re: Review Request 68732: Cached weights in the sorters nodes.

2018-09-21 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68732/ --- (Updated Sept. 21, 2018, 11:24 p.m.) Review request for mesos, Gastón Kleiman

Review Request 68807: Used `Future::discard` to abort an outdated resuorce provider launches.

2018-09-21 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68807/ --- Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.

Review Request 68806: Removed outdated comments for mocking the secret generator.

2018-09-21 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68806/ --- Review request for mesos and Benjamin Bannier. Repository: mesos Description

Review Request 68809: Fixed a typo in `slave.cpp`.

2018-09-21 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68809/ --- Review request for mesos and Gastón Kleiman. Repository: mesos Description

Re: Review Request 68732: Cached weights in the sorters nodes.

2018-09-21 Thread Benjamin Mahler
> On Sept. 19, 2018, 5:54 p.m., Meng Zhu wrote: > > src/master/allocator/sorter/drf/sorter.cpp > > Lines 316 (patched) > > > > > > Can't we just use the client map? > > > >

Re: Review Request 68803: Fixed flaky test `RemoveExecutorUponFailedLaunch`.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68803/#review208905 --- PASS: Mesos patch 68803 was successfully built and tested.

Re: Review Request 68731: Added a ScalarResourceQuantities type to improve sorters performance.

2018-09-21 Thread Benjamin Mahler
> On Sept. 19, 2018, 9:27 p.m., Meng Zhu wrote: > > src/master/allocator/sorter/sorter.hpp > > Lines 163-164 (patched) > > > > > > Why we are enforcing alphabetically sorted? Seems to me unnecessary, > > especially

Re: Review Request 68804: Added Stout `os::readlink` function for POSIX paths.

2018-09-21 Thread Eric Chung
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68804/#review208902 --- Ship it! Ship It! - Eric Chung On Sept. 21, 2018, 10:34

Re: Review Request 68804: Added Stout `os::readlink` function for POSIX paths.

2018-09-21 Thread Jason Lai
> On Sept. 21, 2018, 10:53 p.m., Eric Chung wrote: > > 3rdparty/stout/include/stout/os/posix/readlink.hpp > > Lines 34 (patched) > > > > > > should the commented lines be removed? No. If you look at the rest of the

Re: Review Request 68122: Fixed couple of typos in the allocator.

2018-09-21 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68122/#review208898 --- Ship it! Ship It! - Gastón Kleiman On Sept. 21, 2018, 2:51

Re: Review Request 68804: Added Stout `os::readlink` function for POSIX paths.

2018-09-21 Thread Eric Chung
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68804/#review208899 --- 3rdparty/stout/include/stout/os/posix/readlink.hpp Lines 34

Re: Review Request 68138: Added tests to ensure correct quota accounting.

2018-09-21 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68138/ --- (Updated Sept. 21, 2018, 3:52 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout

2018-09-21 Thread Jason Lai
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65812/ --- (Updated Sept. 21, 2018, 10:45 p.m.) Review request for mesos, Eric Chung,

Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout

2018-09-21 Thread Jason Lai
> On May 18, 2018, 10:12 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 68-81 (patched) > > > > > > IN fact, I would consider making `unprocessed` a `vector` > > (which is a

Re: Review Request 65812: Added an overloaded version of `os::realpath` to stout

2018-09-21 Thread Jason Lai
> On May 18, 2018, 10:08 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/os/posix/realpath.hpp > > Lines 51 (patched) > > > > > > Any reason we need this parameter here? Can we just remove this > > parameter,

Review Request 68804: Added Stout `os::readlink` function for POSIX paths.

2018-09-21 Thread Jason Lai
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68804/ --- Review request for mesos, Eric Chung, Gilbert Song, Jie Yu, James Peach, and

Re: Review Request 68803: Fixed flaky test `RemoveExecutorUponFailedLaunch`.

2018-09-21 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68803/#review208890 --- Ship it! Ship It! - Greg Mann On Sept. 21, 2018, 9:18 p.m.,

Re: Review Request 68801: Fixed flaky test `SlaveAuthenticationRetryBackoff`.

2018-09-21 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68801/#review208889 --- Ship it! Ship It! - Greg Mann On Sept. 21, 2018, 7:18 p.m.,

Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)

2018-09-21 Thread Jason Lai
> On May 18, 2018, 10:08 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 60 (patched) > > > > > > In fact, I'd return `Try` if it escapes the root path, this will allow > > us to avoid some

Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)

2018-09-21 Thread Jason Lai
> On May 18, 2018, 4:57 p.m., Jie Yu wrote: > > 3rdparty/stout/include/stout/path.hpp > > Lines 87 (patched) > > > > > > I small nits, i'd suggest the following to make the logic a bit easier > > to understand: > >

Re: Review Request 68138: Added tests to ensure correct quota accounting.

2018-09-21 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68138/ --- (Updated Sept. 21, 2018, 2:55 p.m.) Review request for mesos and Benjamin

Re: Review Request 65811: Add `path::normalize` to stout for normalizing path (for POSIX only now)

2018-09-21 Thread Jason Lai
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65811/ --- (Updated Sept. 21, 2018, 9:53 p.m.) Review request for mesos, Anish Gupta,

Re: Review Request 68122: Fixed couple of typos in the allocator.

2018-09-21 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68122/ --- (Updated Sept. 21, 2018, 2:51 p.m.) Review request for mesos and Benjamin

Re: Review Request 68762: Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68762/#review208886 --- FAIL: Some of the unit tests failed. Please check the relevant

Review Request 68803: Fixed flaky test `RemoveExecutorUponFailedLaunch`.

2018-09-21 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68803/ --- Review request for mesos and Greg Mann. Repository: mesos Description

Re: Review Request 68800: Removed bundled libev patch.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68800/#review208884 --- PASS: Mesos patch 68800 was successfully built and tested.

Re: Review Request 68801: Fixed flaky test `SlaveAuthenticationRetryBackoff`.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68801/#review208883 --- PASS: Mesos patch 68801 was successfully built and tested.

Re: Review Request 68758: Added unit tests for adding/updating invalid resource provider configs.

2018-09-21 Thread Chun-Hung Hsiao
> On Sept. 21, 2018, 9:41 a.m., Benjamin Bannier wrote: > > src/tests/agent_resource_provider_config_api_tests.cpp > > Lines 240 (patched) > > > > > > Ups, that turned out to be a lot. Thanks for doing this. The

Re: Review Request 68763: Cleaned up residual containers when removing resource provider configs.

2018-09-21 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68763/ --- (Updated Sept. 21, 2018, 7:47 p.m.) Review request for mesos, Benjamin

Re: Review Request 68790: Moved the container ID prefix generation to `LocalResourceProvider`.

2018-09-21 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68790/ --- (Updated Sept. 21, 2018, 7:44 p.m.) Review request for mesos, Benjamin

Re: Review Request 68800: Removed bundled libev patch.

2018-09-21 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68800/#review208881 --- Ship it! Ship It! - Benjamin Mahler On Sept. 21, 2018, 7:18

Re: Review Request 68801: Fixed flaky test `SlaveAuthenticationRetryBackoff`.

2018-09-21 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68801/#review208878 --- Ship it! Ship It! - Gastón Kleiman On Sept. 21, 2018, 12:18

Review Request 68801: Fixed flaky test `SlaveAuthenticationRetryBackoff`.

2018-09-21 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68801/ --- Review request for mesos, Gastón Kleiman and Greg Mann. Repository: mesos

Review Request 68800: Removed bundled libev patch.

2018-09-21 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68800/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-895

Re: Review Request 68763: Cleaned up residual containers when removing resource provider configs.

2018-09-21 Thread Chun-Hung Hsiao
> On Sept. 21, 2018, 10:50 a.m., Benjamin Bannier wrote: > > src/resource_provider/daemon.cpp > > Lines 431-433 (original), 483-485 (patched) > > > > > > How does this hold? It seems we should e.g., insert a

Re: Review Request 68641: Added version check and bundling of libevent within libprocess.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68641/#review208871 --- PASS: Mesos patch 68641 was successfully built and tested.

Re: Review Request 68763: Cleaned up residual containers when removing resource provider configs.

2018-09-21 Thread Chun-Hung Hsiao
> On Sept. 21, 2018, 10:50 a.m., Benjamin Bannier wrote: > > src/resource_provider/daemon.cpp > > Lines 566 (patched) > > > > > > Let's assert here instead of letting the exception propagate. > > Chun-Hung Hsiao

Re: Review Request 68758: Added unit tests for adding/updating invalid resource provider configs.

2018-09-21 Thread Chun-Hung Hsiao
> On Sept. 21, 2018, 9:41 a.m., Benjamin Bannier wrote: > > src/tests/agent_resource_provider_config_api_tests.cpp > > Lines 244-245 (patched) > > > > > > An alternative to the closure we sometimes use in tests is

Re: Review Request 68762: Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.

2018-09-21 Thread Chun-Hung Hsiao
> On Sept. 21, 2018, 10:05 a.m., Benjamin Bannier wrote: > > src/tests/agent_resource_provider_config_api_tests.cpp > > Line 35 (original), 35 (patched) > > > > > > Like you wrote in your _Testing Done_ section, this

Re: Review Request 68763: Cleaned up residual containers when removing resource provider configs.

2018-09-21 Thread Chun-Hung Hsiao
> On Sept. 21, 2018, 10:50 a.m., Benjamin Bannier wrote: > > src/resource_provider/daemon.cpp > > Lines 426-427 (original), 478-481 (patched) > > > > > > Wouldn't this be a situation where we'd like to respond with

Re: Review Request 68641: Added version check and bundling of libevent within libprocess.

2018-09-21 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68641/#review208864 --- You need to copy `libevent.m4` here for the standa-alone

Re: Review Request 68640: Added version check and bundling of libevent to autotools.

2018-09-21 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68640/#review208863 --- Fix it, then Ship it! configure.ac Line 1491 (original), 1491

Re: Review Request 68784: Fixed broken pipe error in IOSwitchboard.

2018-09-21 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68784/#review208859 --- PASS: Mesos patch 68784 was successfully built and tested. All

Re: Review Request 68784: Fixed broken pipe error in IOSwitchboard.

2018-09-21 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68784/#review208854 --- src/slave/containerizer/mesos/io/switchboard.cpp Lines 1757-1761

Re: Review Request 68768: Fixed disconnection while sending acknowledgment to IOSwitchboard.

2018-09-21 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68768/#review208853 --- Ship it! Ship It! - Alexander Rukletsov On Sept. 19, 2018,

Re: Review Request 68784: Fixed broken pipe error in IOSwitchboard.

2018-09-21 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68784/#review208852 --- Ship it! Ship It! - Alexander Rukletsov On Sept. 21, 2018,

Re: Review Request 68784: Fixed broken pipe error in IOSwitchboard.

2018-09-21 Thread Andrei Budnik
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68784/ --- (Updated Sept. 21, 2018, 12:12 p.m.) Review request for mesos and Alexander

Re: Review Request 68679: Fixed stout `FsTest.Used` test.

2018-09-21 Thread Till Toenshoff via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68679/#review208849 --- Ship it! Thanks so much James for staying on this ball. Giving

Re: Review Request 68763: Cleaned up residual containers when removing resource provider configs.

2018-09-21 Thread Benjamin Bannier
> On Sept. 19, 2018, 5:35 p.m., Benjamin Bannier wrote: > > src/resource_provider/daemon.cpp > > Line 290 (original), 319 (patched) > > > > > > Not sure how this will look like once we handle agent failover, but it

Re: Review Request 68763: Cleaned up residual containers when removing resource provider configs.

2018-09-21 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68763/#review208846 --- src/resource_provider/daemon.cpp Lines 426-427 (original),

Re: Review Request 68762: Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.

2018-09-21 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68762/#review208754 --- Fix it, then Ship it!

Re: Review Request 68790: Moved the container ID prefix generation to `LocalResourceProvider`.

2018-09-21 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68790/#review208845 --- Fix it, then Ship it! src/resource_provider/local.cpp Lines

Re: Review Request 68758: Added unit tests for adding/updating invalid resource provider configs.

2018-09-21 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68758/#review208843 --- Ship it!

Re: Review Request 68777: Set master/agent flags in `AgentResourceProviderConfigApiTest` fixture.

2018-09-21 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68777/#review208842 --- Ship it!

Re: Review Request 68756: Performed RP-specific validations when adding/updating RP configs.

2018-09-21 Thread Benjamin Bannier
> On Sept. 19, 2018, 2:43 p.m., Jan Schlicht wrote: > > src/resource_provider/local.hpp > > Lines 47 (patched) > > > > > > This should be defined in `resource_provider/validation.hpp`, not here. > > Implementation

Re: Review Request 68756: Performed RP-specific validations when adding/updating RP configs.

2018-09-21 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68756/#review208841 --- Ship it! Ship It! - Benjamin Bannier On Sept. 19, 2018,