Re: Review Request 34193: Refactored common functionality into BaseFlags

2015-05-21 Thread Marco Massenzio
On May 18, 2015, 10:04 p.m., Joris Van Remoortere wrote: 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp, line 511 https://reviews.apache.org/r/34193/diff/2/?file=963014#file963014line511 Since we're in an implementation file, we can `using std::ostringstream;` and then

Re: Review Request 34068: The test case of extend hashmap to support custom equality and hash

2015-05-21 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34068/#review84668 --- 3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp

Re: Review Request 34068: The test case of extend hashmap to support custom equality and hash

2015-05-21 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34068/#review84667 --- Ship it!

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-21 Thread Colin Williams
On May 20, 2015, 11:47 a.m., Alexander Rojas wrote: src/examples/test_hook_module.cpp, lines 36-38 https://reviews.apache.org/r/34361/diff/1/?file=962951#file962951line36 This constants are used nowhere but in one method. Is there any reason why they are not defined in the method

Review Request 34436: Factor out launch helper for easier reuse

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34436/ --- Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs:

Re: Review Request 34426: Report per-container metrics for network bandwidth throttling

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34426/ --- (Updated May 21, 2015, 5:52 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

2015-05-21 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34309/#review84746 --- Ship it! src/tests/sched_tests.cpp

Re: Review Request 34310: Use IDLE scheduling for revocable CPU in cgroups isolator.

2015-05-21 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34310/#review84745 --- Ship it! Modulo making the resources field optional

Re: Review Request 32660: Report network isolator statistics on a per container basis (MESOS-2332)

2015-05-21 Thread Paul Brett
On April 25, 2015, 1:03 a.m., Chi Zhang wrote: include/mesos/mesos.proto, lines 432-438 https://reviews.apache.org/r/32660/diff/3/?file=941248#file941248line432 maybe make it clear that delayed maps to capping; dropped maps to NIC capacity exceeded? I'm think the interpretation

Re: Review Request 32660: Report network isolator statistics on a per container basis (MESOS-2332)

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32660/ --- (Updated May 21, 2015, 6:06 p.m.) Review request for mesos, Chi Zhang, Ian

Review Request 34558: Move port mapping isolator configuration settings to file local scope for easier sharing.

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34558/ --- Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. Bugs:

Re: Review Request 30774: Fetcher Cache

2015-05-21 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated May 21, 2015, 5:06 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 33823: Protocol file with the HTTP API messages between executor and slave

2015-05-21 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33823/ --- (Updated May 21, 2015, 3:11 p.m.) Review request for mesos, Isabel Jimenez,

Re: Review Request 34563: Allowed delegating constructors in styleguide.

2015-05-21 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34563/ --- (Updated May 21, 2015, 8:48 p.m.) Review request for mesos and Joris Van

Re: Review Request 34563: Allowed delegating constructors in styleguide.

2015-05-21 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34563/ --- (Updated May 21, 2015, 8:55 p.m.) Review request for mesos and Joris Van

Re: Review Request 34534: Reflected in documentation that isolators are only relevant for Mesos Containerizer.

2015-05-21 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34534/ --- (Updated May 21, 2015, 8:45 p.m.) Review request for mesos and Bernd Mathiske.

Re: Review Request 34559: Used the pull model to get estimations from resource estimator.

2015-05-21 Thread Jie Yu
On May 21, 2015, 7:26 p.m., Vinod Kone wrote: src/slave/slave.cpp, line 3986 https://reviews.apache.org/r/34559/diff/1/?file=966887#file966887line3986 Can you add a comment here on why we forward periodically instead of after every update? Discussed with Vinod offline. It's hard

Re: Review Request 34306: Added capabilities field to FrameworkInfo.

2015-05-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34306/#review84785 --- Ship it! Ship It! - Jie Yu On May 21, 2015, 12:42 a.m., Vinod

Review Request 34565: Allowed explicitly-defaulted functions in styleguide.

2015-05-21 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34565/ --- Review request for mesos and Joris Van Remoortere. Bugs: MESOS-2762

Re: Review Request 34534: Reflected in documentation that isolators are only relevant for Mesos Containerizer.

2015-05-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34534/#review84789 --- Patch looks great! Reviews applied: [34534] All tests passed. -

Re: Review Request 32664: Add port mapping isolator statistics tests

2015-05-21 Thread Paul Brett
On April 8, 2015, 7:46 p.m., Ian Downes wrote: src/tests/port_mapping_tests.cpp, line 1788 https://reviews.apache.org/r/32664/diff/1/?file=911866#file911866line1788 Did you consider using iperf3 which makes all features available through a library? I looked at iperf3 but it

Re: Review Request 34309: Support manipulating scheduler policy on Linux.

2015-05-21 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34309/#review84791 --- Ship it! src/tests/sched_tests.cpp

Re: Review Request 34559: Used the pull model to get estimations from resource estimator.

2015-05-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34559/#review84776 --- Patch looks great! Reviews applied: [34559] All tests passed. -

Re: Review Request 34141: AppC provsioning backend.

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34141/#review84774 --- src/slave/containerizer/provisioners/appc/backend.cpp

Re: Review Request 34304: Updated documentation of FrameworkInfo.

2015-05-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34304/#review84783 --- Ship it! Ship It! - Jie Yu On May 21, 2015, 12:34 a.m., Vinod

Re: Review Request 34563: Allowed delegating constructors in styleguide

2015-05-21 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34563/ --- (Updated May 21, 2015, 8:48 p.m.) Review request for mesos and Joris Van

Review Request 34563: Allowed delegating constructors in styleguide

2015-05-21 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34563/ --- Review request for mesos and Joris Van Remoortere. Repository: mesos

Re: Review Request 34559: Used the pull model to get estimations from resource estimator.

2015-05-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34559/ --- (Updated May 21, 2015, 9:26 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 34559: Used the pull model to get estimations from resource estimator.

2015-05-21 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34559/#review84793 --- Ship it! Ship It! - Vinod Kone On May 21, 2015, 9:26 p.m., Jie

Re: Review Request 34563: Allowed delegating constructors in styleguide.

2015-05-21 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34563/#review84795 --- Ship it! Ship It! - Joris Van Remoortere On May 21, 2015, 8:55

Re: Review Request 34361: converted hard-coded strings to consts

2015-05-21 Thread Colin Williams
On May 20, 2015, 11:47 a.m., Alexander Rojas wrote: src/examples/test_hook_module.cpp, lines 36-38 https://reviews.apache.org/r/34361/diff/1/?file=962951#file962951line36 This constants are used nowhere but in one method. Is there any reason why they are not defined in the method

Re: Review Request 34545: Updated the allocator related docs.

2015-05-21 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34545/ --- (Updated May 21, 2015, 2:58 p.m.) Review request for mesos, Adam B, Bernd

Re: Review Request 34563: Allowed delegating constructors in styleguide.

2015-05-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34563/#review84815 --- Patch looks great! Reviews applied: [34563] All tests passed. -

Re: Review Request 34565: Allowed explicitly-defaulted functions in styleguide.

2015-05-21 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34565/#review84797 --- Left a comment in the JIRA about possibly calling out some patterns

Re: Review Request 34565: Allowed explicitly-defaulted functions in styleguide.

2015-05-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34565/#review84801 --- Patch looks great! Reviews applied: [34565] All tests passed. -

Re: Review Request 34559: Used the pull model to get estimations from resource estimator.

2015-05-21 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34559/#review84804 --- Ship it! LGTM - Thanks Jie! src/tests/mesos.hpp

Re: Review Request 34308: Filter revocable resources.

2015-05-21 Thread Vinod Kone
On May 20, 2015, 12:34 a.m., Vinod Kone wrote: Ship It! I'll commit this for you, as I need this patch. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34308/#review84426

Re: Review Request 34529: Add non-const reference version of OptionT::get.

2015-05-21 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34529/#review84808 --- Hi Mark, Can you fold in the operators that are added in:

Re: Review Request 34571: Pushed QoS Correction stub message to mesos.proto

2015-05-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34571/#review84817 --- include/mesos/mesos.proto

Re: Review Request 34571: Pushed QoS Correction stub message to mesos.proto

2015-05-21 Thread Jie Yu
On May 21, 2015, 11:24 p.m., Jie Yu wrote: include/mesos/mesos.proto, line 1189 https://reviews.apache.org/r/34571/diff/1/?file=968376#file968376line1189 This is a protocol between slave and the QoSController, both of which are internal to Mesos. So we probably should move this

Re: Review Request 34432: Remove duplicate constant string references to mesos-containerizer

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34432/ --- (Updated May 21, 2015, 11:32 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34431: Add htb queueing discipline

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34431/ --- (Updated May 21, 2015, 11:32 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34571: Pushed QoS Correction stub message to mesos.proto

2015-05-21 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34571/ --- (Updated May 21, 2015, 4:02 p.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34571: Pushed QoS Correction stub message to mesos.proto

2015-05-21 Thread Bartek Plotka
On May 21, 2015, 11:24 p.m., Jie Yu wrote: include/mesos/mesos.proto, lines 1189-1211 https://reviews.apache.org/r/34571/diff/1/?file=968376#file968376line1189 It reads weired when one wants to create a QoS correction (too many words 'correction'): ```

Re: Review Request 32660: Report network isolator statistics on a per container basis (MESOS-2332)

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32660/ --- (Updated May 21, 2015, 11:30 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34428: Extend queueing discipline wrappers to expose network isolator statistics

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34428/ --- (Updated May 21, 2015, 11:31 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34436: Factor out launch helper for easier reuse

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34436/ --- (Updated May 21, 2015, 11:31 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34558: Move port mapping isolator configuration settings to file local scope for easier sharing.

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34558/ --- (Updated May 21, 2015, 11:31 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34571: Pushed QoS Correction stub message to mesos.proto

2015-05-21 Thread Niklas Nielsen
On May 21, 2015, 4:24 p.m., Jie Yu wrote: include/mesos/mesos.proto, line 1189 https://reviews.apache.org/r/34571/diff/1/?file=968376#file968376line1189 This is a protocol between slave and the QoSController, both of which are internal to Mesos. So we probably should move this

Re: Review Request 34278: Refactor Stout ResultT leveraging TryOptionT to remove the dynamic allocation.

2015-05-21 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34278/ --- (Updated May 21, 2015, 11:45 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 34321: Merge class Handle which is duplicated between filter/handle and queueing/handle.

2015-05-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34321/#review84796 --- Ship it! Looks good to me. A few style nits. Thanks for the

Re: Review Request 34571: Pushed QoS Correction stub message to mesos.proto

2015-05-21 Thread Niklas Nielsen
On May 21, 2015, 4:24 p.m., Jie Yu wrote: include/mesos/mesos.proto, line 1189 https://reviews.apache.org/r/34571/diff/1/?file=968376#file968376line1189 This is a protocol between slave and the QoSController, both of which are internal to Mesos. So we probably should move this

Re: Review Request 32664: Add port mapping isolator statistics tests

2015-05-21 Thread Paul Brett
On April 8, 2015, 7:46 p.m., Ian Downes wrote: src/tests/port_mapping_tests.cpp, lines 2071-2072 https://reviews.apache.org/r/32664/diff/1/?file=911866#file911866line2071 Why does it need to be random if we're inside a temporary directory sandbox? Running in a temporary

Re: Review Request 34276: Use special constructor for OptionT from SomeT.

2015-05-21 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34276/ --- (Updated May 21, 2015, 11:45 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 34277: Refactor Stout TryT leveraging OptionT to remove dynamic allocation.

2015-05-21 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34277/ --- (Updated May 21, 2015, 11:45 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 34321: Merge class Handle which is duplicated between filter/handle and queueing/handle.

2015-05-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34321/#review84835 --- src/linux/routing/queueing/handle.hpp

Re: Review Request 30609: Added a function that reports file size, not following links.

2015-05-21 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30609/#review84853 --- Ship it!

Re: Review Request 30609: Added a function that reports file size, not following links.

2015-05-21 Thread Benjamin Hindman
On May 22, 2015, 2:31 a.m., Benjamin Hindman wrote: These are minor nits so I'll take care of them for you and commit this, thanks Bernd! - Benjamin --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 32664: Add port mapping isolator statistics tests

2015-05-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32664/#review84841 --- Patch looks great! Reviews applied: [34321, 34426, 34428, 34431,

Re: Review Request 34581: Added oversubscription.proto for QoS Controller and slave communication

2015-05-21 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34581/ --- (Updated May 22, 2015, 2:31 a.m.) Review request for mesos, Jie Yu, Niklas

Review Request 34581: Added oversubscription.proto for QoS Controller and slave communication

2015-05-21 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34581/ --- Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod

Re: Review Request 34321: Merge class Handle which is duplicated between filter/handle and queueing/handle.

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34321/ --- (Updated May 22, 2015, 4:42 a.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34581: Added oversubscription.proto for QoS Controller and slave communication

2015-05-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34581/#review84858 --- Patch looks great! Reviews applied: [34581] All tests passed. -

Re: Review Request 34558: Move port mapping isolator configuration settings to file local scope for easier sharing.

2015-05-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34558/#review84766 --- Bad patch! Reviews applied: [34321, 34426, 34321] Failed command:

Re: Review Request 34427: AppC provisioner backend using bind mounts.

2015-05-21 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34427/#review84769 --- LGTM - Paul Brett On May 19, 2015, 6:46 p.m., Ian Downes wrote:

Re: Review Request 34559: Used the pull model to get estimations from resource estimator.

2015-05-21 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34559/#review84768 --- src/slave/slave.cpp

Review Request 34559: Used the pull model to get estimations from resource estimator.

2015-05-21 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34559/ --- Review request for mesos, Benjamin Hindman, Niklas Nielsen, and Vinod Kone.

Re: Review Request 34427: AppC provisioner backend using bind mounts.

2015-05-21 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34427/#review84771 --- src/slave/containerizer/provisioners/appc/bind_backend.hpp

Re: Review Request 34545: Updated the allocator related docs.

2015-05-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34545/#review84711 --- Patch looks great! Reviews applied: [34545] All tests passed. -

Re: Review Request 30774: Fetcher Cache

2015-05-21 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated May 21, 2015, 9:05 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30339: Use flags.hooks.isSome() before calling hooks.

2015-05-21 Thread Niklas Nielsen
On March 4, 2015, 4:03 p.m., Niklas Nielsen wrote: Niklas Nielsen wrote: Do you want this in? If so, please update the review :) Ping - want this in? :) - Niklas --- This is an automatically generated e-mail. To reply,

Re: Review Request 34420: Updated the committer doc regarding checking JIRA before committing.

2015-05-21 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34420/#review84724 --- Ship it! Ship It! - Niklas Nielsen On May 19, 2015, 10:40 a.m.,

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-05-21 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review84726 --- Hey Adam, is this ready for review? - Niklas Nielsen On May 14,

Re: Review Request 34420: Updated the committer doc regarding checking JIRA before committing.

2015-05-21 Thread Jie Yu
On May 19, 2015, 6:53 p.m., Ben Mahler wrote: Looks like we need to add a step for resolving the JIRA ticket (including the commit message in a comment), if applicable? Will add that later. Not sure if it belongs to this file, or we need to have a new doc regarding the pratices we used

Re: Review Request 30612: Added /master/frameworks/{framework}/tasks/{task} endpoint.

2015-05-21 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30612/#review84734 --- Last high level comment - if Adam and/or Till don't agree, feel

Re: Review Request 34306: Added capabilities field to FrameworkInfo.

2015-05-21 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34306/#review84743 --- Ship it! Ship It! - Niklas Nielsen On May 20, 2015, 5:42 p.m.,