Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

2018-10-29 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69146/#review210143 --- Ship it! Ship It! - Joseph Wu On Oct. 25, 2018, 12:34 p.m.,

Re: Review Request 69033: Fixed a comment in SLRP.

2018-10-29 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69033/#review210144 --- Ship it! Ship It! - Benjamin Bannier On Oct. 16, 2018,

Re: Review Request 68706: Added master failover reregistration progress metrics.

2018-10-29 Thread Xudong Ni via Review Board
> On Oct. 15, 2018, 4:45 p.m., James Peach wrote: > > I think that we need test for this as well. At minimum, we ought to update > > `MasterTest.MetricsInMetricsEndpoint`. Best would be a test that registers > > a number of agents, then restarts the master and validates the metrics. Basic

Re: Review Request 69110: Added task health check definitions to master API responses.

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

Review Request 69203: Fixed LongLivedDefaultExecutorRestart GC test.

2018-10-29 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69203/ --- Review request for mesos, Meng Zhu and Qian Zhang. Bugs: MESOS-9217

Re: Review Request 69203: Fixed LongLivedDefaultExecutorRestart GC test.

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

Re: Review Request 69123: Fixed an early fd close in the cgroups event notifier.

2018-10-29 Thread Qian Zhang
> On Oct. 25, 2018, 4:07 p.m., Qian Zhang wrote: > > src/linux/cgroups.cpp > > Lines 1071-1080 (original), 1071-1089 (patched) > > > > > > I see we already have an onAny callback `_listen`, can we close the fd > >

Re: Review Request 68706: Added master failover reregistration progress metrics.

2018-10-29 Thread Jiang Yan Xu
> On Oct. 18, 2018, 11:14 a.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 1874-1875 (patched) > > > > > > If we use equality operator and only set the timer when such a number > > of reregistered

Re: Review Request 68706: Added master failover reregistration progress metrics.

2018-10-29 Thread Xudong Ni via Review Board
> On Oct. 18, 2018, 6:14 p.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 1874-1875 (patched) > > > > > > If we use equality operator and only set the timer when such a number > > of reregistered

Re: Review Request 69085: Stout: Always `fsync` created directories in POSIX `mkdir`.

2018-10-29 Thread Chun-Hung Hsiao
> On Oct. 25, 2018, 1:20 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/os/posix/mkdir.hpp > > Lines 67-72 (patched) > > > > > > Do we want to `fsync` directory entries which existed previously? I >

Re: Review Request 69085: Stout: Always `fsync` created directories in POSIX `mkdir`.

2018-10-29 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69085/ --- (Updated Oct. 29, 2018, 9:12 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 69033: Fixed a comment in SLRP.

2018-10-29 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69033/#review210156 --- Patch looks great! Reviews applied: [69033] Passed command:

Re: Review Request 69203: Fixed LongLivedDefaultExecutorRestart GC test.

2018-10-29 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69203/#review210153 --- Ship it! src/tests/gc_tests.cpp Lines 919 (patched)

Re: Review Request 69171: Added validation of cache files to the URI Fetcher.

2018-10-29 Thread Andrei Budnik
> On Oct. 26, 2018, 11:20 p.m., Joseph Wu wrote: > > src/slave/containerizer/fetcher.cpp > > Lines 1030-1031 (patched) > > > > > > Hum... I think this will work as intended, but the logic and comment > > are

Re: Review Request 69205: Fixed FetcherTest.DuplicateFileURI on OSX.

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

Re: Review Request 69171: Added validation of cache files to the URI Fetcher.

2018-10-29 Thread Andrei Budnik
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69171/ --- (Updated Oct. 29, 2018, 10:33 p.m.) Review request for mesos, Gilbert Song,

Review Request 69210: Used the MS_SILENT mount flag to elide unwanted logging.

2018-10-29 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69210/ --- Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Jiang Yan Xu.

Review Request 69211: Improved the code comments for `getContainerDevicesPath`.

2018-10-29 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69211/ --- Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Jiang Yan Xu.

Re: Review Request 69203: Fixed LongLivedDefaultExecutorRestart GC test.

2018-10-29 Thread Joseph Wu
> On Oct. 29, 2018, 3:10 p.m., Meng Zhu wrote: > > src/tests/gc_tests.cpp > > Lines 919 (patched) > > > > > > NIT: ideally we should do `process::ID::generate()`. Good point. There are several other tests that

Re: Review Request 69086: Move the container `/dev` construction to the isolators.

2018-10-29 Thread James Peach
> On Oct. 29, 2018, 4:42 a.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > > Lines 414 (patched) > > > > > > Not related to this patch. When I review this patch, I was looking at

Re: Review Request 69086: Moved container root construction to the isolators.

2018-10-29 Thread James Peach
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69086/ --- (Updated Oct. 29, 2018, 11:59 p.m.) Review request for mesos, Gilbert Song,

Review Request 69205: Fixed FetcherTest.DuplicateFileURI on OSX.

2018-10-29 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69205/ --- Review request for mesos and Benjamin Bannier. Bugs: MESOS-9357

Re: Review Request 69188: Ensured failed / discarded cgroups OOM notification is logged.

2018-10-29 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69188/#review210137 --- Patch looks great! Reviews applied: [69123, 69188] Passed

Review Request 69207: Moved 'updated_tasks()' to new CLI tests base.

2018-10-29 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69207/ --- Review request for mesos and Kevin Klues. Bugs: MESOS-9363

Re: Review Request 69211: Improved the code comments for `getContainerDevicesPath`.

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

Review Request 69206: Added return value to new Mesos CLI commands.

2018-10-29 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69206/ --- Review request for mesos and Kevin Klues. Bugs: MESOS-9363

Review Request 69208: Updated new CLI task attach/exec exit strategy.

2018-10-29 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69208/ --- Review request for mesos and Kevin Klues. Bugs: MESOS-9363

Re: Review Request 69203: Fixed LongLivedDefaultExecutorRestart GC test.

2018-10-29 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69203/#review210160 --- Patch looks great! Reviews applied: [69203] Passed command:

Re: Review Request 69208: Updated new CLI task attach/exec exit strategy.

2018-10-29 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69208/#review210162 --- FAIL: Failed to apply the dependent review: 69207. Failed

Re: Review Request 69086: Move the container `/dev` construction to the isolators.

2018-10-29 Thread James Peach
> On Oct. 29, 2018, 4:42 a.m., Jie Yu wrote: > > src/linux/fs.hpp > > Lines 397-401 (patched) > > > > > > Any reason need this option? I was thinking just doing dev mounts > > always from linux fileystem isolator.

Re: Review Request 69086: Move the container `/dev` construction to the isolators.

2018-10-29 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69086/#review210141 --- Bad patch! Reviews applied: [69086] Failed command:

Re: Review Request 69110: Added task health check definitions to master API responses.

2018-10-29 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69110/ --- (Updated Oct. 29, 2018, 5:54 p.m.) Review request for mesos, Alexander