Re: Review Request 33875: Removed unused collect cycle in resource monitor

2015-05-11 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33875/ --- (Updated May 11, 2015, 11:18 a.m.) Review request for mesos, Jie Yu and Vinod

Review Request 34048: Fixed disappearing search bar: MESOS-2479

2015-05-11 Thread Ian Babrou
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34048/ --- Review request for mesos, Adam B and Thomas Rampelberg. Bugs: MESOS-2479

Re: Review Request 33875: Removed unused collect cycle in resource monitor

2015-05-11 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33875/#review83266 --- Ship it! Ship It! - Jie Yu On May 11, 2015, 6:20 p.m., Niklas

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

2015-05-11 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33876/ --- (Updated May 11, 2015, 11:20 a.m.) Review request for mesos, Jie Yu and Vinod

Re: Review Request 33875: Removed unused collect cycle in resource monitor

2015-05-11 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33875/ --- (Updated May 11, 2015, 11:20 a.m.) Review request for mesos, Jie Yu and Vinod

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review83265 --- src/slave/containerizer/docker.cpp

Re: Review Request 33513: Added a modules-aware factory for allocators.

2015-05-11 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33513/#review83258 --- src/master/allocator/allocator.cpp

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

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

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

2015-05-11 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33876/ --- (Updated May 11, 2015, 11:18 a.m.) Review request for mesos, Jie Yu and Vinod

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
On May 11, 2015, 12:12 p.m., Timothy Chen wrote: src/tests/slave_tests.cpp, line 325 https://reviews.apache.org/r/33249/diff/7/?file=955468#file955468line325 I don't really understand this comment? And if you simply pause the clock your await won't work right? I'm starting to

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/ --- (Updated May 11, 2015, 3:06 p.m.) Review request for mesos, Benjamin Hindman,

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

2015-05-11 Thread Niklas Nielsen
On May 6, 2015, 11:51 a.m., Jie Yu wrote: src/slave/monitor.cpp, lines 89-90 https://reviews.apache.org/r/33876/diff/1/?file=950753#file950753line89 We've already done `using std::list` above. No need to have the `std::` prefix. ALso, no need for `process::` prefix

Re: Review Request 34048: Fixed disappearing search bar: MESOS-2479

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

Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-11 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32398/ --- (Updated May 11, 2015, 8:35 p.m.) Review request for mesos and Alexander

Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-11 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32398/ --- (Updated May 11, 2015, 8:36 p.m.) Review request for mesos and Alexander

Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-11 Thread Michael Park
On April 8, 2015, 11:49 p.m., Jie Yu wrote: src/common/resources.cpp, lines 442-445 https://reviews.apache.org/r/32398/diff/2/?file=921003#file921003line442 Since we already have `isReserved`. How about calling it `isDynamicallyReserved`? Good idea, Fixed :) On April 8, 2015,

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

2015-05-11 Thread Cong Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31505/ --- (Updated May 12, 2015, 12:14 a.m.) Review request for mesos, Chi Zhang, Ian

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

2015-05-11 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31505/#review83334 --- Patch looks great! Reviews applied: [31502, 31503, 31504, 31505]

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

2015-05-11 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33792/ --- (Updated May 12, 2015, 12:50 a.m.) Review request for mesos, Alexander Rojas

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

2015-05-11 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33792/ --- (Updated May 12, 2015, 12:54 a.m.) Review request for mesos, Alexander Rojas

Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

2015-05-11 Thread haosdent huang
On May 11, 2015, 10:19 p.m., Ben Mahler wrote: src/slave/slave.cpp, line 155 https://reviews.apache.org/r/34016/diff/1/?file=954524#file954524line155 unique_ptr is not a POD, so this will still try to run the destructor of the function during exit of the program. Can

Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

2015-05-11 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34016/ --- (Updated May 12, 2015, 1:17 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
On May 11, 2015, 4:07 p.m., Timothy Chen wrote: Thanks Jay the changes looks reasonable to me, will wait for Jie to comment. Great, thanks! When you commit can you add the three separate commits that are here:

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review83319 --- Ship it! Nice tests! LGTM! - Jie Yu On May 11, 2015, 10:06

Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-11 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32398/#review83321 --- Patch looks great! Reviews applied: [32139, 32140, 32149, 32150,

Re: Review Request 33153: Moved a partition test into partition_tests.cpp.

2015-05-11 Thread Ben Mahler
On April 17, 2015, 12:11 p.m., Alexander Rojas wrote: As in the previous case, can you add the JIRA issue link. I also would like to understand why the move is needed (though I might see it later). Awhile ago we introduced partition_tests.cpp and moved some of the partition related tests

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

2015-05-11 Thread haosdent huang
On May 11, 2015, 10:47 p.m., Ben Mahler wrote: Thanks! Could you split the patch between the implementation and the test? The implementation looks pretty good so would like to give a ship it on that before I review the test. Much appreciated :) Hi, @bmahler. I update the patch and

Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

