Re: Review Request 65072: Fixed handling of terminal operations in `updateSlave` handler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65072/ --- (Updated Jan. 12, 2018, 11:57 a.m.) Review request for mesos, Gaston Kleiman

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65096/ --- (Updated Jan. 12, 2018, 11:53 a.m.) Review request for mesos and Jan Schlicht.

Re: Review Request 65095: Made it possible to update an operation without mutating resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65095/ --- (Updated Jan. 12, 2018, 11:53 a.m.) Review request for mesos, Gaston Kleiman,

Re: Review Request 65095: Made it possible to update an operation without mutating resources.

> On Jan. 11, 2018, 10 p.m., Greg Mann wrote: > > src/master/master.hpp > > Lines 881-885 (original), 881-888 (patched) > > > > > > I'm thinking that `convertResources` might be a more intuitive name for > > the

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

> On Jan. 11, 2018, 10:22 p.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 10343-10344 (patched) > > > > > > What about speculative operations initiated via the operator API on > > resources offered by

Re: Review Request 65072: Fixed handling of terminal operations in `updateSlave` handler.

> On Jan. 11, 2018, 3:07 a.m., Gaston Kleiman wrote: > > src/master/master.cpp > > Line 10419 (original), 10447 (patched) > > > > > > Shouldn't we skip this if the operation was already in a terminal state? Why

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65096/#review195304 --- Fix it, then Ship it! src/master/master.cpp Lines 10306

Re: Review Request 65095: Made it possible to update an operation without mutating resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65095/#review195306 --- Ship it! Ship It! - Jan Schlicht On Jan. 12, 2018, 11:53

Re: Review Request 65072: Fixed handling of terminal operations in `updateSlave` handler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65072/#review195302 --- FAIL: mesos-java failed to build. Reviews applied: `['65093',

Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

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

Re: Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65122/#review195309 --- FAIL: mesos-java failed to build. Reviews applied: `['65122']`

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 12, 2018, 2:24 a.m., Vinod Kone wrote: > > AFAICT, in 1.4.x we show unreachable terminal tasks on a removed agent as > > completed tasks. But now, we show them as unreachable tasks. If that's true > > it's an API change that needs to be called out. Is that really backwards > >

Re: Review Request 64386: Refactored health checks to take in executor information.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64386/ --- (Updated Jan. 12, 2018, 2:36 p.m.) Review request for mesos, Alexander

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65074/ --- (Updated Jan. 12, 2018, 3:40 p.m.) Review request for mesos, James Peach and

Re: Review Request 65126: Added a resource provider test case.

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

Re: Review Request 65126: Added a resource provider test case.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65126/#review195321 --- FAIL: mesos-java failed to build. Reviews applied: `['65125',

Review Request 65127: Windows: Enabled docker health checks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65127/ --- Review request for mesos, Alexander Rukletsov and Andrew Schwartzmeyer.

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

> On Jan. 12, 2018, 2:38 a.m., Vinod Kone wrote: > > src/slave/compatibility.cpp > > Line 156 (original), 135 (patched) > > > > > > s/by Mesos//. Seems redundant? Without the suffix, the comment could be

Review Request 65125: Added a helper function for resource provider tests.

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

Review Request 65126: Added a resource provider test case.

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

Re: Review Request 65127: Windows: Enabled docker health checks.

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

Re: Review Request 63861: Windows: Updated networking doc.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63861/ --- (Updated Jan. 12, 2018, 2:35 p.m.) Review request for mesos, Alexander

Re: Review Request 63860: Windows: Mapped the Docker network info types.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63860/ --- (Updated Jan. 12, 2018, 2:34 p.m.) Review request for mesos, Alexander

Re: Review Request 63859: Windows: Fixed mock signal values in stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63859/ --- (Updated Jan. 12, 2018, 2:33 p.m.) Review request for mesos, Andrew

Re: Review Request 65072: Fixed handling of terminal operations in `updateSlave` handler.

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

