Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

> On June 19, 2015, 10:57 p.m., Jie Yu wrote: > > src/slave/slave.cpp, lines 1579-1583 > > > > > > This should be a CHECK instead? I've created [r35686](https://reviews.apache.org/r/35686/) as a follow-up for this.

Review Request 35686: Change the if statement to a CHECK at the end of _runTask.

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

Re: Review Request 35682: Sped up Allocator::updateSlave() and Allocator::updateAllocation().

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35682/#review88620 --- Ship it! Ship It! - Michael Park On June 20, 2015, 1:08 a.m., Vi

Re: Review Request 35680: Added Sorter::allocation() overload for getting resources of a particular slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35680/#review88619 --- Ship it! Ship It! - Michael Park On June 20, 2015, 1:04 a.m., Vi

Re: Review Request 35680: Added Sorter::allocation() overload for getting resources of a particular slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35680/#review88617 --- Ship it! Ship It! - Jie Yu On June 20, 2015, 1:04 a.m., Vinod Ko

Re: Review Request 35682: Sped up Allocator::updateSlave() and Allocator::updateAllocation().

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35682/#review88618 --- Ship it! Ship It! - Jie Yu On June 20, 2015, 1:08 a.m., Vinod Ko

Re: Review Request 35679: Added benchmark test for Allocator::updateSlave().

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35679/#review88616 --- Ship it! src/tests/hierarchical_allocator_tests.cpp (line 989)

Review Request 35682: Sped up Allocator::updateSlave() and Allocator::updateAllocation().

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35682/ --- Review request for mesos, Ben Mahler and Jie Yu. Bugs: MESOS-2891 https://i

Review Request 35680: Added Sorter::allocation() overload for getting resources of a particular slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35680/ --- Review request for mesos, Ben Mahler and Jie Yu. Bugs: MESOS-2891 https://i

Review Request 35679: Added benchmark test for Allocator::updateSlave().

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35679/ --- Review request for mesos, Ben Mahler and Jie Yu. Bugs: MESOS-2891 https://i

Re: Review Request 34142: AppC provisioner.

> On May 18, 2015, 3:20 p.m., Chi Zhang wrote: > > src/slave/containerizer/provisioners/appc.hpp, line 50 > > > > > > nit: return type could be const? Why? I don't see what it would add? > On May 18, 2015, 3:20 p.m.,

Re: Review Request 34141: AppC provsioning backend.

> On May 21, 2015, 1:25 p.m., Paul Brett wrote: > > src/slave/containerizer/provisioners/appc/backend.cpp, line 136 > > > > > > Running this as an unconstrained subprocess risks impacting the > > performance of every

Re: Review Request 34140: AppC image store

> On May 27, 2015, 4:10 p.m., Paul Brett wrote: > > src/slave/containerizer/provisioners/appc/store.cpp, line 138 > > > > > > And log the error? This is standard clean up, errors will be logged by any preceeding compo

Re: Review Request 34139: AppC image discovery.

> On May 14, 2015, 2:05 p.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/appc/discovery.cpp, line 34 > > > > > > static? This function is only going to be called once anyway though. - Ian

Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

> On June 19, 2015, 2:06 p.m., Jie Yu wrote: > > src/tests/launch_tests.cpp, lines 96-101 > > > > > > Can you explain why this is needed? Maybe add a comment or something? > > If this is not strictly needed, I would re

Re: Review Request 35544: containerizer: statically initialize isolator factories

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35544/#review88603 --- Ship it! Ship It! - Michael Park On June 17, 2015, 12:13 a.m., J

Re: Review Request 34908: Rename --docker_sandbox_directory flag for general use.

> On June 15, 2015, 6:32 p.m., Till Toenshoff wrote: > > docs/configuration.md, line 920 > > > > > > Do you have a followup planned that also updates "docs/upgrades.md" on > > this configuration change? Yes, I'll add

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/#review88595 --- src/slave/slave.cpp (lines 1579 - 1583)

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

> On June 14, 2015, 10:46 a.m., Benjamin Hindman wrote: > > Just so I understand, does this mean if we happen to get in the unfortunate > > situation where a slave has neglected to get the dynamic reservation > > because it was just starting up and then it gets the task launch it will > > shut

