Re: Review Request 33793: HTTP headers should be considered case-insensitive.

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33793/ --- (Updated June 1, 2015, 6:09 a.m.) Review request for mesos, Ben Mahler and

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated June 1, 2015, 6:12 a.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated June 1, 2015, 6:13 a.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34655: Use relative url in /help generated links point

2015-06-01 Thread Adam Bordelon
The only help documentation I know of is what I found in process/help.hpp https://github.com/apache/mesos/blob/0.22.1/3rdparty/libprocess/include/process/help.hpp#L93 I couldn't find an existing JIRA for updating the missing help pages, but I did find: MESOS-1267

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34646/ --- (Updated June 1, 2015, 6:35 a.m.) Review request for mesos and Adam B. Bugs:

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-06-01 Thread haosdent huang
On May 28, 2015, 7:51 a.m., Adam B wrote: src/master/http.cpp, line 1044 https://reviews.apache.org/r/34646/diff/1/?file=971274#file971274line1044 The JIRA suggests that this case should not return OK (200), unless it queries the active master to return the actual tasks list.

Re: Review Request 33792: Extend hashmap to support custom equality and hash

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

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34687/#review85946 --- src/Makefile.am https://reviews.apache.org/r/34687/#comment137765

Review Request 34882: let mesos to compile and run on arm64 servers

2015-06-01 Thread Dong Robin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34882/ --- Review request for mesos. Bugs: MESOS-2786

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34687/#review85948 --- src/common/parse.hpp

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34687/#review85949 --- Ship it! Ship It! - haosdent huang On May 31, 2015, 2:58 a.m.,

Review Request 34886: MESOS-2787: Fixed the exception KeyError: 'mem_rss_bytes' when run command mesos-ps

2015-06-01 Thread weitao zhou
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34886/ --- Review request for mesos. Repository: mesos Description --- The

Review Request 34887: MESOS-2788: Fixed the statistics got by cmd *mesos ps* and webUI is mis-matching

2015-06-01 Thread weitao zhou
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34887/ --- Review request for mesos. Repository: mesos Description --- mesos-ps is

Re: Review Request 34887: MESOS-2788: Fixed the statistics got by cmd *mesos ps* and webUI is mis-matching

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

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

2015-06-01 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33295/#review85950 --- Looks very close! Just some minor nits and suggested rewordings,

Re: Review Request 34655: Use relative url in /help generated links point

2015-06-01 Thread haosdent huang
On May 28, 2015, 5:40 p.m., Adam B wrote: 3rdparty/libprocess/src/help.cpp, line 140 https://reviews.apache.org/r/34655/diff/1/?file=971486#file971486line140 Why does this '/help/' need to be removed? Marco Massenzio wrote: because it's wrong - it shouldn't be there. Adam

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

2015-06-01 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34748/ --- (Updated June 1, 2015, 8:15 a.m.) Review request for mesos, Jie Yu, Joris Van

Re: Review Request 33793: HTTP headers should be considered case-insensitive.

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33793/ --- (Updated June 1, 2015, 9:02 a.m.) Review request for mesos, Ben Mahler and

Re: Review Request 34882: let mesos to compile and run on arm64 servers

2015-06-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34882/#review85963 --- Bad patch! Reviews applied: [34882] Failed command:

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

2015-06-01 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34748/ --- (Updated June 1, 2015, 8:31 a.m.) Review request for mesos, Jie Yu, Joris Van

Re: Review Request 34362: Include ExecutorInfos in master/state.json

2015-06-01 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34362/#review85959 --- Looks good, but it's missing a few performance tricks we've used in

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

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

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34646/ --- (Updated June 1, 2015, 3:07 p.m.) Review request for mesos and Adam B. Bugs:

Re: Review Request 34892: Reintroduced chown stdout stderr in FetcherProcess::run().