Re: Review Request 62965: Added doc for cgroups devices isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62965/ --- (Updated Jan. 12, 2018, 4:24 p.m.) Review request for mesos, Gilbert Song,

Re: Review Request 65122: Fixed race condition in io switchboard for ATTACH_CONTAINER_INPUT call.

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

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

> On Jan. 12, 2018, 6:19 p.m., James Peach wrote: > > The bug we hit was where the resources didn't change but containered mount > > disks. Are we confident that the test here (where a mount disk is added) > > also covers the case where the resources don't change? I think that's what the last

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/ --- (Updated Jan. 12, 2018, 7:24 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 62965: Added doc for cgroups devices isolator.

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

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65096/#review195338 --- Fix it, then Ship it! src/master/master.cpp Lines 10339

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review195339 --- Ship it! LGTM. I still see open issues though, can you please

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

> On Jan. 12, 2018, 6:19 p.m., James Peach wrote: > > src/common/resources.cpp > > Line 263 (original), 263 (patched) > > > > > > Nit: `static bool compareResourceMetadata(...)` I'll fix this before committing. -

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65074/#review195334 --- Fix it, then Ship it! The bug we hit was where the resources

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

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

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review195350 --- Ship it! Will commit with a few touches like the following.

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/#review195358 --- include/mesos/v1/mesos.proto Lines 901-909 (patched)

Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

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

Review Request 65147: Windows: Fixed docker executor `PATH` variable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65147/ --- Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu. Bugs: MESOS-8443

Re: Review Request 64909: Removed unnecessary `SocketManager::dispose` data structure.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64909/#review195374 --- Looks good other than the looping over map values.

Re: Review Request 61158: Introduced http::Server in process.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61158/#review195375 --- Ship it! Can we clarify why we ifdef guarded it? Specifically

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

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

Re: Review Request 65147: Windows: Fixed docker executor `PATH` variable.

> On Jan. 12, 2018, 6:51 p.m., Mesos Reviewbot Windows wrote: > > FAIL: Some Mesos tests failed. > > > > Reviews applied: `['65144', '65145', '65146', '65147']` > > > > Failed command: `D:\DCOS\mesos\src\mesos-tests.exe --verbose` > > > > All the build artifacts available at: > >

Re: Review Request 65147: Windows: Fixed docker executor `PATH` variable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65147/ --- (Updated Jan. 12, 2018, 10:31 p.m.) Review request for mesos, Akash Gupta, Jie

Re: Review Request 65149: MesosTidy: Enabled `mesos-this-capture`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65149/#review195388 --- Fix it, then Ship it! support/clang-tidy Line 26 (original),

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/ --- (Updated Jan. 13, 2018, 1:56 a.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

> On Jan. 13, 2018, 1:14 a.m., Vinod Kone wrote: > > Can you update the `GetMaster` test in api_tests.cpp? Yep, shoulda done this in the first place - thx! Test updated. - Greg --- This is an automatically generated e-mail. To reply,

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65137/ --- (Updated Jan. 12, 2018, 6:29 p.m.) Review request for mesos, Benjamin Bannier

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/#review195370 --- Can you update the `GetMaster` test in api_tests.cpp?

Review Request 65146: Windows: Enabled internet tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65146/ --- Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu. Bugs: MESOS-8225

Review Request 65144: Ported `os::which` to Windows.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65144/ --- Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu. Bugs: MESOS-8225

Review Request 65145: Fixed use of `os::which`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65145/ --- Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu. Bugs: MESOS-8225

Re: Review Request 65147: Windows: Fixed docker executor `PATH` variable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65147/#review195383 --- FAIL: Some Mesos tests failed. Reviews applied: `['65144',

Review Request 65149: MesosTidy: Enabled `mesos-this-capture`.

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

Re: Review Request 65143: Logged some additional information on a master CHECK.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65143/#review195369 --- Ship it! Ship It! - Gilbert Song On Jan. 12, 2018, 4:44

Re: Review Request 65063: Replace `stringify()` with `jsonify()` in http serialization.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65063/#review195377 --- Ship it! Ship It! - Michael Park On Jan. 9, 2018, 11:06

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65137/#review195379 --- src/master/master.cpp Lines 10358-10359 (patched)

Re: Review Request 64908: Refactored `HttpProxy` to not rely on `SocketManager`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64908/#review195365 --- Fix it, then Ship it! Looks good, just looks like the comments

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65137/#review195376 --- FAIL: Some Mesos tests failed. Reviews applied: `['65093',

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/#review195380 --- Ship it! Ship It! - Vinod Kone On Jan. 13, 2018, 1:56 a.m.,

Re: Review Request 65146: Windows: Enabled internet tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65146/#review195390 --- FAIL: Some Mesos tests failed. Reviews applied: `['65144',

Re: Review Request 65147: Windows: Fixed docker executor `PATH` variable.

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

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65137/#review195371 --- Ship it! Ship It! - Greg Mann On Jan. 13, 2018, 12:28 a.m.,

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

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

Re: Review Request 65149: MesosTidy: Enabled `mesos-this-capture`.

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

Re: Review Request 65095: Made it possible to update an operation without mutating resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65095/#review195346 --- Ship it! src/master/master.cpp Line 10318 (original), 10319

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/ --- (Updated Jan. 13, 2018, 12:11 a.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 62965: Added doc for cgroups devices isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62965/#review195355 --- Ship it! Ship It! - Gilbert Song On Jan. 12, 2018, 8:24

Review Request 65143: Logged some additional information on a master CHECK.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65143/ --- Review request for mesos and Andrew Schwartzmeyer. Repository: mesos

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/ --- (Updated Jan. 12, 2018, 11:07 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/ --- (Updated Jan. 12, 2018, 11:07 p.m.) Review request for mesos, Anand Mazumdar,

Review Request 65141: Fixed the default executor flaky testes in tests/cluster.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65141/ --- Review request for mesos, Andrei Budnik, Alexander Rukletsov, Benjamin Mahler,

Review Request 65139: Reverted "Fixed `wait()` and `destroy()` in composing containerizer.".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65139/ --- Review request for mesos, Andrei Budnik, Alexander Rukletsov, Benjamin Mahler,

Review Request 65140: Reverted "Updated composing containerizer tests.".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65140/ --- Review request for mesos, Andrei Budnik, Alexander Rukletsov, Benjamin Mahler,

Re: Review Request 65141: Fixed the default executor flaky testes in tests/cluster.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65141/#review195368 --- Ship it! Ship It! - Vinod Kone On Jan. 13, 2018, 12:15

Re: Review Request 65140: Reverted "Updated composing containerizer tests.".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65140/#review195367 --- Ship it! Ship It! - Vinod Kone On Jan. 13, 2018, 12:15

Re: Review Request 65106: Removed the misleading `isRemovable` helper in the master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65106/#review195349 --- Will need to rebase off of https://reviews.apache.org/r/64940/

Re: Review Request 65104: Updated documentation for `d_type` check regarding `xfs` on `overlay`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65104/#review195364 --- Ship it! Ship It! - Gilbert Song On Jan. 11, 2018, 5:13

Re: Review Request 65139: Reverted "Fixed `wait()` and `destroy()` in composing containerizer.".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65139/#review195366 --- Ship it! Ship It! - Vinod Kone On Jan. 13, 2018, 12:15

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

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

Re: Review Request 64907: Added abandonment and process exit support to `loop()`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64907/#review195362 --- Thanks for fixing this: (1) There seems to be an issue with the

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review195241 --- LGTM modulo Vinod's comments. Sorry we have gone back and forth

Re: Review Request 65096: Fixed master's `updateOperation` for operations without framework ID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65096/ --- (Updated Jan. 12, 2018, 1:49 p.m.) Review request for mesos and Jan Schlicht.