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

2015-05-27 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85336 --- Bad patch! Reviews applied: [34662] Failed command: make -j3

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

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

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

2015-05-27 Thread Bartek Plotka
On May 27, 2015, 9 a.m., Mesos ReviewBot wrote: Bad patch! Reviews applied: [34662] Failed command: make -j3 distcheck Error: make dist-gzip am__post_remove_distdir='@:' make[1]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot' if test -d

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

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

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

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

Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

2015-05-27 Thread Alexander Rojas
On May 20, 2015, 7:10 a.m., Nikita Vetoshkin wrote: 3rdparty/libprocess/include/process/http.hpp, line 352 https://reviews.apache.org/r/30032/diff/7/?file=963338#file963338line352 strftime formatting is locale dependent. This example ``` #include locale.h #include

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

2015-05-27 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34432/ --- (Updated May 27, 2015, 6:05 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-27 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34558/ --- (Updated May 27, 2015, 6:05 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 34431: Add htb queueing discipline

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

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

2015-05-27 Thread Jie Yu
On May 22, 2015, 10:09 p.m., Jie Yu wrote: Two high level comments: 1) We need a slave flag to control if turning on this feature or not 2) Do you think we should at least add some unit test for this? For example, verify that the filters are properly set up? Jie Yu wrote:

Re: Review Request 34724: Doxygen enhancements

2015-05-27 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34724/ --- (Updated May 27, 2015, 11:38 a.m.) Review request for mesos, Ben Mahler, Joerg

Review Request 34728: Changed callbacks to take copies rather than references in monitor.

2015-05-27 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34728/ --- Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone. Repository:

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

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

Re: Review Request 33876: Added usages() to resource monitor

2015-05-27 Thread Niklas Nielsen
On May 11, 2015, 3:49 p.m., Jie Yu wrote: src/slave/monitor.cpp, line 126 https://reviews.apache.org/r/33876/diff/3/?file=955511#file955511line126 Hum, this looks like a bug to me? Since you capture `containerId` by reference, what if by the time `onFailed` is called, the

Re: Review Request 34703: Added stream manipulators for the Time object.

2015-05-27 Thread Nikita Vetoshkin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/#review85380 --- 3rdparty/libprocess/include/process/time.hpp

Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

2015-05-27 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/ --- (Updated May 27, 2015, 4:50 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 34703: Added stream manipulators for the Time object.

2015-05-27 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/ --- (Updated May 27, 2015, 4:48 p.m.) Review request for mesos, Bernd Mathiske,

Re: Review Request 34703: Added stream manipulators for the Time object.

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

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

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

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

2015-05-27 Thread Marco Massenzio
On May 27, 2015, 2:18 a.m., Niklas Nielsen wrote: src/common/protobuf_utils.hpp, line 83 https://reviews.apache.org/r/34687/diff/1/?file=972329#file972329line83 Shouldn't parse() go in https://github.com/apache/mesos/blob/master/src/common/parse.hpp? I don't really know - in the

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

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

Review Request 34721: Added QoS kill executor correction test.

2015-05-27 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34721/ --- Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and

Review Request 34720: Added kill executor correction to slave.

2015-05-27 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34720/ --- Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and

Review Request 34719: Added QOS_KILLED as status reason

2015-05-27 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34719/ --- Review request for mesos, Bartek Plotka, Jie Yu, Joris Van Remoortere, and

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

2015-05-27 Thread Marco Massenzio
On May 27, 2015, 2:18 a.m., Niklas Nielsen wrote: src/common/protobuf_utils.cpp, line 169 https://reviews.apache.org/r/34687/diff/1/?file=972330#file972330line169 We have traditionally used stringify(...) if we needed type T to string conversions. Have you checked if we have one

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

2015-05-27 Thread Cong Wang
On May 22, 2015, 10:09 p.m., Jie Yu wrote: Two high level comments: 1) We need a slave flag to control if turning on this feature or not 2) Do you think we should at least add some unit test for this? For example, verify that the filters are properly set up? Jie Yu wrote:

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

2015-05-27 Thread Jie Yu
On May 22, 2015, 10:09 p.m., Jie Yu wrote: Two high level comments: 1) We need a slave flag to control if turning on this feature or not 2) Do you think we should at least add some unit test for this? For example, verify that the filters are properly set up? Jie Yu wrote:

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

