Re: Review Request 66468: Added tests for operation status reconciliation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66468/#review200794 --- Patch looks great! Reviews applied: [66458, 66459, 66460, 66461,

Re: Review Request 66227: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.

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

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

> On March 27, 2018, 6:58 p.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 11881-11887 (original), 11885-11890 (patched) > > > > > > Currently, it looks like `Slave::usedResources` is the same as the >

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

> On March 27, 2018, 6:05 p.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 10798-10837 (original), 10794-10839 (patched) > > > > > > As discussed in chat, we need to update the agent's resources in the

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66051/ --- (Updated April 9, 2018, 9:32 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 65940: Added test validating hierarchical role accounting.

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

Re: Review Request 66227: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operator API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66227/ --- (Updated April 9, 2018, 9:22 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66051/ --- (Updated April 9, 2018, 9:21 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66050/ --- (Updated April 9, 2018, 9:18 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 65940: Added test validating hierarchical role accounting.

> On April 6, 2018, 12:25 a.m., Kapil Arya wrote: > > This looks good to me. I am wondering if we need more test coverage for > > insertion/deletion of roles implicit roles. Perhaps a TODO? Yes, I was considering adding unit-tests for the `RoleTrackingTree` itself, that way we can avoid the

Re: Review Request 65939: Updated role endpoints for hierarchical accounting.

> On April 6, 2018, 12:25 a.m., Kapil Arya wrote: > > src/master/master.hpp > > Lines 139-140 (patched) > > > > > > Now that we are keeping track of `allocatedResources` and > > `totalAllocatedResources`, we can

Re: Review Request 66468: Added tests for operation status reconciliation.

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

Re: Review Request 66431: Windows: Fixed `os::read()` to use `ReadFile()`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66431/#review200785 --- Ship it! Ship It! - Akash Gupta On April 9, 2018, 10:53

Re: Review Request 66468: Added tests for operation status reconciliation.

> On April 9, 2018, 5:08 p.m., Greg Mann wrote: > > src/tests/operation_reconciliation_tests.cpp > > Lines 364-365 (patched) > > > > > > Is there a reason not to do this with a `for` loop? I copied & pasted this

Re: Review Request 66468: Added tests for operation status reconciliation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66468/ --- (Updated April 9, 2018, 5:30 p.m.) Review request for mesos and Greg Mann.

Re: Review Request 66468: Added tests for operation status reconciliation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66468/#review200769 --- Nice tests! src/tests/operation_reconciliation_tests.cpp Lines

Re: Review Request 66468: Added tests for operation status reconciliation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66468/#review200779 --- Patch looks great! Reviews applied: [66458, 66459, 66460, 66461,

Re: Review Request 66468: Added tests for operation status reconciliation.

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

Re: Review Request 66431: Windows: Fixed `os::read()` to use `ReadFile()`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66431/ --- (Updated April 9, 2018, 3:53 p.m.) Review request for mesos, Akash Gupta, Eric

Re: Review Request 66431: Windows: Fixed `os::read()` to use `ReadFile()`.

> On April 6, 2018, 4:13 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/os/read.hpp > > Lines 117-127 (patched) > > > > > > I can't find much information on the implementation detailfs of > >

Re: Review Request 66440: Replaced `open()` with `os::open()` in `http_proxy.cpp`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66440/ --- (Updated April 9, 2018, 3:09 p.m.) Review request for mesos, Akash Gupta, Eric

Re: Review Request 66440: Replaced `open()` with `os::open()` in `http_proxy.cpp`.

> On April 9, 2018, 2:51 p.m., Akash Gupta wrote: > > 3rdparty/libprocess/src/http_proxy.cpp > > Lines 159 (patched) > > > > > > There's also `ERROR_PATH_NOT_FOUND`. To be safe, you might want to > > check both. I

Review Request 66468: Added tests for operation status reconciliation.

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

Re: Review Request 66440: Replaced `open()` with `os::open()` in `http_proxy.cpp`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66440/#review200764 --- Fix it, then Ship it! 3rdparty/libprocess/src/http_proxy.cpp

Re: Review Request 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66437/#review200763 --- Ship it! Ship It! - Akash Gupta On April 6, 2018, 11:15

Re: Review Request 66432: Windows: Fixed `os::write()` to use `WriteFile()`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66432/#review200762 --- Ship it! Ship It! - Akash Gupta On April 6, 2018, 11:14

Re: Review Request 66464: Implemented operation status reconciliation.

> On April 9, 2018, 12:04 p.m., Greg Mann wrote: > > src/master/http.cpp > > Lines 5097 (patched) > > > > > > s/reconcileOperations/std::move(reconcileOperations)/ > > Gaston Kleiman wrote: > Do you think that

Re: Review Request 66464: Implemented operation status reconciliation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66464/ --- (Updated April 9, 2018, 2:30 p.m.) Review request for mesos and Greg Mann.

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

> On April 9, 2018, 6:13 a.m., Benjamin Bannier wrote: > > cmake/CompilationConfigure.cmake > > Lines 308-311 (patched) > > > > > > I was briefly wondering why we checked for clang below. The reason is > > of

Re: Review Request 66481: Converted `pid` in command executor to `Option`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66481/#review200758 --- Ship it! Thanks Zhitao! - Andrew Schwartzmeyer On April 9,

Re: Review Request 66038: Add usageWithInode to check both disk and inode usage.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66038/#review200754 --- 3rdparty/stout/include/stout/posix/fs.hpp Lines 62 (patched)

Re: Review Request 66455: Windows: Fixed `os::ftruncate()` to use `FileEndOfFileInfo`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66455/#review200752 --- Ship it! Ship It! - Akash Gupta On April 6, 2018, 11:09

Re: Review Request 66038: Add usageWithInode to check both disk and inode usage.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66038/#review200751 --- 3rdparty/stout/include/stout/posix/fs.hpp Lines 61-62 (patched)

Re: Review Request 66424: Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66424/#review200749 --- Ship it! Ship It! - Akash Gupta On April 6, 2018, 10:52

Re: Review Request 66464: Implemented operation status reconciliation.

> On April 9, 2018, 12:04 p.m., Greg Mann wrote: > > src/master/http.cpp > > Lines 5097 (patched) > > > > > > s/reconcileOperations/std::move(reconcileOperations)/ Do you think that we need to use std::move to

Re: Review Request 66464: Implemented operation status reconciliation.

> On April 9, 2018, 12:04 p.m., Greg Mann wrote: > > src/master/http.cpp > > Lines 959-973 (original), 959-969 (patched) > > > > > > Hmm I think we only want to execute this block for calls which will get > > a

Re: Review Request 66464: Implemented operation status reconciliation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66464/#review200746 --- src/master/http.cpp Lines 5097 (patched)

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

> On March 29, 2018, 5:07 a.m., Benjamin Bannier wrote: > > include/mesos/mesos.proto > > Lines 1920-1921 (patched) > > > > > > These enum values are not handled in a number of switch statements in > > the code so

Re: Review Request 66511: Updated tests due to change of containerizer's `destroy()` return type.

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

Re: Review Request 66343: Added test for difference operator of hashset.

> On April 5, 2018, 8:26 p.m., Benjamin Mahler wrote: > > looks like all of these ASSERTs can be EXPECTs? Existing tests are all using ASSERTs. We can perform a sweeping change separately if desired. - Zhitao --- This is an

Re: Review Request 66464: Implemented operation status reconciliation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66464/#review200680 --- src/master/http.cpp Lines 959-973 (original), 959-969 (patched)

Re: Review Request 66481: Converted `pid` in command executor to `Option`.

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

Re: Review Request 66511: Updated tests due to change of containerizer's `destroy()` return type.

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

Re: Review Request 66508: Made sure test agent has reached stable state before starting test.

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

Review Request 66510: Unified return type of `wait` and `destroy` containerizer methods.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66510/ --- Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian

Review Request 66511: Updated tests due to change of containerizer's `destroy()` return type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66511/ --- Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian

Re: Review Request 66508: Made sure test agent has reached stable state before starting test.

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

Re: Review Request 66311: Implement recovery of resource provider manager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66311/#review200735 --- Bad patch! Reviews applied: [66311, 66310, 66309, 66308] Failed

Review Request 66508: Made sure test agent has reached stable state before starting test.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66508/ --- Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. Repository: mesos

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66493/#review200725 --- cmake/CompilationConfigure.cmake Lines 308-311 (patched)

Re: Review Request 66471: Added stringification for resource provider message type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66471/#review200733 --- Ship it! Ship It! - Jan Schlicht On April 5, 2018, 10:48

Re: Review Request 66451: Added devolve function for 'ResourceProviderInfo'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66451/#review200732 --- Ship it! Ship It! - Jan Schlicht On April 4, 2018, 2:35

Re: Review Request 66476: Tightened resource provider manager registrar tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66476/#review200730 --- Ship it! Ship It! - Jan Schlicht On April 5, 2018, 4:14

Re: Review Request 66311: Implement recovery of resource provider manager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66311/#review200729 --- FAIL: Failed to apply the dependent review: 66310. Failed

Re: Review Request 66311: Implement recovery of resource provider manager.

> On April 6, 2018, 4:22 p.m., Benjamin Bannier wrote: > > src/resource_provider/manager.cpp > > Lines 261-263 (patched) > > > > > > The used endpoint detector currently cannot deal with this which can > > e.g.,

Re: Review Request 66311: Implement recovery of resource provider manager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66311/ --- (Updated April 9, 2018, 12:29 p.m.) Review request for mesos, Jie Yu and Jan

Re: Review Request 66450: Added missing gmock expectation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66450/#review200727 --- Ship it! Ship It! - Jan Schlicht On April 4, 2018, 2:21