Re: Review Request 35664: Improved the performance of DRF sorter by caching the scalars.

> On June 19, 2015, 10:31 p.m., Ben Mahler wrote: > > src/master/allocator/sorter/drf/sorter.cpp, line 299 > > > > > > s/scalar/name/ ? It'll hide the function parameter 'name'. I'll do a sweep in a follow up patch t

Re: Review Request 35664: Improved the performance of DRF sorter by caching the scalars.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35664/#review88591 --- Ship it! Thanks! src/master/allocator/sorter/drf/sorter.cpp (line

Re: Review Request 35671: Fixed a bug in test filter that prevent some tests from being launched.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35671/#review88590 --- Ship it! src/tests/environment.cpp (lines 241 - 242)

Re: Review Request 35664: Improved the performance of DRF sorter by caching the scalars.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35664/ --- (Updated June 19, 2015, 10:21 p.m.) Review request for mesos, Ben Mahler and Vi

Review Request 35671: Fixed a bug in test filter that prevent some tests from being launched.

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

Re: Review Request 35664: Improved the performance of DRF sorter by caching the scalars.

> On June 19, 2015, 9:11 p.m., Ben Mahler wrote: > > src/master/allocator/sorter/drf/sorter.cpp, lines 135-151 > > > > > > Might be a bit clearer if all the updates to 'total' are done together, > > and all the update

Re: Review Request 35663: Added a helper in Resources to get all scalar resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35663/#review88588 --- Ship it! Ship It! - Ben Mahler On June 19, 2015, 7:57 p.m., Jie

Re: Review Request 35664: Improved the performance of DRF sorter by caching the scalars.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35664/#review88582 --- Looks good, is it easy to add a test for the bug? src/master/alloc

Re: Review Request 35664: Improved the performance of DRF sorter by caching the scalars.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35664/ --- (Updated June 19, 2015, 9:09 p.m.) Review request for mesos, Ben Mahler and Vin

Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31444/#review88581 --- Ship it! LGTM overall. Had some question about the bind mount in te

Review Request 35668: Report "unevictable" memory in container statistics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35668/ --- Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu. Bugs: mesos-2798

Review Request 35664: Improved the performance of DRF sorter by caching the scalars.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35664/ --- Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2891 https

Review Request 35663: Added a helper in Resources to get all scalar resources.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35663/ --- Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2891 https