2015-05-27 Thread Bartłomiej Płotka
Thx, found the issue (: 2015-05-27 8:39 GMT-07:00 Niklas Nielsen nik...@mesosphere.io: +1 to Vinod's suggestion. The buildbot does a dist build, so also make sure new public headers are in src/Makefile.am On 27 May 2015 at 08:32, Vinod Kone vi...@twitter.com.invalid wrote: You likely

Re: Review Request 30032: Added support for cache control in libprocess when dealing with static files.

2015-05-27 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30032/#review85384 --- Patch looks great! Reviews applied: [34392, 34703, 30032] All

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

2015-05-27 Thread Jie Yu
On May 22, 2015, 10:09 p.m., Jie Yu wrote: Two high level comments: 1) We need a slave flag to control if turning on this feature or not 2) Do you think we should at least add some unit test for this? For example, verify that the filters are properly set up? Ping. Did you see these

Re: Review Request 34654: Send docker inspect output with TaskStatus data.

2015-05-27 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34654/#review85390 --- Ship it! This looks good but I'd like to can clean up the 'output'

Re: Review Request 34141: AppC provsioning backend.

2015-05-27 Thread Ian Downes
On May 18, 2015, 4:38 p.m., Chi Zhang wrote: src/slave/containerizer/provisioners/appc/backend.hpp, line 50 https://reviews.apache.org/r/34141/diff/1/?file=957281#file957281line50 Not a big deal, but would it be more flexible for Bankend to take a rootImage as an argument instead

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

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

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

2015-05-27 Thread Cong Wang
On May 22, 2015, 10:09 p.m., Jie Yu wrote: Two high level comments: 1) We need a slave flag to control if turning on this feature or not 2) Do you think we should at least add some unit test for this? For example, verify that the filters are properly set up? Jie Yu wrote:

Re: Review Request 34730: Added 'updateSlave()' in master to handle oversubscribed resources.

2015-05-27 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34730/#review85468 --- Ship it! Taken it is a stub patch src/master/master.cpp

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

2015-05-27 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated May 28, 2015, 12:04 a.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34729: Updated slave to send total amount of oversubscribed resources.

2015-05-27 Thread Vinod Kone
On May 27, 2015, 11:44 p.m., Jie Yu wrote: src/messages/messages.proto, lines 339-342 https://reviews.apache.org/r/34729/diff/1/?file=973027#file973027line339 It might not be an issue right now, but once we start to support updating slave's total regular resources, how can we

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

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

Re: Review Request 34729: Updated slave to send total amount of oversubscribed resources.

2015-05-27 Thread Jie Yu
On May 27, 2015, 11:44 p.m., Jie Yu wrote: src/messages/messages.proto, lines 339-342 https://reviews.apache.org/r/34729/diff/1/?file=973027#file973027line339 It might not be an issue right now, but once we start to support updating slave's total regular resources, how can we

Review Request 34748: Defined protobuf for usage returned by Resource Monitor. Reused ResourceUsage.

2015-05-27 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34748/ --- Review request for mesos, Jie Yu, Joris Van Remoortere, Niklas Nielsen, Szymon

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

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

Re: Review Request 34729: Updated slave to send total amount of oversubscribed resources.

2015-05-27 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34729/#review85462 --- Ship it! Ship It! - Jie Yu On May 27, 2015, 11:11 p.m., Vinod

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

2015-05-27 Thread Bartek Plotka
On May 28, 2015, 12:24 a.m., Vinod Kone wrote: src/examples/test_resource_estimator_module.cpp, lines 40-44 https://reviews.apache.org/r/34662/diff/6/?file=973211#file973211line40 I'm confused. 'parameters' are unused and the passed 'flags' are empty? Niklas Nielsen wrote:

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

2015-05-27 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34748/ --- (Updated May 28, 2015, 2:37 a.m.) Review request for mesos, Jie Yu, Joris Van

Re: Review Request 34703: Added stream manipulators for the Time object.

2015-05-27 Thread Nikita Vetoshkin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34703/#review85514 --- 3rdparty/libprocess/include/process/time.hpp

Re: Review Request 34140: AppC image store

2015-05-27 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34140/#review85456 --- Disk usage by the store is currently unbounded. Do we need to add

Re: Review Request 34737: Added test to verify fix for MESOS-2771

2015-05-27 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34737/ --- (Updated May 27, 2015, 4:14 p.m.) Review request for mesos, Ian Downes, Jie

Re: Review Request 34724: Doxygen enhancements

2015-05-27 Thread Marco Massenzio
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34724/#review85460 --- Ship it! Awesome! Doxyfile

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

2015-05-27 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85413 --- Do you want to wire up module loading in

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

2015-05-27 Thread Marco Massenzio
On May 27, 2015, 2:18 a.m., Niklas Nielsen wrote: src/Makefile.am, line 1387 https://reviews.apache.org/r/34687/diff/1/?file=972328#file972328line1387 Is the alignment right here? Try to set your tabstop to 8 :) Also, shouldn't the ordering bring this further down? no, it's