2015-06-01 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34892/ --- (Updated June 1, 2015, 8:42 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 34268: stout library - adding support for Solaris

2015-06-01 Thread Till Toenshoff
On June 1, 2015, 10:36 a.m., Till Toenshoff wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp, lines 71-82 https://reviews.apache.org/r/34268/diff/2/?file=969835#file969835line71 Seems we got some options here; A. use your separate, stream-based approach for

Re: Review Request 34892: Reintroduced chown stdout stderr in FetcherProcess::run().

2015-06-01 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34892/#review85991 --- Ship it! As a follow up, let's figure out how to create tests for

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

2015-06-01 Thread Alexander Rojas
On June 1, 2015, 10:06 a.m., Adam B wrote: 3rdparty/libprocess/include/process/firewall.hpp, lines 54-55 https://reviews.apache.org/r/33295/diff/6/?file=964946#file964946line54 Please capitalize The beginning of each @param description. I do completely agree with you, and actually

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

2015-06-01 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33296/ --- (Updated June 1, 2015, 5:54 p.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-06-01 Thread haosdent huang
On June 1, 2015, 8:34 a.m., Adam B wrote: src/master/http.cpp, line 1038 https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038 Could I ask you to write a quick unit test for this? haosdent huang wrote: yes @adam-mesos, I have a newbie problem. In the

Review Request 34894: Add new message for Traffic Control statistics

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

Re: Review Request 34887: MESOS-2788: Fixed the statistics got by cmd *mesos ps* and webUI is mis-matching

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34887/#review85979 --- Ship it! Ship It! - haosdent huang On June 1, 2015, 12:15 p.m.,

Re: Review Request 34887: MESOS-2788: Fixed the statistics got by cmd *mesos ps* and webUI is mis-matching

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34887/#review85978 --- src/cli/mesos-ps https://reviews.apache.org/r/34887/#comment137816

Re: Review Request 34268: stout library - adding support for Solaris

2015-06-01 Thread Stan Teresen
On June 1, 2015, 10:36 a.m., Till Toenshoff wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp, lines 71-82 https://reviews.apache.org/r/34268/diff/2/?file=969835#file969835line71 Seems we got some options here; A. use your separate, stream-based approach for

Re: Review Request 34687: (De)Serializing MasterInfo PB to JSON

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34687/#review85980 --- src/common/parse.hpp

Re: Review Request 34645: Update existing lambdas to meet style guide

2015-06-01 Thread haosdent huang
On May 25, 2015, 11:05 p.m., Joris Van Remoortere wrote: This is an interesting case. We have a proxy to another function, rather than the implementation of that function as a lambda. I'm curious what the community's view is on using the proxy lambda approach as per your patch, versus

Re: Review Request 34631: Added QoS Controller.

2015-06-01 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34631/#review86004 --- Ship it! Ditto Vinod's comment on s/BE/revokable/

Re: Review Request 34830: Fix routing ingress/fq_codel search returning wrong qdisc.

2015-06-01 Thread Chi Zhang
On June 1, 2015, 5:59 p.m., Chi Zhang wrote: src/linux/routing/queueing/fq_codel.cpp, line 102 https://reviews.apache.org/r/34830/diff/2/?file=975051#file975051line102 ignore if you have done in a different patch: egress::ROOT? (You had ingress::ROOT) feel free to drop

Re: Review Request 34835: Add constexpr to C++ whitelist

2015-06-01 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34835/#review86026 --- docs/mesos-c++-style-guide.md

Re: Review Request 34830: Fix routing ingress/fq_codel search returning wrong qdisc.

2015-06-01 Thread Jie Yu
On June 1, 2015, 5:59 p.m., Chi Zhang wrote: src/linux/routing/queueing/ingress.hpp, lines 55-56 https://reviews.apache.org/r/34830/diff/2/?file=975052#file975052line55 a bit confused by an ingress qdisc on the egress side of the link I saw this in other places too. +1

Re: Review Request 34894: Add new message for Traffic Control statistics

2015-06-01 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34894/#review86033 --- include/mesos/mesos.proto

Re: Review Request 34830: Fix routing ingress/fq_codel search returning wrong qdisc.

2015-06-01 Thread Isabel Jimenez
On June 1, 2015, 5:59 p.m., Chi Zhang wrote: src/linux/routing/queueing/ingress.cpp, lines 44-51 https://reviews.apache.org/r/34830/diff/2/?file=975053#file975053line44 Two blank lines between the functions. here and other places. Since the functions are inside a

Re: Review Request 34830: Fix routing ingress/fq_codel search returning wrong qdisc.

2015-06-01 Thread Paul Brett
On June 1, 2015, 5:59 p.m., Chi Zhang wrote: src/linux/routing/queueing/fq_codel.cpp, line 102 https://reviews.apache.org/r/34830/diff/2/?file=975051#file975051line102 ignore if you have done in a different patch: egress::ROOT? (You had ingress::ROOT) Chi Zhang

Re: Review Request 34887: MESOS-2788: Fixed the statistics got by cmd *mesos ps* and webUI is mis-matching

2015-06-01 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34887/#review86037 --- Ship it! Ship It! - Ben Mahler On June 1, 2015, 12:15 p.m.,

Re: Review Request 34362: Include ExecutorInfos in master/state.json

2015-06-01 Thread haosdent huang
On June 1, 2015, 8:53 a.m., Adam B wrote: Looks good, but it's missing a few performance tricks we've used in other json model() functions, to reduce the json construction time, especially for large clusters. We should do some quick benchmarking on a cluster with thousands of

Re: Review Request 34631: Added QoS Controller.

2015-06-01 Thread Jie Yu
On May 29, 2015, 5:39 p.m., Vinod Kone wrote: include/mesos/slave/qos_controller.hpp, line 62 https://reviews.apache.org/r/34631/diff/2/?file=971661#file971661line62 s/QoSCorrection/listQoSCorrection/ ? Probably calling it 'correction' and returns a single QoSCorrection? Can you

Re: Review Request 34644: Update existing lambdas to meet style guide

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34644/ --- (Updated June 1, 2015, 5:45 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-06-01 Thread Adam B
On June 1, 2015, 1:34 a.m., Adam B wrote: src/master/http.cpp, line 1038 https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038 Could I ask you to write a quick unit test for this? haosdent huang wrote: yes haosdent huang wrote: @adam-mesos, I have

Re: Review Request 34894: Add new message for Traffic Control statistics

2015-06-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34894/#review86013 --- Patch looks great! Reviews applied: [34830, 34832, 34426, 34863,

Re: Review Request 34830: Fix routing ingress/fq_codel search returning wrong qdisc.

2015-06-01 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34830/#review86029 --- src/linux/routing/queueing/fq_codel.cpp

Re: Review Request 34362: Include ExecutorInfos in master/state.json

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34362/ --- (Updated June 1, 2015, 5:13 p.m.) Review request for mesos and Adam B. Bugs:

Re: Review Request 34645: Update existing lambdas to meet style guide

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34645/ --- (Updated June 1, 2015, 5:24 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 34830: Fix routing ingress/fq_codel search returning wrong qdisc.

2015-06-01 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34830/#review86006 --- src/linux/routing/queueing/fq_codel.cpp

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

2015-06-01 Thread haosdent huang
On June 1, 2015, 8:34 a.m., Adam B wrote: src/master/http.cpp, line 1038 https://reviews.apache.org/r/34646/diff/3/?file=975522#file975522line1038 Could I ask you to write a quick unit test for this? haosdent huang wrote: yes haosdent huang wrote: @adam-mesos, I have

Re: Review Request 34782: Convert Non-POD static variables in fq_codel and ingress to constexpr

2015-06-01 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34782/#review86023 --- src/linux/routing/handle.hpp

Re: Review Request 34645: Update existing lambdas to meet style guide

2015-06-01 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34645/ --- (Updated June 1, 2015, 5:26 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 34894: Add new message for Traffic Control statistics

2015-06-01 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34894/#review86048 --- include/mesos/mesos.proto

Review Request 34899: Eliminate multiple uses of literal string in fq_codel and ingress queueing disciplines.

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

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-01 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review86039 --- LGTM - Would it make sense to have sane min/max values for the

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

2015-06-01 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34529/#review86055 --- Ship it! I will get this committed for you, but I'll remove the

Re: Review Request 34362: Include ExecutorInfos in master/state.json

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

Re: Review Request 34830: Fix routing ingress/fq_codel search returning wrong qdisc.

2015-06-01 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34830/ --- (Updated June 1, 2015, 7:49 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34900: Convert EGRESS_ROOT to const Handle.

2015-06-01 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34900/#review86050 --- src/linux/routing/handle.hpp

Re: Review Request 34645: Update existing lambdas to meet style guide

2015-06-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34645/#review86056 --- Patch looks great! Reviews applied: [34644, 34645] All tests

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

2015-06-01 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34426/#review86058 --- src/linux/routing/queueing/fq_codel.hpp

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

2015-06-01 Thread Bartek Plotka
On June 1, 2015, 7:34 p.m., Jie Yu wrote: src/tests/oversubscription_tests.cpp, line 96 https://reviews.apache.org/r/34816/diff/4/?file=975492#file975492line96 This looks weired to me since we are actually creating a TestResourceEstimator, but TypeParam == NoopResourceEstimator.

Re: Review Request 33208: Delete detector in MesosSchedulerDriver::stop

2015-06-01 Thread Niklas Nielsen
On April 19, 2015, 10:55 p.m., Adam B wrote: LGTM, barring a question about ordering/synchronization. I'll let another committer take a look before we commit it. Adam B wrote: Would also like to see a successful ReviewBot pass. That MasterFailover segfault seems like it could be

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

2015-06-01 Thread Ben Mahler
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? Jie Yu wrote: Will add that later. Not sure if it belongs to this file, or we need to have a new doc regarding

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

2015-06-01 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34816/#review86051 --- src/tests/oversubscription_tests.cpp

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

2015-06-01 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34529/#review86057 --- 3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp

Re: Review Request 34832: Add new qdisc tests

2015-06-01 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34832/#review86061 --- src/tests/routing_tests.cpp

Re: Review Request 34863: Add tests for new qdisc statistics functions.

2015-06-01 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34863/#review86063 --- src/tests/routing_tests.cpp

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

2015-06-01 Thread Niklas Nielsen
On June 1, 2015, 12:34 p.m., Jie Yu wrote: src/tests/oversubscription_tests.cpp, line 96 https://reviews.apache.org/r/34816/diff/4/?file=975492#file975492line96 This looks weired to me since we are actually creating a TestResourceEstimator, but TypeParam == NoopResourceEstimator.

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

2015-06-01 Thread Niklas Nielsen
On June 1, 2015, 1:46 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, lines 225-226 https://reviews.apache.org/r/34816/diff/4/?file=975492#file975492line225 We can merge these two with multiple put() on a queue :) Bartek Plotka wrote: That's true, but in

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Niklas Nielsen
On June 1, 2015, 1:55 p.m., Jie Yu wrote: Could you please split out the module definition for ResourceEstimator and make this patch not dependent on r34816? I need this patch to build Fixed Resource Estimator. Can't we just get this small chain in a good shape and land it? :) -

Re: Review Request 34317: Updated callers of os::getenv() in /src and removed calls to os::hasenv()

2015-06-01 Thread Ben Mahler
On May 20, 2015, 5:49 p.m., Timothy Chen wrote: Tim, are you able to continue reviewing? - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34317/#review84551

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Jie Yu
On June 1, 2015, 8:55 p.m., Jie Yu wrote: src/tests/module.hpp, lines 76-84 https://reviews.apache.org/r/34662/diff/11/?file=975514#file975514line76 Not your fault. But I found this very unintuitive. It takes me quite a while to figure out why we need this. It would

Review Request 34904: Added help for state.json

2015-06-01 Thread Aditi Dixit
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34904/ --- Review request for mesos. Bugs: mesos-2277

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

2015-06-01 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34816/#review86066 --- src/tests/oversubscription_tests.cpp

Re: Review Request 34832: Add new qdisc tests

2015-06-01 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34832/ --- (Updated June 1, 2015, 8:53 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34900: Convert EGRESS_ROOT to constexpr Handle.

2015-06-01 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34900/#review86073 --- Ship it! Ship It! - Jie Yu On June 1, 2015, 8:29 p.m., Paul

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Niklas Nielsen
On June 1, 2015, 1:55 p.m., Jie Yu wrote: src/tests/module.hpp, lines 76-84 https://reviews.apache.org/r/34662/diff/11/?file=975514#file975514line76 Not your fault. But I found this very unintuitive. It takes me quite a while to figure out why we need this. It would

Re: Review Request 34899: Eliminate multiple uses of literal string in fq_codel and ingress queueing disciplines.

2015-06-01 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34899/ --- (Updated June 1, 2015, 8:20 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34900: Convert EGRESS_ROOT to constexpr Handle.

2015-06-01 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34900/ --- (Updated June 1, 2015, 8:29 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34832: Add new qdisc tests

2015-06-01 Thread Paul Brett
On June 1, 2015, 8:23 p.m., Ian Downes wrote: src/tests/routing_tests.cpp, lines 452-453 https://reviews.apache.org/r/34832/diff/1/?file=974729#file974729line452 move this up to the ingress::create(noSuchInterface)? This and following is three tests are there to check that a remove

Re: Review Request 34816: Changed TestResourceEstimator to be mocked.

2015-06-01 Thread Bartek Plotka
On June 1, 2015, 8:46 p.m., Niklas Nielsen wrote: src/tests/oversubscription_tests.cpp, lines 225-226 https://reviews.apache.org/r/34816/diff/4/?file=975492#file975492line225 We can merge these two with multiple put() on a queue :) That's true, but in previous issue you asked to

Re: Review Request 34900: Convert EGRESS_ROOT to constexpr Handle.

2015-06-01 Thread Jie Yu
On June 1, 2015, 8:56 p.m., Jie Yu wrote: Ship It! Could you rebase this patch. I'll commit it first. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34900/#review86073

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

2015-06-01 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34748/#review86092 --- High level looks good, just some minor comments.

Re: Review Request 34140: AppC image store

2015-06-01 Thread Ian Downes
On May 26, 2015, 11:28 a.m., Timothy Chen wrote: src/slave/containerizer/provisioners/appc/store.cpp, line 79 https://reviews.apache.org/r/34140/diff/1/?file=957277#file957277line79 Check exists first? os::mkdir() ignores EEXIST so won't return an error if it already exists. On

Re: Review Request 31505: Add flow classifiers for fq_codel on egress

2015-06-01 Thread Cong Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31505/ --- (Updated June 1, 2015, 10:45 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/ --- (Updated June 1, 2015, 5:26 p.m.) Review request for mesos, Niklas Nielsen and

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

2015-06-01 Thread Paul Brett
On June 1, 2015, 6:22 p.m., Chi Zhang wrote: src/linux/routing/queueing/fq_codel.hpp, lines 63-64 https://reviews.apache.org/r/34426/diff/8/?file=975297#file975297line63 nit: can probably save the or an error if we cannot find out Disagree, I am trying to provide guidance between

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

2015-06-01 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34748/#review86098 --- include/mesos/mesos.proto

Re: Review Request 34900: Convert EGRESS_ROOT to constexpr Handle.

2015-06-01 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34900/ --- (Updated June 1, 2015, 10:03 p.m.) Review request for mesos, Chi Zhang, Ian

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

2015-06-01 Thread Ian Downes
On May 16, 2015, 1:03 p.m., Joris Van Remoortere wrote: Hi Ian, I'm wondering about the `Note` regarding only setting the scheduling policy to IDLE if the initial resources are revocable. I think this exposes many scenarios where the isolator will seem `enabled` to the operator,

Re: Review Request 34676: Add printing of extended attributes to Resource objects.

2015-06-01 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34676/#review86110 --- Ship it! Ship It! - Vinod Kone On June 1, 2015, 10:24 p.m.,

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

2015-06-01 Thread Paul Brett
On June 1, 2015, 8:11 p.m., Ian Downes wrote: src/linux/routing/queueing/statistics.hpp, lines 26-34 https://reviews.apache.org/r/34426/diff/8/?file=975302#file975302line26 constexpr? Nice :) - Paul --- This is an

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/ --- (Updated June 1, 2015, 5:46 p.m.) Review request for mesos, Niklas Nielsen and

Re: Review Request 30339: Call hookmanager only if some hooks are installed.

2015-06-01 Thread Kapil Arya
On June 1, 2015, 5:41 p.m., Niklas Nielsen wrote: Do you have a JIRA ticket for this change? Is it to avoid memory copies, that you need these to be in the call sites. It clutters the call sites a bit, so I wanted to seee if we could move this into the HooksManager instead.

Re: Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage. Pass ResourceUsage to ResourceEstimator

2015-06-01 Thread Niklas Nielsen
On June 1, 2015, 2:58 p.m., Jie Yu wrote: include/mesos/mesos.proto, line 578 https://reviews.apache.org/r/34748/diff/3/?file=975577#file975577line578 I think we also need to know the total resources allocated to the container. For instance, one may want to know the usage slack.

Re: Review Request 34676: Add printing of extended attributes to Resource objects.

2015-06-01 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34676/ --- (Updated June 1, 2015, 10:24 p.m.) Review request for mesos and Vinod Kone.

  1   2   >