2015-05-11 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34016/ --- (Updated May 12, 2015, 1:26 a.m.) Review request for mesos and Ben Mahler.

Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

2015-05-11 Thread haosdent huang
On May 11, 2015, 10:19 p.m., Ben Mahler wrote: src/slave/slave.cpp, line 155 https://reviews.apache.org/r/34016/diff/1/?file=954524#file954524line155 unique_ptr is not a POD, so this will still try to run the destructor of the function during exit of the program. Can

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

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

Re: Review Request 29748: Added tests for dynamic reservation.

2015-05-11 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29748/#review83313 --- Ship it! src/tests/reservation_tests.cpp

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

2015-05-11 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33792/ --- (Updated May 12, 2015, 12:55 a.m.) Review request for mesos, Alexander Rojas

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

2015-05-11 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34068/ --- Review request for mesos, Alexander Rojas and Ben Mahler. Repository: mesos

Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

2015-05-11 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34016/ --- (Updated May 12, 2015, 1:21 a.m.) Review request for mesos and Ben Mahler.

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

2015-05-11 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33876/#review83292 --- src/slave/monitor.hpp

Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

2015-05-11 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34016/#review83299 --- src/slave/slave.cpp

Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-11 Thread Michael Park
On April 24, 2015, 8:18 a.m., Alexander Rukletsov wrote: src/common/resources_utils.cpp, lines 45-46 https://reviews.apache.org/r/32398/diff/2/?file=921004#file921004line45 I like the name a lot, gives a good understanding on what's going to happen, but let's still leave a

Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-11 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32398/ --- (Updated May 11, 2015, 10:25 p.m.) Review request for mesos and Alexander

Re: Review Request 32398: Persisted the reservation state on the slave.

2015-05-11 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32398/ --- (Updated May 11, 2015, 10:29 p.m.) Review request for mesos and Alexander

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

2015-05-11 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33792/#review83303 --- Thanks! Could you split the patch between the implementation and

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review83310 --- Thanks Jay the changes looks reasonable to me, will wait for Jie to

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review83308 --- Ship it! Ship It! - Timothy Chen On May 11, 2015, 10:06 p.m.,

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

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

Re: Review Request 33109: Allow setting environment variables in mesos-execute

2015-05-11 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33109/#review83358 --- Ship it! Sorry it took me so long to circle back to this. I'll fix

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-11 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29889/#review83305 --- src/docker/docker.cpp

Re: Review Request 33154: Added reason metrics for slave removals.

2015-05-11 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33154/ --- (Updated May 12, 2015, 2:51 a.m.) Review request for mesos and Vinod Kone.

Re: Review Request 33154: Added reason metrics for slave removals.

2015-05-11 Thread Ben Mahler
On April 17, 2015, 12:38 p.m., Alexander Rojas wrote: src/master/master.hpp, line 370 https://reviews.apache.org/r/33154/diff/1/?file=926686#file926686line370 I find this quite counter intuitive for two reasons: 1. If I just check this header, I would expect reason to behave a

Re: Review Request 33155: Added commented-out tests for slave removal metrics.

2015-05-11 Thread Ben Mahler
On April 16, 2015, 11:04 p.m., Vinod Kone wrote: src/tests/master_tests.cpp, lines 1725-1731 https://reviews.apache.org/r/33155/diff/1/?file=926682#file926682line1725 I'm a bit confused on how slave's lifetime affects the metrics collection on master endpoint? There is no master

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
On May 8, 2015, 11:48 p.m., Timothy Chen wrote: src/slave/containerizer/mesos/containerizer.cpp, line 567 https://reviews.apache.org/r/33249/diff/6/?file=954330#file954330line567 No tests for mesos containerizer change? The changes to mesos containerizer were to support the

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/#review83233 --- include/mesos/containerizer/containerizer.proto

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Jay Buffington
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249/ --- (Updated May 11, 2015, 10:10 a.m.) Review request for mesos, Benjamin Hindman,