Re: Review Request 35638: Removed const-ref to temporaries from the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35638/ --- (Updated June 19, 2015, 7:12 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 35638: Removed const-ref to temporaries from the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35638/#review88573 --- Ship it! Ship It! - Benjamin Hindman On June 19, 2015, 11:53 a.m

Re: Review Request 35635: Formatting cleanup in the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35635/#review88571 --- Ship it! Ship It! - Benjamin Hindman On June 19, 2015, 12:39 p.m

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/#review88568 --- Ship it! Ship It! - Benjamin Hindman On June 19, 2015, 2:31 p.m.

Re: Review Request 35129: Refactor Future::Data to use Result. Remove dynamic allocation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35129/#review88563 --- Ship it! Ship It! - Benjamin Hindman On June 19, 2015, 6:38 p.m.

Re: Review Request 35129: Refactor Future::Data to use Result. Remove dynamic allocation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35129/ --- (Updated June 19, 2015, 6:38 p.m.) Review request for mesos, Benjamin Hindman a

Re: Review Request 35571: Adding ability to decode JSON from ZK

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35571/#review88557 --- I will get the last things in before committing. Thanks folks - Nik

Re: Review Request 35571: Adding ability to decode JSON from ZK

> On June 18, 2015, 6:11 p.m., Vinod Kone wrote: > > Please definitely write a test (in a follow up review) to make sure this > > works. I would highly recommend creating a ticket and marking it as a > > blocker for 0.23.0. [MESOS-2898](https://issues.apache.org/jira/browse/MESOS-2898) created

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/#review88541 --- Ship it! Ship It! - Alexander Rukletsov On June 19, 2015, 2:31 p

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 19, 2015, 2:31 p.m.) Review request for mesos, Alexander Rukletso

Re: Review Request 35635: Formatting cleanup in the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35635/ --- (Updated June 19, 2015, 12:39 p.m.) Review request for mesos, Alexander Ruklets

Re: Review Request 35647: Fix logic for calling erase inside loop.

> On June 19, 2015, 12:28 p.m., Alexander Rukletsov wrote: > > Why should we care about that if we `break` right after `erase()`? Oh! I totally missed that! Discarding. - Michael --- This is an automatically generated e-mail. To reply,

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/ --- (Updated June 19, 2015, 12:29 p.m.) Review request for mesos, Alexander Ruklets

Re: Review Request 35647: Fix logic for calling erase inside loop.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35647/#review88532 --- Why should we care about that if we `break` right after `erase()`?

Re: Review Request 35647: Fix logic for calling erase inside loop.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35647/ --- (Updated June 19, 2015, 12:24 p.m.) Review request for mesos, Alexander Ruklets

Re: Review Request 35635: Formatting cleanup in the Slave.

> On June 19, 2015, 9:17 a.m., Alexander Rukletsov wrote: > > Is it manual clean-up or you have fed the file to clang-format? It was a mix of both. The function calls that fit in one line for example, I just stroked `Ctrl-K` to see if it fit, rather than trying it out manually. The incorrect i

Review Request 35647: Fix logic for calling erase inside loop.

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

Re: Review Request 35635: Formatting cleanup in the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35635/ --- (Updated June 19, 2015, 12:11 p.m.) Review request for mesos, Alexander Ruklets

Re: Review Request 35638: Removed const-ref to temporaries from the Slave.

> On June 19, 2015, 9:26 a.m., Alexander Rukletsov wrote: > > src/slave/slave.cpp, line 1280 > > > > > > Though currently we guarantee `executorInfo` lifetime is not shorter > > than `executorId`'s, I think we agreed

Re: Review Request 35638: Removed const-ref to temporaries from the Slave.

> On June 19, 2015, 9:26 a.m., Alexander Rukletsov wrote: > > I think MESOS-2632 would be more appropriate. Fixed. > On June 19, 2015, 9:26 a.m., Alexander Rukletsov wrote: > > src/slave/slave.cpp, line 1280 > > > > >

Re: Review Request 35638: Removed const-ref to temporaries from the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35638/ --- (Updated June 19, 2015, 11:53 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 35571: Adding ability to decode JSON from ZK

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35571/#review88525 --- src/master/detector.cpp (line 447)

Re: Review Request 35554: Rename Stout User Guide to Stout Developer Guide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35554/#review88522 --- Ship it! Ship It! - Alexander Rojas On June 17, 2015, 11:21 a.m.

Re: Review Request 35553: Rename libprocess User Guide to Developer Guide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35553/#review88521 --- Ship it! Ship It! - Alexander Rojas On June 17, 2015, 11:22 a.m.

Re: Review Request 35510: Introduced General and Markdown Documentation Style Guides.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35510/#review88518 --- Ship it! docs/mesos-documentation-guide.md (line 33)

Re: Review Request 35509: Doxygen Style Guide Improvements.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35509/#review88517 --- Ship it! docs/mesos-doxygen-style-guide.md

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35433/#review88513 --- Ship it! Want to link the RR with MESOS-2491 for posterity? src/s

Re: Review Request 35364: Consistent code examples in doxygen style.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35364/#review88516 --- Ship it! Ship It! - Alexander Rojas On June 11, 2015, 10:39 p.m.

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

> On June 14, 2015, 10:46 a.m., Benjamin Hindman wrote: > > Just so I understand, does this mean if we happen to get in the unfortunate > > situation where a slave has neglected to get the dynamic reservation > > because it was just starting up and then it gets the task launch it will > > shut

Re: Review Request 35638: Removed const-ref to temporaries from the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35638/#review88511 --- I think MESOS-2632 would be more appropriate. src/slave/slave.cpp

Re: Review Request 35635: Formatting cleanup in the Slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35635/#review88510 --- Ship it! Is it manual clean-up or you have fed the file to clang-fo