Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract stringify operation

2015-04-03 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated April 3, 2015, 6:29 p.m.) Review request for mesos, Chi Zhang, Ian

Review Request 32820: Fixed the non-POD global variable in perf sampler.

2015-04-03 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32820/ --- Review request for mesos, Ben Mahler and Ian Downes. Repository: mesos

Re: Review Request 30962: Enabled environment decorator to override.

2015-04-03 Thread Niklas Nielsen
On March 30, 2015, 3:28 a.m., Adam B wrote: src/hook/manager.cpp, line 130 https://reviews.apache.org/r/30962/diff/5/?file=894741#file894741line130 And if (result.isNone()), is that really supposed to mean that this hook didn't want to modify the env, so the HookManager can leave

Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract stringify operation

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

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32805/ --- (Updated April 3, 2015, 5:11 p.m.) Review request for mesos, Ben Mahler, Ian

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32805/#review78801 --- Ship it! src/linux/perf.cpp

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32805/#review78809 --- Patch looks great! Reviews applied: [32820, 32805] All tests

Re: Review Request 32797: Kill the executor when docker container is destroyed.

2015-04-03 Thread Timothy Chen
On April 3, 2015, 3:46 p.m., Benjamin Hindman wrote: src/slave/containerizer/docker.cpp, line 1230 https://reviews.apache.org/r/32797/diff/1/?file=914221#file914221line1230 Why kill the executor before doing Docker::stop? Can you comment here why you do it in this order versus

Re: Review Request 32798: Add test to verify executor clean up in docker containerizer.

2015-04-03 Thread Timothy Chen
On April 3, 2015, 3:38 p.m., Benjamin Hindman wrote: src/tests/docker_containerizer_tests.cpp, lines 2625-2627 https://reviews.apache.org/r/32798/diff/1/?file=914222#file914222line2625 My only question here is how do you know the executor is properly killed and cleaned up? Is

Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract stringify operation

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

Re: Review Request 32832: Added CHANGELOG for 0.22.1

2015-04-03 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32832/#review78831 --- Ship it! Ship It! - Timothy Chen On April 3, 2015, 8:43 p.m.,

Re: Review Request 32820: Fixed the non-POD global variable in perf sampler.

2015-04-03 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32820/#review78832 --- Ship it! Ship It! - Ian Downes On April 3, 2015, 10:01 a.m.,

Re: Review Request 32832: Added CHANGELOG for 0.22.1

2015-04-03 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32832/ --- (Updated April 3, 2015, 3:30 p.m.) Review request for mesos, Ben Mahler and

Re: Suggestion: Mesos 0.22.1 point release

2015-04-03 Thread Niklas Nielsen
Hi everyone, I think we have everything for the point release now: https://docs.google.com/a/mesosphere.io/spreadsheets/d/1OzAWNjAL4zKtI-jOJqaQUcDNlnrNik2Dd7dHhwFLKcI/edit#gid=0 We planned on making an RC today. So with that in mind, if you have any urgent issues that needs to go into 0.22.1,

Re: Review Request 32820: Fixed the non-POD global variable in perf sampler.

2015-04-03 Thread Jie Yu
On April 3, 2015, 8:34 p.m., Ben Mahler wrote: src/linux/perf.cpp, lines 46-51 https://reviews.apache.org/r/32820/diff/1/?file=914812#file914812line46 char[] Also noticed many of our other constants are not static, we may want to do a sweep? Done. I'll do the sweep

Re: Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.

