Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69010/ --- (Updated Oct. 15, 2018, 10:08 p.m.) Review request for mesos, Benjamin

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

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

Review Request 69033: Fixed a comment in SLRP.

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

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68956/#review209562 --- Thanks for the patches!! Sorry that it took me a while to review.

Re: Review Request 67841: Updated committer candidate guidelines.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67841/#review209559 --- PASS: Mesos patch 67841 was successfully built and tested.

Re: Review Request 68999: Fixed the XFS project ID labeling to not cross mount points.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68999/#review209567 --- Ship it! Nice trick! LGTM. - Ilya Pronin On Oct. 11, 2018,

Re: Review Request 69003: Added `task exec` to new CLI.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69003/#review209534 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 69003: Added `task exec` to new CLI.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69003/ --- (Updated Oct. 15, 2018, 8:53 a.m.) Review request for mesos and Kevin Klues.

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/os/posix/rename.hpp > > Lines 41 (patched) > > > > > > While POSIX guarantees that `rename` e.g., does not see inconsistent > >

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/os/write.hpp > > Lines 128-130 (original), 138-140 (patched) > > > > > > Not yours and not affecting the `sync=true` case, but

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69010/#review209586 --- PASS: Mesos patch 69010 was successfully built and tested.

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > src/slave/state.hpp > > Line 192 (original), 196 (patched) > > > > > > I agree with James here. It seems totally fine to me to _always `sync`_ > > here.

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/os/posix/rename.hpp > > Lines 41 (patched) > > > > > > While POSIX guarantees that `rename` e.g., does not see inconsistent > >

Re: Review Request 65784: Added validation of QuotaRequest.

> On May 1, 2018, 11:40 p.m., Meng Zhu wrote: > > src/master/quota.cpp > > Lines 199-200 (patched) > > > > > > Add `AllocationInfo` and `SharedInfo` check as well? > > Meng Zhu wrote: > Now we can use

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > src/resource_provider/storage/provider.cpp > > Lines 1158-1171 (patched) > > > > > > Since the connection of this to the surrounding could is not > >

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69010/ --- (Updated Oct. 16, 2018, 2:48 a.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

> On Oct. 13, 2018, 2:49 a.m., James DeFelice wrote: > > src/slave/state.hpp > > Line 192 (original), 196 (patched) > > > > > > Why is the default `false` here? If someone is calling the `checkpoint` > > func

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/os/posix/rename.hpp > > Lines 41 (patched) > > > > > > While POSIX guarantees that `rename` e.g., does not see inconsistent > >

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69009/ --- (Updated Oct. 16, 2018, 2:45 a.m.) Review request for mesos, Andrew

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69033/#review209578 --- PASS: Mesos patch 69033 was successfully built and tested.

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69010/#review209579 --- PASS: Mesos patch 69010 was successfully built and tested.

Review Request 69035: Added a comment for `Resource.provider_id`.

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

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/os/write.hpp > > Lines 133-134 (patched) > > > > > > Is the performance impact in the comment common knowledge? I would have >

Re: Review Request 67841: Updated committer candidate guidelines.

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

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > src/slave/state.hpp > > Line 192 (original), 196 (patched) > > > > > > I agree with James here. It seems totally fine to me to _always `sync`_ > > here.

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69010/ --- (Updated Oct. 16, 2018, 4:46 a.m.) Review request for mesos, Benjamin Bannier,

Review Request 69023: Added `retry` argument to `request()` method for Resource object.

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

Re: Review Request 68978: Added TaskIO object to new CLI for `task exec` and `task attach`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68978/ --- (Updated Oct. 15, 2018, 1:06 p.m.) Review request for mesos and Kevin Klues.

Review Request 69026: Changed usage documentation for new CLI.

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

Re: Review Request 69026: Changed usage documentation for new CLI.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69026/#review209536 --- Ship it! Ship It! - Kevin Klues On Okt. 15, 2018, 11:45

Re: Review Request 69026: Changed usage documentation for new CLI.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69026/#review209539 --- PASS: Mesos patch 69026 was successfully built and tested.

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69009/#review209538 --- 3rdparty/stout/include/stout/os/posix/rename.hpp Lines 41

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69010/#review209542 --- src/resource_provider/storage/provider.cpp Line 663 (original),

Re: Review Request 69036: WIP: Added an optional `profile` field to `CreateDisk`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69036/#review209591 --- PASS: Mesos patch 69036 was successfully built and tested.

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68706/#review209547 --- I think that we need test for this as well. At minimum, we ought

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > src/resource_provider/storage/provider.cpp > > Lines 1158-1171 (patched) > > > > > > Since the connection of this to the surrounding could is not > >

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/os/posix/rename.hpp > > Lines 41 (patched) > > > > > > While POSIX guarantees that `rename` e.g., does not see inconsistent > >

Re: Review Request 67840: Renamed committer checklist into committer guidelines.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67840/#review209551 --- Ship it! Ship It! - Gastón Kleiman On Oct. 15, 2018, 9:25

Re: Review Request 67841: Updated committer candidate guidelines.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67841/#review209552 --- Fix it, then Ship it! docs/committer-candidate-guidelines.md

Re: Review Request 67840: Renamed committer checklist into committer guidelines.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67840/#review209553 --- Ship it! Ship It! - James Peach On Oct. 15, 2018, 4:25

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

> On Oct. 15, 2018, 4:21 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/os/posix/rename.hpp > > Lines 41 (patched) > > > > > > While POSIX guarantees that `rename` e.g., does not see inconsistent > >

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

> On Oct. 15, 2018, 2:21 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/os/write.hpp > > Lines 133-134 (patched) > > > > > > Is the performance impact in the comment common knowledge? I would have >

Re: Review Request 67841: Updated committer candidate guidelines.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67841/ --- (Updated Oct. 15, 2018, 5:43 p.m.) Review request for mesos, Gastón Kleiman

Re: Review Request 67840: Renamed committer checklist into committer guidelines.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67840/ --- (Updated Oct. 15, 2018, 5:43 p.m.) Review request for mesos, Gastón Kleiman

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68706/ --- (Updated Oct. 16, 2018, 4:19 a.m.) Review request for mesos, Benjamin Mahler,