2015-04-03 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32656/ --- (Updated April 3, 2015, 10:36 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 32833: Added os::signals::install to install signal handlers.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32833/#review78836 --- Ship it! Ship It! - Vinod Kone On April 3, 2015, 9:28 p.m., Jie

Re: Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.

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

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32805/ --- (Updated April 3, 2015, 9:29 p.m.) Review request for mesos, Ben Mahler, Ian

Re: Review Request 32805: Terminated the perf subprocess once the parent exits.

2015-04-03 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32805/ --- (Updated April 3, 2015, 9:29 p.m.) Review request for mesos, Ben Mahler, Ian

Re: Review Request 32820: Fixed the non-POD global variable in perf sampler.

2015-04-03 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32820/#review78829 --- Ship it! Feel free to split out the proces namespace change.

Review Request 32832: Added CHANGELOG for 0.22.1

2015-04-03 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32832/ --- Review request for mesos, Ben Mahler and Timothy Chen. Repository: mesos

Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class

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

Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class

2015-04-03 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 3, 2015, 10:36 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 32654: Clean up HostIPNetwork since every use performs the same extract stringify operation

2015-04-03 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32654/ --- (Updated April 3, 2015, 7:19 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 32832: Added CHANGELOG for 0.22.1

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

Review Request 32833: Added os::signals::install to install signal handlers.

2015-04-03 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32833/ --- Review request for mesos and Vinod Kone. Repository: mesos Description

Review Request 32834: Modifiy gdb scripts error message to check gdb is installed.

2015-04-03 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32834/ --- Review request for mesos, Adam B, Cody Maloney, and Niklas Nielsen.

Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class

2015-04-03 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 3, 2015, 10:17 p.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 32507: Moved REGISTER and REREGISTER out of scheduler Calls.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32507/ --- (Updated April 3, 2015, 11:47 p.m.) Review request for mesos and Ben Mahler.

Review Request 32844: Added SUBSCRIBE call and SUBSCRIBED event.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32844/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-1127

Re: Review Request 32508: Added version to the scheduler protobufs.

2015-04-03 Thread Vinod Kone
On March 31, 2015, 11:38 p.m., Ben Mahler wrote: Can we punt on version for the initial HTTP API beta release? I say this because: (1) The protobuf objects seems like the wrong place to place a version. If a backwards-incompatible change occurs, we may not be able to parse the

Re: Review Request 32834: Modifiy gdb scripts error message to check gdb is installed.

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

Review Request 32843: Updated KILL to include SlaveID.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32843/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-1127

Re: Review Request 32656: Refactor statistics helper out of PortMappingIsolatorTest for easier reuse.

2015-04-03 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32656/#review78852 --- src/slave/containerizer/isolators/network/port_mapping.hpp

Re: Review Request 32506: Added output stream operators for scheduler Calls and Events.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32506/ --- (Updated April 3, 2015, 11:42 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32832: Added CHANGELOG for 0.22.1

2015-04-03 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32832/#review78851 --- Ship it! Ship It! - Adam B On April 3, 2015, 3:58 p.m., Niklas

Re: Review Request 32507: Moved REGISTER and REREGISTER out of scheduler Calls.

2015-04-03 Thread Vinod Kone
On March 31, 2015, 10:51 p.m., Ben Mahler wrote: Instead of having a separate /scheduler/events endpoint with a top level Subscribe protobuf, could we just have one /scheduler/call endpoint and have a SUBSCRIBE Call? I prefer this because: (1) There is a single endpoint / Call

Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class

2015-04-03 Thread Paul Brett
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/ --- (Updated April 4, 2015, 12:39 a.m.) Review request for mesos, Chi Zhang, Ian

Re: Review Request 32505: Added SHUTDOWN scheduler call.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32505/ --- (Updated April 3, 2015, 11:38 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 32509: Documented the scheduler Event/Call protobufs.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32509/ --- (Updated April 3, 2015, 11:55 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32845: Renamed UNREGISTER call to UNSUBSCRIBE.

2015-04-03 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32845/#review78869 --- Patch looks great! Reviews applied: [32500, 32501, 32502, 32504,

Re: Review Request 32850: Moved cram-md5 authenticatee process definition into implementation file.

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

Re: Review Request 32505: Added SHUTDOWN scheduler call.

2015-04-03 Thread Vinod Kone
On April 1, 2015, 12:20 a.m., Ben Mahler wrote: Modulo comments. I noticed you added SlaveID on Shutdown, can you add it to Kill as well? Yup. Will send a patch for it as well. On April 1, 2015, 12:20 a.m., Ben Mahler wrote: include/mesos/scheduler/scheduler.proto, line 127

Re: Review Request 32509: Documented the scheduler Event/Call protobufs.

2015-04-03 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32509/#review78874 --- Patch looks great! Reviews applied: [32500, 32501, 32502, 32504,

Review Request 32850: Moved cram-md5 authenticatee process definition into implementation file.

2015-04-03 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32850/ --- Review request for mesos, Joris Van Remoortere and switched to 'mcypark'.

Re: Suggestion: Mesos 0.22.1 point release

2015-04-03 Thread Niklas Nielsen
Based on input from Vinod and Adam; I will reduce the scope on the point release to focus on MESOS-1795 and MESOS-2583. I will move the other tickets back to 0.23.0 if you don't have any objections - let me know if you have any tickets which were regressions in 0.22.0. Also, this will probably

Re: Review Request 32502: Updated RECONCILE call to always specifiy slave id.

2015-04-03 Thread Vinod Kone
On March 31, 2015, 9:49 p.m., Ben Mahler wrote: There is another nice aspect of requiring SlaveID in Reconcile and in Kill, we can make SlaveID in TaskStatus required. Are you planning to add a TODO for that? Will we be ever be able to make it required? Added a TODO. The only way we

Re: Review Request 32502: Updated RECONCILE call to always specifiy slave id.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32502/ --- (Updated April 3, 2015, 11:36 p.m.) Review request for mesos and Ben Mahler.

Re: Review Request 32655: Refactor out launchHelper to make is usable outside PortMappingIsolatorTest class

2015-04-03 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32655/#review78861 --- src/slave/containerizer/mesos/containerizer.hpp

Review Request 32845: Renamed UNREGISTER call to UNSUBSCRIBE.

2015-04-03 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32845/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-1127

Re: Review Request 32832: Added CHANGELOG for 0.22.1

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

Re: Review Request 32583: Marked RunTaskMessage::framework_id as optional.

2015-04-03 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32583/ --- (Updated April 3, 2015, 10:05 a.m.) Review request for mesos, Adam B and

Re: Review Request 32796: Only update docker container when resources differs.

2015-04-03 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32796/#review78780 --- Ship it! Ship It! - Benjamin Hindman On April 2, 2015, 11:37

Re: Review Request 32797: Kill the executor when docker container is destroyed.

2015-04-03 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32797/#review78781 --- Ship it! Ship It! - Benjamin Hindman On April 2, 2015, 11:38

Re: Review Request 32798: Add test to verify executor clean up in docker containerizer.

2015-04-03 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32798/#review78783 --- Ship it! src/tests/docker_containerizer_tests.cpp

Re: Review Request 32797: Kill the executor when docker container is destroyed.

2015-04-03 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32797/#review78785 --- Ship it! I had another thought after my first review, see below